Hi Laurent,
On Fri, Jul 01, 2022 at 02:06:56AM +0300, Laurent Pinchart wrote:
> The ISP supports multiple source subdevs, but can only capture from a
> single one at a time. The source is selected through link setup. The
> driver finds the active source in its .s_stream() handler using the
> media_entity_remote_pad() function. This fails to reject invalid
> configurations with multiple active sources. Fix it by using the
> media_entity_remote_source_pad() helper instead, and inline
> rkisp1_isp_get_source() in rkisp1_isp_s_stream() as the function is
> small and has a single caller.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Reviewed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> ---
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 30 ++++++++-----------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 37623b73b1d9..d7e2802d11f5 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -58,20 +58,6 @@
> * Helpers
> */
>
> -static struct v4l2_subdev *rkisp1_isp_get_source(struct v4l2_subdev *sd)
> -{
> - struct media_pad *local, *remote;
> - struct media_entity *sensor_me;
> -
> - local = &sd->entity.pads[RKISP1_ISP_PAD_SINK_VIDEO];
> - remote = media_pad_remote_pad_first(local);
> - if (!remote)
> - return NULL;
> -
> - sensor_me = remote->entity;
> - return media_entity_to_v4l2_subdev(sensor_me);
> -}
> -
> static struct v4l2_mbus_framefmt *
> rkisp1_isp_get_pad_fmt(struct rkisp1_isp *isp,
> struct v4l2_subdev_state *sd_state,
> @@ -743,6 +729,8 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> const struct rkisp1_sensor_async *asd;
> + struct media_pad *source_pad;
> + struct media_pad *sink_pad;
> int ret;
>
> if (!enable) {
> @@ -754,10 +742,18 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> return 0;
> }
>
> - rkisp1->source = rkisp1_isp_get_source(sd);
> + sink_pad = &isp->pads[RKISP1_ISP_PAD_SINK_VIDEO];
> + source_pad = media_pad_remote_pad_unique(sink_pad);
> + if (IS_ERR(source_pad)) {
> + dev_dbg(rkisp1->dev, "Failed to get source for ISP: %ld\n",
> + PTR_ERR(source_pad));
> + return -EPIPE;
> + }
> +
> + rkisp1->source = media_entity_to_v4l2_subdev(source_pad->entity);
> if (!rkisp1->source) {
> - dev_warn(rkisp1->dev, "No link between isp and source\n");
> - return -ENODEV;
> + /* This should really not happen, so is not worth a message. */
> + return -EPIPE;
> }
>
> asd = container_of(rkisp1->source->asd, struct rkisp1_sensor_async,
