diff mbox series

[24/28] media: ti-vpe: cal: add mbus_code support to cal_mc_enum_fmt_vid_cap

Message ID 20210412113457.328012-25-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series media: ti-vpe: cal: prepare for multistream support | expand

Commit Message

Tomi Valkeinen April 12, 2021, 11:34 a.m. UTC
Commit e5b6b07a1b45dd9d19bec1fa1d60750b0fcf2fb0 ("media: v4l2: Extend
VIDIOC_ENUM_FMT to support MC-centric devices") added support to
enumerate formats based in mbus-code.

Add this feature to cal driver.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal-video.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart April 18, 2021, 1:17 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 02:34:53PM +0300, Tomi Valkeinen wrote:
> Commit e5b6b07a1b45dd9d19bec1fa1d60750b0fcf2fb0 ("media: v4l2: Extend


You can abbreviate that to 12 characters if desired.

> VIDIOC_ENUM_FMT to support MC-centric devices") added support to

> enumerate formats based in mbus-code.

> 

> Add this feature to cal driver.

> 

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

> ---

>  drivers/media/platform/ti-vpe/cal-video.c | 21 ++++++++++++++++++---

>  1 file changed, 18 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c

> index ea9b13c16a06..1d9c0fce4b03 100644

> --- a/drivers/media/platform/ti-vpe/cal-video.c

> +++ b/drivers/media/platform/ti-vpe/cal-video.c

> @@ -437,13 +437,28 @@ static const struct v4l2_ioctl_ops cal_ioctl_video_ops = {

>  static int cal_mc_enum_fmt_vid_cap(struct file *file, void  *priv,

>  				   struct v4l2_fmtdesc *f)

>  {

> +	unsigned int i;

> +	unsigned int idx;

> +

>  	if (f->index >= cal_num_formats)

>  		return -EINVAL;

>  

> -	f->pixelformat = cal_formats[f->index].fourcc;

> -	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;


As a shortcut, you could have

	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

	if (!f->mbus_code) {
		f->pixelformat = cal_formats[f->index].fourcc;
		return 0;
	}

> +	idx = 0;

>  

> -	return 0;

> +	for (i = 0; i < cal_num_formats; ++i) {

> +		if (f->mbus_code && cal_formats[i].code != f->mbus_code)


And drop the first condition here, as well as f->type below. Up to you.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


> +			continue;

> +

> +		if (idx == f->index) {

> +			f->pixelformat = cal_formats[i].fourcc;

> +			f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

> +			return 0;

> +		}

> +

> +		idx++;

> +	}

> +

> +	return -EINVAL;

>  }

>  

>  static void cal_mc_try_fmt(struct cal_ctx *ctx, struct v4l2_format *f,


-- 
Regards,

Laurent Pinchart
Tomi Valkeinen April 19, 2021, 12:50 p.m. UTC | #2
On 18/04/2021 16:17, Laurent Pinchart wrote:
> Hi Tomi,

> 

> Thank you for the patch.

> 

> On Mon, Apr 12, 2021 at 02:34:53PM +0300, Tomi Valkeinen wrote:

>> Commit e5b6b07a1b45dd9d19bec1fa1d60750b0fcf2fb0 ("media: v4l2: Extend

> 

> You can abbreviate that to 12 characters if desired.

> 

>> VIDIOC_ENUM_FMT to support MC-centric devices") added support to

>> enumerate formats based in mbus-code.

>>

>> Add this feature to cal driver.

>>

>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

>> ---

>>   drivers/media/platform/ti-vpe/cal-video.c | 21 ++++++++++++++++++---

>>   1 file changed, 18 insertions(+), 3 deletions(-)

>>

>> diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c

>> index ea9b13c16a06..1d9c0fce4b03 100644

>> --- a/drivers/media/platform/ti-vpe/cal-video.c

>> +++ b/drivers/media/platform/ti-vpe/cal-video.c

>> @@ -437,13 +437,28 @@ static const struct v4l2_ioctl_ops cal_ioctl_video_ops = {

>>   static int cal_mc_enum_fmt_vid_cap(struct file *file, void  *priv,

>>   				   struct v4l2_fmtdesc *f)

>>   {

>> +	unsigned int i;

>> +	unsigned int idx;

>> +

>>   	if (f->index >= cal_num_formats)

>>   		return -EINVAL;

>>   

>> -	f->pixelformat = cal_formats[f->index].fourcc;

>> -	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

> 

> As a shortcut, you could have

> 

> 	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

> 

> 	if (!f->mbus_code) {

> 		f->pixelformat = cal_formats[f->index].fourcc;

> 		return 0;

> 	}

> 

>> +	idx = 0;

>>   

>> -	return 0;

>> +	for (i = 0; i < cal_num_formats; ++i) {

>> +		if (f->mbus_code && cal_formats[i].code != f->mbus_code)

> 

> And drop the first condition here, as well as f->type below. Up to you.


True, but in a patch that adds metadata support (not posted) I add also 
metadata formats into the cal_formats array, and I need to iterate and 
skip the possible metadata-formats even for !f->mbus_code case.

I'm not sure if that approach is good or not (I also tried separate 
metadata format array but that just caused mess elsewhere), but for the 
time being I'd rather keep this as it is to easily allow adding the 
metadata formats.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c
index ea9b13c16a06..1d9c0fce4b03 100644
--- a/drivers/media/platform/ti-vpe/cal-video.c
+++ b/drivers/media/platform/ti-vpe/cal-video.c
@@ -437,13 +437,28 @@  static const struct v4l2_ioctl_ops cal_ioctl_video_ops = {
 static int cal_mc_enum_fmt_vid_cap(struct file *file, void  *priv,
 				   struct v4l2_fmtdesc *f)
 {
+	unsigned int i;
+	unsigned int idx;
+
 	if (f->index >= cal_num_formats)
 		return -EINVAL;
 
-	f->pixelformat = cal_formats[f->index].fourcc;
-	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	idx = 0;
 
-	return 0;
+	for (i = 0; i < cal_num_formats; ++i) {
+		if (f->mbus_code && cal_formats[i].code != f->mbus_code)
+			continue;
+
+		if (idx == f->index) {
+			f->pixelformat = cal_formats[i].fourcc;
+			f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+			return 0;
+		}
+
+		idx++;
+	}
+
+	return -EINVAL;
 }
 
 static void cal_mc_try_fmt(struct cal_ctx *ctx, struct v4l2_format *f,