lkml.org 
[lkml]   [2019]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] media: i2c: adv7180: fix adv7280 BT.656-4 compatibility
Hi Tim,

On 2019-09-27 12:26:40 -0700, Tim Harvey wrote:
> On Fri, Sep 27, 2019 at 12:04 PM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hi Tim,
> >
> > Sorry for taking to so long to look at this.
> >
> > On 2019-09-23 15:04:47 -0700, Tim Harvey wrote:
> > > On Thu, Aug 29, 2019 at 7:29 AM Niklas Söderlund
> > > <niklas.soderlund@ragnatech.se> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On 2019-08-29 13:43:49 +0200, Hans Verkuil wrote:
> > > > > Adding Niklas.
> > > > >
> > > > > Niklas, can you take a look at this?
> > > >
> > > > I'm happy to have a look at this. I'm currently moving so all my boards
> > > > are in a box somewhere. I hope to have my lab up and running next week,
> > > > so if this is not urgent I will look at it then.
> > > >
> > >
> > > Niklas,
> > >
> > > Have you looked at this yet? Without this patch the ADV7280A does not
> > > output proper BT.656. We tested this on a Gateworks Ventana GW5404-G
> > > which uses the ADV7280A connected to the IMX6 CSI parallel bus. I'm
> > > hoping to see this get merged and perhaps backported to older kernels.
> >
> > I only have access to an adv7180 so I was unable to test this patch.
> > After reviewing the documentation I think the patch is OK if what you
> > want is to unconditionally switch the driver from outputting BT.656-3 to
> > outputting BT.656-4.
> >
> > As this change would effect a large number of compat strings (adv7280,
> > adv7280-m, adv7281, adv7281-m, adv7281-ma, adv7282, adv7282-m) and the
> > goal is to back port it I'm a bit reluctant to adding my tag to this
> > patch as I'm not sure if this will break other setups.
> >
> > From the documentation about the BT.656-4 register (address 0x04 bit 7):
> >
> > Between Revision 3 and Revision 4 of the ITU-R BT.656 standards,
> > the ITU has changed the toggling position for the V bit within
> > the SAV EAV codes for NTSC. The ITU-R BT.656-4 standard
> > bit allows the user to select an output mode that is compliant
> > with either the previous or new standard. For further information,
> > visit the International Telecommunication Union website.
> >
> > Note that the standard change only affects NTSC and has no
> > bearing on PAL.
> >
> > When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3
> > specification is used. The V bit goes low at EAV of Line 10
> > and Line 273.
> >
> > When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is
> > used. The V bit goes low at EAV of Line 20 and Line 283.
> >
> > Do you know what effects such a change would bring? Looking at the
> > driver BT.656-4 seems to be set unconditionally for some adv7180 chips.
> >
>
> Niklas,
>
> Quite simply, we have a board that has an ADV7180 attached to the
> parallel CSI of an IMX6 that worked fine with mainline drivers then
> when we revised this board to attach an ADV7280A in the same way
> capture failed to sync. Investigation showed that the NEWAVMODE
> differed between the two.

I understand your problem, the driver configures adv7180 and adv7280
differently.

>
> So if the point of the driver is to configure the variants in the same
> way, this patch needs to be applied.

I'm not sure that is the point of the driver. As the driver today
configures different compatible strings differently. Some as ITU-R
BT.656-3 and some as ITU-R BT.656-4, I can only assume there is a reason
for that.

>
> I would maintain that the adv7180 comes up with NEWAVMODE enabled and
> in order to be compatible we must configure the adv7282 the same.
>
> The same argument can be made for setting the V bit end position in
> NTSC mode - its done for the adv7180 so for compatible output it
> should be done for the adv7282.

I understand that this is needed to make it a drop-in replacement for
the adv7180 in your use-case. But I'm not sure it is a good idea for
other users of the driver. What if someone is already using a adv7282 on
a board and depends on it providing ITU-R BT.656-3 and the old settings?
If this patch is picked up there use-cases may break.

I'm not sure what the best way forward is I'm afraid. Looking at
video-interfaces.txt we have a device tree property bus-type which is
used to describe the bus is a BT.656 bus but not which revision of it.

I'm not really found of driver specific bus descriptions, but maybe this
is a case where one might consider adding one? Hans what do you think?

>
> > >
> > > Regards,
> > >
> > > Tim
> > >
> > > > >
> > > > > Regards,
> > > > >
> > > > > Hans
> > > > >
> > > > > On 8/27/19 11:55 PM, Matthew Michilot wrote:
> > > > > > From: Matthew Michilot <matthew.michilot@gmail.com>
> > > > > >
> > > > > > Captured video would be out of sync when using the adv7280 with
> > > > > > the BT.656-4 protocol. Certain registers (0x04, 0x31, 0xE6) had to
> > > > > > be configured properly to ensure BT.656-4 compatibility.
> > > > > >
> > > > > > An error in the adv7280 reference manual suggested that EAV/SAV mode
> > > > > > was enabled by default, however upon inspecting register 0x31, it was
> > > > > > determined to be disabled by default.
> >
> > The manual I have [1] states that NEWAVMODE is switched off by default.
> > I'm only asking as I would like to know if there is an error in that
> > datasheet or not.
> >
> > 1. https://www.analog.com/media/en/technical-documentation/user-guides/ADV7280_7281_7282_7283_UG-637.pdf
> >
>
> Table 99 in that document shows NEVAVMODE disabled on power-up
> (0x31=0x02) yet Page 77 shows it enabled at power-up. Looking at an
> actual device we find it is indeed disabled on powerup (0x31=0x02) so
> Table 99 is correct, and Page 77 is not.
>
> If you look at the ADV7180 datasheet
> (https://www.analog.com/media/en/technical-documentation/data-sheets/ADV7180.pdf)
> Table 105 shows NEWAVMODE enabled by default which is also reflected
> in the register details on Page 91 and is what you find on an actual
> device.
>
> Regards,
>
> Tim
>
> --
>
>
> CONFIDENTIALITY NOTICE: This email constitutes an electronic
> communication within the meaning of the Electronic Communications Privacy
> Act, 18 U.S.C. 2510, and its disclosure is strictly limited to the named
> recipient(s) intended by the sender of this message. This email, and any
> attachments, may contain confidential and/or proprietary information. If
> you are not a named recipient, any copying, using, disclosing or
> distributing to others the information in this email and attachments is
> STRICTLY PROHIBITED. If you have received this email in error, please
> notify the sender immediately and permanently delete the email, any
> attachments, and all copies thereof from any drives or storage media and
> destroy any printouts or hard copies of the email and attachments.
>
>  
>
>
> EXPORT COMPLIANCE NOTICE: This email and any attachments may contain
> technical data subject to U.S export restrictions under the International
> Traffic in Arms Regulations (ITAR) or the Export Administration Regulations
> (EAR). Export or transfer of this technical data and/or related information
> to any foreign person(s) or entity(ies), either within the U.S. or outside
> of the U.S., may require advance export authorization by the appropriate
> U.S. Government agency prior to export or transfer. In addition, technical
> data may not be exported or transferred to certain countries or specified
> designated nationals identified by U.S. embargo controls without prior
> export authorization. By accepting this email and any attachments, all
> recipients confirm that they understand and will comply with all applicable
> ITAR, EAR and embargo compliance requirements.

--
Regards,
Niklas Söderlund

\
 
 \ /
  Last update: 2019-09-27 22:44    [W:0.049 / U:36.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site