diff mbox series

[v3,05/14] media: vimc: cap: refactor singleplanar as a subset of multiplanar

Message ID 20190424135642.31973-6-andrealmeid@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: vimc: Add support for multiplanar formats | expand

Commit Message

André Almeida April 24, 2019, 1:56 p.m. UTC 
Since multiplanar is a superset of single planar formats, instead of
having different implementations for them, treat all formats as
multiplanar. If we need to work with single planar formats, convert them to
multiplanar (with num_planes = 1), re-use the multiplanar code, and
transform them back to single planar. This is implemented with
v4l2_fmt_sp2mp_func().

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
Changes in v3:
- Commit title renamed
- Get rid of `if (IS_MULTIPLANAR(vcap))` checks and functions to do this
check

Changes in v2:
- Make more clear that the generic function _vimc_cap_try_fmt_vid_cap_mp
expects a multiplanar format as input 

 drivers/media/platform/vimc/vimc-capture.c | 51 +++++++++++++---------
 1 file changed, 30 insertions(+), 21 deletions(-)

Comments

Helen Mae Koike Fornazier April 24, 2019, 9:34 p.m. UTC | #1 
On 4/24/19 10:56 AM, André Almeida wrote:
> Since multiplanar is a superset of single planar formats, instead of
> having different implementations for them, treat all formats as
> multiplanar. If we need to work with single planar formats, convert them to
> multiplanar (with num_planes = 1), re-use the multiplanar code, and
> transform them back to single planar. This is implemented with
> v4l2_fmt_sp2mp_func().
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
> Changes in v3:
> - Commit title renamed
> - Get rid of `if (IS_MULTIPLANAR(vcap))` checks and functions to do this
> check
> 
> Changes in v2:
> - Make more clear that the generic function _vimc_cap_try_fmt_vid_cap_mp
> expects a multiplanar format as input 
> 
>  drivers/media/platform/vimc/vimc-capture.c | 51 +++++++++++++---------
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index ae703c52d3f8..fbe51004aba8 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -128,10 +128,10 @@ static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
> -				    struct v4l2_format *f)
> +static int vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
> +				       struct v4l2_format *f)
>  {
> -	struct v4l2_pix_format *format = &f->fmt.pix;
> +	struct v4l2_pix_format_mplane *format = &f->fmt.pix_mp;
>  
>  	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
>  				VIMC_FRAME_MAX_WIDTH) & ~1;
> @@ -149,12 +149,32 @@ static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
>  	if (!v4l2_format_info(format->pixelformat))
>  		format->pixelformat = fmt_default.fmt.pix.pixelformat;
>  
> -	return v4l2_fill_pixfmt(format, format->pixelformat,
> -				format->width, format->height);
> +	return v4l2_fill_pixfmt_mp(format, format->pixelformat,
> +				   format->width, format->height);
>  }
>  
> -static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
> -				  struct v4l2_format *f)
> +static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
> +				     struct v4l2_fmtdesc *f)
> +{
> +	if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt))
> +		return -EINVAL;
> +
> +	f->pixelformat = vimc_cap_supported_pixfmt[f->index];
> +
> +	return 0;
> +}
> +
> +/*
> + * VIDIOC FMT handlers for single-planar
> + */
> +static int vimc_cap_try_fmt_vid_cap_sp(struct file *file, void *priv,
> +				       struct v4l2_format *f)
> +{
> +	return v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp);
> +}
> +
> +static int vimc_cap_s_fmt_vid_cap_sp(struct file *file, void *priv,
> +				     struct v4l2_format *f)
>  {
>  	struct vimc_cap_device *vcap = video_drvdata(file);
>  
> @@ -162,7 +182,7 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
>  	if (vb2_is_busy(&vcap->queue))
>  		return -EBUSY;
>  
> -	vimc_cap_try_fmt_vid_cap(file, priv, f);
> +	v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp);

I know the original code wasn't checking for errors in this func, but if
you could add a check here it would be great.

Helen

>  
>  	dev_dbg(vcap->dev, "%s: format update: "
>  		"old:%dx%d (0x%x, %d, %d, %d, %d) "
> @@ -185,17 +205,6 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
> -				     struct v4l2_fmtdesc *f)
> -{
> -	if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt))
> -		return -EINVAL;
> -
> -	f->pixelformat = vimc_cap_supported_pixfmt[f->index];
> -
> -	return 0;
> -}
> -
>  static bool vimc_cap_is_pixfmt_supported(u32 pixelformat)
>  {
>  	unsigned int i;
> @@ -240,8 +249,8 @@ static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
>  	.vidioc_querycap = vimc_cap_querycap,
>  
>  	.vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap,
> -	.vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap,
> -	.vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap_sp,
> +	.vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap_sp,
>  	.vidioc_enum_fmt_vid_cap = vimc_cap_enum_fmt_vid_cap,
>  	.vidioc_enum_framesizes = vimc_cap_enum_framesizes,
>  
>
diff mbox series

Patch

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index ae703c52d3f8..fbe51004aba8 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -128,10 +128,10 @@  static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
-				    struct v4l2_format *f)
+static int vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
+				       struct v4l2_format *f)
 {
-	struct v4l2_pix_format *format = &f->fmt.pix;
+	struct v4l2_pix_format_mplane *format = &f->fmt.pix_mp;
 
 	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
 				VIMC_FRAME_MAX_WIDTH) & ~1;
@@ -149,12 +149,32 @@  static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
 	if (!v4l2_format_info(format->pixelformat))
 		format->pixelformat = fmt_default.fmt.pix.pixelformat;
 
-	return v4l2_fill_pixfmt(format, format->pixelformat,
-				format->width, format->height);
+	return v4l2_fill_pixfmt_mp(format, format->pixelformat,
+				   format->width, format->height);
 }
 
-static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
-				  struct v4l2_format *f)
+static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
+				     struct v4l2_fmtdesc *f)
+{
+	if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt))
+		return -EINVAL;
+
+	f->pixelformat = vimc_cap_supported_pixfmt[f->index];
+
+	return 0;
+}
+
+/*
+ * VIDIOC FMT handlers for single-planar
+ */
+static int vimc_cap_try_fmt_vid_cap_sp(struct file *file, void *priv,
+				       struct v4l2_format *f)
+{
+	return v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp);
+}
+
+static int vimc_cap_s_fmt_vid_cap_sp(struct file *file, void *priv,
+				     struct v4l2_format *f)
 {
 	struct vimc_cap_device *vcap = video_drvdata(file);
 
@@ -162,7 +182,7 @@  static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
 	if (vb2_is_busy(&vcap->queue))
 		return -EBUSY;
 
-	vimc_cap_try_fmt_vid_cap(file, priv, f);
+	v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp);
 
 	dev_dbg(vcap->dev, "%s: format update: "
 		"old:%dx%d (0x%x, %d, %d, %d, %d) "
@@ -185,17 +205,6 @@  static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
-				     struct v4l2_fmtdesc *f)
-{
-	if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt))
-		return -EINVAL;
-
-	f->pixelformat = vimc_cap_supported_pixfmt[f->index];
-
-	return 0;
-}
-
 static bool vimc_cap_is_pixfmt_supported(u32 pixelformat)
 {
 	unsigned int i;
@@ -240,8 +249,8 @@  static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
 	.vidioc_querycap = vimc_cap_querycap,
 
 	.vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap,
-	.vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap,
-	.vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap_sp,
+	.vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap_sp,
 	.vidioc_enum_fmt_vid_cap = vimc_cap_enum_fmt_vid_cap,
 	.vidioc_enum_framesizes = vimc_cap_enum_framesizes,