| Message ID | 20210516024216.4576-4-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx7-media-csi: i.MX8MM support | expand |
Hi Laurent, On Sun May 16, 2021 at 3:42 AM WEST, Laurent Pinchart wrote: > The PIXEL_BIT field of the CSICR1 register is documented as setting the > Bayer data width to 10 bits, and is set by the driver for all non-YUV > pixel formats. Test code from NXP showed that the bit shouldn't be set > for Bayer formats, and this was confirmed by experimentation with RAW8 > capture (which doesn't work when setting the field) and RAW10 capture > (for which setting the field doesn't seem to make a difference) on > i.MX8MM with an OV5640 sensor connected over CSI-2. Don't set it. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/staging/media/imx/imx7-media-csi.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c > index 256b9aa978f0..94ee8d9838ee 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -495,21 +495,6 @@ static void imx7_csi_configure(struct imx7_csi *csi) > cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B; > break; > } > - > - switch (out_pix->pixelformat) { > - case V4L2_PIX_FMT_Y10: > - case V4L2_PIX_FMT_Y12: > - case V4L2_PIX_FMT_SBGGR8: > - case V4L2_PIX_FMT_SGBRG8: > - case V4L2_PIX_FMT_SGRBG8: > - case V4L2_PIX_FMT_SRGGB8: > - case V4L2_PIX_FMT_SBGGR16: > - case V4L2_PIX_FMT_SGBRG16: > - case V4L2_PIX_FMT_SGRBG16: > - case V4L2_PIX_FMT_SRGGB16: > - cr1 |= BIT_PIXEL_BIT; > - break; > - } LGTM Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com> ------ Cheers, Rui > } > > imx7_csi_reg_write(csi, cr1, CSI_CSICR1); > -- > Regards, > > Laurent Pinchart
Am Sonntag, dem 16.05.2021 um 05:42 +0300 schrieb Laurent Pinchart: > The PIXEL_BIT field of the CSICR1 register is documented as setting > the > Bayer data width to 10 bits, and is set by the driver for all non-YUV > pixel formats. Test code from NXP showed that the bit shouldn't be > set > for Bayer formats, and this was confirmed by experimentation with > RAW8 > capture (which doesn't work when setting the field) and RAW10 capture > (for which setting the field doesn't seem to make a difference) on > i.MX8MM with an OV5640 sensor connected over CSI-2. Don't set it. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/staging/media/imx/imx7-media-csi.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c > b/drivers/staging/media/imx/imx7-media-csi.c > index 256b9aa978f0..94ee8d9838ee 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -495,21 +495,6 @@ static void imx7_csi_configure(struct imx7_csi > *csi) > cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B; > break; > } > - > - switch (out_pix->pixelformat) { > - case V4L2_PIX_FMT_Y10: > - case V4L2_PIX_FMT_Y12: > - case V4L2_PIX_FMT_SBGGR8: > - case V4L2_PIX_FMT_SGBRG8: > - case V4L2_PIX_FMT_SGRBG8: > - case V4L2_PIX_FMT_SRGGB8: > - case V4L2_PIX_FMT_SBGGR16: > - case V4L2_PIX_FMT_SGBRG16: > - case V4L2_PIX_FMT_SGRBG16: > - case V4L2_PIX_FMT_SRGGB16: > - cr1 |= BIT_PIXEL_BIT; > - break; > - } > } > > imx7_csi_reg_write(csi, cr1, CSI_CSICR1); I can confirm I never needed to set that bit despite said documentation for 10bit bayer (on imx8mq). Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> thanks, martin
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c index 256b9aa978f0..94ee8d9838ee 100644 --- a/drivers/staging/media/imx/imx7-media-csi.c +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -495,21 +495,6 @@ static void imx7_csi_configure(struct imx7_csi *csi) cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B; break; } - - switch (out_pix->pixelformat) { - case V4L2_PIX_FMT_Y10: - case V4L2_PIX_FMT_Y12: - case V4L2_PIX_FMT_SBGGR8: - case V4L2_PIX_FMT_SGBRG8: - case V4L2_PIX_FMT_SGRBG8: - case V4L2_PIX_FMT_SRGGB8: - case V4L2_PIX_FMT_SBGGR16: - case V4L2_PIX_FMT_SGBRG16: - case V4L2_PIX_FMT_SGRBG16: - case V4L2_PIX_FMT_SRGGB16: - cr1 |= BIT_PIXEL_BIT; - break; - } } imx7_csi_reg_write(csi, cr1, CSI_CSICR1);
The PIXEL_BIT field of the CSICR1 register is documented as setting the Bayer data width to 10 bits, and is set by the driver for all non-YUV pixel formats. Test code from NXP showed that the bit shouldn't be set for Bayer formats, and this was confirmed by experimentation with RAW8 capture (which doesn't work when setting the field) and RAW10 capture (for which setting the field doesn't seem to make a difference) on i.MX8MM with an OV5640 sensor connected over CSI-2. Don't set it. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/staging/media/imx/imx7-media-csi.c | 15 --------------- 1 file changed, 15 deletions(-)