Hi Laurent,
On 3/9/20 1:52 PM, Laurent Pinchart wrote:
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 :-)
Sorry, you're right, 4791bd7d6adc should have placed some sane field
value back at sink pad in imx7_csi_try_fmt(). But the problem is that at
the time, I didn't know what a sane field value for imx7 would be (your
patch resolved that question, by stating only NONE and INTERLACED are
supported). But I must stand by my statement that field was never
propagated to source. I guess I was waiting for someone to implement
basic field handling, instead of possibly doing the wrong thing myself.
I should have clarified at the time that there needed to be basic field
handling added.
Steve
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;
