Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1439004 > unrolled thread

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

Started bySteve Longerbeam <slongerbeam@gmail.com>
First post2016-07-08 01:10 +0200
Last post2016-07-20 02:10 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <slongerbeam@gmail.com> - 2016-07-08 01:10 +0200
    Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-08 19:40 +0200
      Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <steve_longerbeam@mentor.com> - 2016-07-10 18:40 +0200
        Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <steve_longerbeam@mentor.com> - 2016-07-14 01:10 +0200
          Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-15 15:00 +0200
            Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <steve_longerbeam@mentor.com> - 2016-07-16 01:10 +0200
              Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <steve_longerbeam@mentor.com> - 2016-07-16 22:30 +0200
                Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-19 15:40 +0200
                  Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Steve Longerbeam <steve_longerbeam@mentor.com> - 2016-07-20 02:10 +0200

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

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2016-07-08 01:10 +0200
Subject[PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format
Message-ID<rSqAF-51z-9@gated-at.bofh.it>
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,
-- 
1.9.1

[toc] | [next] | [standalone]


#1439719

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-07-08 19:40 +0200
Message-ID<rSHUT-7Ss-41@gated-at.bofh.it>
In reply to#1439004
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.

regards
Philipp

[toc] | [prev] | [next] | [standalone]


#1440131

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2016-07-10 18:40 +0200
Message-ID<rTpVT-34V-1@gated-at.bofh.it>
In reply to#1439719

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.

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.

Steve

[toc] | [prev] | [next] | [standalone]


#1442934

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2016-07-14 01:10 +0200
Message-ID<rUBrX-1jq-5@gated-at.bofh.it>
In reply to#1440131
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.

Steve

[toc] | [prev] | [next] | [standalone]


#1444281

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-07-15 15:00 +0200
Message-ID<rVaSK-70X-27@gated-at.bofh.it>
In reply to#1442934
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.

regards
Philipp

[toc] | [prev] | [next] | [standalone]


#1444658

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2016-07-16 01:10 +0200
Message-ID<rVkp3-4zC-9@gated-at.bofh.it>
In reply to#1444281

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.

Adding Hans.

Steve

[toc] | [prev] | [next] | [standalone]


#1444909

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2016-07-16 22:30 +0200
Message-ID<rVEnL-8l8-1@gated-at.bofh.it>
In reply to#1444658

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

[toc] | [prev] | [next] | [standalone]


#1446478

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-07-19 15:40 +0200
Message-ID<rWDpE-4AM-27@gated-at.bofh.it>
In reply to#1444909
Am Samstag, den 16.07.2016, 13:24 -0700 schrieb Steve Longerbeam:
[...]
> > 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.

Sounds good to me, the V4L2_MBUS_BT656 documentation comment in
include/media/v4l2-mediabus.h should be extended to include the new
non-standard mode then.

regards
Philipp

[toc] | [prev] | [next] | [standalone]


#1446817

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2016-07-20 02:10 +0200
Message-ID<rWNfj-2pz-21@gated-at.bofh.it>
In reply to#1446478
On 07/19/2016 06:32 AM, Philipp Zabel wrote:
> Am Samstag, den 16.07.2016, 13:24 -0700 schrieb Steve Longerbeam:
> [...]
>>> 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.
> Sounds good to me, the V4L2_MBUS_BT656 documentation comment in
> include/media/v4l2-mediabus.h should be extended to include the new
> non-standard mode then.

Hi Philipp, Ok I just sent that off to linux-media. If approved there,
we can use the new flag V4L2_MBUS_NEWAVMODE to make the
adjustment to CSI_CCIR_CODE_2.

Steve

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web