| Message ID | 20210107104726.1294538-3-rmfrfs@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: im7: csi: fix csi for i.MX6UL/i.MX6ULL | expand |
On 07/01/2021 11:47, Rui Miguel Silva wrote: > We can not make the assumption that the bound subdev is always a CSI > mux, in i.MX6UL/i.MX6ULL that is not the case. So, just get the entity > selected by source directly upstream from the CSI. scripts/checkpatch.pl --strict: WARNING: From:/Signed-off-by: email address mismatch: 'From: Rui Miguel Silva <rui.silva@linaro.org>' != 'Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>' I can fix this if you let me know which email address I should pick. Regards, Hans > > Fixes: 86e02d07871c ("media: imx5/6/7: csi: Mark a bound video mux as a CSI mux") > Reported-by: Fabio Estevam <festevam@gmail.com> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com> > Tested-by: Fabio Estevam <festevam@gmail.com> > Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com> > --- > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c > index 31e36168f9d0..ac52b1daf991 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -499,6 +499,7 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > struct v4l2_subdev_format *sink_fmt) > { > struct imx7_csi *csi = v4l2_get_subdevdata(sd); > + struct media_entity *src; > struct media_pad *pad; > int ret; > > @@ -509,11 +510,21 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > 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 > + */ > + if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE && > + src->function != MEDIA_ENT_F_VID_MUX) > + src = &csi->sd.entity; > + > /* > - * find the entity that is selected by the CSI mux. This is needed > + * find the entity that is selected by the source. This is needed > * to distinguish between a parallel or CSI-2 pipeline. > */ > - pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true); > + pad = imx_media_pipeline_pad(src, 0, 0, true); > if (!pad) > return -ENODEV; > >
Hi Hans, On Wed, Jan 13, 2021 at 01:11:15PM +0100, Hans Verkuil wrote: > On 07/01/2021 11:47, Rui Miguel Silva wrote: > > We can not make the assumption that the bound subdev is always a CSI > > mux, in i.MX6UL/i.MX6ULL that is not the case. So, just get the entity > > selected by source directly upstream from the CSI. > > scripts/checkpatch.pl --strict: > > WARNING: From:/Signed-off-by: email address mismatch: 'From: Rui Miguel Silva <rui.silva@linaro.org>' != 'Signed-off-by: Rui Miguel Silva > <rmfrfs@gmail.com>' > > I can fix this if you let me know which email address I should pick. Errr... Sorry about that, pick the gmail.com one please. Thanks. ------ Cheers, Rui > > Regards, > > Hans > > > > > Fixes: 86e02d07871c ("media: imx5/6/7: csi: Mark a bound video mux as a CSI mux") > > Reported-by: Fabio Estevam <festevam@gmail.com> > > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com> > > Tested-by: Fabio Estevam <festevam@gmail.com> > > Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com> > > --- > > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c > > index 31e36168f9d0..ac52b1daf991 100644 > > --- a/drivers/staging/media/imx/imx7-media-csi.c > > +++ b/drivers/staging/media/imx/imx7-media-csi.c > > @@ -499,6 +499,7 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > > struct v4l2_subdev_format *sink_fmt) > > { > > struct imx7_csi *csi = v4l2_get_subdevdata(sd); > > + struct media_entity *src; > > struct media_pad *pad; > > int ret; > > > > @@ -509,11 +510,21 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > > 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 > > + */ > > + if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE && > > + src->function != MEDIA_ENT_F_VID_MUX) > > + src = &csi->sd.entity; > > + > > /* > > - * find the entity that is selected by the CSI mux. This is needed > > + * find the entity that is selected by the source. This is needed > > * to distinguish between a parallel or CSI-2 pipeline. > > */ > > - pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true); > > + pad = imx_media_pipeline_pad(src, 0, 0, true); > > if (!pad) > > return -ENODEV; > > > > >
Hi Rui, Thank you for the patch. On Thu, Jan 07, 2021 at 10:47:26AM +0000, Rui Miguel Silva wrote: > We can not make the assumption that the bound subdev is always a CSI > mux, in i.MX6UL/i.MX6ULL that is not the case. So, just get the entity > selected by source directly upstream from the CSI. > > Fixes: 86e02d07871c ("media: imx5/6/7: csi: Mark a bound video mux as a CSI mux") > Reported-by: Fabio Estevam <festevam@gmail.com> > Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com> > Tested-by: Fabio Estevam <festevam@gmail.com> > Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com> > --- > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c > index 31e36168f9d0..ac52b1daf991 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -499,6 +499,7 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > struct v4l2_subdev_format *sink_fmt) > { > struct imx7_csi *csi = v4l2_get_subdevdata(sd); > + struct media_entity *src; > struct media_pad *pad; > int ret; > > @@ -509,11 +510,21 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, > 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 > + */ > + if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE && > + src->function != MEDIA_ENT_F_VID_MUX) > + src = &csi->sd.entity; > + > /* > - * find the entity that is selected by the CSI mux. This is needed > + * find the entity that is selected by the source. This is needed > * to distinguish between a parallel or CSI-2 pipeline. > */ > - pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true); > + pad = imx_media_pipeline_pad(src, 0, 0, true); > if (!pad) > return -ENODEV; This is a bit over-complicated. If the source entity isn't a mux or bridge, then the input is always parallel. I'll send a follow up patch to simplify this, as this one has been merged already. By the way, it would be entirely possible for an external entity, connected through a parallel interface, to have a function set to MEDIA_ENT_F_VID_IF_BRIDGE or MEDIA_ENT_F_VID_MUX. This code is thus not very robust, and will need to be fixed.
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c index 31e36168f9d0..ac52b1daf991 100644 --- a/drivers/staging/media/imx/imx7-media-csi.c +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -499,6 +499,7 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, struct v4l2_subdev_format *sink_fmt) { struct imx7_csi *csi = v4l2_get_subdevdata(sd); + struct media_entity *src; struct media_pad *pad; int ret; @@ -509,11 +510,21 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd, 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 + */ + if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE && + src->function != MEDIA_ENT_F_VID_MUX) + src = &csi->sd.entity; + /* - * find the entity that is selected by the CSI mux. This is needed + * find the entity that is selected by the source. This is needed * to distinguish between a parallel or CSI-2 pipeline. */ - pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true); + pad = imx_media_pipeline_pad(src, 0, 0, true); if (!pad) return -ENODEV;
We can not make the assumption that the bound subdev is always a CSI mux, in i.MX6UL/i.MX6ULL that is not the case. So, just get the entity selected by source directly upstream from the CSI. Fixes: 86e02d07871c ("media: imx5/6/7: csi: Mark a bound video mux as a CSI mux") Reported-by: Fabio Estevam <festevam@gmail.com> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com> Tested-by: Fabio Estevam <festevam@gmail.com> Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com> --- drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)