diff mbox series

[4/4] rcar-vin: Clean up how format is set on subdevice

Message ID 20190705045557.25463-5-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New, archived
Headers show
Series rcar-vin: Add support for V4L2_FIELD_ALTERNATE | expand

Commit Message

Niklas Söderlund July 5, 2019, 4:55 a.m. UTC 
With support for V4L2_FIELD_ALTERNATE added it's possible to clean up
how formats are set on the subdevice. This makes the code easier to read
as variable names now more clearly express their intent.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++-----------
 1 file changed, 21 insertions(+), 23 deletions(-)

Comments

Kieran Bingham July 16, 2019, 2:56 p.m. UTC | #1 
Hi Niklas,

On 05/07/2019 05:55, Niklas Söderlund wrote:
> With support for V4L2_FIELD_ALTERNATE added it's possible to clean up
> how formats are set on the subdevice. This makes the code easier to read
> as variable names now more clearly express their intent.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++-----------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index bc96ed563e365448..fa6cc1b76f02133e 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -170,7 +170,7 @@ static int rvin_reset_format(struct rvin_dev *vin)
>  
>  static int rvin_try_format(struct rvin_dev *vin, u32 which,
>  			   struct v4l2_pix_format *pix,
> -			   struct v4l2_rect *crop, struct v4l2_rect *compose)
> +			   struct v4l2_rect *src_rect)
>  {
>  	struct v4l2_subdev *sd = vin_to_source(vin);
>  	struct v4l2_subdev_pad_config *pad_cfg;
> @@ -189,24 +189,24 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which,
>  	if (!rvin_format_from_pixel(vin, pix->pixelformat))
>  		pix->pixelformat = RVIN_DEFAULT_FORMAT;
>  
> -	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
> -
>  	/* Allow the video device to override field and to scale */
>  	field = pix->field;
>  	width = pix->width;
>  	height = pix->height;
>  
> +	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
> +
>  	ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
>  	if (ret < 0 && ret != -ENOIOCTLCMD)
>  		goto done;
>  
>  	v4l2_fill_pix_format(pix, &format.format);
>  
> -	if (crop) {
> -		crop->top = 0;
> -		crop->left = 0;
> -		crop->width = pix->width;
> -		crop->height = pix->height;
> +	if (src_rect) {
> +		src_rect->top = 0;
> +		src_rect->left = 0;
> +		src_rect->width = pix->width;
> +		src_rect->height = pix->height;
>  	}
>  
>  	if (field != V4L2_FIELD_ANY)
> @@ -216,17 +216,10 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which,
>  	pix->height = height;
>  
>  	rvin_format_align(vin, pix);
> -
> -	if (compose) {
> -		compose->top = 0;
> -		compose->left = 0;
> -		compose->width = pix->width;
> -		compose->height = pix->height;
> -	}
>  done:
>  	v4l2_subdev_free_pad_config(pad_cfg);
>  
> -	return 0;
> +	return ret;

This return value change looks suspiciously like a small bug-fix.
Does it deserve it's own patch and fixes tag for the stable trees?



>  }
>  
>  static int rvin_querycap(struct file *file, void *priv,
> @@ -246,29 +239,34 @@ static int rvin_try_fmt_vid_cap(struct file *file, void *priv,
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
>  
> -	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL,
> -			       NULL);
> +	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL);
>  }
>  
>  static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
>  			      struct v4l2_format *f)
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_rect crop, compose;
> +	struct v4l2_rect fmt_rect, src_rect;
>  	int ret;
>  
>  	if (vb2_is_busy(&vin->queue))
>  		return -EBUSY;
>  
>  	ret = rvin_try_format(vin, V4L2_SUBDEV_FORMAT_ACTIVE, &f->fmt.pix,
> -			      &crop, &compose);
> +			      &src_rect);
>  	if (ret)
>  		return ret;
>  
>  	vin->format = f->fmt.pix;
> -	v4l2_rect_map_inside(&vin->crop, &crop);
> -	v4l2_rect_map_inside(&vin->compose, &compose);
> -	vin->src_rect = crop;
> +
> +	fmt_rect.top = 0;
> +	fmt_rect.left = 0;
> +	fmt_rect.width = vin->format.width;
> +	fmt_rect.height = vin->format.height;
> +
> +	v4l2_rect_map_inside(&vin->crop, &src_rect);
> +	v4l2_rect_map_inside(&vin->compose, &fmt_rect);
> +	vin->src_rect = src_rect;
>  
>  	return 0;
>  }
>
diff mbox series

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index bc96ed563e365448..fa6cc1b76f02133e 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -170,7 +170,7 @@  static int rvin_reset_format(struct rvin_dev *vin)
 
 static int rvin_try_format(struct rvin_dev *vin, u32 which,
 			   struct v4l2_pix_format *pix,
-			   struct v4l2_rect *crop, struct v4l2_rect *compose)
+			   struct v4l2_rect *src_rect)
 {
 	struct v4l2_subdev *sd = vin_to_source(vin);
 	struct v4l2_subdev_pad_config *pad_cfg;
@@ -189,24 +189,24 @@  static int rvin_try_format(struct rvin_dev *vin, u32 which,
 	if (!rvin_format_from_pixel(vin, pix->pixelformat))
 		pix->pixelformat = RVIN_DEFAULT_FORMAT;
 
-	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
-
 	/* Allow the video device to override field and to scale */
 	field = pix->field;
 	width = pix->width;
 	height = pix->height;
 
+	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
+
 	ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
 	if (ret < 0 && ret != -ENOIOCTLCMD)
 		goto done;
 
 	v4l2_fill_pix_format(pix, &format.format);
 
-	if (crop) {
-		crop->top = 0;
-		crop->left = 0;
-		crop->width = pix->width;
-		crop->height = pix->height;
+	if (src_rect) {
+		src_rect->top = 0;
+		src_rect->left = 0;
+		src_rect->width = pix->width;
+		src_rect->height = pix->height;
 	}
 
 	if (field != V4L2_FIELD_ANY)
@@ -216,17 +216,10 @@  static int rvin_try_format(struct rvin_dev *vin, u32 which,
 	pix->height = height;
 
 	rvin_format_align(vin, pix);
-
-	if (compose) {
-		compose->top = 0;
-		compose->left = 0;
-		compose->width = pix->width;
-		compose->height = pix->height;
-	}
 done:
 	v4l2_subdev_free_pad_config(pad_cfg);
 
-	return 0;
+	return ret;
 }
 
 static int rvin_querycap(struct file *file, void *priv,
@@ -246,29 +239,34 @@  static int rvin_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct rvin_dev *vin = video_drvdata(file);
 
-	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL,
-			       NULL);
+	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL);
 }
 
 static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
 			      struct v4l2_format *f)
 {
 	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_rect crop, compose;
+	struct v4l2_rect fmt_rect, src_rect;
 	int ret;
 
 	if (vb2_is_busy(&vin->queue))
 		return -EBUSY;
 
 	ret = rvin_try_format(vin, V4L2_SUBDEV_FORMAT_ACTIVE, &f->fmt.pix,
-			      &crop, &compose);
+			      &src_rect);
 	if (ret)
 		return ret;
 
 	vin->format = f->fmt.pix;
-	v4l2_rect_map_inside(&vin->crop, &crop);
-	v4l2_rect_map_inside(&vin->compose, &compose);
-	vin->src_rect = crop;
+
+	fmt_rect.top = 0;
+	fmt_rect.left = 0;
+	fmt_rect.width = vin->format.width;
+	fmt_rect.height = vin->format.height;
+
+	v4l2_rect_map_inside(&vin->crop, &src_rect);
+	v4l2_rect_map_inside(&vin->compose, &fmt_rect);
+	vin->src_rect = src_rect;
 
 	return 0;
 }