Hi Steve,
On Thu, Oct 24, 2019 at 08:11:20AM -0700, Steve Longerbeam wrote:
> On 10/23/19 5:41 PM, Laurent Pinchart wrote:
> > Commit 4791bd7d6adc ("media: imx: Try colorimetry at both sink and
> > source pads") reworked the way that formats are set on the sink pad of
> > the CSI subdevice, and accidentally removed video field handling.
>
> Well, let me restate the problem. This driver never did have correct
> field handling (as you demonstrate in this patch, the driver never
> set/propagated the field type to its source pad). So it's not accurate
> to say the patch is a fix. A better description is that it adds
> rudimentary field handling.
Didn't it ? The above commit removed a call to
imx_media_fill_default_mbus_fields() from imx7_csi_try_fmt(), and that
function had rudimentary field handling. The Fixes: tag isn't there to
blame you :-)
> > Restore it by defaulting to V4L2_FIELD_NONE if the field value isn't
> > supported, with the only two supported value being V4L2_FIELD_NONE and
> > V4L2_FIELD_ALTERNATE.
> >
> > Fixes: 4791bd7d6adc ("media: imx: Try colorimetry at both sink and source pads")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/staging/media/imx/imx7-media-csi.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> > index ac07f55a63e3..0db6473caf13 100644
> > --- a/drivers/staging/media/imx/imx7-media-csi.c
> > +++ b/drivers/staging/media/imx/imx7-media-csi.c
> > @@ -1017,6 +1017,7 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
> > sdformat->format.width = in_fmt->width;
> > sdformat->format.height = in_fmt->height;
> > sdformat->format.code = in_fmt->code;
> > + sdformat->format.field = in_fmt->field;
> > *cc = in_cc;
> >
> > sdformat->format.colorspace = in_fmt->colorspace;
> > @@ -1031,6 +1032,9 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
> > false);
> > sdformat->format.code = (*cc)->codes[0];
> > }
> > +
> > + if (sdformat->format.field != V4L2_FIELD_INTERLACED)
> > + sdformat->format.field = V4L2_FIELD_NONE;
> > break;
> > default:
> > return -EINVAL;
--
Regards,
Laurent Pinchart
