Hi Jacopo,
Thank you for the patch.
On Mon, Feb 14, 2022 at 07:43:15PM +0100, Jacopo Mondi wrote:
> The pixel sampling mode controls the size of data sampled from the CSI
> Rx queue. The supported sample size depends on the configuration of the
> preceding block in the capture pipeline and is then dependent on the SoC
> version the CSI peripheral is integrated on.
>
> When capturing YUV422 data if dual sample mode is available use it.
>
> This change is particularly relevant for the IMX8MM SoC which uses the
> CSIS CSI-2 receiver which operates in dual pixel mode.
>
> Other SoCs should be unaffected by this change and should continue to
> operate as before.
>
> Signed-off-by: Xavier Roumegue <xavier.roumegue@xxxxxxxxxxx>
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> ---
> drivers/staging/media/imx/imx7-media-csi.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 112096774961..a8bdfb0bb0ee 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -426,6 +426,7 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> {
> struct imx_media_video_dev *vdev = csi->vdev;
> struct v4l2_pix_format *out_pix = &vdev->fmt;
> + struct imx_media_dev *imxmd = csi->imxmd;
> int width = out_pix->width;
> u32 stride = 0;
> u32 cr3 = BIT_FRMCNT_RST;
> @@ -436,7 +437,7 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> cr18 &= ~(BIT_CSI_HW_ENABLE | BIT_MIPI_DATA_FORMAT_MASK |
> BIT_DATA_FROM_MIPI | BIT_BASEADDR_CHG_ERR_EN |
> BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> - BIT_DEINTERLACE_EN);
> + BIT_DEINTERLACE_EN | BIT_MIPI_DOUBLE_CMPNT);
>
> if (out_pix->field == V4L2_FIELD_INTERLACED) {
> cr18 |= BIT_DEINTERLACE_EN;
> @@ -500,6 +501,13 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> case MEDIA_BUS_FMT_YUYV8_2X8:
> case MEDIA_BUS_FMT_YUYV8_1X16:
> cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
> +
> + /* If dual mode is supported use it. */
> + if (imxmd->info->sample_modes & MODE_DUAL) {
> + cr18 |= BIT_MIPI_DOUBLE_CMPNT;
> + cr3 |= BIT_TWO_8BIT_SENSOR;
> + }
I would implement this differently:
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_YUYV8_2X8:
cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
break;
case MEDIA_BUS_FMT_UYVY8_1X16:
case MEDIA_BUS_FMT_YUYV8_1X16:
cr3 |= BIT_TWO_8BIT_SENSOR;
cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B
| BIT_MIPI_DOUBLE_CMPNT;
break;
This would support either option here. What you will then need to change
is imx7_csi_enum_mbus_code() and imx7_csi_try_fmt(), to allow/disallow
the 2X8 and 1X16 variants based on the SoC. This is important for the
i.MX7, which has both a CSI-2 input and a parallel input. When using the
CSIS it can (and should) use double component mode, while when using the
parallel input it can work in 8-bit or 16-bit mode depending on how the
sensor is wired.
> +
> break;
> }
> }
--
Regards,
Laurent Pinchart
