Re: [PATCH 2/4] media: i2c: max9286: Get format from remote ends

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

Hi Jacopo,

Thank you for the patch.

On Mon, Aug 17, 2020 at 04:35:38PM +0200, Jacopo Mondi wrote:
> The MAX9286 chip does not allow any modification to the image stream
> format it de-serializes from the GMSL bus to its MIPI CSI-2 output
> interface.
> 
> For this reason, when the format is queried from on any of the MAX9286
> pads, get the remote subdevice format and return it.

That's not how MC-based drivers are supposed to work though. Userspace
has to propagate formats between subdevs, it must not be done internally
in the kernel.

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/max9286.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 7c292f2e2704..e6a70dbd27df 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -742,8 +742,10 @@ static int max9286_get_fmt(struct v4l2_subdev *sd,
>  			   struct v4l2_subdev_format *format)
>  {
>  	struct max9286_priv *priv = sd_to_max9286(sd);
> -	struct v4l2_mbus_framefmt *cfg_fmt;
> +	struct v4l2_subdev_format remote_fmt = {};
> +	struct device *dev = &priv->client->dev;
>  	unsigned int pad = format->pad;
> +	int ret;
>  
>  	/*
>  	 * Multiplexed Stream Support: Support link validation by returning the
> @@ -754,12 +756,26 @@ static int max9286_get_fmt(struct v4l2_subdev *sd,
>  	if (pad == MAX9286_SRC_PAD)
>  		pad = __ffs(priv->bound_sources);
>  
> -	cfg_fmt = max9286_get_pad_format(priv, cfg, pad, format->which);
> -	if (!cfg_fmt)
> -		return -EINVAL;
> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> +		mutex_lock(&priv->mutex);
> +		format->format = *v4l2_subdev_get_try_format(&priv->sd,
> +							     cfg, pad);
> +		mutex_unlock(&priv->mutex);
> +
> +		return 0;
> +	}
> +
> +	remote_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	remote_fmt.pad = 0;
> +	ret = v4l2_subdev_call(priv->sources[pad].sd, pad, get_fmt, NULL,
> +			       &remote_fmt);
> +	if (ret) {
> +		dev_err(dev, "Unable get format on source %d\n", pad);
> +		return ret;
> +	}
>  
>  	mutex_lock(&priv->mutex);
> -	format->format = *cfg_fmt;
> +	format->format = remote_fmt.format;
>  	mutex_unlock(&priv->mutex);
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart