Re: [libcamera-devel] [PATCH 19/25] media: ov5647: Implement set_fmt pad operation

[Dafna Hirschfeld <dafna.hirschfeld@collabora.com>]

On 23.06.20 18:49, Jacopo Mondi wrote:
> Now that the driver supports more than a single mode, implement the
> .set_fmt pad operation and adjust the existing .get_fmt one to report
> the currently applied format.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>   drivers/media/i2c/ov5647.c | 67 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 62 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index af9e6d43967d8..39e320f321bd8 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -1016,15 +1016,72 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,
>   	return 0;
>   }
>   
> -static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
> +static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,
>   			      struct v4l2_subdev_pad_config *cfg,
>   			      struct v4l2_subdev_format *format)
>   {
>   	struct v4l2_mbus_framefmt *fmt = &format->format;
> +	struct v4l2_mbus_framefmt *sensor_format;
> +	struct ov5647 *sensor = to_sensor(sd);
>   
> -	/* Only one format is supported, so return that. */
> +	mutex_lock(&sensor->lock);
>   	memset(fmt, 0, sizeof(*fmt));
> -	*fmt = OV5647_DEFAULT_FORMAT;
> +
> +	switch (format->which) {
> +	case V4L2_SUBDEV_FORMAT_TRY:
> +		sensor_format = v4l2_subdev_get_try_format(sd, cfg, format->pad);
> +		break;
> +	default:
> +		sensor_format = &sensor->mode->format;
> +		break;
> +	}
> +
> +	*fmt = *sensor_format;
> +	mutex_unlock(&sensor->lock);
> +
> +	return 0;
> +}
> +
> +static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
> +			      struct v4l2_subdev_pad_config *cfg,
> +			      struct v4l2_subdev_format *format)
> +{
> +	struct v4l2_mbus_framefmt *fmt = &format->format;
> +	struct ov5647 *sensor = to_sensor(sd);
> +	struct ov5647_mode *ov5647_mode_list;
> +	struct ov5647_mode *mode;
> +	unsigned int num_modes;
> +
> +	/*
> +	 * Default mbus code MEDIA_BUS_FMT_SBGGR10_1X10 if the requested one
> +	 * is not supported.

In previous patch you added macros OV5647_DEFAULT_MODE, OV5647_DEFAULT_FORMAT
which comes from first format in the array 'ov5647_formats' which is MEDIA_BUS_FMT_SBGGR8_1X8.
But here you set the default format to MEDIA_BUS_FMT_SBGGR10_1X10

> +	 */
> +	if (fmt->code == MEDIA_BUS_FMT_SBGGR8_1X8) {
> +		ov5647_mode_list = ov5647_sbggr8_modes;
> +		num_modes = ARRAY_SIZE(ov5647_sbggr8_modes);
> +	} else {
> +		ov5647_mode_list = ov5647_sbggr10_modes;
> +		num_modes = ARRAY_SIZE(ov5647_sbggr10_modes);
> +	}
> +
> +	mode = v4l2_find_nearest_size(ov5647_mode_list, num_modes,
> +				      format.width, format.height,
> +				      fmt->width, fmt->height);
> +
> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> +		mutex_lock(&sensor->lock);
> +		*v4l2_subdev_get_try_format(sd, cfg, format->pad) = mode->format;
> +		*fmt = mode->format;
> +		mutex_unlock(&sensor->lock);
> +
> +		return 0;
> +	}
> +
> +	/* Update the sensor mode and apply at it at streamon time. */
> +	mutex_lock(&sensor->lock);
> +	sensor->mode = mode;
> +	*fmt = mode->format;
> +	mutex_unlock(&sensor->lock);
>   
>   	return 0;
>   }
> @@ -1068,8 +1125,8 @@ static int ov5647_get_selection(struct v4l2_subdev *sd,
>   static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
>   	.enum_mbus_code		= ov5647_enum_mbus_code,
>   	.enum_frame_size	= ov5647_enum_frame_size,
> -	.set_fmt		= ov5647_set_get_fmt,
> -	.get_fmt		= ov5647_set_get_fmt,
> +	.set_fmt		= ov5647_set_pad_fmt,
> +	.get_fmt		= ov5647_get_pad_fmt,
>   	.get_selection		= ov5647_get_selection,
>   };
>   
>