2024-04-05 14:22:32

by Paweł Anikiel

[permalink] [raw]
Subject: [PATCH 00/16] Switch subdev dv timing callbacks to pad ops

Currently, subdev dv timing calls (i.e. g/s/query_dv_timings) are video
ops 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).

This patchset changes subdev dv timing callbacks to include a pad
argument, and moves them from video to pad ops. All the affected
drivers are updated to use the new pad ops version.

The affected drivers were updated in a way that mathes how they deal
with the pad argument in other callbacks (mainly enum_dv_timings,
dv_timings_cap, get/set_edid).

This was originally a part of a larger patchset:
https://lore.kernel.org/lkml/[email protected]/

Paweł Anikiel (16):
media: v4l2-subdev: Add pad versions of dv timing subdev calls
media: i2c: adv748x: Switch dv timing callbacks to pad ops
media: i2c: adv7511: Switch dv timing callbacks to pad ops
media: i2c: adv7604: Switch dv timing callbacks to pad ops
media: i2c: adv7842: Switch dv timing callbacks to pad ops
media: i2c: tc358743: Switch dv timing callbacks to pad ops
media: i2c: tda1997x: Switch dv timing callbacks to pad ops
media: i2c: ths7303: Switch dv timing callbacks to pad ops
media: i2c: ths8200: Switch dv timing callbacks to pad ops
media: i2c: tvp7002: Switch dv timing callbacks to pad ops
media: spi: gs1662: Switch dv timing callbacks to pad ops
media: cobalt: Use pad variant of dv timing subdev calls
media: rcar-vin: Use pad variant of dv timing subdev calls
media: vpif_capture: Use pad variant of dv timing subdev calls
media: tegra-video: Use pad variant of dv timing subdev calls
media: v4l2-subdev: Remove non-pad dv timing callbacks

drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++----
drivers/media/i2c/adv7511-v4l2.c | 14 +++++--
drivers/media/i2c/adv7604.c | 14 +++----
drivers/media/i2c/adv7842.c | 25 ++++++++----
drivers/media/i2c/tc358743.c | 23 +++++++----
drivers/media/i2c/tda1997x.c | 12 +++---
drivers/media/i2c/ths7303.c | 8 +++-
drivers/media/i2c/ths8200.c | 14 +++++--
drivers/media/i2c/tvp7002.c | 26 +++++++++----
drivers/media/pci/cobalt/cobalt-v4l2.c | 12 +++---
.../platform/renesas/rcar-vin/rcar-v4l2.c | 9 +++--
.../media/platform/ti/davinci/vpif_capture.c | 4 +-
drivers/media/spi/gs1662.c | 21 +++++++---
drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++--
drivers/staging/media/tegra-video/vi.c | 12 +++---
include/media/v4l2-subdev.h | 28 ++++++-------
16 files changed, 183 insertions(+), 94 deletions(-)

--
2.44.0.478.gd926399ef9-goog



2024-04-05 14:22:48

by Paweł Anikiel

[permalink] [raw]
Subject: [PATCH 02/16] media: i2c: adv748x: Switch dv timing callbacks to pad ops

Change all (s|g|query)_dv_timings subdev callbacks to include
a pad argument.

Signed-off-by: Paweł Anikiel <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
index ec151dc69c23..a4db9bae5f79 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -214,7 +214,7 @@ static int adv748x_hdmi_set_video_timings(struct adv748x_state *state,
* v4l2_subdev_video_ops
*/

-static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd,
+static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
struct v4l2_dv_timings *timings)
{
struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
@@ -254,7 +254,7 @@ static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd,
return ret;
}

-static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
+static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
struct v4l2_dv_timings *timings)
{
struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
@@ -269,7 +269,7 @@ static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
return 0;
}

-static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd,
+static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
struct v4l2_dv_timings *timings)
{
struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
@@ -392,9 +392,6 @@ static int adv748x_hdmi_g_pixelaspect(struct v4l2_subdev *sd,
}

static const struct v4l2_subdev_video_ops adv748x_video_ops_hdmi = {
- .s_dv_timings = adv748x_hdmi_s_dv_timings,
- .g_dv_timings = adv748x_hdmi_g_dv_timings,
- .query_dv_timings = adv748x_hdmi_query_dv_timings,
.g_input_status = adv748x_hdmi_g_input_status,
.s_stream = adv748x_hdmi_s_stream,
.g_pixelaspect = adv748x_hdmi_g_pixelaspect,
@@ -413,7 +410,7 @@ static int adv748x_hdmi_propagate_pixelrate(struct adv748x_hdmi *hdmi)
if (!tx)
return -ENOLINK;

- adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
+ adv748x_hdmi_query_dv_timings(&hdmi->sd, 0, &timings);

return adv748x_csi2_set_pixelrate(tx, timings.bt.pixelclock);
}
@@ -610,6 +607,9 @@ static const struct v4l2_subdev_pad_ops adv748x_pad_ops_hdmi = {
.get_fmt = adv748x_hdmi_get_format,
.get_edid = adv748x_hdmi_get_edid,
.set_edid = adv748x_hdmi_set_edid,
+ .s_dv_timings = adv748x_hdmi_s_dv_timings,
+ .g_dv_timings = adv748x_hdmi_g_dv_timings,
+ .query_dv_timings = adv748x_hdmi_query_dv_timings,
.dv_timings_cap = adv748x_hdmi_dv_timings_cap,
.enum_dv_timings = adv748x_hdmi_enum_dv_timings,
};
@@ -734,7 +734,7 @@ int adv748x_hdmi_init(struct adv748x_hdmi *hdmi)
struct v4l2_dv_timings cea1280x720 = V4L2_DV_BT_CEA_1280X720P30;
int ret;

- adv748x_hdmi_s_dv_timings(&hdmi->sd, &cea1280x720);
+ adv748x_hdmi_s_dv_timings(&hdmi->sd, 0, &cea1280x720);

/* Initialise a default 16:9 aspect ratio */
hdmi->aspect_ratio.numerator = 16;
--
2.44.0.478.gd926399ef9-goog


2024-04-05 14:23:33

by Paweł Anikiel

[permalink] [raw]
Subject: [PATCH 08/16] media: i2c: ths7303: Switch dv timing callbacks to pad ops

Change all (s|g|query)_dv_timings subdev callbacks to include
a pad argument.

Signed-off-by: Paweł Anikiel <[email protected]>
---
drivers/media/i2c/ths7303.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c
index ea70c1c13872..5e4a76eaf507 100644
--- a/drivers/media/i2c/ths7303.c
+++ b/drivers/media/i2c/ths7303.c
@@ -193,7 +193,7 @@ static int ths7303_s_stream(struct v4l2_subdev *sd, int enable)
}

/* for setting filter for HD output */
-static int ths7303_s_dv_timings(struct v4l2_subdev *sd,
+static int ths7303_s_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
struct v4l2_dv_timings *dv_timings)
{
struct ths7303_state *state = to_state(sd);
@@ -210,7 +210,6 @@ static int ths7303_s_dv_timings(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops ths7303_video_ops = {
.s_stream = ths7303_s_stream,
.s_std_output = ths7303_s_std_output,
- .s_dv_timings = ths7303_s_dv_timings,
};

#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -317,9 +316,14 @@ static const struct v4l2_subdev_core_ops ths7303_core_ops = {
#endif
};

+static const struct v4l2_subdev_pad_ops ths7303_pad_ops = {
+ .s_dv_timings = ths7303_s_dv_timings,
+};
+
static const struct v4l2_subdev_ops ths7303_ops = {
.core = &ths7303_core_ops,
.video = &ths7303_video_ops,
+ .pad = &ths7303_pad_ops,
};

static int ths7303_probe(struct i2c_client *client)
--
2.44.0.478.gd926399ef9-goog


2024-04-06 14:04:20

by Niklas Söderlund

[permalink] [raw]
Subject: Re: [PATCH 02/16] media: i2c: adv748x: Switch dv timing callbacks to pad ops

Hi Paweł,

Thanks for your work.

On 2024-04-05 14:13:57 +0000, Paweł Anikiel wrote:
> Change all (s|g|query)_dv_timings subdev callbacks to include
> a pad argument.
>
> Signed-off-by: Paweł Anikiel <[email protected]>

Tested-by: Niklas Söderlund <[email protected]>

> ---
> drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> index ec151dc69c23..a4db9bae5f79 100644
> --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> @@ -214,7 +214,7 @@ static int adv748x_hdmi_set_video_timings(struct adv748x_state *state,
> * v4l2_subdev_video_ops
> */
>
> -static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd,
> +static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
> struct v4l2_dv_timings *timings)
> {
> struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
> @@ -254,7 +254,7 @@ static int adv748x_hdmi_s_dv_timings(struct v4l2_subdev *sd,
> return ret;
> }
>
> -static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
> +static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
> struct v4l2_dv_timings *timings)
> {
> struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
> @@ -269,7 +269,7 @@ static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd,
> +static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
> struct v4l2_dv_timings *timings)
> {
> struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
> @@ -392,9 +392,6 @@ static int adv748x_hdmi_g_pixelaspect(struct v4l2_subdev *sd,
> }
>
> static const struct v4l2_subdev_video_ops adv748x_video_ops_hdmi = {
> - .s_dv_timings = adv748x_hdmi_s_dv_timings,
> - .g_dv_timings = adv748x_hdmi_g_dv_timings,
> - .query_dv_timings = adv748x_hdmi_query_dv_timings,
> .g_input_status = adv748x_hdmi_g_input_status,
> .s_stream = adv748x_hdmi_s_stream,
> .g_pixelaspect = adv748x_hdmi_g_pixelaspect,
> @@ -413,7 +410,7 @@ static int adv748x_hdmi_propagate_pixelrate(struct adv748x_hdmi *hdmi)
> if (!tx)
> return -ENOLINK;
>
> - adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
> + adv748x_hdmi_query_dv_timings(&hdmi->sd, 0, &timings);
>
> return adv748x_csi2_set_pixelrate(tx, timings.bt.pixelclock);
> }
> @@ -610,6 +607,9 @@ static const struct v4l2_subdev_pad_ops adv748x_pad_ops_hdmi = {
> .get_fmt = adv748x_hdmi_get_format,
> .get_edid = adv748x_hdmi_get_edid,
> .set_edid = adv748x_hdmi_set_edid,
> + .s_dv_timings = adv748x_hdmi_s_dv_timings,
> + .g_dv_timings = adv748x_hdmi_g_dv_timings,
> + .query_dv_timings = adv748x_hdmi_query_dv_timings,
> .dv_timings_cap = adv748x_hdmi_dv_timings_cap,
> .enum_dv_timings = adv748x_hdmi_enum_dv_timings,
> };
> @@ -734,7 +734,7 @@ int adv748x_hdmi_init(struct adv748x_hdmi *hdmi)
> struct v4l2_dv_timings cea1280x720 = V4L2_DV_BT_CEA_1280X720P30;
> int ret;
>
> - adv748x_hdmi_s_dv_timings(&hdmi->sd, &cea1280x720);
> + adv748x_hdmi_s_dv_timings(&hdmi->sd, 0, &cea1280x720);
>
> /* Initialise a default 16:9 aspect ratio */
> hdmi->aspect_ratio.numerator = 16;
> --
> 2.44.0.478.gd926399ef9-goog
>

--
Kind Regards,
Niklas Söderlund

2024-04-08 14:16:24

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 00/16] Switch subdev dv timing callbacks to pad ops

On 05/04/2024 16:13, Paweł Anikiel wrote:
> Currently, subdev dv timing calls (i.e. g/s/query_dv_timings) are video
> ops 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).
>
> This patchset changes subdev dv timing callbacks to include a pad
> argument, and moves them from video to pad ops. All the affected
> drivers are updated to use the new pad ops version.
>
> The affected drivers were updated in a way that mathes how they deal
> with the pad argument in other callbacks (mainly enum_dv_timings,
> dv_timings_cap, get/set_edid).
>
> This was originally a part of a larger patchset:
> https://lore.kernel.org/lkml/[email protected]/
>
> Paweł Anikiel (16):
> media: v4l2-subdev: Add pad versions of dv timing subdev calls
> media: i2c: adv748x: Switch dv timing callbacks to pad ops
> media: i2c: adv7511: Switch dv timing callbacks to pad ops
> media: i2c: adv7604: Switch dv timing callbacks to pad ops
> media: i2c: adv7842: Switch dv timing callbacks to pad ops
> media: i2c: tc358743: Switch dv timing callbacks to pad ops
> media: i2c: tda1997x: Switch dv timing callbacks to pad ops
> media: i2c: ths7303: Switch dv timing callbacks to pad ops
> media: i2c: ths8200: Switch dv timing callbacks to pad ops
> media: i2c: tvp7002: Switch dv timing callbacks to pad ops
> media: spi: gs1662: Switch dv timing callbacks to pad ops
> media: cobalt: Use pad variant of dv timing subdev calls
> media: rcar-vin: Use pad variant of dv timing subdev calls
> media: vpif_capture: Use pad variant of dv timing subdev calls
> media: tegra-video: Use pad variant of dv timing subdev calls
> media: v4l2-subdev: Remove non-pad dv timing callbacks

You missed one:

In file included from include/media/v4l2-device.h:13,
from drivers/media/platform/ti/davinci/vpif_display.h:13,
from drivers/media/platform/ti/davinci/vpif_display.c:26:
drivers/media/platform/ti/davinci/vpif_display.c: In function 'vpif_s_dv_timings':
include/media/v4l2-subdev.h:1816:56: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
1816 | else if (!(__sd->ops->o && __sd->ops->o->f)) \
| ^~
drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
| ^~~~~~~~~~~~~~~~
include/media/v4l2-subdev.h:1819:53: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
1819 | v4l2_subdev_call_wrappers.o->f) \
| ^~
drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
| ^~~~~~~~~~~~~~~~
include/media/v4l2-subdev.h:1820:63: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
1820 | __result = v4l2_subdev_call_wrappers.o->f( \
| ^~
drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
| ^~~~~~~~~~~~~~~~
include/media/v4l2-subdev.h:1823:48: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
1823 | __result = __sd->ops->o->f(__sd, ##args); \
| ^~
drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
| ^~~~~~~~~~~~~~~~
drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops

Regards,

Hans

>
> drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++----
> drivers/media/i2c/adv7511-v4l2.c | 14 +++++--
> drivers/media/i2c/adv7604.c | 14 +++----
> drivers/media/i2c/adv7842.c | 25 ++++++++----
> drivers/media/i2c/tc358743.c | 23 +++++++----
> drivers/media/i2c/tda1997x.c | 12 +++---
> drivers/media/i2c/ths7303.c | 8 +++-
> drivers/media/i2c/ths8200.c | 14 +++++--
> drivers/media/i2c/tvp7002.c | 26 +++++++++----
> drivers/media/pci/cobalt/cobalt-v4l2.c | 12 +++---
> .../platform/renesas/rcar-vin/rcar-v4l2.c | 9 +++--
> .../media/platform/ti/davinci/vpif_capture.c | 4 +-
> drivers/media/spi/gs1662.c | 21 +++++++---
> drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++--
> drivers/staging/media/tegra-video/vi.c | 12 +++---
> include/media/v4l2-subdev.h | 28 ++++++-------
> 16 files changed, 183 insertions(+), 94 deletions(-)
>


2024-04-08 14:41:53

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 00/16] Switch subdev dv timing callbacks to pad ops

On 08/04/2024 16:15, Hans Verkuil wrote:
> On 05/04/2024 16:13, Paweł Anikiel wrote:
>> Currently, subdev dv timing calls (i.e. g/s/query_dv_timings) are video
>> ops 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).
>>
>> This patchset changes subdev dv timing callbacks to include a pad
>> argument, and moves them from video to pad ops. All the affected
>> drivers are updated to use the new pad ops version.
>>
>> The affected drivers were updated in a way that mathes how they deal
>> with the pad argument in other callbacks (mainly enum_dv_timings,
>> dv_timings_cap, get/set_edid).
>>
>> This was originally a part of a larger patchset:
>> https://lore.kernel.org/lkml/[email protected]/
>>
>> Paweł Anikiel (16):
>> media: v4l2-subdev: Add pad versions of dv timing subdev calls
>> media: i2c: adv748x: Switch dv timing callbacks to pad ops
>> media: i2c: adv7511: Switch dv timing callbacks to pad ops
>> media: i2c: adv7604: Switch dv timing callbacks to pad ops
>> media: i2c: adv7842: Switch dv timing callbacks to pad ops
>> media: i2c: tc358743: Switch dv timing callbacks to pad ops
>> media: i2c: tda1997x: Switch dv timing callbacks to pad ops
>> media: i2c: ths7303: Switch dv timing callbacks to pad ops
>> media: i2c: ths8200: Switch dv timing callbacks to pad ops
>> media: i2c: tvp7002: Switch dv timing callbacks to pad ops
>> media: spi: gs1662: Switch dv timing callbacks to pad ops
>> media: cobalt: Use pad variant of dv timing subdev calls
>> media: rcar-vin: Use pad variant of dv timing subdev calls
>> media: vpif_capture: Use pad variant of dv timing subdev calls
>> media: tegra-video: Use pad variant of dv timing subdev calls
>> media: v4l2-subdev: Remove non-pad dv timing callbacks
>
> You missed one:
>
> In file included from include/media/v4l2-device.h:13,
> from drivers/media/platform/ti/davinci/vpif_display.h:13,
> from drivers/media/platform/ti/davinci/vpif_display.c:26:
> drivers/media/platform/ti/davinci/vpif_display.c: In function 'vpif_s_dv_timings':
> include/media/v4l2-subdev.h:1816:56: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> 1816 | else if (!(__sd->ops->o && __sd->ops->o->f)) \
> | ^~
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> | ^~~~~~~~~~~~~~~~
> include/media/v4l2-subdev.h:1819:53: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> 1819 | v4l2_subdev_call_wrappers.o->f) \
> | ^~
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> | ^~~~~~~~~~~~~~~~
> include/media/v4l2-subdev.h:1820:63: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> 1820 | __result = v4l2_subdev_call_wrappers.o->f( \
> | ^~
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> | ^~~~~~~~~~~~~~~~
> include/media/v4l2-subdev.h:1823:48: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> 1823 | __result = __sd->ops->o->f(__sd, ##args); \
> | ^~
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> | ^~~~~~~~~~~~~~~~
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops

No need to post a v2. Just post a single patch '14.5/16' to convert vpif_display that
will be added after patch [14/16], then I'll take care of it.

Regards,

Hans

>
> Regards,
>
> Hans
>
>>
>> drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++----
>> drivers/media/i2c/adv7511-v4l2.c | 14 +++++--
>> drivers/media/i2c/adv7604.c | 14 +++----
>> drivers/media/i2c/adv7842.c | 25 ++++++++----
>> drivers/media/i2c/tc358743.c | 23 +++++++----
>> drivers/media/i2c/tda1997x.c | 12 +++---
>> drivers/media/i2c/ths7303.c | 8 +++-
>> drivers/media/i2c/ths8200.c | 14 +++++--
>> drivers/media/i2c/tvp7002.c | 26 +++++++++----
>> drivers/media/pci/cobalt/cobalt-v4l2.c | 12 +++---
>> .../platform/renesas/rcar-vin/rcar-v4l2.c | 9 +++--
>> .../media/platform/ti/davinci/vpif_capture.c | 4 +-
>> drivers/media/spi/gs1662.c | 21 +++++++---
>> drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++--
>> drivers/staging/media/tegra-video/vi.c | 12 +++---
>> include/media/v4l2-subdev.h | 28 ++++++-------
>> 16 files changed, 183 insertions(+), 94 deletions(-)
>>
>
>


2024-04-08 15:25:52

by Paweł Anikiel

[permalink] [raw]
Subject: Re: [PATCH 00/16] Switch subdev dv timing callbacks to pad ops

On Mon, Apr 8, 2024 at 4:18 PM Hans Verkuil <[email protected]> wrote:
>
> On 08/04/2024 16:15, Hans Verkuil wrote:
> > On 05/04/2024 16:13, Paweł Anikiel wrote:
> >> Currently, subdev dv timing calls (i.e. g/s/query_dv_timings) are video
> >> ops 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).
> >>
> >> This patchset changes subdev dv timing callbacks to include a pad
> >> argument, and moves them from video to pad ops. All the affected
> >> drivers are updated to use the new pad ops version.
> >>
> >> The affected drivers were updated in a way that mathes how they deal
> >> with the pad argument in other callbacks (mainly enum_dv_timings,
> >> dv_timings_cap, get/set_edid).
> >>
> >> This was originally a part of a larger patchset:
> >> https://lore.kernel.org/lkml/[email protected]/
> >>
> >> Paweł Anikiel (16):
> >> media: v4l2-subdev: Add pad versions of dv timing subdev calls
> >> media: i2c: adv748x: Switch dv timing callbacks to pad ops
> >> media: i2c: adv7511: Switch dv timing callbacks to pad ops
> >> media: i2c: adv7604: Switch dv timing callbacks to pad ops
> >> media: i2c: adv7842: Switch dv timing callbacks to pad ops
> >> media: i2c: tc358743: Switch dv timing callbacks to pad ops
> >> media: i2c: tda1997x: Switch dv timing callbacks to pad ops
> >> media: i2c: ths7303: Switch dv timing callbacks to pad ops
> >> media: i2c: ths8200: Switch dv timing callbacks to pad ops
> >> media: i2c: tvp7002: Switch dv timing callbacks to pad ops
> >> media: spi: gs1662: Switch dv timing callbacks to pad ops
> >> media: cobalt: Use pad variant of dv timing subdev calls
> >> media: rcar-vin: Use pad variant of dv timing subdev calls
> >> media: vpif_capture: Use pad variant of dv timing subdev calls
> >> media: tegra-video: Use pad variant of dv timing subdev calls
> >> media: v4l2-subdev: Remove non-pad dv timing callbacks
> >
> > You missed one:
> >
> > In file included from include/media/v4l2-device.h:13,
> > from drivers/media/platform/ti/davinci/vpif_display.h:13,
> > from drivers/media/platform/ti/davinci/vpif_display.c:26:
> > drivers/media/platform/ti/davinci/vpif_display.c: In function 'vpif_s_dv_timings':
> > include/media/v4l2-subdev.h:1816:56: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> > 1816 | else if (!(__sd->ops->o && __sd->ops->o->f)) \
> > | ^~
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> > 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> > | ^~~~~~~~~~~~~~~~
> > include/media/v4l2-subdev.h:1819:53: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> > 1819 | v4l2_subdev_call_wrappers.o->f) \
> > | ^~
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> > 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> > | ^~~~~~~~~~~~~~~~
> > include/media/v4l2-subdev.h:1820:63: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> > 1820 | __result = v4l2_subdev_call_wrappers.o->f( \
> > | ^~
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> > 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> > | ^~~~~~~~~~~~~~~~
> > include/media/v4l2-subdev.h:1823:48: error: 'const struct v4l2_subdev_video_ops' has no member named 's_dv_timings'
> > 1823 | __result = __sd->ops->o->f(__sd, ##args); \
> > | ^~
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: note: in expansion of macro 'v4l2_subdev_call'
> > 937 | ret = v4l2_subdev_call(ch->sd, video, s_dv_timings, timings);
> > | ^~~~~~~~~~~~~~~~
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
> > drivers/media/platform/ti/davinci/vpif_display.c:937:15: error: no member 's_dv_timings' in struct v4l2_subdev_video_ops
>
> No need to post a v2. Just post a single patch '14.5/16' to convert vpif_display that
> will be added after patch [14/16], then I'll take care of it.

I probably thought vpif-capture.c and vpif-display.c were the same
file. I sent the 14.5 patch as you requested.

As an aside, how did you run the compile test? It would be nice to
know for the future. For this, I did a few greps and manually enabled
all the drivers in menuconfig. Is there a better way?

Regards,
Paweł

>
> Regards,
>
> Hans
>
> >
> > Regards,
> >
> > Hans
> >
> >>
> >> drivers/media/i2c/adv748x/adv748x-hdmi.c | 16 ++++----
> >> drivers/media/i2c/adv7511-v4l2.c | 14 +++++--
> >> drivers/media/i2c/adv7604.c | 14 +++----
> >> drivers/media/i2c/adv7842.c | 25 ++++++++----
> >> drivers/media/i2c/tc358743.c | 23 +++++++----
> >> drivers/media/i2c/tda1997x.c | 12 +++---
> >> drivers/media/i2c/ths7303.c | 8 +++-
> >> drivers/media/i2c/ths8200.c | 14 +++++--
> >> drivers/media/i2c/tvp7002.c | 26 +++++++++----
> >> drivers/media/pci/cobalt/cobalt-v4l2.c | 12 +++---
> >> .../platform/renesas/rcar-vin/rcar-v4l2.c | 9 +++--
> >> .../media/platform/ti/davinci/vpif_capture.c | 4 +-
> >> drivers/media/spi/gs1662.c | 21 +++++++---
> >> drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++--
> >> drivers/staging/media/tegra-video/vi.c | 12 +++---
> >> include/media/v4l2-subdev.h | 28 ++++++-------
> >> 16 files changed, 183 insertions(+), 94 deletions(-)
> >>
> >
> >
>