linux-kernel - Re: [PATCH v2 1/9] media: v4l2-subdev: Add a pad variant of .query_dv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ce262cda-84ba-4d8f-a916-76488c94066d@xs4all.nl>
Date: Wed, 28 Feb 2024 12:25:44 +0100
From: Hans Verkuil <hverkuil-cisco@...all.nl>
To: Paweł Anikiel <panikiel@...gle.com>, airlied@...il.com,
 akpm@...ux-foundation.org, conor+dt@...nel.org, daniel@...ll.ch,
 dinguyen@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 maarten.lankhorst@...ux.intel.com, mchehab@...nel.org, mripard@...nel.org,
 robh+dt@...nel.org, tzimmermann@...e.de
Cc: devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
 chromeos-krk-upstreaming@...gle.com, ribalda@...omium.org
Subject: Re: [PATCH v2 1/9] media: v4l2-subdev: Add a pad variant of
 .query_dv_timings()

Hi Paweł,

On 21/02/2024 17:02, Paweł Anikiel wrote:
> Currently, .query_dv_timings() is defined as a video callback without
> a pad argument. This is a problem if the subdevice can have different
> dv timings for each pad (e.g. a DisplayPort receiver with multiple
> virtual channels).
> 
> To solve this, add a pad variant of this callback which includes
> the pad number as an argument.

So now we have two query_dv_timings ops: one for video ops, and one
for pad ops. That's not very maintainable. I would suggest switching
all current users of the video op over to the pad op.

Regards,

	Hans

> 
> Signed-off-by: Paweł Anikiel <panikiel@...gle.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 11 +++++++++++
>  include/media/v4l2-subdev.h           |  5 +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 4c6198c48dd6..11f865dd19b4 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -389,6 +389,16 @@ static int call_enum_dv_timings(struct v4l2_subdev *sd,
>  	       sd->ops->pad->enum_dv_timings(sd, dvt);
>  }
>  
> +static int call_query_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
> +				 struct v4l2_dv_timings *timings)
> +{
> +	if (!timings)
> +		return -EINVAL;
> +
> +	return check_pad(sd, pad) ? :
> +	       sd->ops->pad->query_dv_timings(sd, pad, timings);
> +}
> +
>  static int call_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
>  				struct v4l2_mbus_config *config)
>  {
> @@ -489,6 +499,7 @@ static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
>  	.set_edid		= call_set_edid,
>  	.dv_timings_cap		= call_dv_timings_cap,
>  	.enum_dv_timings	= call_enum_dv_timings,
> +	.query_dv_timings	= call_query_dv_timings,
>  	.get_frame_desc		= call_get_frame_desc,
>  	.get_mbus_config	= call_get_mbus_config,
>  };
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a9e6b8146279..dc8963fa5a06 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -797,6 +797,9 @@ struct v4l2_subdev_state {
>   * @enum_dv_timings: callback for VIDIOC_SUBDEV_ENUM_DV_TIMINGS() ioctl handler
>   *		     code.
>   *
> + * @query_dv_timings: same as query_dv_timings() from v4l2_subdev_video_ops,
> + *		      but with additional pad argument.
> + *
>   * @link_validate: used by the media controller code to check if the links
>   *		   that belongs to a pipeline can be used for stream.
>   *
> @@ -868,6 +871,8 @@ struct v4l2_subdev_pad_ops {
>  			      struct v4l2_dv_timings_cap *cap);
>  	int (*enum_dv_timings)(struct v4l2_subdev *sd,
>  			       struct v4l2_enum_dv_timings *timings);
> +	int (*query_dv_timings)(struct v4l2_subdev *sd, unsigned int pad,
> +				struct v4l2_dv_timings *timings);
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	int (*link_validate)(struct v4l2_subdev *sd, struct media_link *link,
>  			     struct v4l2_subdev_format *source_fmt,