* [PATCH 1/7] media: atmel-isi: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-23 21:40 ` [PATCH 2/7] media: microchip-isc: " Laurent Pinchart
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Hans Verkuil, Eugen Hristev, Nicolas Ferre
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/atmel/atmel-isi.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 4046212d48b4..bb83117bab2e 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -559,11 +559,13 @@ static const struct isi_format *find_format_by_fourcc(struct atmel_isi *isi,
static void isi_try_fse(struct atmel_isi *isi, const struct isi_format *isi_fmt,
struct v4l2_subdev_state *sd_state)
{
- int ret;
+ struct v4l2_rect *try_crop =
+ v4l2_subdev_get_pad_crop(isi->entity.subdev, sd_state, 0);
struct v4l2_subdev_frame_size_enum fse = {
.code = isi_fmt->mbus_code,
.which = V4L2_SUBDEV_FORMAT_TRY,
};
+ int ret;
ret = v4l2_subdev_call(isi->entity.subdev, pad, enum_frame_size,
sd_state, &fse);
@@ -572,11 +574,11 @@ static void isi_try_fse(struct atmel_isi *isi, const struct isi_format *isi_fmt,
* just use the maximum ISI can receive.
*/
if (ret) {
- sd_state->pads->try_crop.width = MAX_SUPPORT_WIDTH;
- sd_state->pads->try_crop.height = MAX_SUPPORT_HEIGHT;
+ try_crop->width = MAX_SUPPORT_WIDTH;
+ try_crop->height = MAX_SUPPORT_HEIGHT;
} else {
- sd_state->pads->try_crop.width = fse.max_width;
- sd_state->pads->try_crop.height = fse.max_height;
+ try_crop->width = fse.max_width;
+ try_crop->height = fse.max_height;
}
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/7] media: microchip-isc: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
2023-10-23 21:40 ` [PATCH 1/7] media: atmel-isi: Use accessors for " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-23 21:40 ` [PATCH 3/7] media: atmel-isc: " Laurent Pinchart
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Hans Verkuil, Eugen Hristev, Nicolas Ferre
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/microchip/microchip-isc-base.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 1f8528844497..a901be02e2f6 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -854,6 +854,8 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
static void isc_try_fse(struct isc_device *isc,
struct v4l2_subdev_state *sd_state)
{
+ struct v4l2_rect *try_crop =
+ v4l2_subdev_get_pad_crop(isc->current_subdev->sd, sd_state, 0);
struct v4l2_subdev_frame_size_enum fse = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
@@ -875,11 +877,11 @@ static void isc_try_fse(struct isc_device *isc,
* just use the maximum ISC can receive.
*/
if (ret) {
- sd_state->pads->try_crop.width = isc->max_width;
- sd_state->pads->try_crop.height = isc->max_height;
+ try_crop->width = isc->max_width;
+ try_crop->height = isc->max_height;
} else {
- sd_state->pads->try_crop.width = fse.max_width;
- sd_state->pads->try_crop.height = fse.max_height;
+ try_crop->width = fse.max_width;
+ try_crop->height = fse.max_height;
}
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/7] media: atmel-isc: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
2023-10-23 21:40 ` [PATCH 1/7] media: atmel-isi: Use accessors for " Laurent Pinchart
2023-10-23 21:40 ` [PATCH 2/7] media: microchip-isc: " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-23 21:40 ` [PATCH 4/7] media: atomisp: " Laurent Pinchart
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Hans Verkuil, Eugen Hristev, Nicolas Ferre
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../staging/media/deprecated/atmel/atmel-isc-base.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/deprecated/atmel/atmel-isc-base.c b/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
index 8e26663cecb6..b63eea8f4fc0 100644
--- a/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
+++ b/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
@@ -820,6 +820,8 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
static void isc_try_fse(struct isc_device *isc,
struct v4l2_subdev_state *sd_state)
{
+ struct v4l2_rect *try_crop =
+ v4l2_subdev_get_pad_crop(isc->current_subdev->sd, sd_state, 0);
struct v4l2_subdev_frame_size_enum fse = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
@@ -841,11 +843,11 @@ static void isc_try_fse(struct isc_device *isc,
* just use the maximum ISC can receive.
*/
if (ret) {
- sd_state->pads->try_crop.width = isc->max_width;
- sd_state->pads->try_crop.height = isc->max_height;
+ try_crop->width = isc->max_width;
+ try_crop->height = isc->max_height;
} else {
- sd_state->pads->try_crop.width = fse.max_width;
- sd_state->pads->try_crop.height = fse.max_height;
+ try_crop->width = fse.max_width;
+ try_crop->height = fse.max_height;
}
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/7] media: atomisp: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
` (2 preceding siblings ...)
2023-10-23 21:40 ` [PATCH 3/7] media: atmel-isc: " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-23 21:40 ` [PATCH 5/7] media: tegra-video: " Laurent Pinchart
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Hans Verkuil, Hans de Goede
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 2 +-
drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 2 +-
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +-
drivers/staging/media/atomisp/pci/atomisp_tpg.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 9fa390fbc5f3..5e438c5fd4a9 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -561,7 +561,7 @@ static int gc2235_set_fmt(struct v4l2_subdev *sd,
fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
mutex_unlock(&dev->input_lock);
return 0;
}
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index 1c6643c442ef..db76f52e1dc8 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -666,7 +666,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
fmt->height = res->height;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
return 0;
}
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 6a72691ed5b7..ae70e04040dd 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -671,7 +671,7 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
return 0;
}
diff --git a/drivers/staging/media/atomisp/pci/atomisp_tpg.c b/drivers/staging/media/atomisp/pci/atomisp_tpg.c
index 074826a5b706..b2376ebf45a1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_tpg.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_tpg.c
@@ -47,7 +47,7 @@ static int tpg_set_fmt(struct v4l2_subdev *sd,
/* only raw8 grbg is supported by TPG */
fmt->code = MEDIA_BUS_FMT_SGRBG8_1X8;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
return 0;
}
return 0;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/7] media: tegra-video: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
` (3 preceding siblings ...)
2023-10-23 21:40 ` [PATCH 4/7] media: atomisp: " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-25 7:27 ` Luca Ceresoli
2023-10-23 21:40 ` [PATCH 6/7] media: i2c: " Laurent Pinchart
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Hans Verkuil, Thierry Reding, Jonathan Hunter,
Sowjanya Komatineni, Luca Ceresoli
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/staging/media/tegra-video/vi.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
index 94171e62dee9..a2f21c70a5bc 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -439,6 +439,7 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
.target = V4L2_SEL_TGT_CROP_BOUNDS,
};
+ struct v4l2_rect *try_crop;
int ret;
subdev = tegra_channel_get_remote_source_subdev(chan);
@@ -473,24 +474,25 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
* Attempt to obtain the format size from subdev.
* If not available, try to get crop boundary from subdev.
*/
+ try_crop = v4l2_subdev_get_pad_crop(subdev, sd_state, 0);
fse.code = fmtinfo->code;
ret = v4l2_subdev_call(subdev, pad, enum_frame_size, sd_state, &fse);
if (ret) {
if (!v4l2_subdev_has_op(subdev, pad, get_selection)) {
- sd_state->pads->try_crop.width = 0;
- sd_state->pads->try_crop.height = 0;
+ try_crop->width = 0;
+ try_crop->height = 0;
} else {
ret = v4l2_subdev_call(subdev, pad, get_selection,
NULL, &sdsel);
if (ret)
return -EINVAL;
- sd_state->pads->try_crop.width = sdsel.r.width;
- sd_state->pads->try_crop.height = sdsel.r.height;
+ try_crop->width = sdsel.r.width;
+ try_crop->height = sdsel.r.height;
}
} else {
- sd_state->pads->try_crop.width = fse.max_width;
- sd_state->pads->try_crop.height = fse.max_height;
+ try_crop->width = fse.max_width;
+ try_crop->height = fse.max_height;
}
ret = v4l2_subdev_call(subdev, pad, set_fmt, sd_state, &fmt);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/7] media: tegra-video: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 ` [PATCH 5/7] media: tegra-video: " Laurent Pinchart
@ 2023-10-25 7:27 ` Luca Ceresoli
0 siblings, 0 replies; 13+ messages in thread
From: Luca Ceresoli @ 2023-10-25 7:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Sakari Ailus, Hans Verkuil, Thierry Reding,
Jonathan Hunter, Sowjanya Komatineni
Hi Laurent,
On Tue, 24 Oct 2023 00:40:09 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
> be accessed through helper functions. Replace direct access with usage
> of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
> v4l2_subdev_get_pad_compose() helpers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/7] media: i2c: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
` (4 preceding siblings ...)
2023-10-23 21:40 ` [PATCH 5/7] media: tegra-video: " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-27 20:21 ` kernel test robot
2023-10-23 21:40 ` [PATCH 7/7] media: v4l2-subdev: Rename " Laurent Pinchart
2023-10-24 6:55 ` [PATCH 0/7] " Eugen Hristev
7 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Hans Verkuil, Leon Luo, Jacopo Mondi,
Rui Miguel Silva, Hans de Goede, Petr Cvek
The 'try_*' fields of the v4l2_subdev_pad_config structure are meant to
be accessed through helper functions. Replace direct access with usage
of the v4l2_subdev_get_pad_format(), v4l2_subdev_get_pad_crop() and
v4l2_subdev_get_pad_compose() helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/adv7183.c | 2 +-
drivers/media/i2c/imx274.c | 12 ++++++------
drivers/media/i2c/mt9m001.c | 2 +-
drivers/media/i2c/mt9m111.c | 2 +-
drivers/media/i2c/mt9t112.c | 2 +-
drivers/media/i2c/mt9v011.c | 2 +-
drivers/media/i2c/mt9v111.c | 2 +-
drivers/media/i2c/ov2640.c | 2 +-
drivers/media/i2c/ov2680.c | 4 ++--
drivers/media/i2c/ov6650.c | 34 +++++++++++++++++++++-------------
drivers/media/i2c/ov772x.c | 2 +-
drivers/media/i2c/ov9640.c | 2 +-
drivers/media/i2c/rj54n1cb0c.c | 2 +-
drivers/media/i2c/saa6752hs.c | 2 +-
drivers/media/i2c/tw9910.c | 2 +-
15 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c
index 3659feafac69..0754bfefa073 100644
--- a/drivers/media/i2c/adv7183.c
+++ b/drivers/media/i2c/adv7183.c
@@ -443,7 +443,7 @@ static int adv7183_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
decoder->fmt = *fmt;
else
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
return 0;
}
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index f33b692e6951..1886eaab1c24 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1018,8 +1018,8 @@ static int __imx274_change_compose(struct stimx274 *imx274,
int best_goodness = INT_MIN;
if (which == V4L2_SUBDEV_FORMAT_TRY) {
- cur_crop = &sd_state->pads->try_crop;
- tgt_fmt = &sd_state->pads->try_fmt;
+ cur_crop = v4l2_subdev_get_pad_crop(&imx274->sd, sd_state, 0);
+ tgt_fmt = v4l2_subdev_get_pad_format(&imx274->sd, sd_state, 0);
} else {
cur_crop = &imx274->crop;
tgt_fmt = &imx274->format;
@@ -1112,7 +1112,7 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
*/
fmt->field = V4L2_FIELD_NONE;
if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
else
imx274->format = *fmt;
@@ -1143,8 +1143,8 @@ static int imx274_get_selection(struct v4l2_subdev *sd,
}
if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
- src_crop = &sd_state->pads->try_crop;
- src_fmt = &sd_state->pads->try_fmt;
+ src_crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0);
+ src_fmt = v4l2_subdev_get_pad_format(sd, sd_state, 0);
} else {
src_crop = &imx274->crop;
src_fmt = &imx274->format;
@@ -1215,7 +1215,7 @@ static int imx274_set_selection_crop(struct stimx274 *imx274,
sel->r = new_crop;
if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
- tgt_crop = &sd_state->pads->try_crop;
+ tgt_crop = v4l2_subdev_get_pad_crop(&imx274->sd, sd_state, 0);
else
tgt_crop = &imx274->crop;
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index 79192cf79d28..5c137fc2034e 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -405,7 +405,7 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return mt9m001_s_fmt(sd, fmt, mf);
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 1f44b72e8a70..e17ff41ebba0 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -671,7 +671,7 @@ static int mt9m111_set_fmt(struct v4l2_subdev *sd,
mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
index 93f34b767027..e3b9ff374500 100644
--- a/drivers/media/i2c/mt9t112.c
+++ b/drivers/media/i2c/mt9t112.c
@@ -982,7 +982,7 @@ static int mt9t112_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return mt9t112_s_fmt(sd, mf);
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
index 37a634b92cd5..7f313d731839 100644
--- a/drivers/media/i2c/mt9v011.c
+++ b/drivers/media/i2c/mt9v011.c
@@ -356,7 +356,7 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
set_res(sd);
} else {
- sd_state->pads->try_fmt = *fmt;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *fmt;
}
return 0;
diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
index f859b49e13bf..2c321e4884be 100644
--- a/drivers/media/i2c/mt9v111.c
+++ b/drivers/media/i2c/mt9v111.c
@@ -951,7 +951,7 @@ static int mt9v111_set_format(struct v4l2_subdev *subdev,
static int mt9v111_init_cfg(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *sd_state)
{
- sd_state->pads->try_fmt = mt9v111_def_fmt;
+ *v4l2_subdev_get_pad_format(subdev, sd_state, 0) = mt9v111_def_fmt;
return 0;
}
diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 28a01c6eff64..389b6119ddec 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -988,7 +988,7 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
/* select format */
priv->cfmt_code = mf->code;
} else {
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
}
out:
mutex_unlock(&priv->lock);
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index 72bab0ff8a36..f58fa2eb86ee 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -760,9 +760,9 @@ static int ov2680_init_cfg(struct v4l2_subdev *sd,
{
struct ov2680_dev *sensor = to_ov2680_dev(sd);
- sd_state->pads[0].try_crop = ov2680_default_crop;
+ *v4l2_subdev_get_pad_crop(sd, sd_state, 0) = ov2680_default_crop;
- ov2680_fill_format(sensor, &sd_state->pads[0].try_fmt,
+ ov2680_fill_format(sensor, v4l2_subdev_get_pad_format(sd, sd_state, 0),
OV2680_DEFAULT_WIDTH, OV2680_DEFAULT_HEIGHT);
return 0;
}
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 1ad07935f046..38d21b40f5d8 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -476,7 +476,7 @@ static int ov6650_get_selection(struct v4l2_subdev *sd,
if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
/* pre-select try crop rectangle */
- rect = &sd_state->pads->try_crop;
+ rect = v4l2_subdev_get_pad_crop(sd, sd_state, 0);
} else {
/* pre-select active crop rectangle */
@@ -531,8 +531,10 @@ static int ov6650_set_selection(struct v4l2_subdev *sd,
ov6650_bind_align_crop_rectangle(&sel->r);
if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
- struct v4l2_rect *crop = &sd_state->pads->try_crop;
- struct v4l2_mbus_framefmt *mf = &sd_state->pads->try_fmt;
+ struct v4l2_rect *crop =
+ v4l2_subdev_get_pad_crop(sd, sd_state, 0);
+ struct v4l2_mbus_framefmt *mf =
+ v4l2_subdev_get_pad_format(sd, sd_state, 0);
/* detect current pad config scaling factor */
bool half_scale = !is_unscaled_ok(mf->width, mf->height, crop);
@@ -588,9 +590,12 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd,
/* update media bus format code and frame size */
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- mf->width = sd_state->pads->try_fmt.width;
- mf->height = sd_state->pads->try_fmt.height;
- mf->code = sd_state->pads->try_fmt.code;
+ struct v4l2_mbus_framefmt *try_fmt =
+ v4l2_subdev_get_pad_format(sd, sd_state, 0);
+
+ mf->width = try_fmt->width;
+ mf->height = try_fmt->height;
+ mf->code = try_fmt->code;
} else {
mf->width = priv->rect.width >> priv->half_scale;
@@ -717,23 +722,26 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
}
if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- crop = &sd_state->pads->try_crop;
+ crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0);
else
crop = &priv->rect;
half_scale = !is_unscaled_ok(mf->width, mf->height, crop);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ struct v4l2_mbus_framefmt *try_fmt =
+ v4l2_subdev_get_pad_format(sd, sd_state, 0);
+
/* store new mbus frame format code and size in pad config */
- sd_state->pads->try_fmt.width = crop->width >> half_scale;
- sd_state->pads->try_fmt.height = crop->height >> half_scale;
- sd_state->pads->try_fmt.code = mf->code;
+ try_fmt->width = crop->width >> half_scale;
+ try_fmt->height = crop->height >> half_scale;
+ try_fmt->code = mf->code;
/* return default mbus frame format updated with pad config */
*mf = ov6650_def_fmt;
- mf->width = sd_state->pads->try_fmt.width;
- mf->height = sd_state->pads->try_fmt.height;
- mf->code = sd_state->pads->try_fmt.code;
+ mf->width = try_fmt->width;
+ mf->height = try_fmt->height;
+ mf->code = try_fmt->code;
} else {
int ret = 0;
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 7618b58a7ad0..1fbf72152956 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1220,7 +1220,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
index cbaea049531d..a9adddde1006 100644
--- a/drivers/media/i2c/ov9640.c
+++ b/drivers/media/i2c/ov9640.c
@@ -547,7 +547,7 @@ static int ov9640_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return ov9640_s_fmt(sd, mf);
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/rj54n1cb0c.c b/drivers/media/i2c/rj54n1cb0c.c
index b430046f9e2a..dbddb7a9f211 100644
--- a/drivers/media/i2c/rj54n1cb0c.c
+++ b/drivers/media/i2c/rj54n1cb0c.c
@@ -1009,7 +1009,7 @@ static int rj54n1_set_fmt(struct v4l2_subdev *sd,
&mf->height, 84, RJ54N1_MAX_HEIGHT, align, 0);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
diff --git a/drivers/media/i2c/saa6752hs.c b/drivers/media/i2c/saa6752hs.c
index c106e7a7d1f4..82976aee5e86 100644
--- a/drivers/media/i2c/saa6752hs.c
+++ b/drivers/media/i2c/saa6752hs.c
@@ -595,7 +595,7 @@ static int saa6752hs_set_fmt(struct v4l2_subdev *sd,
f->colorspace = V4L2_COLORSPACE_SMPTE170M;
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- sd_state->pads->try_fmt = *f;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *f;
return 0;
}
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 477a64d8f8ab..5233c33b93df 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -829,7 +829,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return tw9910_s_fmt(sd, mf);
- sd_state->pads->try_fmt = *mf;
+ *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *mf;
return 0;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 6/7] media: i2c: Use accessors for pad config 'try_*' fields
2023-10-23 21:40 ` [PATCH 6/7] media: i2c: " Laurent Pinchart
@ 2023-10-27 20:21 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-10-27 20:21 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: oe-kbuild-all
Hi Laurent,
kernel test robot noticed the following build errors:
[auto build test ERROR on 94e27fbeca27d8c772fc2bc807730aaee5886055]
url: https://github.com/intel-lab-lkp/linux/commits/Laurent-Pinchart/media-atmel-isi-Use-accessors-for-pad-config-try_-fields/20231024-054150
base: 94e27fbeca27d8c772fc2bc807730aaee5886055
patch link: https://lore.kernel.org/r/20231023214011.17730-7-laurent.pinchart%40ideasonboard.com
patch subject: [PATCH 6/7] media: i2c: Use accessors for pad config 'try_*' fields
config: x86_64-buildonly-randconfig-006-20231027 (https://download.01.org/0day-ci/archive/20231028/202310280440.IDSYRK6B-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231028/202310280440.IDSYRK6B-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310280440.IDSYRK6B-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/media/i2c/saa6752hs.c: In function 'saa6752hs_set_fmt':
>> drivers/media/i2c/saa6752hs.c:598:18: error: implicit declaration of function 'v4l2_subdev_get_pad_format'; did you mean 'v4l2_subdev_has_pad_interdep'? [-Werror=implicit-function-declaration]
598 | *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *f;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| v4l2_subdev_has_pad_interdep
>> drivers/media/i2c/saa6752hs.c:598:17: error: invalid type argument of unary '*' (have 'int')
598 | *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *f;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +598 drivers/media/i2c/saa6752hs.c
564
565 static int saa6752hs_set_fmt(struct v4l2_subdev *sd,
566 struct v4l2_subdev_state *sd_state,
567 struct v4l2_subdev_format *format)
568 {
569 struct v4l2_mbus_framefmt *f = &format->format;
570 struct saa6752hs_state *h = to_state(sd);
571 int dist_352, dist_480, dist_720;
572
573 if (format->pad)
574 return -EINVAL;
575
576 f->code = MEDIA_BUS_FMT_FIXED;
577
578 dist_352 = abs(f->width - 352);
579 dist_480 = abs(f->width - 480);
580 dist_720 = abs(f->width - 720);
581 if (dist_720 < dist_480) {
582 f->width = 720;
583 f->height = 576;
584 } else if (dist_480 < dist_352) {
585 f->width = 480;
586 f->height = 576;
587 } else {
588 f->width = 352;
589 if (abs(f->height - 576) < abs(f->height - 288))
590 f->height = 576;
591 else
592 f->height = 288;
593 }
594 f->field = V4L2_FIELD_INTERLACED;
595 f->colorspace = V4L2_COLORSPACE_SMPTE170M;
596
597 if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> 598 *v4l2_subdev_get_pad_format(sd, sd_state, 0) = *f;
599 return 0;
600 }
601
602 /*
603 FIXME: translate and round width/height into EMPRESS
604 subsample type:
605
606 type | PAL | NTSC
607 ---------------------------
608 SIF | 352x288 | 352x240
609 1/2 D1 | 352x576 | 352x480
610 2/3 D1 | 480x576 | 480x480
611 D1 | 720x576 | 720x480
612 */
613
614 if (f->code != MEDIA_BUS_FMT_FIXED)
615 return -EINVAL;
616
617 if (f->width == 720)
618 h->video_format = SAA6752HS_VF_D1;
619 else if (f->width == 480)
620 h->video_format = SAA6752HS_VF_2_3_D1;
621 else if (f->height == 576)
622 h->video_format = SAA6752HS_VF_1_2_D1;
623 else
624 h->video_format = SAA6752HS_VF_SIF;
625 return 0;
626 }
627
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/7] media: v4l2-subdev: Rename pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
` (5 preceding siblings ...)
2023-10-23 21:40 ` [PATCH 6/7] media: i2c: " Laurent Pinchart
@ 2023-10-23 21:40 ` Laurent Pinchart
2023-10-24 6:55 ` [PATCH 0/7] " Eugen Hristev
7 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-23 21:40 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Hans Verkuil
The try_fmt, try_crop and try_compose fields of the
v4l2_subdev_pad_config structure are misnamed (for historical reason) as
they also store data for the subdev active configuration. Rename them to
format, crop and compose respectively and update the accessor helpers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
include/media/v4l2-subdev.h | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index c1f90c1223a7..bb1aad264756 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -688,21 +688,18 @@ struct v4l2_subdev_ir_ops {
/**
* struct v4l2_subdev_pad_config - Used for storing subdev pad information.
*
- * @try_fmt: &struct v4l2_mbus_framefmt
- * @try_crop: &struct v4l2_rect to be used for crop
- * @try_compose: &struct v4l2_rect to be used for compose
+ * @format: &struct v4l2_mbus_framefmt
+ * @crop: &struct v4l2_rect to be used for crop
+ * @compose: &struct v4l2_rect to be used for compose
*
* This structure only needs to be passed to the pad op if the 'which' field
* of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
* %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
- *
- * Note: This struct is also used in active state, and the 'try' prefix is
- * historical and to be removed.
*/
struct v4l2_subdev_pad_config {
- struct v4l2_mbus_framefmt try_fmt;
- struct v4l2_rect try_crop;
- struct v4l2_rect try_compose;
+ struct v4l2_mbus_framefmt format;
+ struct v4l2_rect crop;
+ struct v4l2_rect compose;
};
/**
@@ -1145,8 +1142,8 @@ struct v4l2_subdev_fh {
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
/**
- * v4l2_subdev_get_pad_format - ancillary routine to call
- * &struct v4l2_subdev_pad_config->try_fmt
+ * v4l2_subdev_get_pad_format - ancillary routine to get
+ * &struct v4l2_subdev_pad_config->format
*
* @sd: pointer to &struct v4l2_subdev
* @state: pointer to &struct v4l2_subdev_state
@@ -1161,12 +1158,12 @@ v4l2_subdev_get_pad_format(struct v4l2_subdev *sd,
return NULL;
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
- return &state->pads[pad].try_fmt;
+ return &state->pads[pad].format;
}
/**
- * v4l2_subdev_get_pad_crop - ancillary routine to call
- * &struct v4l2_subdev_pad_config->try_crop
+ * v4l2_subdev_get_pad_crop - ancillary routine to get
+ * &struct v4l2_subdev_pad_config->crop
*
* @sd: pointer to &struct v4l2_subdev
* @state: pointer to &struct v4l2_subdev_state.
@@ -1181,12 +1178,12 @@ v4l2_subdev_get_pad_crop(struct v4l2_subdev *sd,
return NULL;
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
- return &state->pads[pad].try_crop;
+ return &state->pads[pad].crop;
}
/**
- * v4l2_subdev_get_pad_compose - ancillary routine to call
- * &struct v4l2_subdev_pad_config->try_compose
+ * v4l2_subdev_get_pad_compose - ancillary routine to get
+ * &struct v4l2_subdev_pad_config->compose
*
* @sd: pointer to &struct v4l2_subdev
* @state: pointer to &struct v4l2_subdev_state.
@@ -1201,7 +1198,7 @@ v4l2_subdev_get_pad_compose(struct v4l2_subdev *sd,
return NULL;
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
- return &state->pads[pad].try_compose;
+ return &state->pads[pad].compose;
}
/*
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields
2023-10-23 21:40 [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields Laurent Pinchart
` (6 preceding siblings ...)
2023-10-23 21:40 ` [PATCH 7/7] media: v4l2-subdev: Rename " Laurent Pinchart
@ 2023-10-24 6:55 ` Eugen Hristev
2023-10-24 8:41 ` Tomi Valkeinen
7 siblings, 1 reply; 13+ messages in thread
From: Eugen Hristev @ 2023-10-24 6:55 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Sakari Ailus, Hans Verkuil, Nicolas Ferre, Hans de Goede,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Luca Ceresoli, Leon Luo, Jacopo Mondi, Rui Miguel Silva,
Hans de Goede, Petr Cvek
On 10/24/23 00:40, Laurent Pinchart wrote:
> Hello,
Hello Laurent,
>
> This series is the result of me getting bothered by the following note
> in the documentation of the v4l2_subdev_pad_config structure:
>
> * Note: This struct is also used in active state, and the 'try' prefix is
> * historical and to be removed.
>
> So I decided to drop the prefix.
>
> Patches 1/7 to 6/7 replace direct usage of the fields in drivers with
> the corresponding accessor functions. There was a relatively large
> number of them in sensor drivers (in 6/7), but more worryingly, the
> atmel-isi (1/7), microchip-isc (2/7) and tegra-video (5/7) should really
> not have messed up with creating a v4l2_subdev_pad_config structure
> manually. I urge the maintainers of those drivers to address the issue.
Could you hint a bit about how the issue should be addressed ?
Instead of creating a `v4l2_subdev_pad_config`, one should use
v4l2_subdev_lock_and_get_active_state() ? Is this the right way to do it ?
Thanks for looking into this,
Eugen
>
> Finally, patch 7/7 renames the fields, which becomes easy after
> addressing all the drivers.
>
> The patches have been compile-tested only.
>
> Sakari, this conflicts with your "[PATCH v3 0/8] Unify sub-device state
> access functions" series. I don't mind rebasing on top if it gets merged
> first.
>
> Laurent Pinchart (7):
> media: atmel-isi: Use accessors for pad config 'try_*' fields
> media: microchip-isc: Use accessors for pad config 'try_*' fields
> media: atmel-isc: Use accessors for pad config 'try_*' fields
> media: atomisp: Use accessors for pad config 'try_*' fields
> media: tegra-video: Use accessors for pad config 'try_*' fields
> media: i2c: Use accessors for pad config 'try_*' fields
> media: v4l2-subdev: Rename pad config 'try_*' fields
>
> drivers/media/i2c/adv7183.c | 2 +-
> drivers/media/i2c/imx274.c | 12 +++----
> drivers/media/i2c/mt9m001.c | 2 +-
> drivers/media/i2c/mt9m111.c | 2 +-
> drivers/media/i2c/mt9t112.c | 2 +-
> drivers/media/i2c/mt9v011.c | 2 +-
> drivers/media/i2c/mt9v111.c | 2 +-
> drivers/media/i2c/ov2640.c | 2 +-
> drivers/media/i2c/ov2680.c | 4 +--
> drivers/media/i2c/ov6650.c | 34 ++++++++++++-------
> drivers/media/i2c/ov772x.c | 2 +-
> drivers/media/i2c/ov9640.c | 2 +-
> drivers/media/i2c/rj54n1cb0c.c | 2 +-
> drivers/media/i2c/saa6752hs.c | 2 +-
> drivers/media/i2c/tw9910.c | 2 +-
> drivers/media/platform/atmel/atmel-isi.c | 12 ++++---
> .../platform/microchip/microchip-isc-base.c | 10 +++---
> .../media/atomisp/i2c/atomisp-gc2235.c | 2 +-
> .../media/atomisp/i2c/atomisp-mt9m114.c | 2 +-
> .../media/atomisp/i2c/atomisp-ov2722.c | 2 +-
> .../staging/media/atomisp/pci/atomisp_tpg.c | 2 +-
> .../media/deprecated/atmel/atmel-isc-base.c | 10 +++---
> drivers/staging/media/tegra-video/vi.c | 14 ++++----
> include/media/v4l2-subdev.h | 33 ++++++++----------
> 24 files changed, 87 insertions(+), 74 deletions(-)
>
>
> base-commit: 94e27fbeca27d8c772fc2bc807730aaee5886055
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields
2023-10-24 6:55 ` [PATCH 0/7] " Eugen Hristev
@ 2023-10-24 8:41 ` Tomi Valkeinen
2023-10-24 8:47 ` Laurent Pinchart
0 siblings, 1 reply; 13+ messages in thread
From: Tomi Valkeinen @ 2023-10-24 8:41 UTC (permalink / raw)
To: Eugen Hristev, Laurent Pinchart, linux-media
Cc: Sakari Ailus, Hans Verkuil, Nicolas Ferre, Hans de Goede,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Luca Ceresoli, Leon Luo, Jacopo Mondi, Rui Miguel Silva,
Hans de Goede, Petr Cvek
On 24/10/2023 09:55, Eugen Hristev wrote:
> On 10/24/23 00:40, Laurent Pinchart wrote:
>> Hello,
>
> Hello Laurent,
>
>>
>> This series is the result of me getting bothered by the following note
>> in the documentation of the v4l2_subdev_pad_config structure:
>>
>> * Note: This struct is also used in active state, and the 'try'
>> prefix is
>> * historical and to be removed.
>>
>> So I decided to drop the prefix.
>>
>> Patches 1/7 to 6/7 replace direct usage of the fields in drivers with
>> the corresponding accessor functions. There was a relatively large
>> number of them in sensor drivers (in 6/7), but more worryingly, the
>> atmel-isi (1/7), microchip-isc (2/7) and tegra-video (5/7) should really
>> not have messed up with creating a v4l2_subdev_pad_config structure
>> manually. I urge the maintainers of those drivers to address the issue.
>
> Could you hint a bit about how the issue should be addressed ?
> Instead of creating a `v4l2_subdev_pad_config`, one should use
> v4l2_subdev_lock_and_get_active_state() ? Is this the right way to do it ?
I had a quick look at atmel-isi. If I understand it right, it does not
expose the subdevs to the userspace, and 'isi->entity.subdev' refers to
the sensor.
In that case I think using v4l2_subdev_call_state_active() and
v4l2_subdev_call_state_try() should usually work.
If there are cases where the same try state needs to be used for
multiple calls, then the state management has to be done manually with
__v4l2_subdev_state_alloc() and __v4l2_subdev_state_free() (e.g.
drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c).
Tomi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/7] media: v4l2-subdev: Rename pad config 'try_*' fields
2023-10-24 8:41 ` Tomi Valkeinen
@ 2023-10-24 8:47 ` Laurent Pinchart
0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2023-10-24 8:47 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Eugen Hristev, linux-media, Sakari Ailus, Hans Verkuil,
Nicolas Ferre, Hans de Goede, Thierry Reding, Jonathan Hunter,
Sowjanya Komatineni, Luca Ceresoli, Leon Luo, Jacopo Mondi,
Rui Miguel Silva, Hans de Goede, Petr Cvek
On Tue, Oct 24, 2023 at 11:41:57AM +0300, Tomi Valkeinen wrote:
> On 24/10/2023 09:55, Eugen Hristev wrote:
> > On 10/24/23 00:40, Laurent Pinchart wrote:
> >> Hello,
> >
> > Hello Laurent,
> >
> >>
> >> This series is the result of me getting bothered by the following note
> >> in the documentation of the v4l2_subdev_pad_config structure:
> >>
> >> * Note: This struct is also used in active state, and the 'try'
> >> prefix is
> >> * historical and to be removed.
> >>
> >> So I decided to drop the prefix.
> >>
> >> Patches 1/7 to 6/7 replace direct usage of the fields in drivers with
> >> the corresponding accessor functions. There was a relatively large
> >> number of them in sensor drivers (in 6/7), but more worryingly, the
> >> atmel-isi (1/7), microchip-isc (2/7) and tegra-video (5/7) should really
> >> not have messed up with creating a v4l2_subdev_pad_config structure
> >> manually. I urge the maintainers of those drivers to address the issue.
> >
> > Could you hint a bit about how the issue should be addressed ?
> > Instead of creating a `v4l2_subdev_pad_config`, one should use
> > v4l2_subdev_lock_and_get_active_state() ? Is this the right way to do it ?
>
> I had a quick look at atmel-isi. If I understand it right, it does not
> expose the subdevs to the userspace, and 'isi->entity.subdev' refers to
> the sensor.
>
> In that case I think using v4l2_subdev_call_state_active() and
> v4l2_subdev_call_state_try() should usually work.
>
> If there are cases where the same try state needs to be used for
> multiple calls, then the state management has to be done manually with
> __v4l2_subdev_state_alloc() and __v4l2_subdev_state_free() (e.g.
> drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c).
And for the microchip-isc driver, my understanding is that it's an
MC-centric driver. It should therefore not call the sensor's
.enum_frame_size() operation, which would remove the stack-allocated
state in the isc_validate() function.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread