From: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
Bingbu Cao <bingbu.cao@intel.com>
Subject: Re: [PATCH] media: intel/ipu6: Move isys_subdev functions to common code
Date: Tue, 1 Apr 2025 11:37:24 +0200 [thread overview]
Message-ID: <Z+uz1DPz3gI48+L1@linux.intel.com> (raw)
In-Reply-To: <20250401090953.473339-2-stanislaw.gruszka@linux.intel.com>
This one should be dropped from the set. Send accidentally.
Sorry about that.
Regards
Stanislaw
On Tue, Apr 01, 2025 at 11:09:48AM +0200, Stanislaw Gruszka wrote:
> Move more functions (those that utilize isys_subdev structure internally),
> to common code to be shared with ipu7 driver.
>
> Additionally use isd variable name for isys_subdevice, asd name is used
> for historical reason - precursor of the drier was atomisp driver, the
> variables names stayed unchanged, what is a little confusing.
>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 8 +-
> .../media/pci/intel/ipu6/ipu6-isys-subdev.c | 179 +-----------------
> .../media/pci/intel/ipu6/ipu6-isys-subdev.h | 11 +-
> drivers/media/pci/intel/ipu6/isys-subdev.c | 159 ++++++++++++++++
> drivers/media/pci/intel/ipu6/isys.h | 9 +
> 5 files changed, 174 insertions(+), 192 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> index 2441f47e8742..5c4baaffa092 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> @@ -487,7 +487,7 @@ static const struct v4l2_subdev_pad_ops csi2_sd_pad_ops = {
> .set_fmt = ipu6_isys_subdev_set_fmt,
> .get_selection = ipu6_isys_csi2_get_sel,
> .set_selection = ipu6_isys_csi2_set_sel,
> - .enum_mbus_code = ipu6_isys_subdev_enum_mbus_code,
> + .enum_mbus_code = isys_subdev_enum_mbus_code,
> .set_routing = isys_subdev_set_routing,
> .enable_streams = ipu6_isys_csi2_enable_streams,
> .disable_streams = ipu6_isys_csi2_disable_streams,
> @@ -510,7 +510,7 @@ void ipu6_isys_csi2_cleanup(struct ipu6_isys_csi2 *csi2)
>
> v4l2_device_unregister_subdev(&csi2->asd.sd);
> v4l2_subdev_cleanup(&csi2->asd.sd);
> - ipu6_isys_subdev_cleanup(&csi2->asd);
> + isys_subdev_cleanup(&csi2->asd);
> csi2->isys = NULL;
> }
>
> @@ -526,8 +526,8 @@ int ipu6_isys_csi2_init(struct ipu6_isys_csi2 *csi2,
> csi2->port = index;
>
> csi2->asd.sd.entity.ops = &csi2_entity_ops;
> - ret = ipu6_isys_subdev_init(&csi2->asd, dev, &csi2_sd_ops, 0,
> - NR_OF_CSI2_SINK_PADS, NR_OF_CSI2_SRC_PADS);
> + ret = isys_subdev_init(&csi2->asd, dev, &csi2_sd_ops, 0,
> + NR_OF_CSI2_SINK_PADS, NR_OF_CSI2_SRC_PADS);
> if (ret)
> goto fail;
>
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
> index 020c25925ca0..655057d54785 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
> @@ -141,14 +141,6 @@ int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *format)
> {
> - struct isys_subdev *asd = to_isys_subdev(sd);
> - struct v4l2_mbus_framefmt *fmt;
> - struct v4l2_rect *crop;
> - u32 code = asd->supported_codes[0];
> - u32 other_pad, other_stream;
> - unsigned int i;
> - int ret;
> -
> /* No transcoding, source and sink formats must match. */
> if ((sd->entity.pads[format->pad].flags & MEDIA_PAD_FL_SOURCE) &&
> sd->entity.num_pads > 1)
> @@ -160,174 +152,5 @@ int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
> IPU6_ISYS_MIN_HEIGHT,
> IPU6_ISYS_MAX_HEIGHT);
>
> - for (i = 0; asd->supported_codes[i]; i++) {
> - if (asd->supported_codes[i] == format->format.code) {
> - code = asd->supported_codes[i];
> - break;
> - }
> - }
> - format->format.code = code;
> - format->format.field = V4L2_FIELD_NONE;
> -
> - /* Store the format and propagate it to the source pad. */
> - fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> - if (!fmt)
> - return -EINVAL;
> -
> - *fmt = format->format;
> -
> - if (!(sd->entity.pads[format->pad].flags & MEDIA_PAD_FL_SINK))
> - return 0;
> -
> - /* propagate format to following source pad */
> - fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> - format->stream);
> - if (!fmt)
> - return -EINVAL;
> -
> - *fmt = format->format;
> -
> - ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
> - format->pad,
> - format->stream,
> - &other_pad,
> - &other_stream);
> - if (ret)
> - return -EINVAL;
> -
> - crop = v4l2_subdev_state_get_crop(state, other_pad, other_stream);
> - /* reset crop */
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->width;
> - crop->height = fmt->height;
> -
> - return 0;
> -}
> -
> -int ipu6_isys_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - struct v4l2_subdev_mbus_code_enum *code)
> -{
> - struct isys_subdev *asd = to_isys_subdev(sd);
> - const u32 *supported_codes = asd->supported_codes;
> - u32 index;
> -
> - for (index = 0; supported_codes[index]; index++) {
> - if (index == code->index) {
> - code->code = supported_codes[index];
> - return 0;
> - }
> - }
> -
> - return -EINVAL;
> -}
> -
> -static int subdev_set_routing(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - struct v4l2_subdev_krouting *routing)
> -{
> - static const struct v4l2_mbus_framefmt format = {
> - .width = 4096,
> - .height = 3072,
> - .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> - .field = V4L2_FIELD_NONE,
> - };
> - int ret;
> -
> - ret = v4l2_subdev_routing_validate(sd, routing,
> - V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> - if (ret)
> - return ret;
> -
> - return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> -}
> -
> -static int ipu6_isys_subdev_init_state(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state)
> -{
> - struct v4l2_subdev_route route = {
> - .sink_pad = 0,
> - .sink_stream = 0,
> - .source_pad = 1,
> - .source_stream = 0,
> - .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> - };
> - struct v4l2_subdev_krouting routing = {
> - .num_routes = 1,
> - .routes = &route,
> - };
> -
> - return subdev_set_routing(sd, state, &routing);
> -}
> -
> -static const struct v4l2_subdev_internal_ops ipu6_isys_subdev_internal_ops = {
> - .init_state = ipu6_isys_subdev_init_state,
> -};
> -
> -int ipu6_isys_subdev_init(struct isys_subdev *asd, struct device *dev,
> - const struct v4l2_subdev_ops *ops,
> - unsigned int nr_ctrls, unsigned int num_sink_pads,
> - unsigned int num_source_pads)
> -{
> - unsigned int num_pads = num_sink_pads + num_source_pads;
> - unsigned int i;
> - int ret;
> -
> - v4l2_subdev_init(&asd->sd, ops);
> -
> - asd->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> - V4L2_SUBDEV_FL_HAS_EVENTS |
> - V4L2_SUBDEV_FL_STREAMS;
> - asd->sd.owner = THIS_MODULE;
> - asd->sd.dev = dev;
> - asd->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> - asd->sd.internal_ops = &ipu6_isys_subdev_internal_ops;
> -
> - asd->pad = devm_kcalloc(dev, num_pads, sizeof(*asd->pad), GFP_KERNEL);
> - if (!asd->pad)
> - return -ENOMEM;
> -
> - for (i = 0; i < num_sink_pads; i++)
> - asd->pad[i].flags = MEDIA_PAD_FL_SINK |
> - MEDIA_PAD_FL_MUST_CONNECT;
> -
> - for (i = num_sink_pads; i < num_pads; i++)
> - asd->pad[i].flags = MEDIA_PAD_FL_SOURCE;
> -
> - ret = media_entity_pads_init(&asd->sd.entity, num_pads, asd->pad);
> - if (ret)
> - return ret;
> -
> - if (asd->ctrl_init) {
> - ret = v4l2_ctrl_handler_init(&asd->ctrl_handler, nr_ctrls);
> - if (ret)
> - goto out_media_entity_cleanup;
> -
> - asd->ctrl_init(&asd->sd);
> - if (asd->ctrl_handler.error) {
> - ret = asd->ctrl_handler.error;
> - goto out_v4l2_ctrl_handler_free;
> - }
> -
> - asd->sd.ctrl_handler = &asd->ctrl_handler;
> - }
> -
> - asd->source = -1;
> -
> - return 0;
> -
> -out_v4l2_ctrl_handler_free:
> - v4l2_ctrl_handler_free(&asd->ctrl_handler);
> -
> -out_media_entity_cleanup:
> - media_entity_cleanup(&asd->sd.entity);
> -
> - return ret;
> -}
> -
> -void ipu6_isys_subdev_cleanup(struct isys_subdev *asd)
> -{
> - media_entity_cleanup(&asd->sd.entity);
> - v4l2_ctrl_handler_free(&asd->ctrl_handler);
> + return isys_subdev_set_fmt(sd, state, format);
> }
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
> index f3835d873991..e8d1ff181a9d 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
> @@ -1,5 +1,5 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> -/* Copyright (C) 2013--2024 Intel Corporation */
> +/* Copyright (C) 2013-2025 Intel Corporation */
>
> #ifndef IPU6_ISYS_SUBDEV_H
> #define IPU6_ISYS_SUBDEV_H
> @@ -16,13 +16,4 @@ u32 ipu6_isys_convert_bayer_order(u32 code, int x, int y);
> int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *fmt);
> -int ipu6_isys_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - struct v4l2_subdev_mbus_code_enum
> - *code);
> -int ipu6_isys_subdev_init(struct isys_subdev *asd, struct device *dev,
> - const struct v4l2_subdev_ops *ops,
> - unsigned int nr_ctrls, unsigned int num_sink_pads,
> - unsigned int num_source_pads);
> -void ipu6_isys_subdev_cleanup(struct isys_subdev *asd);
> #endif /* IPU6_ISYS_SUBDEV_H */
> diff --git a/drivers/media/pci/intel/ipu6/isys-subdev.c b/drivers/media/pci/intel/ipu6/isys-subdev.c
> index 483d718f2ea4..7635d768067f 100644
> --- a/drivers/media/pci/intel/ipu6/isys-subdev.c
> +++ b/drivers/media/pci/intel/ipu6/isys-subdev.c
> @@ -92,3 +92,162 @@ int isys_subdev_set_routing(struct v4l2_subdev *sd,
> {
> return subdev_set_routing(sd, state, routing);
> }
> +
> +int isys_subdev_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format)
> +{
> + struct isys_subdev *isd = to_isys_subdev(sd);
> + struct v4l2_mbus_framefmt *fmt;
> + struct v4l2_rect *crop;
> + u32 code = isd->supported_codes[0];
> + u32 other_pad, other_stream;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; isd->supported_codes[i]; i++) {
> + if (isd->supported_codes[i] == format->format.code) {
> + code = isd->supported_codes[i];
> + break;
> + }
> + }
> + format->format.code = code;
> + format->format.field = V4L2_FIELD_NONE;
> +
> + /* Store the format and propagate it to the source pad. */
> + fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> + *fmt = format->format;
> +
> + if (!(sd->entity.pads[format->pad].flags & MEDIA_PAD_FL_SINK))
> + return 0;
> +
> + /* propagate format to following source pad */
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> + format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> + *fmt = format->format;
> +
> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
> + format->pad, format->stream,
> + &other_pad, &other_stream);
> + if (ret)
> + return -EINVAL;
> +
> + crop = v4l2_subdev_state_get_crop(state, other_pad, other_stream);
> + /* reset crop */
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->width;
> + crop->height = fmt->height;
> +
> + return 0;
> +}
> +
> +int isys_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct isys_subdev *isd = to_isys_subdev(sd);
> + const u32 *supported_codes = isd->supported_codes;
> + u32 index;
> +
> + for (index = 0; supported_codes[index]; index++) {
> + if (index == code->index) {
> + code->code = supported_codes[index];
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int isys_subdev_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct v4l2_subdev_route route = {
> + .sink_pad = 0,
> + .sink_stream = 0,
> + .source_pad = 1,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + };
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = 1,
> + .routes = &route,
> + };
> +
> + return subdev_set_routing(sd, state, &routing);
> +}
> +
> +static const struct v4l2_subdev_internal_ops isys_subdev_internal_ops = {
> + .init_state = isys_subdev_init_state,
> +};
> +
> +int isys_subdev_init(struct isys_subdev *isd, struct device *dev,
> + const struct v4l2_subdev_ops *ops, unsigned int nr_ctrls,
> + unsigned int num_sink_pads, unsigned int num_source_pads)
> +{
> + unsigned int num_pads = num_sink_pads + num_source_pads;
> + unsigned int i;
> + int ret;
> +
> + v4l2_subdev_init(&isd->sd, ops);
> +
> + isd->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_STREAMS;
> + isd->sd.owner = THIS_MODULE;
> + isd->sd.dev = dev;
> + isd->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> + isd->sd.internal_ops = &isys_subdev_internal_ops;
> +
> + isd->pad = devm_kcalloc(dev, num_pads, sizeof(*isd->pad), GFP_KERNEL);
> + if (!isd->pad)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_sink_pads; i++)
> + isd->pad[i].flags = MEDIA_PAD_FL_SINK |
> + MEDIA_PAD_FL_MUST_CONNECT;
> +
> + for (i = num_sink_pads; i < num_pads; i++)
> + isd->pad[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&isd->sd.entity, num_pads, isd->pad);
> + if (ret)
> + return ret;
> +
> + if (isd->ctrl_init) {
> + ret = v4l2_ctrl_handler_init(&isd->ctrl_handler, nr_ctrls);
> + if (ret)
> + goto out_media_entity_cleanup;
> +
> + isd->ctrl_init(&isd->sd);
> + if (isd->ctrl_handler.error) {
> + ret = isd->ctrl_handler.error;
> + goto out_v4l2_ctrl_handler_free;
> + }
> +
> + isd->sd.ctrl_handler = &isd->ctrl_handler;
> + }
> +
> + isd->source = -1;
> +
> + return 0;
> +
> +out_v4l2_ctrl_handler_free:
> + v4l2_ctrl_handler_free(&isd->ctrl_handler);
> +
> +out_media_entity_cleanup:
> + media_entity_cleanup(&isd->sd.entity);
> +
> + return ret;
> +}
> +
> +void isys_subdev_cleanup(struct isys_subdev *isd)
> +{
> + media_entity_cleanup(&isd->sd.entity);
> + v4l2_ctrl_handler_free(&isd->ctrl_handler);
> +}
> diff --git a/drivers/media/pci/intel/ipu6/isys.h b/drivers/media/pci/intel/ipu6/isys.h
> index 76f64439952b..4bde32f6d767 100644
> --- a/drivers/media/pci/intel/ipu6/isys.h
> +++ b/drivers/media/pci/intel/ipu6/isys.h
> @@ -30,4 +30,13 @@ int isys_subdev_set_routing(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> enum v4l2_subdev_format_whence which,
> struct v4l2_subdev_krouting *routing);
> +int isys_subdev_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format);
> +int isys_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_mbus_code_enum *code);
> +int isys_subdev_init(struct isys_subdev *asd, struct device *dev,
> + const struct v4l2_subdev_ops *ops, unsigned int nr_ctrls,
> + unsigned int num_sink_pads, unsigned int num_source_pads);
> +void isys_subdev_cleanup(struct isys_subdev *asd);
> #endif
> --
> 2.34.1
>
>
next prev parent reply other threads:[~2025-04-01 9:37 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-01 9:09 [PATCH 0/5] media: intel/ipu6: initial ipu7 code sharing preparation Stanislaw Gruszka
2025-04-01 9:09 ` [PATCH] media: intel/ipu6: Move isys_subdev functions to common code Stanislaw Gruszka
2025-04-01 9:37 ` Stanislaw Gruszka [this message]
2025-04-01 9:09 ` [PATCH 1/5] media: intel/ipu6: Separate ipu6 subdev functions Stanislaw Gruszka
2025-04-01 9:09 ` [PATCH 2/5] media: intel/ipu6: Remove ipu6_isys dependency from ipu6_isys_subdev Stanislaw Gruszka
2025-04-01 9:09 ` [PATCH 3/5] media: intel/ipu6: Remove redundant ipu6_isys_subdev_to_csi2 macro Stanislaw Gruszka
2025-04-01 9:09 ` [PATCH 4/5] media: intel/ipu6: Rename ipu6_isys_subdev Stanislaw Gruszka
2025-04-01 9:09 ` [PATCH 5/5] media: intel/ipu6: Move isys_subdev functions to common code Stanislaw Gruszka
2025-04-08 11:01 ` Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z+uz1DPz3gI48+L1@linux.intel.com \
--to=stanislaw.gruszka@linux.intel.com \
--cc=bingbu.cao@intel.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox