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@xxxxxxxxxx>
> ---
> 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,
