* [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state
@ 2023-11-26 2:09 Laurent Pinchart
2023-12-05 9:32 ` Tomi Valkeinen
2023-12-05 9:57 ` Paul Elder
0 siblings, 2 replies; 3+ messages in thread
From: Laurent Pinchart @ 2023-11-26 2:09 UTC (permalink / raw)
To: linux-media; +Cc: Dafna Hirschfeld, Paul Elder, Kieran Bingham, linux-rockchip
Supported media bus codes on the resizer sink pad are identical to the
ISP source pad. The .enum_mbus_code() handler thus delegates the
enumeration to the ISP's operation. This is problematic for two
reasons:
- Format enumeration on the ISP source pad is dependent on the format
configured on the ISP sink pad for the same subdev state (TRY or
ACTIVE), while format enumeration on the resizer sink pad should
return all formats supported by the resizer subdev, regardless of the
ISP configuration.
- Delegating the operation involves creating a fake v4l2_subdev_state on
the stack to pass to the ISP .enum_mbus_code() handler. This gets in
the way of evolution of both the ISP enumeration handler and, more
generally, the V4L2 subdev state infrastructure.
Fix those two issues by implementing format enumeration manually for the
resizer.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Paul, Kieran, would you be able to test this for regressions ?
---
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 38 ++++++++++++-------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index f7af360dcb28..a8e377701302 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -330,12 +330,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
{
struct rkisp1_resizer *rsz =
container_of(sd, struct rkisp1_resizer, sd);
- struct v4l2_subdev_pad_config dummy_cfg;
- struct v4l2_subdev_state pad_state = {
- .pads = &dummy_cfg
- };
- u32 pad = code->pad;
- int ret;
+ unsigned int index = code->index;
+ unsigned int i;
if (code->pad == RKISP1_RSZ_PAD_SRC) {
/* supported mbus codes on the src are the same as in the capture */
@@ -355,15 +351,29 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
}
- /* supported mbus codes on the sink pad are the same as isp src pad */
- code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
- ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
- &pad_state, code);
+ /*
+ * Supported mbus codes on the sink pad are the same as on the ISP
+ * source pad.
+ */
+ for (i = 0; ; i++) {
+ const struct rkisp1_mbus_info *fmt =
+ rkisp1_mbus_info_get_by_index(i);
- /* restore pad */
- code->pad = pad;
- code->flags = 0;
- return ret;
+ if (!fmt)
+ break;
+
+ if (!(fmt->direction & RKISP1_ISP_SD_SRC))
+ continue;
+
+ if (!index) {
+ code->code = fmt->mbus_code;
+ return 0;
+ }
+
+ index--;
+ }
+
+ return -EINVAL;
}
static int rkisp1_rsz_init_state(struct v4l2_subdev *sd,
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state
2023-11-26 2:09 [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state Laurent Pinchart
@ 2023-12-05 9:32 ` Tomi Valkeinen
2023-12-05 9:57 ` Paul Elder
1 sibling, 0 replies; 3+ messages in thread
From: Tomi Valkeinen @ 2023-12-05 9:32 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Dafna Hirschfeld, Paul Elder, Kieran Bingham, linux-rockchip
On 26/11/2023 04:09, Laurent Pinchart wrote:
> Supported media bus codes on the resizer sink pad are identical to the
> ISP source pad. The .enum_mbus_code() handler thus delegates the
> enumeration to the ISP's operation. This is problematic for two
> reasons:
>
> - Format enumeration on the ISP source pad is dependent on the format
> configured on the ISP sink pad for the same subdev state (TRY or
> ACTIVE), while format enumeration on the resizer sink pad should
> return all formats supported by the resizer subdev, regardless of the
> ISP configuration.
>
> - Delegating the operation involves creating a fake v4l2_subdev_state on
> the stack to pass to the ISP .enum_mbus_code() handler. This gets in
> the way of evolution of both the ISP enumeration handler and, more
> generally, the V4L2 subdev state infrastructure.
>
> Fix those two issues by implementing format enumeration manually for the
> resizer.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Paul, Kieran, would you be able to test this for regressions ?
> ---
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 38 ++++++++++++-------
> 1 file changed, 24 insertions(+), 14 deletions(-)
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
I tested on my i.MX8MP board, although my kernel branch has quite a bit
of non-upstream patches to get everything working. But I don't think
those affect this patch.
Tomi
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> index f7af360dcb28..a8e377701302 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> @@ -330,12 +330,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
> {
> struct rkisp1_resizer *rsz =
> container_of(sd, struct rkisp1_resizer, sd);
> - struct v4l2_subdev_pad_config dummy_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &dummy_cfg
> - };
> - u32 pad = code->pad;
> - int ret;
> + unsigned int index = code->index;
> + unsigned int i;
>
> if (code->pad == RKISP1_RSZ_PAD_SRC) {
> /* supported mbus codes on the src are the same as in the capture */
> @@ -355,15 +351,29 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
> return 0;
> }
>
> - /* supported mbus codes on the sink pad are the same as isp src pad */
> - code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
> - ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
> - &pad_state, code);
> + /*
> + * Supported mbus codes on the sink pad are the same as on the ISP
> + * source pad.
> + */
> + for (i = 0; ; i++) {
> + const struct rkisp1_mbus_info *fmt =
> + rkisp1_mbus_info_get_by_index(i);
>
> - /* restore pad */
> - code->pad = pad;
> - code->flags = 0;
> - return ret;
> + if (!fmt)
> + break;
> +
> + if (!(fmt->direction & RKISP1_ISP_SD_SRC))
> + continue;
> +
> + if (!index) {
> + code->code = fmt->mbus_code;
> + return 0;
> + }
> +
> + index--;
> + }
> +
> + return -EINVAL;
> }
>
> static int rkisp1_rsz_init_state(struct v4l2_subdev *sd,
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state
2023-11-26 2:09 [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state Laurent Pinchart
2023-12-05 9:32 ` Tomi Valkeinen
@ 2023-12-05 9:57 ` Paul Elder
1 sibling, 0 replies; 3+ messages in thread
From: Paul Elder @ 2023-12-05 9:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dafna Hirschfeld, Kieran Bingham, linux-rockchip
On Sun, Nov 26, 2023 at 04:09:48AM +0200, Laurent Pinchart wrote:
> Supported media bus codes on the resizer sink pad are identical to the
> ISP source pad. The .enum_mbus_code() handler thus delegates the
> enumeration to the ISP's operation. This is problematic for two
> reasons:
>
> - Format enumeration on the ISP source pad is dependent on the format
> configured on the ISP sink pad for the same subdev state (TRY or
> ACTIVE), while format enumeration on the resizer sink pad should
> return all formats supported by the resizer subdev, regardless of the
> ISP configuration.
>
> - Delegating the operation involves creating a fake v4l2_subdev_state on
> the stack to pass to the ISP .enum_mbus_code() handler. This gets in
> the way of evolution of both the ISP enumeration handler and, more
> generally, the V4L2 subdev state infrastructure.
>
> Fix those two issues by implementing format enumeration manually for the
> resizer.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Paul, Kieran, would you be able to test this for regressions ?
> ---
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 38 ++++++++++++-------
> 1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> index f7af360dcb28..a8e377701302 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> @@ -330,12 +330,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
> {
> struct rkisp1_resizer *rsz =
> container_of(sd, struct rkisp1_resizer, sd);
> - struct v4l2_subdev_pad_config dummy_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &dummy_cfg
> - };
> - u32 pad = code->pad;
> - int ret;
> + unsigned int index = code->index;
> + unsigned int i;
>
> if (code->pad == RKISP1_RSZ_PAD_SRC) {
> /* supported mbus codes on the src are the same as in the capture */
> @@ -355,15 +351,29 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
> return 0;
> }
>
> - /* supported mbus codes on the sink pad are the same as isp src pad */
> - code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
> - ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
> - &pad_state, code);
> + /*
> + * Supported mbus codes on the sink pad are the same as on the ISP
> + * source pad.
> + */
> + for (i = 0; ; i++) {
> + const struct rkisp1_mbus_info *fmt =
> + rkisp1_mbus_info_get_by_index(i);
>
> - /* restore pad */
> - code->pad = pad;
> - code->flags = 0;
> - return ret;
> + if (!fmt)
> + break;
> +
> + if (!(fmt->direction & RKISP1_ISP_SD_SRC))
> + continue;
> +
> + if (!index) {
> + code->code = fmt->mbus_code;
> + return 0;
> + }
> +
> + index--;
> + }
> +
> + return -EINVAL;
> }
>
> static int rkisp1_rsz_init_state(struct v4l2_subdev *sd,
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-05 9:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-26 2:09 [PATCH] media: rkisp1: resizer: Stop manual allocation of v4l2_subdev_state Laurent Pinchart
2023-12-05 9:32 ` Tomi Valkeinen
2023-12-05 9:57 ` Paul Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox