Hi Laurent,
Thanks for the patch.
On Mon, Feb 15, 2021 at 06:27:02AM +0200, Laurent Pinchart wrote:
> The code in imx7_csi_pad_link_validate() that checks the type of the
> source incorrectly handles devices that have no parallel input. In that
> case, the source entity of the CSI is the CSI-2 receiver, not the video
> mux, and the driver will proceed to check the type of the source of the
> CSI-2 receiver.
>
> Make the code more explicit to fix this, by handling the three cases
> (parallel input only, CSI-2 receiver only, and video mux) separately.
>
> Note that the driver will not correctly handle the case where only a
> parallel input is present, and the external entity connected to the
> parallel input reports a MEDIA_ENT_F_VID_IF_BRIDGE or
> MEDIA_ENT_F_VID_MUX function. This was broken already, and should be
> fixed separately.
>
> Fixes: f168e796bf40 ("media: imx7: csi: Fix pad link validation")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Reviewed-by: Rui Miguel Silva <rmfrfs@xxxxxxxxx>
------
Cheers,
Rui
> ---
> drivers/staging/media/imx/imx7-media-csi.c | 52 +++++++++++++---------
> 1 file changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index de7b93317a47..2a4b69cc0178 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1000,39 +1000,47 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
> struct imx7_csi *csi = v4l2_get_subdevdata(sd);
> struct imx_media_video_dev *vdev = csi->vdev;
> const struct v4l2_pix_format *out_pix = &vdev->fmt;
> - struct media_entity *src;
> struct media_pad *pad;
> + bool is_csi2;
> int ret;
>
> - /*
> - * Validate the source link, and record whether the CSI mux selects the
> - * parallel input or the CSI-2 receiver.
> - */
> - ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt);
> - if (ret)
> - return ret;
> -
> if (!csi->src_sd)
> return -EPIPE;
>
> - src = &csi->src_sd->entity;
> -
> /*
> - * if the source is neither a CSI MUX or CSI-2 get the one directly
> - * upstream from this CSI
> + * Validate the source link, and record whether the source uses the
> + * parallel input or the CSI-2 receiver.
> */
> - if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE &&
> - src->function != MEDIA_ENT_F_VID_MUX)
> - src = &csi->sd.entity;
> + ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt);
> + if (ret)
> + return ret;
>
> - pad = imx_media_pipeline_pad(src, 0, 0, true);
> - if (!pad)
> - return -ENODEV;
> + switch (csi->src_sd->entity.function) {
> + case MEDIA_ENT_F_VID_IF_BRIDGE:
> + /* The input is the CSI-2 receiver. */
> + is_csi2 = true;
> + break;
> +
> + case MEDIA_ENT_F_VID_MUX:
> + /* The input is the mux, check its input. */
> + pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true);
> + if (!pad)
> + return -ENODEV;
> +
> + is_csi2 = pad->entity->function == MEDIA_ENT_F_VID_IF_BRIDGE;
> + break;
> +
> + default:
> + /*
> + * The input is an external entity, it must use the parallel
> + * bus.
> + */
> + is_csi2 = false;
> + break;
> + }
>
> mutex_lock(&csi->lock);
> -
> - csi->is_csi2 = (pad->entity->function == MEDIA_ENT_F_VID_IF_BRIDGE);
> -
> + csi->is_csi2 = is_csi2;
> mutex_unlock(&csi->lock);
>
> /* Validate the sink link, ensure the pixel format is supported. */
> --
> Regards,
>
> Laurent Pinchart
>
