lkml.org 
[lkml]   [2016]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format
    From
    Date


    On 07/15/2016 04:09 PM, Steve Longerbeam wrote:
    >
    >
    > On 07/15/2016 05:58 AM, Philipp Zabel wrote:
    >> 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@in.bosch.com>
    >>>>>>
    >>>>>> 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@in.bosch.com>
    >>>>>> ---
    >>>>>> 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.
    >
    > Hmm, do you mean define something like a V4L2_MBUS_BT656_NEWAVMODE,
    > and then add a new "newavmode" boolean DT binding parsed by
    > v4l2_of_parse_endpoint()?
    >
    > I don't know if that would make sense given that this NEWAVMODE is a kind
    > of hack of the BT.656 standard, only used by Analog Devices
    > encoders/decoders.
    >
    > Although there a _lot_ of AD encoder/decoder chips (and subdev drivers
    > written for
    > them), so maybe it would make sense to do this.
    >

    I don't think a "newavmode" boolean property would necessitate a whole new
    mbus type, but perhaps just a new parallel bus flag. I will propose a
    patch at
    linux-media that adds this.

    Steve

    \
     
     \ /
      Last update: 2016-07-16 23:01    [W:4.765 / U:1.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site