diff mbox series

[v2,38/77] media: imx: imx7-media-csi: Fix source type identification

Message ID 20210215042741.28850-39-laurent.pinchart@ideasonboard.com (mailing list archive)
State New
Headers show
Series media: imx: Miscellaneous fixes and cleanups for i.MX7 | expand

Commit Message

Laurent Pinchart Feb. 15, 2021, 4:27 a.m. UTC
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@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-media-csi.c | 52 +++++++++++++---------
 1 file changed, 30 insertions(+), 22 deletions(-)

Comments

Rui Miguel Silva March 1, 2021, 9:30 a.m. UTC | #1
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@ideasonboard.com>

Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
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
>
diff mbox series

Patch

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. */