This series is a continuation of the series that converts the atmel-isc driver
to media controller paradigm:
https://lore.kernel.org/lkml/[email protected]/T/#mccad96a3122f2817bf1ae1db7eddf1a8cecb2749
As discussed on ML, the current Atmel ISC driver is moved to staging to keep the
existing users happy, and readded to platform/microchip under a different
Kconfig symbol and with the new media controller support which was reviewed in
v10.
I kept the original v10 patches on top of the movement of the driver
to make it more clear what the conversion to media controller brings.
Please let me know if this is okay or acceptable to take as it is,
and if it complies with the requirements for the subsystem/ABI breakage, etc.
Thanks to everyone for reviewing and helping in the discussions !
PLEASE NOTE: the series depends on the patch:
vb2: add support for (un)prepare_streaming queue ops
by Hans Verkuil
I have a different patch as the last in the series that uses the new callbacks
(and only that patch is dependant on the
vb2: add support for (un)prepare_streaming queue ops
)
Eugen
Changes in v12:
- moved patch 'move to staging' as last patch in the series
- renamed exported symbols by Microchip ISC by adding prefix microchip_
- added TODO in staging dir
- make old ISC kconfigs dependant to !VIDEO_MICROCHIP_ISC_BASE
- move to staging deprecated dir
Changes in v11:
- moved atmel isc to staging
- created platform/microchip to host the new MICROCHIP_ISC driver
- it was natural to move the MICROCHIP_CSI2DC here
- on top, add the patches that move to media controller
Full series history:
Changes in v10:
-> split the series into this first fixes part.
-> moved IO_MC addition from first patch to the second patch on the driver changes
-> edited commit messages
-> DT nodes now disabled by default.
Changes in v9:
-> kernel robot reported isc_link_validate is not static, changed to static.
Changes in v8:
-> scaler: modified crop bounds to have the exact source size
Changes in v7:
-> scaler: modified crop bounds to have maximum isc size
-> format propagation: did small changes as per Jacopo review
Changes in v6:
-> worked a bit on scaler, added try crop and other changes as per Jacopo review
-> worked on isc-base enum_fmt , reworked as per Jacopo review
Changes in v5:
-> removed patch that removed the 'stop' variable as it was still required
-> added two new trivial patches
-> reworked some parts of the scaler and format propagation after discussions with Jacopo
Changes in v4:
-> as reviewed by Hans, added new patch to remove the 'stop' variable and reworked
one patch that was using it
-> as reviewed by Jacopo, reworked some parts of the media controller implementation
Changes in v3:
- change in bindings, small fixes in csi2dc driver and conversion to mc
for the isc-base.
- removed some MAINTAINERS patches and used patterns in MAINTAINERS
Changes in v2:
- integrated many changes suggested by Jacopo in the review of the v1 series.
- add a few new patches
Eugen Hristev (6):
media: atmel: move microchip_csi2dc to dedicated microchip platform
media: microchip: add ISC driver as Microchip ISC
media: microchip: microchip-isc: prepare for media controller support
media: microchip: microchip-isc: implement media controller
media: microchip: microchip-isc: move media_pipeline_* to (un)prepare
cb
media: atmel: atmel-isc: move to staging
MAINTAINERS | 8 +-
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/atmel/Kconfig | 51 -
drivers/media/platform/atmel/Makefile | 7 -
drivers/media/platform/microchip/Kconfig | 61 +
drivers/media/platform/microchip/Makefile | 9 +
.../{atmel => microchip}/microchip-csi2dc.c | 0
.../platform/microchip/microchip-isc-base.c | 2040 +++++++++++++++++
.../platform/microchip/microchip-isc-clk.c | 311 +++
.../platform/microchip/microchip-isc-regs.h | 413 ++++
.../platform/microchip/microchip-isc-scaler.c | 267 +++
.../media/platform/microchip/microchip-isc.h | 400 ++++
.../microchip/microchip-sama5d2-isc.c | 683 ++++++
.../microchip/microchip-sama7g5-isc.c | 646 ++++++
drivers/staging/media/Kconfig | 1 +
drivers/staging/media/Makefile | 1 +
.../staging/media/deprecated/atmel/Kconfig | 48 +
.../staging/media/deprecated/atmel/Makefile | 8 +
drivers/staging/media/deprecated/atmel/TODO | 34 +
.../media/deprecated}/atmel/atmel-isc-base.c | 20 +-
.../media/deprecated}/atmel/atmel-isc-clk.c | 8 +-
.../media/deprecated}/atmel/atmel-isc-regs.h | 0
.../media/deprecated}/atmel/atmel-isc.h | 16 +-
.../deprecated}/atmel/atmel-sama5d2-isc.c | 18 +-
.../deprecated}/atmel/atmel-sama7g5-isc.c | 18 +-
26 files changed, 4969 insertions(+), 101 deletions(-)
create mode 100644 drivers/media/platform/microchip/Kconfig
create mode 100644 drivers/media/platform/microchip/Makefile
rename drivers/media/platform/{atmel => microchip}/microchip-csi2dc.c (100%)
create mode 100644 drivers/media/platform/microchip/microchip-isc-base.c
create mode 100644 drivers/media/platform/microchip/microchip-isc-clk.c
create mode 100644 drivers/media/platform/microchip/microchip-isc-regs.h
create mode 100644 drivers/media/platform/microchip/microchip-isc-scaler.c
create mode 100644 drivers/media/platform/microchip/microchip-isc.h
create mode 100644 drivers/media/platform/microchip/microchip-sama5d2-isc.c
create mode 100644 drivers/media/platform/microchip/microchip-sama7g5-isc.c
create mode 100644 drivers/staging/media/deprecated/atmel/Kconfig
create mode 100644 drivers/staging/media/deprecated/atmel/Makefile
create mode 100644 drivers/staging/media/deprecated/atmel/TODO
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-isc-base.c (99%)
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-isc-clk.c (97%)
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-isc-regs.h (100%)
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-isc.h (96%)
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-sama5d2-isc.c (97%)
rename drivers/{media/platform => staging/media/deprecated}/atmel/atmel-sama7g5-isc.c (97%)
--
2.25.1
Prepare the support for media-controller.
This means that the capabilities of the driver have changed and now it's
capable of media controller operations.
The driver will register its media device, and add the video entity to this
media device. The subdevices are registered to the same media device.
The ISC will have a base entity which is auto-detected as
microchip_isc_base.
It will also register a subdevice that allows cropping of the incoming
frame to the maximum frame size supported by the ISC.
The ISC will create a link between the subdevice that is asynchronously
registered and the microchip_isc_scaler entity.
Then, the microchip_isc_scaler and microchip_isc_base are connected
through another link.
This patch does not change the previous capability of the driver, the
fact that the format is still being propagated from the top video node
down to the sensor.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
Changes in v12:
- all exported symbols now prefixed by microchip_
Changes in v11:
- changed to apply on top of MICROCHIP_ISC
- changed naming s/Atmel/Microchip
Changes in v10:
- edited commit message, and patch name
- moved IO_MC to another patch
Changes in v8:
- use source format size as bounds always
Changes in v7:
- use maximum isc frame size as bounds always
Changes in v6:
- reworked a bit as suggested by Jacopo
- add try crops
Changes in v5:
- reworked s_fmt to pass the same format from sink to source
- simplified enum_mbus_code
- separated tgt and bounds to report correctly in g_sel
Changes in v4:
As suggested by Jacopo:
- renamed atmel_isc_mc to atmel_isc_scaler.c
- moved init_mc/clean_mc to isc_base file
Changes in v2:
- implement try formats
drivers/media/platform/microchip/Makefile | 2 +-
.../platform/microchip/microchip-isc-base.c | 72 ++++-
.../platform/microchip/microchip-isc-scaler.c | 262 ++++++++++++++++++
.../media/platform/microchip/microchip-isc.h | 37 +++
.../microchip/microchip-sama5d2-isc.c | 12 +-
.../microchip/microchip-sama7g5-isc.c | 12 +-
6 files changed, 391 insertions(+), 6 deletions(-)
create mode 100644 drivers/media/platform/microchip/microchip-isc-scaler.c
diff --git a/drivers/media/platform/microchip/Makefile b/drivers/media/platform/microchip/Makefile
index 498a0fafc1b3..bd8d6e779c51 100644
--- a/drivers/media/platform/microchip/Makefile
+++ b/drivers/media/platform/microchip/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
microchip-isc-objs = microchip-sama5d2-isc.o
microchip-xisc-objs = microchip-sama7g5-isc.o
-microchip-isc-common-objs = microchip-isc-base.o microchip-isc-clk.o
+microchip-isc-common-objs = microchip-isc-base.o microchip-isc-clk.o microchip-isc-scaler.o
obj-$(CONFIG_VIDEO_MICROCHIP_ISC_BASE) += microchip-isc-common.o
obj-$(CONFIG_VIDEO_MICROCHIP_ISC) += microchip-isc.o
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 8b5318fb72f3..9034d8f97831 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -1732,6 +1732,7 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier,
struct isc_device, v4l2_dev);
struct isc_subdev_entity *subdev_entity =
container_of(notifier, struct isc_subdev_entity, notifier);
+ int pad;
if (video_is_registered(&isc->video_dev)) {
v4l2_err(&isc->v4l2_dev, "only supports one sub-device.\n");
@@ -1740,6 +1741,16 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier,
subdev_entity->sd = subdev;
+ pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
+ MEDIA_PAD_FL_SOURCE);
+ if (pad < 0) {
+ v4l2_err(&isc->v4l2_dev, "failed to find pad for %s\n",
+ subdev->name);
+ return pad;
+ }
+
+ isc->remote_pad = pad;
+
return 0;
}
@@ -1755,8 +1766,8 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
v4l2_ctrl_handler_free(&isc->ctrls.handler);
}
-static struct isc_format *find_format_by_code(struct isc_device *isc,
- unsigned int code, int *index)
+struct isc_format *isc_find_format_by_code(struct isc_device *isc,
+ unsigned int code, int *index)
{
struct isc_format *fmt = &isc->formats_list[0];
unsigned int i;
@@ -1772,6 +1783,7 @@ static struct isc_format *find_format_by_code(struct isc_device *isc,
return NULL;
}
+EXPORT_SYMBOL_GPL(isc_find_format_by_code);
static int isc_formats_init(struct isc_device *isc)
{
@@ -1788,7 +1800,7 @@ static int isc_formats_init(struct isc_device *isc)
NULL, &mbus_code)) {
mbus_code.index++;
- fmt = find_format_by_code(isc, mbus_code.code, &i);
+ fmt = isc_find_format_by_code(isc, mbus_code.code, &i);
if (!fmt) {
v4l2_warn(&isc->v4l2_dev, "Mbus code %x not supported\n",
mbus_code.code);
@@ -1926,8 +1938,19 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
goto isc_async_complete_err;
}
+ ret = isc_scaler_link(isc);
+ if (ret < 0)
+ goto isc_async_complete_unregister_device;
+
+ ret = media_device_register(&isc->mdev);
+ if (ret < 0)
+ goto isc_async_complete_unregister_device;
+
return 0;
+isc_async_complete_unregister_device:
+ video_unregister_device(vdev);
+
isc_async_complete_err:
mutex_destroy(&isc->awb_mutex);
mutex_destroy(&isc->lock);
@@ -1995,6 +2018,49 @@ int microchip_isc_pipeline_init(struct isc_device *isc)
}
EXPORT_SYMBOL_GPL(microchip_isc_pipeline_init);
+int isc_mc_init(struct isc_device *isc, u32 ver)
+{
+ const struct of_device_id *match;
+ int ret;
+
+ isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L;
+ isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT;
+ isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+
+ ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM,
+ isc->pads);
+ if (ret < 0) {
+ dev_err(isc->dev, "media entity init failed\n");
+ return ret;
+ }
+
+ isc->mdev.dev = isc->dev;
+
+ match = of_match_node(isc->dev->driver->of_match_table,
+ isc->dev->of_node);
+
+ strscpy(isc->mdev.driver_name, KBUILD_MODNAME,
+ sizeof(isc->mdev.driver_name));
+ strscpy(isc->mdev.model, match->compatible, sizeof(isc->mdev.model));
+ snprintf(isc->mdev.bus_info, sizeof(isc->mdev.bus_info), "platform:%s",
+ isc->v4l2_dev.name);
+ isc->mdev.hw_revision = ver;
+
+ media_device_init(&isc->mdev);
+
+ isc->v4l2_dev.mdev = &isc->mdev;
+
+ return isc_scaler_init(isc);
+}
+EXPORT_SYMBOL_GPL(isc_mc_init);
+
+void isc_mc_cleanup(struct isc_device *isc)
+{
+ media_entity_cleanup(&isc->video_dev.entity);
+ media_device_cleanup(&isc->mdev);
+}
+EXPORT_SYMBOL_GPL(isc_mc_cleanup);
+
/* regmap configuration */
#define MICROCHIP_ISC_REG_MAX 0xd5c
const struct regmap_config microchip_isc_regmap_config = {
diff --git a/drivers/media/platform/microchip/microchip-isc-scaler.c b/drivers/media/platform/microchip/microchip-isc-scaler.c
new file mode 100644
index 000000000000..80fff803e6bb
--- /dev/null
+++ b/drivers/media/platform/microchip/microchip-isc-scaler.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Microchip Image Sensor Controller (ISC) Scaler entity support
+ *
+ * Copyright (C) 2022 Microchip Technology, Inc.
+ *
+ * Author: Eugen Hristev <[email protected]>
+ *
+ */
+
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+#include "microchip-isc-regs.h"
+#include "microchip-isc.h"
+
+static void isc_scaler_prepare_fmt(struct v4l2_mbus_framefmt *framefmt)
+{
+ framefmt->colorspace = V4L2_COLORSPACE_SRGB;
+ framefmt->field = V4L2_FIELD_NONE;
+ framefmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ framefmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+ framefmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+};
+
+static int isc_scaler_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_format *format)
+{
+ struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+ struct v4l2_mbus_framefmt *v4l2_try_fmt;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ format->pad);
+ format->format = *v4l2_try_fmt;
+
+ return 0;
+ }
+
+ format->format = isc->scaler_format[format->pad];
+
+ return 0;
+}
+
+static int isc_scaler_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_format *req_fmt)
+{
+ struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+ struct v4l2_mbus_framefmt *v4l2_try_fmt;
+ struct isc_format *fmt;
+ unsigned int i;
+
+ /* Source format is fixed, we cannot change it */
+ if (req_fmt->pad == ISC_SCALER_PAD_SOURCE) {
+ req_fmt->format = isc->scaler_format[ISC_SCALER_PAD_SOURCE];
+ return 0;
+ }
+
+ /* There is no limit on the frame size on the sink pad */
+ v4l_bound_align_image(&req_fmt->format.width, 16, UINT_MAX, 0,
+ &req_fmt->format.height, 16, UINT_MAX, 0, 0);
+
+ isc_scaler_prepare_fmt(&req_fmt->format);
+
+ fmt = isc_find_format_by_code(isc, req_fmt->format.code, &i);
+
+ if (!fmt)
+ fmt = &isc->formats_list[0];
+
+ req_fmt->format.code = fmt->mbus_code;
+
+ if (req_fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+ v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ req_fmt->pad);
+ *v4l2_try_fmt = req_fmt->format;
+ /* Trying on the sink pad makes the source pad change too */
+ v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+ ISC_SCALER_PAD_SOURCE);
+ *v4l2_try_fmt = req_fmt->format;
+
+ v4l_bound_align_image(&v4l2_try_fmt->width,
+ 16, isc->max_width, 0,
+ &v4l2_try_fmt->height,
+ 16, isc->max_height, 0, 0);
+ /* if we are just trying, we are done */
+ return 0;
+ }
+
+ isc->scaler_format[ISC_SCALER_PAD_SINK] = req_fmt->format;
+
+ /* The source pad is the same as the sink, but we have to crop it */
+ isc->scaler_format[ISC_SCALER_PAD_SOURCE] =
+ isc->scaler_format[ISC_SCALER_PAD_SINK];
+ v4l_bound_align_image
+ (&isc->scaler_format[ISC_SCALER_PAD_SOURCE].width, 16,
+ isc->max_width, 0,
+ &isc->scaler_format[ISC_SCALER_PAD_SOURCE].height, 16,
+ isc->max_height, 0, 0);
+
+ return 0;
+}
+
+static int isc_scaler_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+
+ /*
+ * All formats supported by the ISC are supported by the scaler.
+ * Advertise the formats which the ISC can take as input, as the scaler
+ * entity cropping is part of the PFE module (parallel front end)
+ */
+ if (code->index < isc->formats_list_size) {
+ code->code = isc->formats_list[code->index].mbus_code;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int isc_scaler_g_sel(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_selection *sel)
+{
+ struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+
+ if (sel->pad == ISC_SCALER_PAD_SOURCE)
+ return -EINVAL;
+
+ if (sel->target != V4L2_SEL_TGT_CROP_BOUNDS &&
+ sel->target != V4L2_SEL_TGT_CROP)
+ return -EINVAL;
+
+ sel->r.height = isc->scaler_format[ISC_SCALER_PAD_SOURCE].height;
+ sel->r.width = isc->scaler_format[ISC_SCALER_PAD_SOURCE].width;
+
+ sel->r.left = 0;
+ sel->r.top = 0;
+
+ return 0;
+}
+
+static int isc_scaler_init_cfg(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state)
+{
+ struct v4l2_mbus_framefmt *v4l2_try_fmt =
+ v4l2_subdev_get_try_format(sd, sd_state, 0);
+ struct v4l2_rect *try_crop;
+ struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+
+ *v4l2_try_fmt = isc->scaler_format[ISC_SCALER_PAD_SOURCE];
+
+ try_crop = v4l2_subdev_get_try_crop(sd, sd_state, 0);
+
+ try_crop->top = 0;
+ try_crop->left = 0;
+ try_crop->width = v4l2_try_fmt->width;
+ try_crop->height = v4l2_try_fmt->height;
+
+ return 0;
+}
+
+static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = {
+ .enum_mbus_code = isc_scaler_enum_mbus_code,
+ .set_fmt = isc_scaler_set_fmt,
+ .get_fmt = isc_scaler_get_fmt,
+ .get_selection = isc_scaler_g_sel,
+ .init_cfg = isc_scaler_init_cfg,
+};
+
+static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = {
+ .pad = &isc_scaler_pad_ops,
+};
+
+int isc_scaler_init(struct isc_device *isc)
+{
+ int ret;
+
+ v4l2_subdev_init(&isc->scaler_sd, &xisc_scaler_subdev_ops);
+
+ isc->scaler_sd.owner = THIS_MODULE;
+ isc->scaler_sd.dev = isc->dev;
+ snprintf(isc->scaler_sd.name, sizeof(isc->scaler_sd.name),
+ "microchip_isc_scaler");
+
+ isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
+ isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+
+ isc_scaler_prepare_fmt(&isc->scaler_format[ISC_SCALER_PAD_SOURCE]);
+ isc->scaler_format[ISC_SCALER_PAD_SOURCE].height = isc->max_height;
+ isc->scaler_format[ISC_SCALER_PAD_SOURCE].width = isc->max_width;
+ isc->scaler_format[ISC_SCALER_PAD_SOURCE].code =
+ isc->formats_list[0].mbus_code;
+
+ isc->scaler_format[ISC_SCALER_PAD_SINK] =
+ isc->scaler_format[ISC_SCALER_PAD_SOURCE];
+
+ ret = media_entity_pads_init(&isc->scaler_sd.entity,
+ ISC_SCALER_PADS_NUM,
+ isc->scaler_pads);
+ if (ret < 0) {
+ dev_err(isc->dev, "scaler sd media entity init failed\n");
+ return ret;
+ }
+
+ ret = v4l2_device_register_subdev(&isc->v4l2_dev, &isc->scaler_sd);
+ if (ret < 0) {
+ dev_err(isc->dev, "scaler sd failed to register subdev\n");
+ return ret;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(isc_scaler_init);
+
+int isc_scaler_link(struct isc_device *isc)
+{
+ int ret;
+
+ ret = media_create_pad_link(&isc->current_subdev->sd->entity,
+ isc->remote_pad, &isc->scaler_sd.entity,
+ ISC_SCALER_PAD_SINK,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+
+ if (ret < 0) {
+ dev_err(isc->dev, "Failed to create pad link: %s to %s\n",
+ isc->current_subdev->sd->entity.name,
+ isc->scaler_sd.entity.name);
+ return ret;
+ }
+
+ dev_dbg(isc->dev, "link with %s pad: %d\n",
+ isc->current_subdev->sd->name, isc->remote_pad);
+
+ ret = media_create_pad_link(&isc->scaler_sd.entity,
+ ISC_SCALER_PAD_SOURCE,
+ &isc->video_dev.entity, ISC_PAD_SINK,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+
+ if (ret < 0) {
+ dev_err(isc->dev, "Failed to create pad link: %s to %s\n",
+ isc->scaler_sd.entity.name,
+ isc->video_dev.entity.name);
+ return ret;
+ }
+
+ dev_dbg(isc->dev, "link with %s pad: %d\n", isc->scaler_sd.name,
+ ISC_SCALER_PAD_SOURCE);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(isc_scaler_link);
+
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index 32969a3f79c5..1ba951df4f15 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -183,6 +183,17 @@ struct isc_reg_offsets {
u32 his_entry;
};
+enum isc_mc_pads {
+ ISC_PAD_SINK = 0,
+ ISC_PADS_NUM = 1,
+};
+
+enum isc_scaler_pads {
+ ISC_SCALER_PAD_SINK = 0,
+ ISC_SCALER_PAD_SOURCE = 1,
+ ISC_SCALER_PADS_NUM = 2,
+};
+
/*
* struct isc_device - ISC device driver data/config struct
* @regmap: Register map
@@ -259,6 +270,12 @@ struct isc_reg_offsets {
* be used as an input to the controller
* @controller_formats_size: size of controller_formats array
* @formats_list_size: size of formats_list array
+ * @pads: media controller pads for isc video entity
+ * @mdev: media device that is registered by the isc
+ * @remote_pad: remote pad on the connected subdevice
+ * @scaler_sd: subdevice for the scaler that isc registers
+ * @scaler_pads: media controller pads for the scaler subdevice
+ * @scaler_format: current format for the scaler subdevice
*/
struct isc_device {
struct regmap *regmap;
@@ -348,6 +365,19 @@ struct isc_device {
struct isc_format *formats_list;
u32 controller_formats_size;
u32 formats_list_size;
+
+ struct {
+ struct media_pad pads[ISC_PADS_NUM];
+ struct media_device mdev;
+
+ u32 remote_pad;
+ };
+
+ struct {
+ struct v4l2_subdev scaler_sd;
+ struct media_pad scaler_pads[ISC_SCALER_PADS_NUM];
+ struct v4l2_mbus_framefmt scaler_format[ISC_SCALER_PADS_NUM];
+ };
};
extern const struct regmap_config microchip_isc_regmap_config;
@@ -359,4 +389,11 @@ int microchip_isc_clk_init(struct isc_device *isc);
void microchip_isc_subdev_cleanup(struct isc_device *isc);
void microchip_isc_clk_cleanup(struct isc_device *isc);
+int isc_scaler_link(struct isc_device *isc);
+int isc_scaler_init(struct isc_device *isc);
+int isc_mc_init(struct isc_device *isc, u32 ver);
+void isc_mc_cleanup(struct isc_device *isc);
+
+struct isc_format *isc_find_format_by_code(struct isc_device *isc,
+ unsigned int code, int *index);
#endif
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index 8c7ceee41fd5..c702d2537738 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -536,6 +536,12 @@ static int microchip_isc_probe(struct platform_device *pdev)
break;
}
+ regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
+
+ ret = isc_mc_init(isc, ver);
+ if (ret < 0)
+ goto isc_probe_mc_init_err;
+
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
pm_request_idle(dev);
@@ -555,7 +561,6 @@ static int microchip_isc_probe(struct platform_device *pdev)
goto unprepare_clk;
}
- regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
dev_info(dev, "Microchip ISC version %x\n", ver);
return 0;
@@ -566,6 +571,9 @@ static int microchip_isc_probe(struct platform_device *pdev)
disable_pm:
pm_runtime_disable(dev);
+isc_probe_mc_init_err:
+ isc_mc_cleanup(isc);
+
cleanup_subdev:
microchip_isc_subdev_cleanup(isc);
@@ -586,6 +594,8 @@ static int microchip_isc_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
+ isc_mc_cleanup(isc);
+
microchip_isc_subdev_cleanup(isc);
v4l2_device_unregister(&isc->v4l2_dev);
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index 3408f6fb61e8..75a0a5caa8d8 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -526,15 +526,23 @@ static int microchip_xisc_probe(struct platform_device *pdev)
break;
}
+ regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
+
+ ret = isc_mc_init(isc, ver);
+ if (ret < 0)
+ goto isc_probe_mc_init_err;
+
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
pm_request_idle(dev);
- regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
dev_info(dev, "Microchip XISC version %x\n", ver);
return 0;
+isc_probe_mc_init_err:
+ isc_mc_cleanup(isc);
+
cleanup_subdev:
microchip_isc_subdev_cleanup(isc);
@@ -555,6 +563,8 @@ static int microchip_xisc_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
+ isc_mc_cleanup(isc);
+
microchip_isc_subdev_cleanup(isc);
v4l2_device_unregister(&isc->v4l2_dev);
--
2.25.1
As a top MC video driver, the microchip-isc should not propagate the format
to the subdevice, it should rather check at start_streaming() time if the
subdev is properly configured with a compatible format.
Removed the whole format finding logic, and reworked the format
verification at start_streaming time, such that the ISC will return an
error if the subdevice is not properly configured.
To achieve this, media_pipeline_start is called and a link_validate
callback is created to check the formats.
With this being done, the module parameter 'sensor_preferred' makes no
sense anymore. The ISC should not decide which format the sensor is using.
The ISC should only cope with the situation and inform userspace if the
streaming is possible in the current configuration.
The redesign of the format propagation has also risen the question of the
enumfmt callback. If enumfmt is called with an mbus_code, the enumfmt
handler should only return the formats that are supported for this
mbus_code. Otherwise, the enumfmt will report all the formats that the ISC
could output.
With this rework, the dynamic list of user formats is removed. It makes no
more sense to identify at complete time which formats the sensor could
emit, and add those into a separate dynamic list.
The ISC will start with a simple preconfigured default format, and at
link validate time, decide whether it can use the format that is
configured on the sink or not.
From now on, the driver also advertises the IO_MC capability.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
Changes in v12:
- all exported symbols now prefixed by microchip_
Changes in v11:
- changed to apply on top of MICROCHIP_ISC
- changed media pipeline_stop/start to use new type of first parameter
- s/Atmel/Microchip
Changes in v10:
- moved IO_MC to this patch
- edited commit message
Changes in v9:
- isc_link_validate now static
Changes in v7:
- minor typos as suggested by Jacopo
- small changes, reduce some indentation, modified an index, as suggested by
Jacopo
Changes in v6:
- reworked a bit enum_fmt as suggested by Jacopo
Changes in v5:
- removed user_formats dynamic list as it is now pointless
- greatly simplified the enum_fmt function
- removed some init code that was useless now
Changes in v4:
- moved validation code into link_validate and used media_pipeline_start
- merged this patch with the enum_fmt patch which was previously in v3 of
the series
Changes in v3:
- clamp to maximum resolution once the frame size from the subdev is found
drivers/media/platform/microchip/Kconfig | 2 +-
.../platform/microchip/microchip-isc-base.c | 416 ++++++++----------
.../platform/microchip/microchip-isc-scaler.c | 5 +
.../media/platform/microchip/microchip-isc.h | 13 +-
.../microchip/microchip-sama5d2-isc.c | 20 +
.../microchip/microchip-sama7g5-isc.c | 20 +
6 files changed, 239 insertions(+), 237 deletions(-)
diff --git a/drivers/media/platform/microchip/Kconfig b/drivers/media/platform/microchip/Kconfig
index 4f30807aae43..4734ecced029 100644
--- a/drivers/media/platform/microchip/Kconfig
+++ b/drivers/media/platform/microchip/Kconfig
@@ -23,7 +23,7 @@ config VIDEO_MICROCHIP_ISC
config VIDEO_MICROCHIP_XISC
tristate "Microchip eXtended Image Sensor Controller (XISC) support"
depends on V4L_PLATFORM_DRIVERS
- depends on VIDEO_DEV && COMMON_CLK && VIDEO_V4L2_SUBDEV_API
+ depends on VIDEO_DEV && COMMON_CLK
depends on ARCH_AT91 || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select REGMAP_MMIO
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 9034d8f97831..28749a09b0c3 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -36,11 +36,6 @@ static unsigned int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "debug level (0-2)");
-static unsigned int sensor_preferred = 1;
-module_param(sensor_preferred, uint, 0644);
-MODULE_PARM_DESC(sensor_preferred,
- "Sensor is preferred to output the specified format (1-on 0-off), default 1");
-
#define ISC_IS_FORMAT_RAW(mbus_code) \
(((mbus_code) & 0xf000) == 0x3000)
@@ -341,6 +336,10 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
unsigned long flags;
int ret;
+ ret = media_pipeline_start(isc->video_dev.entity.pads, &isc->mpipe);
+ if (ret)
+ goto err_pipeline_start;
+
/* Enable stream on the sub device */
ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD) {
@@ -390,6 +389,9 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 0);
err_start_stream:
+ media_pipeline_stop(isc->video_dev.entity.pads);
+
+err_pipeline_start:
spin_lock_irqsave(&isc->dma_queue_lock, flags);
list_for_each_entry(buf, &isc->dma_queue, list)
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -428,6 +430,9 @@ static void isc_stop_streaming(struct vb2_queue *vq)
if (ret && ret != -ENOIOCTLCMD)
v4l2_err(&isc->v4l2_dev, "stream off failed in subdev\n");
+ /* Stop media pipeline */
+ media_pipeline_stop(isc->video_dev.entity.pads);
+
/* Release all active buffers */
spin_lock_irqsave(&isc->dma_queue_lock, flags);
if (unlikely(isc->cur_frm)) {
@@ -459,22 +464,6 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
}
-static struct isc_format *find_format_by_fourcc(struct isc_device *isc,
- unsigned int fourcc)
-{
- unsigned int num_formats = isc->num_user_formats;
- struct isc_format *fmt;
- unsigned int i;
-
- for (i = 0; i < num_formats; i++) {
- fmt = isc->user_formats[i];
- if (fmt->fourcc == fourcc)
- return fmt;
- }
-
- return NULL;
-}
-
static const struct vb2_ops isc_vb2_ops = {
.queue_setup = isc_queue_setup,
.wait_prepare = vb2_ops_wait_prepare,
@@ -503,23 +492,57 @@ static int isc_enum_fmt_vid_cap(struct file *file, void *priv,
{
struct isc_device *isc = video_drvdata(file);
u32 index = f->index;
- u32 i, supported_index;
+ u32 i, supported_index = 0;
+ struct isc_format *fmt;
+
+ /*
+ * If we are not asked a specific mbus_code, we have to report all
+ * the formats that we can output.
+ */
+ if (!f->mbus_code) {
+ if (index >= isc->controller_formats_size)
+ return -EINVAL;
- if (index < isc->controller_formats_size) {
f->pixelformat = isc->controller_formats[index].fourcc;
+
+ return 0;
+ }
+
+ /*
+ * If a specific mbus_code is requested, check if we support
+ * this mbus_code as input for the ISC.
+ * If it's supported, then we report the corresponding pixelformat
+ * as first possible option for the ISC.
+ * E.g. mbus MEDIA_BUS_FMT_YUYV8_2X8 and report
+ * 'YUYV' (YUYV 4:2:2)
+ */
+ fmt = isc_find_format_by_code(isc, f->mbus_code, &i);
+ if (!fmt)
+ return -EINVAL;
+
+ if (!index) {
+ f->pixelformat = fmt->fourcc;
+
return 0;
}
- index -= isc->controller_formats_size;
+ supported_index++;
- supported_index = 0;
+ /* If the index is not raw, we don't have anymore formats to report */
+ if (!ISC_IS_FORMAT_RAW(f->mbus_code))
+ return -EINVAL;
- for (i = 0; i < isc->formats_list_size; i++) {
- if (!ISC_IS_FORMAT_RAW(isc->formats_list[i].mbus_code) ||
- !isc->formats_list[i].sd_support)
+ /*
+ * We are asked for a specific mbus code, which is raw.
+ * We have to search through the formats we can convert to.
+ * We have to skip the raw formats, we cannot convert to raw.
+ * E.g. 'AR12' (16-bit ARGB 4-4-4-4), 'AR15' (16-bit ARGB 1-5-5-5), etc.
+ */
+ for (i = 0; i < isc->controller_formats_size; i++) {
+ if (isc->controller_formats[i].raw)
continue;
- if (supported_index == index) {
- f->pixelformat = isc->formats_list[i].fourcc;
+ if (index == supported_index) {
+ f->pixelformat = isc->controller_formats[i].fourcc;
return 0;
}
supported_index++;
@@ -590,20 +613,30 @@ static int isc_try_validate_formats(struct isc_device *isc)
break;
default:
/* any other different formats are not supported */
+ v4l2_err(&isc->v4l2_dev, "Requested unsupported format.\n");
ret = -EINVAL;
}
v4l2_dbg(1, debug, &isc->v4l2_dev,
"Format validation, requested rgb=%u, yuv=%u, grey=%u, bayer=%u\n",
rgb, yuv, grey, bayer);
- /* we cannot output RAW if we do not receive RAW */
- if ((bayer) && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
+ if (bayer &&
+ !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
+ v4l2_err(&isc->v4l2_dev, "Cannot output RAW if we do not receive RAW.\n");
return -EINVAL;
+ }
- /* we cannot output GREY if we do not receive RAW/GREY */
if (grey && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code) &&
- !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code))
+ !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) {
+ v4l2_err(&isc->v4l2_dev, "Cannot output GREY if we do not receive RAW/GREY.\n");
return -EINVAL;
+ }
+
+ if ((rgb || bayer || yuv) &&
+ ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) {
+ v4l2_err(&isc->v4l2_dev, "Cannot convert GREY to another format.\n");
+ return -EINVAL;
+ }
return ret;
}
@@ -831,7 +864,7 @@ static void isc_try_fse(struct isc_device *isc,
* If we do not know yet which format the subdev is using, we cannot
* do anything.
*/
- if (!isc->try_config.sd_format)
+ if (!isc->config.sd_format)
return;
fse.code = isc->try_config.sd_format->mbus_code;
@@ -852,180 +885,139 @@ static void isc_try_fse(struct isc_device *isc,
}
}
-static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
- u32 *code)
+static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
{
- int i;
- struct isc_format *sd_fmt = NULL, *direct_fmt = NULL;
struct v4l2_pix_format *pixfmt = &f->fmt.pix;
- struct v4l2_subdev_pad_config pad_cfg = {};
- struct v4l2_subdev_state pad_state = {
- .pads = &pad_cfg
- };
- struct v4l2_subdev_format format = {
- .which = V4L2_SUBDEV_FORMAT_TRY,
- };
- u32 mbus_code;
- int ret;
- bool rlp_dma_direct_dump = false;
+ unsigned int i;
if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
- /* Step 1: find a RAW format that is supported */
- for (i = 0; i < isc->num_user_formats; i++) {
- if (ISC_IS_FORMAT_RAW(isc->user_formats[i]->mbus_code)) {
- sd_fmt = isc->user_formats[i];
+ isc->try_config.fourcc = isc->controller_formats[0].fourcc;
+
+ /* find if the format requested is supported */
+ for (i = 0; i < isc->controller_formats_size; i++)
+ if (isc->controller_formats[i].fourcc == pixfmt->pixelformat) {
+ isc->try_config.fourcc = pixfmt->pixelformat;
break;
}
- }
- /* Step 2: We can continue with this RAW format, or we can look
- * for better: maybe sensor supports directly what we need.
- */
- direct_fmt = find_format_by_fourcc(isc, pixfmt->pixelformat);
-
- /* Step 3: We have both. We decide given the module parameter which
- * one to use.
- */
- if (direct_fmt && sd_fmt && sensor_preferred)
- sd_fmt = direct_fmt;
-
- /* Step 4: we do not have RAW but we have a direct format. Use it. */
- if (direct_fmt && !sd_fmt)
- sd_fmt = direct_fmt;
-
- /* Step 5: if we are using a direct format, we need to package
- * everything as 8 bit data and just dump it
- */
- if (sd_fmt == direct_fmt)
- rlp_dma_direct_dump = true;
-
- /* Step 6: We have no format. This can happen if the userspace
- * requests some weird/invalid format.
- * In this case, default to whatever we have
- */
- if (!sd_fmt && !direct_fmt) {
- sd_fmt = isc->user_formats[isc->num_user_formats - 1];
- v4l2_dbg(1, debug, &isc->v4l2_dev,
- "Sensor not supporting %.4s, using %.4s\n",
- (char *)&pixfmt->pixelformat, (char *)&sd_fmt->fourcc);
- }
-
- if (!sd_fmt) {
- ret = -EINVAL;
- goto isc_try_fmt_err;
- }
-
- /* Step 7: Print out what we decided for debugging */
- v4l2_dbg(1, debug, &isc->v4l2_dev,
- "Preferring to have sensor using format %.4s\n",
- (char *)&sd_fmt->fourcc);
-
- /* Step 8: at this moment we decided which format the subdev will use */
- isc->try_config.sd_format = sd_fmt;
-
- /* Limit to Microchip ISC hardware capabilities */
- if (pixfmt->width > isc->max_width)
- pixfmt->width = isc->max_width;
- if (pixfmt->height > isc->max_height)
- pixfmt->height = isc->max_height;
-
- /*
- * The mbus format is the one the subdev outputs.
- * The pixels will be transferred in this format Sensor -> ISC
- */
- mbus_code = sd_fmt->mbus_code;
-
- /*
- * Validate formats. If the required format is not OK, default to raw.
- */
-
- isc->try_config.fourcc = pixfmt->pixelformat;
-
- if (isc_try_validate_formats(isc)) {
- pixfmt->pixelformat = isc->try_config.fourcc = sd_fmt->fourcc;
- /* Re-try to validate the new format */
- ret = isc_try_validate_formats(isc);
- if (ret)
- goto isc_try_fmt_err;
- }
-
- ret = isc_try_configure_rlp_dma(isc, rlp_dma_direct_dump);
- if (ret)
- goto isc_try_fmt_err;
-
- ret = isc_try_configure_pipeline(isc);
- if (ret)
- goto isc_try_fmt_err;
-
- /* Obtain frame sizes if possible to have crop requirements ready */
- isc_try_fse(isc, &pad_state);
- v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code);
- ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
- &pad_state, &format);
- if (ret < 0)
- goto isc_try_fmt_subdev_err;
-
- v4l2_fill_pix_format(pixfmt, &format.format);
+ isc_try_configure_rlp_dma(isc, false);
/* Limit to Microchip ISC hardware capabilities */
- if (pixfmt->width > isc->max_width)
- pixfmt->width = isc->max_width;
- if (pixfmt->height > isc->max_height)
- pixfmt->height = isc->max_height;
-
+ v4l_bound_align_image(&pixfmt->width, 16, isc->max_width, 0,
+ &pixfmt->height, 16, isc->max_height, 0, 0);
+ /* If we did not find the requested format, we will fallback here */
+ pixfmt->pixelformat = isc->try_config.fourcc;
+ pixfmt->colorspace = V4L2_COLORSPACE_SRGB;
pixfmt->field = V4L2_FIELD_NONE;
+
pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp_v4l2) >> 3;
pixfmt->sizeimage = ((pixfmt->width * isc->try_config.bpp) >> 3) *
pixfmt->height;
- if (code)
- *code = mbus_code;
+ isc->try_fmt = *f;
return 0;
+}
-isc_try_fmt_err:
- v4l2_err(&isc->v4l2_dev, "Could not find any possible format for a working pipeline\n");
-isc_try_fmt_subdev_err:
- memset(&isc->try_config, 0, sizeof(isc->try_config));
+static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
+{
+ isc_try_fmt(isc, f);
- return ret;
+ /* make the try configuration active */
+ isc->config = isc->try_config;
+ isc->fmt = isc->try_fmt;
+
+ v4l2_dbg(1, debug, &isc->v4l2_dev, "ISC set_fmt to %.4s @%dx%d\n",
+ (char *)&f->fmt.pix.pixelformat,
+ f->fmt.pix.width, f->fmt.pix.height);
+
+ return 0;
}
-static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
+static int isc_validate(struct isc_device *isc)
{
+ int ret;
+ int i;
+ struct isc_format *sd_fmt = NULL;
+ struct v4l2_pix_format *pixfmt = &isc->fmt.fmt.pix;
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = isc->remote_pad,
+ };
+ struct v4l2_subdev_pad_config pad_cfg = {};
+ struct v4l2_subdev_state pad_state = {
+ .pads = &pad_cfg,
};
- u32 mbus_code = 0;
- int ret;
- ret = isc_try_fmt(isc, f, &mbus_code);
+ /* Get current format from subdev */
+ ret = v4l2_subdev_call(isc->current_subdev->sd, pad, get_fmt, NULL,
+ &format);
if (ret)
return ret;
- v4l2_fill_mbus_format(&format.format, &f->fmt.pix, mbus_code);
- ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
- set_fmt, NULL, &format);
- if (ret < 0)
- return ret;
+ /* Identify the subdev's format configuration */
+ for (i = 0; i < isc->formats_list_size; i++)
+ if (isc->formats_list[i].mbus_code == format.format.code) {
+ sd_fmt = &isc->formats_list[i];
+ break;
+ }
+
+ /* Check if the format is not supported */
+ if (!sd_fmt) {
+ v4l2_err(&isc->v4l2_dev,
+ "Current subdevice is streaming a media bus code that is not supported 0x%x\n",
+ format.format.code);
+ return -EPIPE;
+ }
+
+ /* At this moment we know which format the subdev will use */
+ isc->try_config.sd_format = sd_fmt;
+
+ /* If the sensor is not RAW, we can only do a direct dump */
+ if (!ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
+ isc_try_configure_rlp_dma(isc, true);
/* Limit to Microchip ISC hardware capabilities */
- if (f->fmt.pix.width > isc->max_width)
- f->fmt.pix.width = isc->max_width;
- if (f->fmt.pix.height > isc->max_height)
- f->fmt.pix.height = isc->max_height;
+ v4l_bound_align_image(&format.format.width, 16, isc->max_width, 0,
+ &format.format.height, 16, isc->max_height, 0, 0);
- isc->fmt = *f;
+ /* Check if the frame size is the same. Otherwise we may overflow */
+ if (pixfmt->height != format.format.height ||
+ pixfmt->width != format.format.width) {
+ v4l2_err(&isc->v4l2_dev,
+ "ISC not configured with the proper frame size: %dx%d\n",
+ format.format.width, format.format.height);
+ return -EPIPE;
+ }
+
+ v4l2_dbg(1, debug, &isc->v4l2_dev,
+ "Identified subdev using format %.4s with %dx%d %d bpp\n",
+ (char *)&sd_fmt->fourcc, pixfmt->width, pixfmt->height,
+ isc->try_config.bpp);
+ /* Reset and restart AWB if the subdevice changed the format */
if (isc->try_config.sd_format && isc->config.sd_format &&
isc->try_config.sd_format != isc->config.sd_format) {
isc->ctrls.hist_stat = HIST_INIT;
isc_reset_awb_ctrls(isc);
isc_update_v4l2_ctrls(isc);
}
- /* make the try configuration active */
+
+ /* Validate formats */
+ ret = isc_try_validate_formats(isc);
+ if (ret)
+ return ret;
+
+ /* Obtain frame sizes if possible to have crop requirements ready */
+ isc_try_fse(isc, &pad_state);
+
+ /* Configure ISC pipeline for the config */
+ ret = isc_try_configure_pipeline(isc);
+ if (ret)
+ return ret;
+
isc->config = isc->try_config;
v4l2_dbg(1, debug, &isc->v4l2_dev, "New ISC configuration in place\n");
@@ -1049,7 +1041,7 @@ static int isc_try_fmt_vid_cap(struct file *file, void *priv,
{
struct isc_device *isc = video_drvdata(file);
- return isc_try_fmt(isc, f, NULL);
+ return isc_try_fmt(isc, f);
}
static int isc_enum_input(struct file *file, void *priv,
@@ -1104,10 +1096,6 @@ static int isc_enum_framesizes(struct file *file, void *fh,
if (fsize->index)
return -EINVAL;
- for (i = 0; i < isc->num_user_formats; i++)
- if (isc->user_formats[i]->fourcc == fsize->pixel_format)
- ret = 0;
-
for (i = 0; i < isc->controller_formats_size; i++)
if (isc->controller_formats[i].fourcc == fsize->pixel_format)
ret = 0;
@@ -1785,52 +1773,6 @@ struct isc_format *isc_find_format_by_code(struct isc_device *isc,
}
EXPORT_SYMBOL_GPL(isc_find_format_by_code);
-static int isc_formats_init(struct isc_device *isc)
-{
- struct isc_format *fmt;
- struct v4l2_subdev *subdev = isc->current_subdev->sd;
- unsigned int num_fmts, i, j;
- u32 list_size = isc->formats_list_size;
- struct v4l2_subdev_mbus_code_enum mbus_code = {
- .which = V4L2_SUBDEV_FORMAT_ACTIVE,
- };
-
- num_fmts = 0;
- while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
- NULL, &mbus_code)) {
- mbus_code.index++;
-
- fmt = isc_find_format_by_code(isc, mbus_code.code, &i);
- if (!fmt) {
- v4l2_warn(&isc->v4l2_dev, "Mbus code %x not supported\n",
- mbus_code.code);
- continue;
- }
-
- fmt->sd_support = true;
- num_fmts++;
- }
-
- if (!num_fmts)
- return -ENXIO;
-
- isc->num_user_formats = num_fmts;
- isc->user_formats = devm_kcalloc(isc->dev,
- num_fmts, sizeof(*isc->user_formats),
- GFP_KERNEL);
- if (!isc->user_formats)
- return -ENOMEM;
-
- fmt = &isc->formats_list[0];
- for (i = 0, j = 0; i < list_size; i++) {
- if (fmt->sd_support)
- isc->user_formats[j++] = fmt;
- fmt++;
- }
-
- return 0;
-}
-
static int isc_set_default_fmt(struct isc_device *isc)
{
struct v4l2_format f = {
@@ -1839,12 +1781,12 @@ static int isc_set_default_fmt(struct isc_device *isc)
.width = VGA_WIDTH,
.height = VGA_HEIGHT,
.field = V4L2_FIELD_NONE,
- .pixelformat = isc->user_formats[0]->fourcc,
+ .pixelformat = isc->controller_formats[0].fourcc,
},
};
int ret;
- ret = isc_try_fmt(isc, &f, NULL);
+ ret = isc_try_fmt(isc, &f);
if (ret)
return ret;
@@ -1899,13 +1841,6 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
spin_lock_init(&isc->dma_queue_lock);
spin_lock_init(&isc->awb_lock);
- ret = isc_formats_init(isc);
- if (ret < 0) {
- v4l2_err(&isc->v4l2_dev,
- "Init format failed: %d\n", ret);
- goto isc_async_complete_err;
- }
-
ret = isc_set_default_fmt(isc);
if (ret) {
v4l2_err(&isc->v4l2_dev, "Could not set default format\n");
@@ -1928,7 +1863,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
vdev->queue = q;
vdev->lock = &isc->lock;
vdev->ctrl_handler = &isc->ctrls.handler;
- vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
+ vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE |
+ V4L2_CAP_IO_MC;
video_set_drvdata(vdev, isc);
ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
@@ -2018,6 +1954,24 @@ int microchip_isc_pipeline_init(struct isc_device *isc)
}
EXPORT_SYMBOL_GPL(microchip_isc_pipeline_init);
+static int isc_link_validate(struct media_link *link)
+{
+ struct video_device *vdev =
+ media_entity_to_video_device(link->sink->entity);
+ struct isc_device *isc = video_get_drvdata(vdev);
+ int ret;
+
+ ret = v4l2_subdev_link_validate(link);
+ if (ret)
+ return ret;
+
+ return isc_validate(isc);
+}
+
+static const struct media_entity_operations isc_entity_operations = {
+ .link_validate = isc_link_validate,
+};
+
int isc_mc_init(struct isc_device *isc, u32 ver)
{
const struct of_device_id *match;
@@ -2025,6 +1979,8 @@ int isc_mc_init(struct isc_device *isc, u32 ver)
isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L;
isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT;
+ isc->video_dev.entity.ops = &isc_entity_operations;
+
isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM,
diff --git a/drivers/media/platform/microchip/microchip-isc-scaler.c b/drivers/media/platform/microchip/microchip-isc-scaler.c
index 80fff803e6bb..0f29a32d15ce 100644
--- a/drivers/media/platform/microchip/microchip-isc-scaler.c
+++ b/drivers/media/platform/microchip/microchip-isc-scaler.c
@@ -173,6 +173,10 @@ static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = {
.init_cfg = isc_scaler_init_cfg,
};
+static const struct media_entity_operations isc_scaler_entity_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+};
+
static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = {
.pad = &isc_scaler_pad_ops,
};
@@ -190,6 +194,7 @@ int isc_scaler_init(struct isc_device *isc)
isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
+ isc->scaler_sd.entity.ops = &isc_scaler_entity_ops;
isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index 1ba951df4f15..e3a6c7367e70 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -63,15 +63,16 @@ struct isc_subdev_entity {
* @cfa_baycfg: If this format is RAW BAYER, indicate the type of bayer.
this is either BGBG, RGRG, etc.
* @pfe_cfg0_bps: Number of hardware data lines connected to the ISC
+ * @raw: If the format is raw bayer.
*/
struct isc_format {
u32 fourcc;
u32 mbus_code;
u32 cfa_baycfg;
-
- bool sd_support;
u32 pfe_cfg0_bps;
+
+ bool raw;
};
/* Pipeline bitmap */
@@ -216,8 +217,7 @@ enum isc_scaler_pads {
* @comp: completion reference that signals frame completion
*
* @fmt: current v42l format
- * @user_formats: list of formats that are supported and agreed with sd
- * @num_user_formats: how many formats are in user_formats
+ * @try_fmt: current v4l2 try format
*
* @config: current ISC format configuration
* @try_config: the current ISC try format , not yet activated
@@ -272,6 +272,7 @@ enum isc_scaler_pads {
* @formats_list_size: size of formats_list array
* @pads: media controller pads for isc video entity
* @mdev: media device that is registered by the isc
+ * @mpipe: media device pipeline used by the isc
* @remote_pad: remote pad on the connected subdevice
* @scaler_sd: subdevice for the scaler that isc registers
* @scaler_pads: media controller pads for the scaler subdevice
@@ -298,8 +299,7 @@ struct isc_device {
struct completion comp;
struct v4l2_format fmt;
- struct isc_format **user_formats;
- unsigned int num_user_formats;
+ struct v4l2_format try_fmt;
struct fmt_config config;
struct fmt_config try_config;
@@ -369,6 +369,7 @@ struct isc_device {
struct {
struct media_pad pads[ISC_PADS_NUM];
struct media_device mdev;
+ struct media_pipeline mpipe;
u32 remote_pad;
};
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index c702d2537738..ac4715d91de6 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -80,20 +80,40 @@ static const struct isc_format sama5d2_controller_formats[] = {
.fourcc = V4L2_PIX_FMT_Y10,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB10,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SBGGR12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGBRG12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGRBG12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SRGGB12,
+ .raw = true,
},
};
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index 75a0a5caa8d8..d583eafe5cc1 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -89,20 +89,40 @@ static const struct isc_format sama7g5_controller_formats[] = {
.fourcc = V4L2_PIX_FMT_Y16,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB8,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG10,
+ .raw = true,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB10,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SBGGR12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGBRG12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGRBG12,
+ .raw = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SRGGB12,
+ .raw = true,
},
};
--
2.25.1
Move the media_pipeline_start/stop calls from start/stop streaming to
the new prepare_streaming and unprepare_streaming callbacks.
Signed-off-by: Eugen Hristev <[email protected]>
---
.../platform/microchip/microchip-isc-base.c | 27 ++++++++++++-------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 28749a09b0c3..e2994d48f10c 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -328,6 +328,13 @@ static int isc_configure(struct isc_device *isc)
return isc_update_profile(isc);
}
+static int isc_prepare_streaming(struct vb2_queue *vq)
+{
+ struct isc_device *isc = vb2_get_drv_priv(vq);
+
+ return media_pipeline_start(isc->video_dev.entity.pads, &isc->mpipe);
+}
+
static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct isc_device *isc = vb2_get_drv_priv(vq);
@@ -336,10 +343,6 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
unsigned long flags;
int ret;
- ret = media_pipeline_start(isc->video_dev.entity.pads, &isc->mpipe);
- if (ret)
- goto err_pipeline_start;
-
/* Enable stream on the sub device */
ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD) {
@@ -389,9 +392,6 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 0);
err_start_stream:
- media_pipeline_stop(isc->video_dev.entity.pads);
-
-err_pipeline_start:
spin_lock_irqsave(&isc->dma_queue_lock, flags);
list_for_each_entry(buf, &isc->dma_queue, list)
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -401,6 +401,14 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
return ret;
}
+static void isc_unprepare_streaming(struct vb2_queue *vq)
+{
+ struct isc_device *isc = vb2_get_drv_priv(vq);
+
+ /* Stop media pipeline */
+ media_pipeline_stop(isc->video_dev.entity.pads);
+}
+
static void isc_stop_streaming(struct vb2_queue *vq)
{
struct isc_device *isc = vb2_get_drv_priv(vq);
@@ -430,9 +438,6 @@ static void isc_stop_streaming(struct vb2_queue *vq)
if (ret && ret != -ENOIOCTLCMD)
v4l2_err(&isc->v4l2_dev, "stream off failed in subdev\n");
- /* Stop media pipeline */
- media_pipeline_stop(isc->video_dev.entity.pads);
-
/* Release all active buffers */
spin_lock_irqsave(&isc->dma_queue_lock, flags);
if (unlikely(isc->cur_frm)) {
@@ -472,6 +477,8 @@ static const struct vb2_ops isc_vb2_ops = {
.start_streaming = isc_start_streaming,
.stop_streaming = isc_stop_streaming,
.buf_queue = isc_buffer_queue,
+ .prepare_streaming = isc_prepare_streaming,
+ .unprepare_streaming = isc_unprepare_streaming,
};
static int isc_querycap(struct file *file, void *priv,
--
2.25.1
Hi Eugen,
I love your patch! Yet something to improve:
[auto build test ERROR on media-tree/master]
[also build test ERROR on soc/for-next linus/master sailus-media-tree/streams v6.1-rc4 next-20221111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugen-Hristev/media-atmel-atmel-isc-driver-redesign/20221107-222554
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20221107141818.104937-6-eugen.hristev%40microchip.com
patch subject: [PATCH v12 5/6] media: microchip: microchip-isc: move media_pipeline_* to (un)prepare cb
config: ia64-allmodconfig
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/03b56a6f5e4cea93bf3bb2072be7a1d2ca8c5449
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Eugen-Hristev/media-atmel-atmel-isc-driver-redesign/20221107-222554
git checkout 03b56a6f5e4cea93bf3bb2072be7a1d2ca8c5449
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/media/platform/microchip/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/media/platform/microchip/microchip-isc-base.c:480:10: error: 'const struct vb2_ops' has no member named 'prepare_streaming'; did you mean 'start_streaming'?
480 | .prepare_streaming = isc_prepare_streaming,
| ^~~~~~~~~~~~~~~~~
| start_streaming
>> drivers/media/platform/microchip/microchip-isc-base.c:480:35: error: initialization of 'void (*)(struct vb2_buffer *)' from incompatible pointer type 'int (*)(struct vb2_queue *)' [-Werror=incompatible-pointer-types]
480 | .prepare_streaming = isc_prepare_streaming,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/media/platform/microchip/microchip-isc-base.c:480:35: note: (near initialization for 'isc_vb2_ops.buf_request_complete')
>> drivers/media/platform/microchip/microchip-isc-base.c:481:10: error: 'const struct vb2_ops' has no member named 'unprepare_streaming'; did you mean 'start_streaming'?
481 | .unprepare_streaming = isc_unprepare_streaming,
| ^~~~~~~~~~~~~~~~~~~
| start_streaming
drivers/media/platform/microchip/microchip-isc-base.c:481:35: warning: excess elements in struct initializer
481 | .unprepare_streaming = isc_unprepare_streaming,
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/media/platform/microchip/microchip-isc-base.c:481:35: note: (near initialization for 'isc_vb2_ops')
cc1: some warnings being treated as errors
vim +480 drivers/media/platform/microchip/microchip-isc-base.c
471
472 static const struct vb2_ops isc_vb2_ops = {
473 .queue_setup = isc_queue_setup,
474 .wait_prepare = vb2_ops_wait_prepare,
475 .wait_finish = vb2_ops_wait_finish,
476 .buf_prepare = isc_buffer_prepare,
477 .start_streaming = isc_start_streaming,
478 .stop_streaming = isc_stop_streaming,
479 .buf_queue = isc_buffer_queue,
> 480 .prepare_streaming = isc_prepare_streaming,
> 481 .unprepare_streaming = isc_unprepare_streaming,
482 };
483
--
0-DAY CI Kernel Test Service
https://01.org/lkp