Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format

[Philipp Zabel]

Am Mittwoch, den 13.07.2016, 16:02 -0700 schrieb Steve Longerbeam:
> On 07/10/2016 09:33 AM, Steve Longerbeam wrote:
> >
> >
> > On 07/08/2016 10:34 AM, Philipp Zabel wrote:
> >> Am Donnerstag, den 07.07.2016, 16:03 -0700 schrieb Steve Longerbeam:
> >>> From: Suresh Dhandapani <Suresh.Dhandapani@xxxxxxxxxxxx>
> >>>
> >>> This patch will change the register IPU_CSI0_CCIR_CODE_2 value from
> >>> 0x40596 to 0x405A6. The change is related to the Start of field 1
> >>> first blanking line command bit[5-3] for NTSC format only. This
> >>> change is dependent with ADV chip where the NEWAVMODE is set to 0
> >>> in register 0x31. Setting NEWAVMODE to "0" in ADV means "EAV/SAV
> >>> codes generated to suit analog devices encoders".
> >>>
> >>> Signed-off-by: Suresh Dhandapani <Suresh.Dhandapani@xxxxxxxxxxxx>
> >>> ---
> >>>   drivers/gpu/ipu-v3/ipu-csi.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> >>> index 0eac28c..ec81958 100644
> >>> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> >>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> >>> @@ -422,7 +422,7 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> >>>                 ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> >>>                         CSI_CCIR_CODE_1);
> >>> -            ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> >>> +            ipu_csi_write(csi, 0x405A6, CSI_CCIR_CODE_2);
> >>>               ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> >>>           } else {
> >>>               dev_err(csi->ipu->dev,
> >> This looks like a very hardware specific hack? I'll at least have to
> >> test if that also works with other analog decoders.
> >
> > Hi Philipp,
> >
> > Yes it's a hack, but it has always been a hack (hardcoded values). And the
> > reason is simple, nobody AFAIK (including me) understands how to program
> > these CSI_CCIR_CODE registers, the description in the reference manual is
> > complete gibberish.
> 
> Hi Philipp, Ian over at linux-media helped me to understand these registers a
> little better, although there are still mysteries given the poor documentation.
> You should have been copied on that linux-media thread.
> 
> >
> > The reason we made this change is that, in discussions with Analog Devices,
> > they recommended setting NEWAVMODE, which changes the positions of
> > the AV codes sent by the ADV7180 on the bt.656 bus. It took Suresh at least
> > a full day of reverse engineering (Suresh correct me if I am wrong) to hit
> > on the correct values in these registers to regain stable video after switching
> > the ADV7180 to NEWAVMODE.
> 
> So this NEWAVMODE is somehow breaking from the BT.656 standard, which
> necessitated the change to CSI_CCIR_CODE_2. So NEWAVMODE if enabled in
> the ADV7180 will break other capture backends that are expecting standard
> BT.656 SAV/EAV codes. So NEWAVMODE should not be used and I will remove
> this patch in the next version.

Ok. To use that mode, first a new v4l2 mbus type and corresponding DT
bindings would have to be added.

regards
Philipp