diff mbox series

[v2,08/15] media: vimc: cap: Add multiplanar default format

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

Commit Message

André Almeida March 27, 2019, 3:17 p.m. UTC 
vimc already have a default single planar default format.
Add a multiplanar default pixel format to perform those
same actions. Change where the default pixelformat is set to make sure
that the vcap is with the right capabilities.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
Change in v2:
- Move here the default format is set to verify if the device have a
multiplanar capability before assign the default format

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

Comments

Helen Mae Koike Fornazier April 12, 2019, 10:49 p.m. UTC | #1 
On 3/27/19 12:17 PM, André Almeida wrote:
> vimc already have a default single planar default format.

The word "default" is repeated twice.

> Add a multiplanar default pixel format to perform those
> same actions. Change where the default pixelformat is set to make sure
> that the vcap is with the right capabilities.
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
> Change in v2:
> - Move here the default format is set to verify if the device have a
> multiplanar capability before assign the default format
> 
>  drivers/media/platform/vimc/vimc-capture.c | 38 +++++++++++++++++-----
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index a65611be63bb..c344d04ed8ea 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -104,6 +104,15 @@ static const struct v4l2_format fmt_default = {
>  	.fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT,
>  };
>  
> +static const struct v4l2_format fmt_default_mp = {

I would just add a "_sp" sufix to the sp version to make it consistent
with previous patches.

> +	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +	.fmt.pix_mp.width = 640,
> +	.fmt.pix_mp.height = 480,
> +	.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YVU420M,
> +	.fmt.pix_mp.field = V4L2_FIELD_NONE,
> +	.fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT,
> +};
> +
>  struct vimc_cap_buffer {
>  	/*
>  	 * struct vb2_v4l2_buffer must be the first element
> @@ -153,6 +162,7 @@ static int _vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
>  				    struct v4l2_format *f)
>  {
>  	struct v4l2_pix_format_mplane *format = &f->fmt.pix_mp;
> +	struct vimc_cap_device *vcap = video_drvdata(file);
>  
>  	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
>  				VIMC_FRAME_MAX_WIDTH) & ~1;
> @@ -168,7 +178,9 @@ static int _vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
>  
>  	/* Don't accept a pixelformat that is not on the table */
>  	if (!v4l2_format_info(format->pixelformat))
> -		format->pixelformat = fmt_default.fmt.pix.pixelformat;
> +		format->pixelformat = IS_MULTIPLANAR(vcap) ?
> +				fmt_default_mp.fmt.pix_mp.pixelformat :
> +				fmt_default.fmt.pix.pixelformat;
>  
>  	return v4l2_fill_pixfmt_mp(format, format->pixelformat,
>  				   format->width, format->height);
> @@ -651,13 +663,6 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
>  	INIT_LIST_HEAD(&vcap->buf_list);
>  	spin_lock_init(&vcap->qlock);
>  
> -	/* Set default frame format */
> -	vcap->format = fmt_default;
> -	v4l2_fill_pixfmt(&vcap->format.fmt.pix,
> -			 vcap->format.fmt.pix.pixelformat,
> -			 vcap->format.fmt.pix.width,
> -			 vcap->format.fmt.pix.height);
> -
>  	/* Fill the vimc_ent_device struct */
>  	vcap->ved.ent = &vcap->vdev.entity;
>  	vcap->ved.process_frame = vimc_cap_process_frame;
> @@ -679,6 +684,23 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
>  	strscpy(vdev->name, pdata->entity_name, sizeof(vdev->name));
>  	video_set_drvdata(vdev, &vcap->ved);
>  
> +	dev_dbg(vcap->dev, "vimc-capture is in %s mode", IS_MULTIPLANAR(vcap) ?
> +		"multiplanar" : "singleplanar");

I think if you put the check in the second line it would be better:

IS_MULTIPLANAR(vcap) ? "multiplanar" : "singleplanar"

(this is just a suggestion, feel free to take it or not).

Helen

> +
> +	if (IS_MULTIPLANAR(vcap)) {
> +		vcap->format = fmt_default_mp;
> +		v4l2_fill_pixfmt_mp(&vcap->format.fmt.pix_mp,
> +				vcap->format.fmt.pix_mp.pixelformat,
> +				vcap->format.fmt.pix_mp.width,
> +				vcap->format.fmt.pix_mp.height);
> +	} else {
> +		vcap->format = fmt_default;
> +		v4l2_fill_pixfmt(&vcap->format.fmt.pix,
> +				vcap->format.fmt.pix.pixelformat,
> +				vcap->format.fmt.pix.width,
> +				vcap->format.fmt.pix.height);
> +	}
> +
>  	/* Register the video_device with the v4l2 and the media framework */
>  	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
>  	if (ret) {
>
diff mbox series

Patch

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index a65611be63bb..c344d04ed8ea 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -104,6 +104,15 @@  static const struct v4l2_format fmt_default = {
 	.fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT,
 };
 
+static const struct v4l2_format fmt_default_mp = {
+	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+	.fmt.pix_mp.width = 640,
+	.fmt.pix_mp.height = 480,
+	.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YVU420M,
+	.fmt.pix_mp.field = V4L2_FIELD_NONE,
+	.fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT,
+};
+
 struct vimc_cap_buffer {
 	/*
 	 * struct vb2_v4l2_buffer must be the first element
@@ -153,6 +162,7 @@  static int _vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
 				    struct v4l2_format *f)
 {
 	struct v4l2_pix_format_mplane *format = &f->fmt.pix_mp;
+	struct vimc_cap_device *vcap = video_drvdata(file);
 
 	format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
 				VIMC_FRAME_MAX_WIDTH) & ~1;
@@ -168,7 +178,9 @@  static int _vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
 
 	/* Don't accept a pixelformat that is not on the table */
 	if (!v4l2_format_info(format->pixelformat))
-		format->pixelformat = fmt_default.fmt.pix.pixelformat;
+		format->pixelformat = IS_MULTIPLANAR(vcap) ?
+				fmt_default_mp.fmt.pix_mp.pixelformat :
+				fmt_default.fmt.pix.pixelformat;
 
 	return v4l2_fill_pixfmt_mp(format, format->pixelformat,
 				   format->width, format->height);
@@ -651,13 +663,6 @@  static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 	INIT_LIST_HEAD(&vcap->buf_list);
 	spin_lock_init(&vcap->qlock);
 
-	/* Set default frame format */
-	vcap->format = fmt_default;
-	v4l2_fill_pixfmt(&vcap->format.fmt.pix,
-			 vcap->format.fmt.pix.pixelformat,
-			 vcap->format.fmt.pix.width,
-			 vcap->format.fmt.pix.height);
-
 	/* Fill the vimc_ent_device struct */
 	vcap->ved.ent = &vcap->vdev.entity;
 	vcap->ved.process_frame = vimc_cap_process_frame;
@@ -679,6 +684,23 @@  static int vimc_cap_comp_bind(struct device *comp, struct device *master,
 	strscpy(vdev->name, pdata->entity_name, sizeof(vdev->name));
 	video_set_drvdata(vdev, &vcap->ved);
 
+	dev_dbg(vcap->dev, "vimc-capture is in %s mode", IS_MULTIPLANAR(vcap) ?
+		"multiplanar" : "singleplanar");
+
+	if (IS_MULTIPLANAR(vcap)) {
+		vcap->format = fmt_default_mp;
+		v4l2_fill_pixfmt_mp(&vcap->format.fmt.pix_mp,
+				vcap->format.fmt.pix_mp.pixelformat,
+				vcap->format.fmt.pix_mp.width,
+				vcap->format.fmt.pix_mp.height);
+	} else {
+		vcap->format = fmt_default;
+		v4l2_fill_pixfmt(&vcap->format.fmt.pix,
+				vcap->format.fmt.pix.pixelformat,
+				vcap->format.fmt.pix.width,
+				vcap->format.fmt.pix.height);
+	}
+
 	/* Register the video_device with the v4l2 and the media framework */
 	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
 	if (ret) {