Re: [PATCH v7 06/11] media: cedrus: Cleanup control initialization

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: linux-media@vger.kernel.org, kernel@collabora.com,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Tomasz Figa <tfiga@chromium.org>,
	linux-rockchip@lists.infradead.org,
	Heiko Stuebner <heiko@sntech.de>, Jonas Karlman <jonas@kwiboo.se>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Alexandre Courbot <acourbot@chromium.org>,
	fbuergisser@chromium.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 06/11] media: cedrus: Cleanup control initialization
Date: Mon, 19 Aug 2019 14:31:31 +0200	[thread overview]
Message-ID: <20190819123131.GA32182@aptenodytes> (raw)
In-Reply-To: <20190816160132.7352-7-ezequiel@collabora.com>

[-- Attachment #1: Type: text/plain, Size: 4443 bytes --]

Hi,

On Fri 16 Aug 19, 13:01, Ezequiel Garcia wrote:
> In order to introduce other controls, the control initialization
> needs to support an initial struct v4l2_ctrl_control.
> 
> While here, let's cleanup the control initialization,
> removing unneeded fields.

Thanks for the change!

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes in v7:
> * None.
> Changes in v6:
> * None.
> Changes in v5:
> * None.
> Changes in v4:
> * New patch.
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c | 45 +++++++++++----------
>  drivers/staging/media/sunxi/cedrus/cedrus.h |  3 +-
>  2 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 370937edfc14..7bdc413bf727 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -29,44 +29,51 @@
>  
>  static const struct cedrus_control cedrus_controls[] = {
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS,
> -		.elem_size	= sizeof(struct v4l2_ctrl_mpeg2_slice_params),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS,
> +		},
>  		.codec		= CEDRUS_CODEC_MPEG2,
>  		.required	= true,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION,
> -		.elem_size	= sizeof(struct v4l2_ctrl_mpeg2_quantization),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION,
> +		},
>  		.codec		= CEDRUS_CODEC_MPEG2,
>  		.required	= false,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
> -		.elem_size	= sizeof(struct v4l2_ctrl_h264_decode_params),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
> +		},
>  		.codec		= CEDRUS_CODEC_H264,
>  		.required	= true,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
> -		.elem_size	= sizeof(struct v4l2_ctrl_h264_slice_params),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
> +		},
>  		.codec		= CEDRUS_CODEC_H264,
>  		.required	= true,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_H264_SPS,
> -		.elem_size	= sizeof(struct v4l2_ctrl_h264_sps),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_H264_SPS,
> +		},
>  		.codec		= CEDRUS_CODEC_H264,
>  		.required	= true,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_H264_PPS,
> -		.elem_size	= sizeof(struct v4l2_ctrl_h264_pps),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_H264_PPS,
> +		},
>  		.codec		= CEDRUS_CODEC_H264,
>  		.required	= true,
>  	},
>  	{
> -		.id		= V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
> -		.elem_size	= sizeof(struct v4l2_ctrl_h264_scaling_matrix),
> +		.cfg = {
> +			.id	= V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
> +		},
>  		.codec		= CEDRUS_CODEC_H264,
>  		.required	= true,
>  	},
> @@ -106,12 +113,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
>  		return -ENOMEM;
>  
>  	for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
> -		struct v4l2_ctrl_config cfg = {};
> -
> -		cfg.elem_size = cedrus_controls[i].elem_size;
> -		cfg.id = cedrus_controls[i].id;
> -
> -		ctrl = v4l2_ctrl_new_custom(hdl, &cfg, NULL);
> +		ctrl = v4l2_ctrl_new_custom(hdl, &cedrus_controls[i].cfg,
> +					    NULL);
>  		if (hdl->error) {
>  			v4l2_err(&dev->v4l2_dev,
>  				 "Failed to create new custom control\n");
> @@ -178,7 +181,7 @@ static int cedrus_request_validate(struct media_request *req)
>  			continue;
>  
>  		ctrl_test = v4l2_ctrl_request_hdl_ctrl_find(hdl,
> -							    cedrus_controls[i].id);
> +							    cedrus_controls[i].cfg.id);
>  		if (!ctrl_test) {
>  			v4l2_info(&ctx->dev->v4l2_dev,
>  				  "Missing required codec control\n");
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index d8e6777e5e27..2f017a651848 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -49,8 +49,7 @@ enum cedrus_h264_pic_type {
>  };
>  
>  struct cedrus_control {
> -	u32			id;
> -	u32			elem_size;
> +	struct v4l2_ctrl_config cfg;
>  	enum cedrus_codec	codec;
>  	unsigned char		required:1;
>  };
> -- 
> 2.22.0
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]