Re: [PATCH v4 08/17] media: imx: Add imx_media_create_fwnode_pad_link()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Steve,

Thank you for the patch.

On Tue, Mar 03, 2020 at 03:42:47PM -0800, Steve Longerbeam wrote:
> Add functions to create media links between a source and sink subdevice
> based on fwnode endpoint connections between them.
> 
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/staging/media/imx/imx-media-utils.c | 91 +++++++++++++++++++++
>  drivers/staging/media/imx/imx-media.h       |  4 +
>  2 files changed, 95 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 0788a1874557..87152bd9af22 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -729,6 +729,97 @@ void imx_media_grp_id_to_sd_name(char *sd_name, int sz, u32 grp_id, int ipu_id)
>  }
>  EXPORT_SYMBOL_GPL(imx_media_grp_id_to_sd_name);
>  
> +/*
> + * Look for and create a single fwnode link that connects a source
> + * subdevice to a sink pad.
> + */
> +int imx_media_create_fwnode_pad_link(struct v4l2_subdev *src_sd,
> +				     struct media_pad *sink)
> +{
> +	struct fwnode_handle *endpoint;
> +
> +	/* loop thru the source's fwnode endpoints */
> +	fwnode_graph_for_each_endpoint(dev_fwnode(src_sd->dev), endpoint) {
> +		struct fwnode_handle *remote_ep;
> +		int src_idx, sink_idx, ret;
> +		struct media_pad *src;
> +
> +		remote_ep = fwnode_graph_get_remote_endpoint(endpoint);
> +		if (!remote_ep)
> +			continue;
> +
> +		/*
> +		 * ask the sink entity to verify that this fwnode link
> +		 * actually does connect with the entity, and if so that
> +		 * it connects to its requested sink pad.
> +		 */
> +		sink_idx = media_entity_get_fwnode_pad(sink->entity,
> +						       remote_ep,
> +						       MEDIA_PAD_FL_SINK);
> +		fwnode_handle_put(remote_ep);
> +
> +		if (sink_idx < 0 || sink_idx != sink->index)
> +			continue;
> +
> +		src_idx = media_entity_get_fwnode_pad(&src_sd->entity,
> +						      endpoint,
> +						      MEDIA_PAD_FL_SOURCE);
> +		if (src_idx < 0)
> +			continue;
> +
> +		/*
> +		 * found the fwnode link that works, create the media
> +		 * link for it.
> +		 */
> +
> +		fwnode_handle_put(endpoint);
> +
> +		src = &src_sd->entity.pads[src_idx];
> +
> +		/* success if it already exists */
> +		if (media_entity_find_link(src, sink))
> +			return 0;
> +
> +		dev_dbg(src_sd->dev, "%s:%d -> %s:%d\n",
> +			src_sd->entity.name, src_idx,
> +			sink->entity->name, sink_idx);
> +
> +		ret = media_create_pad_link(&src_sd->entity, src_idx,
> +					    sink->entity, sink_idx, 0);
> +		if (ret)
> +			dev_err(src_sd->dev,
> +				"%s:%d -> %s:%d failed with %d\n",
> +				src_sd->entity.name, src_idx,
> +				sink->entity->name, sink_idx, ret);
> +
> +		return ret;
> +	}
> +
> +	return -ENXIO;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_create_fwnode_pad_link);
> +
> +int imx_media_create_fwnode_pad_links(struct v4l2_subdev *src_sd,
> +				      struct v4l2_subdev *sink_sd)
> +{
> +	int i;
> +
> +	for (i = 0; i < sink_sd->entity.num_pads; i++) {
> +		struct media_pad *pad = &sink_sd->entity.pads[i];
> +		int ret;
> +
> +		if (!(pad->flags & MEDIA_PAD_FL_SINK))
> +			continue;
> +
> +		ret = imx_media_create_fwnode_pad_link(src_sd, pad);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_create_fwnode_pad_links);

As commented in 09/17, I think these should be core helpers.

> +
>  struct v4l2_subdev *
>  imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
>  				struct fwnode_handle *fwnode)
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index 11861191324a..f90a65ba4ced 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -183,6 +183,10 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
>  				    struct ipu_image *image);
>  void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
>  				 u32 grp_id, int ipu_id);
> +int imx_media_create_fwnode_pad_link(struct v4l2_subdev *src_sd,
> +				     struct media_pad *sink);
> +int imx_media_create_fwnode_pad_links(struct v4l2_subdev *src_sd,
> +				      struct v4l2_subdev *sink_sd);
>  struct v4l2_subdev *
>  imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
>  				struct fwnode_handle *fwnode);

-- 
Regards,

Laurent Pinchart