| Message ID | 20210215042741.28850-39-laurent.pinchart@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/77] media: imx: Drop dependency on I2C | expand |
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 --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. */
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(-)