Re: [PATCH v2 2/2] media: v4l: xilinx: Add Xilinx UHD-SDI Rx Subsystem driver

From: Hans Verkuil
Date: Tue Jun 02 2020 - 06:44:37 EST

On 01/06/2020 16:59, Vishal Sagar wrote:
> Hi Hans,
> Thanks for reviewing!
>>> + if (!xsdirxss->vidlocked) {
>>> + dev_err(dev, "Can't get values when video not
>> locked!\n");
>>> + return -EINVAL;
>>> + }
>>> + ctrl->val = xsdirxss->ts_is_interlaced;
>> This control makes no sense: the v4l2_dv_timings struct will already tell you
>> if it is an interlaced format or not. Same for v4l2_mbus_framefmt.
> The SDI has a concept of supporting progressive, interlaced (both as we know normally) and a progressive segmented frames(psf).
> The progressive segmented frames have their video content in progressive format but the transport stream is interlaced.
> This is distinguished using the bit 6 and 7 of Byte 2 in the 4 byte ST352 payload.
> Refer to sec 5.3 in SMPTE ST 352:2010.
> This control can be used by the application to distinguish normal interlaced and progressive segmented frames.

Ah, interesting. So this relies on the receiver to reconstruct the progressive
frame by combining the top and bottom field, right?

I think this deserves a new v4l2_field value:


Basically this is identical to V4L2_FIELD_ALTERNATE, except that the two fields
combine to a single progressive frame.



PS: I'll look at your other comments separately