Message ID | 20220630230713.10580-1-laurent.pinchart@ideasonboard.com |
---|---|
Headers | show |
Series | media: rkisp1: Cleanups and add support for i.MX8MP | expand |
On Fri, 01 Jul 2022 02:07:05 +0300, Laurent Pinchart wrote: > From: Paul Elder <paul.elder@ideasonboard.com> > > The i.MX8MP ISP is compatbile with the rkisp1 driver. Add it to the list > of compatible strings. While at it, expand on the description of the > clocks to make it clear which clock in the i.MX8MP ISP they map to, > based on the names from the datasheet (which are confusing). > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Documentation/devicetree/bindings/media/rockchip-isp1.yaml | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Laurent, On Fri, Jul 01, 2022 at 02:06:25AM +0300, Laurent Pinchart wrote: > There's no need to read the ID register every time streaming is started. > Do it once, at probe time. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 10 ++++++++++ > drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c | 4 ---- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > index 248f0172ca62..ba773c0784fb 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > @@ -467,6 +467,7 @@ static int rkisp1_probe(struct platform_device *pdev) > struct v4l2_device *v4l2_dev; > unsigned int i; > int ret, irq; > + u32 cif_id; > > match_data = of_device_get_match_data(&pdev->dev); > if (!match_data) > @@ -509,6 +510,15 @@ static int rkisp1_probe(struct platform_device *pdev) > > pm_runtime_enable(&pdev->dev); > > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret) > + goto err_pm_runtime_disable; > + > + cif_id = rkisp1_read(rkisp1, RKISP1_CIF_VI_ID); > + dev_dbg(rkisp1->dev, "CIF_ID 0x%08x\n", cif_id); > + > + pm_runtime_put(&pdev->dev); > + > rkisp1->media_dev.hw_revision = match_data->isp_ver; > strscpy(rkisp1->media_dev.model, RKISP1_DRIVER_NAME, > sizeof(rkisp1->media_dev.model)); > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > index a97c145bad98..bc94a51124b0 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > @@ -473,12 +473,8 @@ static int rkisp1_config_path(struct rkisp1_device *rkisp1) > /* Hardware configure Entry */ > static int rkisp1_config_cif(struct rkisp1_device *rkisp1) > { > - u32 cif_id; > int ret; > > - cif_id = rkisp1_read(rkisp1, RKISP1_CIF_VI_ID); > - dev_dbg(rkisp1->dev, "CIF_ID 0x%08x\n", cif_id); > - > ret = rkisp1_config_isp(rkisp1); > if (ret) > return ret;
Hi Laurent, On Fri, Jul 01, 2022 at 02:06:23AM +0300, Laurent Pinchart wrote: > The ISP used by the Rockchip RK3399 is also found in the NXP i.MX8MP. > Enable compilation of the driver for the MXC architecture in addition to > ARCH_ROCKCHIP. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > drivers/media/platform/rockchip/rkisp1/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/Kconfig b/drivers/media/platform/rockchip/rkisp1/Kconfig > index dabd7e42c193..731c9acbf6ef 100644 > --- a/drivers/media/platform/rockchip/rkisp1/Kconfig > +++ b/drivers/media/platform/rockchip/rkisp1/Kconfig > @@ -3,7 +3,7 @@ config VIDEO_ROCKCHIP_ISP1 > tristate "Rockchip Image Signal Processing v1 Unit driver" > depends on V4L_PLATFORM_DRIVERS > depends on VIDEO_DEV && OF > - depends on ARCH_ROCKCHIP || COMPILE_TEST > + depends on ARCH_ROCKCHIP || ARCH_MXC || COMPILE_TEST > select MEDIA_CONTROLLER > select VIDEO_V4L2_SUBDEV_API > select VIDEOBUF2_DMA_CONTIG
Hi Laurent, On Fri, Jul 01, 2022 at 02:06:52AM +0300, Laurent Pinchart wrote: > Initialize the src_fmt and sink_fmt variable when declaring them in > rkisp1_config_isp(). > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > Changes since v1: > > - Fix typo in commit message > --- > drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > index 938541ce52ce..53f0516594ef 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > @@ -142,12 +142,11 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > { > struct rkisp1_device *rkisp1 = isp->rkisp1; > u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, acq_prop = 0; > - const struct rkisp1_mbus_info *src_fmt, *sink_fmt; > + const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt; > + const struct rkisp1_mbus_info *src_fmt = isp->src_fmt; > struct v4l2_mbus_framefmt *sink_frm; > struct v4l2_rect *sink_crop; > > - sink_fmt = isp->sink_fmt; > - src_fmt = isp->src_fmt; > sink_frm = rkisp1_isp_get_pad_fmt(isp, NULL, > RKISP1_ISP_PAD_SINK_VIDEO, > V4L2_SUBDEV_FORMAT_ACTIVE);
On 01.07.2022 02:06, Laurent Pinchart wrote: >When a link validation failure occurs, print a debug message to help >diagnosing the cause. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- >Changes since v1: > >- Add missing \n >--- > .../media/platform/rockchip/rkisp1/rkisp1-capture.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >index 94819e6c23e2..fb14c8aa154c 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >@@ -1294,8 +1294,16 @@ static int rkisp1_capture_link_validate(struct media_link *link) > > if (sd_fmt.format.height != cap->pix.fmt.height || > sd_fmt.format.width != cap->pix.fmt.width || >- sd_fmt.format.code != fmt->mbus) >+ sd_fmt.format.code != fmt->mbus) { >+ dev_dbg(cap->rkisp1->dev, >+ "link '%s':%u -> '%s':%u not valid: 0x%04x/%ux%u != 0x%04x/%ux%u\n", >+ link->source->entity->name, link->source->index, >+ link->sink->entity->name, link->sink->index, >+ sd_fmt.format.code, sd_fmt.format.width, >+ sd_fmt.format.height, fmt->mbus, cap->pix.fmt.width, >+ cap->pix.fmt.height); > return -EPIPE; >+ } > > return 0; > } >-- >Regards, > >Laurent Pinchart >
On 01.07.2022 02:06, Laurent Pinchart wrote: >From: Paul Elder <paul.elder@ideasonboard.com> > >Not all ISP instances include a MIPI CSI-2 receiver. To prepare for >making it optional, move code related to the CSI-2 receiver to a >separate file. > >Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- >Changes since v1: > >- Initialize csi->rkisp1 >- Fix dev_err_probe() usage >- Fix white space issue >--- > .../media/platform/rockchip/rkisp1/Makefile | 1 + > .../platform/rockchip/rkisp1/rkisp1-common.h | 17 +- > .../platform/rockchip/rkisp1/rkisp1-csi.c | 193 ++++++++++++++++++ > .../platform/rockchip/rkisp1/rkisp1-csi.h | 28 +++ > .../platform/rockchip/rkisp1/rkisp1-dev.c | 32 +-- > .../platform/rockchip/rkisp1/rkisp1-isp.c | 153 +------------- > 6 files changed, 257 insertions(+), 167 deletions(-) > create mode 100644 drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c > create mode 100644 drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h > >diff --git a/drivers/media/platform/rockchip/rkisp1/Makefile b/drivers/media/platform/rockchip/rkisp1/Makefile >index f7543a82aa10..b3844c4f7623 100644 >--- a/drivers/media/platform/rockchip/rkisp1/Makefile >+++ b/drivers/media/platform/rockchip/rkisp1/Makefile >@@ -2,6 +2,7 @@ > > rockchip-isp1-y := rkisp1-capture.o \ > rkisp1-common.o \ >+ rkisp1-csi.o \ > rkisp1-dev.o \ > rkisp1-isp.o \ > rkisp1-resizer.o \ >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >index f08b3dec1465..4ba30f172c8b 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >@@ -123,7 +123,6 @@ struct rkisp1_info { > * @mbus_flags: media bus (V4L2_MBUS_*) flags > * @sd: a pointer to v4l2_subdev struct of the sensor > * @pixel_rate_ctrl: pixel rate of the sensor, used to initialize the phy >- * @dphy: a pointer to the phy > */ > struct rkisp1_sensor_async { > struct v4l2_async_subdev asd; >@@ -134,7 +133,19 @@ struct rkisp1_sensor_async { > unsigned int mbus_flags; > struct v4l2_subdev *sd; > struct v4l2_ctrl *pixel_rate_ctrl; >+}; >+ >+/* >+ * struct rkisp1_csi - CSI receiver subdev >+ * >+ * @rkisp1: pointer to the rkisp1 device >+ * @dphy: a pointer to the phy >+ * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt) >+ */ >+struct rkisp1_csi { >+ struct rkisp1_device *rkisp1; > struct phy *dphy; >+ bool is_dphy_errctrl_disabled; > }; > > /* >@@ -147,7 +158,6 @@ struct rkisp1_sensor_async { > * @sink_fmt: input format > * @src_fmt: output format > * @ops_lock: ops serialization >- * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt) > * @frame_sequence: used to synchronize frame_id between video devices. > */ > struct rkisp1_isp { >@@ -157,7 +167,6 @@ struct rkisp1_isp { > const struct rkisp1_mbus_info *sink_fmt; > const struct rkisp1_mbus_info *src_fmt; > struct mutex ops_lock; /* serialize the subdevice ops */ >- bool is_dphy_errctrl_disabled; > __u32 frame_sequence; > }; > >@@ -402,6 +411,7 @@ struct rkisp1_debug { > * @media_dev: media_device variable > * @notifier: a notifier to register on the v4l2-async API to be notified on the sensor > * @active_sensor: sensor in-use, set when streaming on >+ * @csi: internal CSI-2 receiver > * @isp: ISP sub-device > * @resizer_devs: resizer sub-devices > * @capture_devs: capture devices >@@ -421,6 +431,7 @@ struct rkisp1_device { > struct media_device media_dev; > struct v4l2_async_notifier notifier; > struct rkisp1_sensor_async *active_sensor; >+ struct rkisp1_csi csi; > struct rkisp1_isp isp; > struct rkisp1_resizer resizer_devs[2]; > struct rkisp1_capture capture_devs[2]; >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c >new file mode 100644 >index 000000000000..b5732511459f >--- /dev/null >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c >@@ -0,0 +1,193 @@ >+// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >+/* >+ * Rockchip ISP1 Driver - CSI-2 Receiver >+ * >+ * Copyright (C) 2019 Collabora, Ltd. >+ * Copyright (C) 2022 Ideas on Board >+ * >+ * Based on Rockchip ISP1 driver by Rockchip Electronics Co., Ltd. >+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd. >+ */ >+ >+#include <linux/device.h> >+#include <linux/phy/phy.h> >+#include <linux/phy/phy-mipi-dphy.h> >+ >+#include <media/v4l2-ctrls.h> >+ >+#include "rkisp1-common.h" >+#include "rkisp1-csi.h" >+ >+int rkisp1_config_mipi(struct rkisp1_csi *csi) >+{ >+ struct rkisp1_device *rkisp1 = csi->rkisp1; >+ const struct rkisp1_mbus_info *sink_fmt = rkisp1->isp.sink_fmt; >+ unsigned int lanes = rkisp1->active_sensor->lanes; >+ u32 mipi_ctrl; >+ >+ if (lanes < 1 || lanes > 4) >+ return -EINVAL; >+ >+ mipi_ctrl = RKISP1_CIF_MIPI_CTRL_NUM_LANES(lanes - 1) | >+ RKISP1_CIF_MIPI_CTRL_SHUTDOWNLANES(0xf) | >+ RKISP1_CIF_MIPI_CTRL_ERR_SOT_SYNC_HS_SKIP | >+ RKISP1_CIF_MIPI_CTRL_CLOCKLANE_ENA; >+ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, mipi_ctrl); >+ >+ /* V12 could also use a newer csi2-host, but we don't want that yet */ >+ if (rkisp1->info->isp_ver == RKISP1_V12) >+ rkisp1_write(rkisp1, RKISP1_CIF_ISP_CSI0_CTRL0, 0); >+ >+ /* Configure Data Type and Virtual Channel */ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL, >+ RKISP1_CIF_MIPI_DATA_SEL_DT(sink_fmt->mipi_dt) | >+ RKISP1_CIF_MIPI_DATA_SEL_VC(0)); >+ >+ /* Clear MIPI interrupts */ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, ~0); >+ >+ /* >+ * Disable RKISP1_CIF_MIPI_ERR_DPHY interrupt here temporary for >+ * isp bus may be dead when switch isp. >+ */ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, >+ RKISP1_CIF_MIPI_FRAME_END | RKISP1_CIF_MIPI_ERR_CSI | >+ RKISP1_CIF_MIPI_ERR_DPHY | >+ RKISP1_CIF_MIPI_SYNC_FIFO_OVFLW(0x03) | >+ RKISP1_CIF_MIPI_ADD_DATA_OVFLW); >+ >+ dev_dbg(rkisp1->dev, "\n MIPI_CTRL 0x%08x\n" >+ " MIPI_IMG_DATA_SEL 0x%08x\n" >+ " MIPI_STATUS 0x%08x\n" >+ " MIPI_IMSC 0x%08x\n", >+ rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL), >+ rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL), >+ rkisp1_read(rkisp1, RKISP1_CIF_MIPI_STATUS), >+ rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC)); >+ >+ return 0; >+} >+ >+int rkisp1_mipi_csi2_start(struct rkisp1_csi *csi, >+ struct rkisp1_sensor_async *sensor) >+{ >+ struct rkisp1_device *rkisp1 = csi->rkisp1; >+ union phy_configure_opts opts; >+ struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; >+ s64 pixel_clock; >+ >+ pixel_clock = v4l2_ctrl_g_ctrl_int64(sensor->pixel_rate_ctrl); >+ if (!pixel_clock) { >+ dev_err(rkisp1->dev, "Invalid pixel rate value\n"); >+ return -EINVAL; >+ } >+ >+ phy_mipi_dphy_get_default_config(pixel_clock, >+ rkisp1->isp.sink_fmt->bus_width, >+ sensor->lanes, cfg); >+ phy_set_mode(csi->dphy, PHY_MODE_MIPI_DPHY); >+ phy_configure(csi->dphy, &opts); >+ phy_power_on(csi->dphy); >+ >+ return 0; >+} >+ >+void rkisp1_mipi_csi2_stop(struct rkisp1_csi *csi) >+{ >+ phy_power_off(csi->dphy); >+} >+ >+void rkisp1_mipi_start(struct rkisp1_csi *csi) >+{ >+ struct rkisp1_device *rkisp1 = csi->rkisp1; >+ u32 val; >+ >+ val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL); >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, >+ val | RKISP1_CIF_MIPI_CTRL_OUTPUT_ENA); >+} >+ >+void rkisp1_mipi_stop(struct rkisp1_csi *csi) >+{ >+ struct rkisp1_device *rkisp1 = csi->rkisp1; >+ u32 val; >+ >+ /* Mask and clear interrupts. */ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, 0); >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, ~0); >+ >+ val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL); >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, >+ val & (~RKISP1_CIF_MIPI_CTRL_OUTPUT_ENA)); >+} >+ >+irqreturn_t rkisp1_mipi_isr(int irq, void *ctx) >+{ >+ struct device *dev = ctx; >+ struct rkisp1_device *rkisp1 = dev_get_drvdata(dev); >+ u32 val, status; >+ >+ status = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_MIS); >+ if (!status) >+ return IRQ_NONE; >+ >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, status); >+ >+ /* >+ * Disable DPHY errctrl interrupt, because this dphy >+ * erctrl signal is asserted until the next changes >+ * of line state. This time is may be too long and cpu >+ * is hold in this interrupt. >+ */ >+ if (status & RKISP1_CIF_MIPI_ERR_CTRL(0x0f)) { >+ val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC); >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, >+ val & ~RKISP1_CIF_MIPI_ERR_CTRL(0x0f)); >+ rkisp1->csi.is_dphy_errctrl_disabled = true; >+ } >+ >+ /* >+ * Enable DPHY errctrl interrupt again, if mipi have receive >+ * the whole frame without any error. >+ */ >+ if (status == RKISP1_CIF_MIPI_FRAME_END) { >+ /* >+ * Enable DPHY errctrl interrupt again, if mipi have receive >+ * the whole frame without any error. >+ */ >+ if (rkisp1->csi.is_dphy_errctrl_disabled) { >+ val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC); >+ val |= RKISP1_CIF_MIPI_ERR_CTRL(0x0f); >+ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, val); >+ rkisp1->csi.is_dphy_errctrl_disabled = false; >+ } >+ } else { >+ rkisp1->debug.mipi_error++; >+ } >+ >+ return IRQ_HANDLED; >+} >+ >+int rkisp1_csi_init(struct rkisp1_device *rkisp1) >+{ >+ struct rkisp1_csi *csi = &rkisp1->csi; >+ >+ csi->rkisp1 = rkisp1; >+ >+ csi->dphy = devm_phy_get(rkisp1->dev, "dphy"); >+ if (IS_ERR(csi->dphy)) >+ return dev_err_probe(rkisp1->dev, PTR_ERR(csi->dphy), >+ "Couldn't get the MIPI D-PHY\n"); >+ >+ phy_init(csi->dphy); >+ >+ return 0; >+} >+ >+void rkisp1_csi_cleanup(struct rkisp1_device *rkisp1) >+{ >+ struct rkisp1_csi *csi = &rkisp1->csi; >+ >+ phy_exit(csi->dphy); >+} >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h >new file mode 100644 >index 000000000000..d97a4ee5c002 >--- /dev/null >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h >@@ -0,0 +1,28 @@ >+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ >+/* >+ * Rockchip ISP1 Driver - CSI-2 Receiver >+ * >+ * Copyright (C) 2019 Collabora, Ltd. >+ * Copyright (C) 2022 Ideas on Board >+ * >+ * Based on Rockchip ISP1 driver by Rockchip Electronics Co., Ltd. >+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd. >+ */ >+#ifndef _RKISP1_CSI_H >+#define _RKISP1_CSI_H >+ >+struct rkisp1_device; >+struct rkisp1_sensor_async; >+ >+int rkisp1_csi_init(struct rkisp1_device *rkisp1); >+void rkisp1_csi_cleanup(struct rkisp1_device *rkisp1); >+ >+int rkisp1_config_mipi(struct rkisp1_csi *csi); >+ >+int rkisp1_mipi_csi2_start(struct rkisp1_csi *csi, >+ struct rkisp1_sensor_async *sensor); >+void rkisp1_mipi_csi2_stop(struct rkisp1_csi *csi); >+void rkisp1_mipi_start(struct rkisp1_csi *csi); >+void rkisp1_mipi_stop(struct rkisp1_csi *csi); >+ >+#endif /* _RKISP1_CSI_H */ >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index 813c013139ea..2afaa9f26f29 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -15,11 +15,11 @@ > #include <linux/of_graph.h> > #include <linux/of_platform.h> > #include <linux/pinctrl/consumer.h> >-#include <linux/phy/phy.h> >-#include <linux/phy/phy-mipi-dphy.h> >+#include <linux/pm_runtime.h> > #include <media/v4l2-fwnode.h> > > #include "rkisp1-common.h" >+#include "rkisp1-csi.h" > > /* > * ISP Details >@@ -128,14 +128,6 @@ static int rkisp1_subdev_notifier_bound(struct v4l2_async_notifier *notifier, > } > > s_asd->sd = sd; >- s_asd->dphy = devm_phy_get(rkisp1->dev, "dphy"); >- if (IS_ERR(s_asd->dphy)) { >- if (PTR_ERR(s_asd->dphy) != -EPROBE_DEFER) >- dev_err(rkisp1->dev, "Couldn't get the MIPI D-PHY\n"); >- return PTR_ERR(s_asd->dphy); >- } >- >- phy_init(s_asd->dphy); > > /* Create the link to the sensor. */ > source_pad = media_entity_get_fwnode_pad(&sd->entity, s_asd->source_ep, >@@ -152,16 +144,6 @@ static int rkisp1_subdev_notifier_bound(struct v4l2_async_notifier *notifier, > !s_asd->index ? MEDIA_LNK_FL_ENABLED : 0); > } > >-static void rkisp1_subdev_notifier_unbind(struct v4l2_async_notifier *notifier, >- struct v4l2_subdev *sd, >- struct v4l2_async_subdev *asd) >-{ >- struct rkisp1_sensor_async *s_asd = >- container_of(asd, struct rkisp1_sensor_async, asd); >- >- phy_exit(s_asd->dphy); >-} >- > static int rkisp1_subdev_notifier_complete(struct v4l2_async_notifier *notifier) > { > struct rkisp1_device *rkisp1 = >@@ -180,7 +162,6 @@ static void rkisp1_subdev_notifier_destroy(struct v4l2_async_subdev *asd) > > static const struct v4l2_async_notifier_operations rkisp1_subdev_notifier_ops = { > .bound = rkisp1_subdev_notifier_bound, >- .unbind = rkisp1_subdev_notifier_unbind, > .complete = rkisp1_subdev_notifier_complete, > .destroy = rkisp1_subdev_notifier_destroy, > }; >@@ -540,14 +521,20 @@ static int rkisp1_probe(struct platform_device *pdev) > goto err_unreg_v4l2_dev; > } > >- ret = rkisp1_entities_register(rkisp1); >+ ret = rkisp1_csi_init(rkisp1); > if (ret) > goto err_unreg_media_dev; > >+ ret = rkisp1_entities_register(rkisp1); >+ if (ret) >+ goto err_cleanup_csi; >+ > rkisp1_debug_init(rkisp1); > > return 0; > >+err_cleanup_csi: >+ rkisp1_csi_cleanup(rkisp1); > err_unreg_media_dev: > media_device_unregister(&rkisp1->media_dev); > err_unreg_v4l2_dev: >@@ -565,6 +552,7 @@ static int rkisp1_remove(struct platform_device *pdev) > v4l2_async_nf_cleanup(&rkisp1->notifier); > > rkisp1_entities_unregister(rkisp1); >+ rkisp1_csi_cleanup(rkisp1); > rkisp1_debug_cleanup(rkisp1); > > media_device_unregister(&rkisp1->media_dev); >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >index 56781b53dd83..c05148dd32c0 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >@@ -9,8 +9,6 @@ > */ > > #include <linux/iopoll.h> >-#include <linux/phy/phy.h> >-#include <linux/phy/phy-mipi-dphy.h> > #include <linux/pm_runtime.h> > #include <linux/videodev2.h> > #include <linux/vmalloc.h> >@@ -18,6 +16,7 @@ > #include <media/v4l2-event.h> > > #include "rkisp1-common.h" >+#include "rkisp1-csi.h" > > #define RKISP1_DEF_SINK_PAD_FMT MEDIA_BUS_FMT_SRGGB10_1X10 > #define RKISP1_DEF_SRC_PAD_FMT MEDIA_BUS_FMT_YUYV8_2X8 >@@ -265,55 +264,6 @@ static int rkisp1_config_dvp(struct rkisp1_device *rkisp1) > return 0; > } > >-static int rkisp1_config_mipi(struct rkisp1_device *rkisp1) >-{ >- const struct rkisp1_mbus_info *sink_fmt = rkisp1->isp.sink_fmt; >- unsigned int lanes = rkisp1->active_sensor->lanes; >- u32 mipi_ctrl; >- >- if (lanes < 1 || lanes > 4) >- return -EINVAL; >- >- mipi_ctrl = RKISP1_CIF_MIPI_CTRL_NUM_LANES(lanes - 1) | >- RKISP1_CIF_MIPI_CTRL_SHUTDOWNLANES(0xf) | >- RKISP1_CIF_MIPI_CTRL_ERR_SOT_SYNC_HS_SKIP | >- RKISP1_CIF_MIPI_CTRL_CLOCKLANE_ENA; >- >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, mipi_ctrl); >- >- /* V12 could also use a newer csi2-host, but we don't want that yet */ >- if (rkisp1->info->isp_ver == RKISP1_V12) >- rkisp1_write(rkisp1, RKISP1_CIF_ISP_CSI0_CTRL0, 0); >- >- /* Configure Data Type and Virtual Channel */ >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL, >- RKISP1_CIF_MIPI_DATA_SEL_DT(sink_fmt->mipi_dt) | >- RKISP1_CIF_MIPI_DATA_SEL_VC(0)); >- >- /* Clear MIPI interrupts */ >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, ~0); >- /* >- * Disable RKISP1_CIF_MIPI_ERR_DPHY interrupt here temporary for >- * isp bus may be dead when switch isp. >- */ >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, >- RKISP1_CIF_MIPI_FRAME_END | RKISP1_CIF_MIPI_ERR_CSI | >- RKISP1_CIF_MIPI_ERR_DPHY | >- RKISP1_CIF_MIPI_SYNC_FIFO_OVFLW(0x03) | >- RKISP1_CIF_MIPI_ADD_DATA_OVFLW); >- >- dev_dbg(rkisp1->dev, "\n MIPI_CTRL 0x%08x\n" >- " MIPI_IMG_DATA_SEL 0x%08x\n" >- " MIPI_STATUS 0x%08x\n" >- " MIPI_IMSC 0x%08x\n", >- rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL), >- rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMG_DATA_SEL), >- rkisp1_read(rkisp1, RKISP1_CIF_MIPI_STATUS), >- rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC)); >- >- return 0; >-} >- > /* Configure MUX */ > static int rkisp1_config_path(struct rkisp1_device *rkisp1) > { >@@ -326,7 +276,7 @@ static int rkisp1_config_path(struct rkisp1_device *rkisp1) > ret = rkisp1_config_dvp(rkisp1); > dpcl |= RKISP1_CIF_VI_DPCL_IF_SEL_PARALLEL; > } else if (sensor->mbus_type == V4L2_MBUS_CSI2_DPHY) { >- ret = rkisp1_config_mipi(rkisp1); >+ ret = rkisp1_config_mipi(&rkisp1->csi); > dpcl |= RKISP1_CIF_VI_DPCL_IF_SEL_MIPI; > } > >@@ -360,17 +310,14 @@ static void rkisp1_isp_stop(struct rkisp1_device *rkisp1) > * Stop ISP(isp) ->wait for ISP isp off > */ > /* stop and clear MI, MIPI, and ISP interrupts */ >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, 0); >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, ~0); >- > rkisp1_write(rkisp1, RKISP1_CIF_ISP_IMSC, 0); > rkisp1_write(rkisp1, RKISP1_CIF_ISP_ICR, ~0); > > rkisp1_write(rkisp1, RKISP1_CIF_MI_IMSC, 0); > rkisp1_write(rkisp1, RKISP1_CIF_MI_ICR, ~0); >- val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL); >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, >- val & (~RKISP1_CIF_MIPI_CTRL_OUTPUT_ENA)); >+ >+ rkisp1_mipi_stop(&rkisp1->csi); >+ > /* stop ISP */ > val = rkisp1_read(rkisp1, RKISP1_CIF_ISP_CTRL); > val &= ~(RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE | >@@ -417,11 +364,9 @@ static void rkisp1_isp_start(struct rkisp1_device *rkisp1) > rkisp1_config_clk(rkisp1); > > /* Activate MIPI */ >- if (sensor->mbus_type == V4L2_MBUS_CSI2_DPHY) { >- val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL); >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_CTRL, >- val | RKISP1_CIF_MIPI_CTRL_OUTPUT_ENA); >- } >+ if (sensor->mbus_type == V4L2_MBUS_CSI2_DPHY) >+ rkisp1_mipi_start(&rkisp1->csi); >+ > /* Activate ISP */ > val = rkisp1_read(rkisp1, RKISP1_CIF_ISP_CTRL); > val |= RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD | >@@ -814,35 +759,6 @@ static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = { > * Stream operations > */ > >-static int rkisp1_mipi_csi2_start(struct rkisp1_isp *isp, >- struct rkisp1_sensor_async *sensor) >-{ >- struct rkisp1_device *rkisp1 = >- container_of(isp->sd.v4l2_dev, struct rkisp1_device, v4l2_dev); >- union phy_configure_opts opts; >- struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; >- s64 pixel_clock; >- >- pixel_clock = v4l2_ctrl_g_ctrl_int64(sensor->pixel_rate_ctrl); >- if (!pixel_clock) { >- dev_err(rkisp1->dev, "Invalid pixel rate value\n"); >- return -EINVAL; >- } >- >- phy_mipi_dphy_get_default_config(pixel_clock, isp->sink_fmt->bus_width, >- sensor->lanes, cfg); >- phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY); >- phy_configure(sensor->dphy, &opts); >- phy_power_on(sensor->dphy); >- >- return 0; >-} >- >-static void rkisp1_mipi_csi2_stop(struct rkisp1_sensor_async *sensor) >-{ >- phy_power_off(sensor->dphy); >-} >- > static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable) > { > struct rkisp1_device *rkisp1 = >@@ -856,7 +772,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable) > false); > > rkisp1_isp_stop(rkisp1); >- rkisp1_mipi_csi2_stop(rkisp1->active_sensor); >+ rkisp1_mipi_csi2_stop(&rkisp1->csi); > return 0; > } > >@@ -878,7 +794,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable) > if (ret) > goto mutex_unlock; > >- ret = rkisp1_mipi_csi2_start(&rkisp1->isp, rkisp1->active_sensor); >+ ret = rkisp1_mipi_csi2_start(&rkisp1->csi, rkisp1->active_sensor); > if (ret) > goto mutex_unlock; > >@@ -888,7 +804,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable) > true); > if (ret) { > rkisp1_isp_stop(rkisp1); >- rkisp1_mipi_csi2_stop(rkisp1->active_sensor); >+ rkisp1_mipi_csi2_stop(&rkisp1->csi); > goto mutex_unlock; > } > >@@ -993,53 +909,6 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1) > * Interrupt handlers > */ > >-irqreturn_t rkisp1_mipi_isr(int irq, void *ctx) >-{ >- struct device *dev = ctx; >- struct rkisp1_device *rkisp1 = dev_get_drvdata(dev); >- u32 val, status; >- >- status = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_MIS); >- if (!status) >- return IRQ_NONE; >- >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, status); >- >- /* >- * Disable DPHY errctrl interrupt, because this dphy >- * erctrl signal is asserted until the next changes >- * of line state. This time is may be too long and cpu >- * is hold in this interrupt. >- */ >- if (status & RKISP1_CIF_MIPI_ERR_CTRL(0x0f)) { >- val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC); >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, >- val & ~RKISP1_CIF_MIPI_ERR_CTRL(0x0f)); >- rkisp1->isp.is_dphy_errctrl_disabled = true; >- } >- >- /* >- * Enable DPHY errctrl interrupt again, if mipi have receive >- * the whole frame without any error. >- */ >- if (status == RKISP1_CIF_MIPI_FRAME_END) { >- /* >- * Enable DPHY errctrl interrupt again, if mipi have receive >- * the whole frame without any error. >- */ >- if (rkisp1->isp.is_dphy_errctrl_disabled) { >- val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC); >- val |= RKISP1_CIF_MIPI_ERR_CTRL(0x0f); >- rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, val); >- rkisp1->isp.is_dphy_errctrl_disabled = false; >- } >- } else { >- rkisp1->debug.mipi_error++; >- } >- >- return IRQ_HANDLED; >-} >- > static void rkisp1_isp_queue_event_sof(struct rkisp1_isp *isp) > { > struct v4l2_event event = { >-- >Regards, > >Laurent Pinchart >
On 01.07.2022 02:06, Laurent Pinchart wrote: >The ISP_ACQ_PROP register is set twice, once in rkisp1_config_isp() for >most of its fields, and once in rkisp1_config_dvp() (called from >rkisp1_config_path()) to configure the input selection field. Move the >latter to rkisp1_config_isp() to write the register once only, and drop >the now empty rkisp1_config_dvp() function. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by Dafna Hirschfeld <dafna@fastmail.com> >--- >Changes since v1: > >- Print the value of unsupported bus width >- Remove unneeded curly braces >--- > .../platform/rockchip/rkisp1/rkisp1-isp.c | 66 +++++++------------ > 1 file changed, 24 insertions(+), 42 deletions(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >index 9b32ae585de8..85c1995bb5c2 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >@@ -141,7 +141,7 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > enum v4l2_mbus_type mbus_type, u32 mbus_flags) > { > struct rkisp1_device *rkisp1 = isp->rkisp1; >- u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, signal = 0; >+ u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, signal = 0, input_sel = 0; > const struct rkisp1_mbus_info *src_fmt, *sink_fmt; > struct v4l2_mbus_framefmt *sink_frm; > struct v4l2_rect *sink_crop; >@@ -189,6 +189,22 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > if (mbus_type == V4L2_MBUS_BT656 || mbus_type == V4L2_MBUS_PARALLEL) { > if (mbus_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > signal = RKISP1_CIF_ISP_ACQ_PROP_POS_EDGE; >+ >+ switch (sink_fmt->bus_width) { >+ case 8: >+ input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_8B_ZERO; >+ break; >+ case 10: >+ input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_10B_ZERO; >+ break; >+ case 12: >+ input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_12B; >+ break; >+ default: >+ dev_err(rkisp1->dev, "Invalid bus width %u\n", >+ sink_fmt->bus_width); >+ return -EINVAL; >+ } > } > > if (mbus_type == V4L2_MBUS_PARALLEL) { >@@ -201,7 +217,7 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > > rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, isp_ctrl); > rkisp1_write(rkisp1, RKISP1_CIF_ISP_ACQ_PROP, >- signal | sink_fmt->yuv_seq | >+ signal | sink_fmt->yuv_seq | input_sel | > RKISP1_CIF_ISP_ACQ_PROP_BAYER_PAT(sink_fmt->bayer_pat) | > RKISP1_CIF_ISP_ACQ_PROP_FIELD_SEL_ALL); > rkisp1_write(rkisp1, RKISP1_CIF_ISP_ACQ_NR_FRAMES, 0); >@@ -238,52 +254,19 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > return 0; > } > >-static int rkisp1_config_dvp(struct rkisp1_isp *isp) >-{ >- struct rkisp1_device *rkisp1 = isp->rkisp1; >- const struct rkisp1_mbus_info *sink_fmt = isp->sink_fmt; >- u32 val, input_sel; >- >- switch (sink_fmt->bus_width) { >- case 8: >- input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_8B_ZERO; >- break; >- case 10: >- input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_10B_ZERO; >- break; >- case 12: >- input_sel = RKISP1_CIF_ISP_ACQ_PROP_IN_SEL_12B; >- break; >- default: >- dev_err(rkisp1->dev, "Invalid bus width\n"); >- return -EINVAL; >- } >- >- val = rkisp1_read(rkisp1, RKISP1_CIF_ISP_ACQ_PROP); >- rkisp1_write(rkisp1, RKISP1_CIF_ISP_ACQ_PROP, val | input_sel); >- >- return 0; >-} >- > /* Configure MUX */ >-static int rkisp1_config_path(struct rkisp1_isp *isp, >- enum v4l2_mbus_type mbus_type) >+static void rkisp1_config_path(struct rkisp1_isp *isp, >+ enum v4l2_mbus_type mbus_type) > { > struct rkisp1_device *rkisp1 = isp->rkisp1; > u32 dpcl = rkisp1_read(rkisp1, RKISP1_CIF_VI_DPCL); >- int ret = 0; > >- if (mbus_type == V4L2_MBUS_BT656 || >- mbus_type == V4L2_MBUS_PARALLEL) { >- ret = rkisp1_config_dvp(isp); >+ if (mbus_type == V4L2_MBUS_BT656 || mbus_type == V4L2_MBUS_PARALLEL) > dpcl |= RKISP1_CIF_VI_DPCL_IF_SEL_PARALLEL; >- } else if (mbus_type == V4L2_MBUS_CSI2_DPHY) { >+ else if (mbus_type == V4L2_MBUS_CSI2_DPHY) > dpcl |= RKISP1_CIF_VI_DPCL_IF_SEL_MIPI; >- } > > rkisp1_write(rkisp1, RKISP1_CIF_VI_DPCL, dpcl); >- >- return ret; > } > > /* Hardware configure Entry */ >@@ -295,9 +278,8 @@ static int rkisp1_config_cif(struct rkisp1_isp *isp, > ret = rkisp1_config_isp(isp, mbus_type, mbus_flags); > if (ret) > return ret; >- ret = rkisp1_config_path(isp, mbus_type); >- if (ret) >- return ret; >+ >+ rkisp1_config_path(isp, mbus_type); > rkisp1_config_ism(isp); > > return 0; >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:06, Laurent Pinchart wrote: >From: Paul Elder <paul.elder@ideasonboard.com> > >The CSI receiver is a component separate from the ISP or the resizers. >It is actually optional, not all device model include a CSI receiver. On >some SoCs CSI-2 support can be provided through an external CSI-2 >receiver, connected to the ISP's parallel input. > >To support those use cases, create a V4L2 subdev to model the CSI >receiver. It will allow the driver to handle both internal and external >CSI receivers the same way. > >The next commit will plumb the CSI subdev to the rest of the driver, >replacing direct function calls. > >Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >--- >Changes since v1: > >- Rename RKISP1_CSI_PAD_MAX to RKISP1_CSI_PAD_NUM >- Simplify format propagation >- Fix usage of uninitialized variables >- White space fixes >--- > .../platform/rockchip/rkisp1/rkisp1-common.h | 17 ++ > .../platform/rockchip/rkisp1/rkisp1-csi.c | 288 ++++++++++++++++++ > .../platform/rockchip/rkisp1/rkisp1-csi.h | 4 + > .../platform/rockchip/rkisp1/rkisp1-dev.c | 5 + > 4 files changed, 314 insertions(+) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >index 5301461d3ea3..84832e1367ff 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >@@ -68,6 +68,13 @@ enum rkisp1_rsz_pad { > RKISP1_RSZ_PAD_MAX > }; > >+/* enum for the csi receiver pads */ >+enum rkisp1_csi_pad { >+ RKISP1_CSI_PAD_SINK, >+ RKISP1_CSI_PAD_SRC, >+ RKISP1_CSI_PAD_NUM >+}; >+ > /* enum for the capture id */ > enum rkisp1_stream_id { > RKISP1_MAINPATH, >@@ -141,11 +148,21 @@ struct rkisp1_sensor_async { > * @rkisp1: pointer to the rkisp1 device > * @dphy: a pointer to the phy > * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt) >+ * @sd: v4l2_subdev variable >+ * @pads: media pads >+ * @pad_cfg: configurations for the pads >+ * @ops_lock: a lock for the subdev ops >+ * @source: source in-use, set when starting streaming > */ > struct rkisp1_csi { > struct rkisp1_device *rkisp1; > struct phy *dphy; > bool is_dphy_errctrl_disabled; >+ struct v4l2_subdev sd; >+ struct media_pad pads[RKISP1_CSI_PAD_NUM]; >+ struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM]; >+ struct mutex ops_lock; /* serialize the subdevice ops */ >+ struct rkisp1_sensor_async *source; > }; > > /* >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c >index 81849c8e9c73..173a0550af5c 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c >@@ -15,10 +15,35 @@ > #include <linux/phy/phy-mipi-dphy.h> > > #include <media/v4l2-ctrls.h> >+#include <media/v4l2-fwnode.h> > > #include "rkisp1-common.h" > #include "rkisp1-csi.h" > >+#define RKISP1_CSI_DEV_NAME RKISP1_DRIVER_NAME "_csi" >+ >+#define RKISP1_CSI_DEF_FMT MEDIA_BUS_FMT_SRGGB10_1X10 >+ >+static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd) >+{ >+ return container_of(sd, struct rkisp1_csi, sd); >+} >+ >+static struct v4l2_mbus_framefmt * >+rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi, >+ struct v4l2_subdev_state *sd_state, >+ unsigned int pad, u32 which) >+{ >+ struct v4l2_subdev_state state = { >+ .pads = csi->pad_cfg >+ }; >+ >+ if (which == V4L2_SUBDEV_FORMAT_TRY) >+ return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad); >+ else >+ return v4l2_subdev_get_try_format(&csi->sd, &state, pad); >+} >+ > static int rkisp1_csi_config(struct rkisp1_csi *csi, > const struct rkisp1_sensor_async *sensor) > { >@@ -186,6 +211,269 @@ irqreturn_t rkisp1_csi_isr(int irq, void *ctx) > return IRQ_HANDLED; > } > >+/* ---------------------------------------------------------------------------- >+ * Subdev pad operations >+ */ >+ >+static void rkisp1_csi_set_src_fmt(struct rkisp1_csi *csi, >+ struct v4l2_subdev_state *sd_state, >+ struct v4l2_mbus_framefmt *format, >+ unsigned int which) >+{ >+ struct v4l2_mbus_framefmt *sink_fmt; >+ >+ /* We don't set the src format directly; take it from the sink format */ >+ sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK, >+ which); >+ >+ *format = *sink_fmt; >+} >+ >+static void rkisp1_csi_set_sink_fmt(struct rkisp1_csi *csi, >+ struct v4l2_subdev_state *sd_state, >+ struct v4l2_mbus_framefmt *format, >+ unsigned int which) >+{ >+ const struct rkisp1_mbus_info *mbus_info; >+ struct v4l2_mbus_framefmt *sink_fmt, *src_fmt; >+ >+ sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK, >+ which); >+ src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC, >+ which); >+ >+ sink_fmt->code = format->code; >+ >+ mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code); >+ if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) { >+ sink_fmt->code = RKISP1_CSI_DEF_FMT; >+ mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code); >+ } >+ >+ sink_fmt->width = clamp_t(u32, format->width, >+ RKISP1_ISP_MIN_WIDTH, >+ RKISP1_ISP_MAX_WIDTH); >+ sink_fmt->height = clamp_t(u32, format->height, >+ RKISP1_ISP_MIN_HEIGHT, >+ RKISP1_ISP_MAX_HEIGHT); >+ >+ /* Propagate to source pad */ >+ *src_fmt = *sink_fmt; >+ >+ *format = *sink_fmt; >+} >+ >+static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd, >+ struct v4l2_subdev_state *sd_state, >+ struct v4l2_subdev_mbus_code_enum *code) >+{ >+ unsigned int i; >+ int pos = 0; >+ >+ for (i = 0; ; i++) { >+ const struct rkisp1_mbus_info *fmt = >+ rkisp1_mbus_info_get_by_index(i); >+ >+ if (!fmt) >+ return -EINVAL; >+ >+ if (fmt->direction & RKISP1_ISP_SD_SINK) >+ pos++; >+ >+ if (code->index == pos - 1) { >+ code->code = fmt->mbus_code; >+ return 0; >+ } >+ } >+ >+ return -EINVAL; >+} >+ >+static int rkisp1_csi_init_config(struct v4l2_subdev *sd, >+ struct v4l2_subdev_state *sd_state) >+{ >+ struct v4l2_mbus_framefmt *sink_fmt, *src_fmt; >+ >+ sink_fmt = v4l2_subdev_get_try_format(sd, sd_state, >+ RKISP1_CSI_PAD_SINK); >+ src_fmt = v4l2_subdev_get_try_format(sd, sd_state, >+ RKISP1_CSI_PAD_SRC); >+ >+ sink_fmt->width = RKISP1_DEFAULT_WIDTH; >+ sink_fmt->height = RKISP1_DEFAULT_HEIGHT; >+ sink_fmt->field = V4L2_FIELD_NONE; >+ sink_fmt->code = RKISP1_CSI_DEF_FMT; >+ >+ *src_fmt = *sink_fmt; >+ >+ return 0; >+} >+ >+static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd, >+ struct v4l2_subdev_state *sd_state, >+ struct v4l2_subdev_format *fmt) >+{ >+ struct rkisp1_csi *csi = to_rkisp1_csi(sd); >+ >+ mutex_lock(&csi->ops_lock); >+ fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad, >+ fmt->which); >+ mutex_unlock(&csi->ops_lock); >+ return 0; >+} >+ >+static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd, >+ struct v4l2_subdev_state *sd_state, >+ struct v4l2_subdev_format *fmt) >+{ >+ struct rkisp1_csi *csi = to_rkisp1_csi(sd); >+ >+ mutex_lock(&csi->ops_lock); >+ if (fmt->pad == RKISP1_CSI_PAD_SINK) >+ rkisp1_csi_set_sink_fmt(csi, sd_state, &fmt->format, >+ fmt->which); >+ else >+ rkisp1_csi_set_src_fmt(csi, sd_state, &fmt->format, >+ fmt->which); >+ >+ mutex_unlock(&csi->ops_lock); >+ return 0; >+} >+ >+/* ---------------------------------------------------------------------------- >+ * Subdev video operations >+ */ >+ >+static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable) >+{ >+ struct rkisp1_csi *csi = to_rkisp1_csi(sd); >+ struct rkisp1_device *rkisp1 = csi->rkisp1; >+ struct media_pad *source_pad; >+ struct v4l2_subdev *source; >+ int ret; >+ >+ if (!enable) { >+ v4l2_subdev_call(csi->source->sd, video, s_stream, >+ false); >+ >+ rkisp1_csi_stop(csi); >+ >+ return 0; >+ } >+ >+ source_pad = media_entity_remote_source_pad_unique(&sd->entity); >+ if (IS_ERR(source_pad)) { >+ dev_dbg(rkisp1->dev, "Failed to get source for CSI: %d\n", >+ IS_ERR(source_pad)); s/IS_ERR/PTR_ERR/ >+ return -EPIPE; >+ } >+ >+ source = media_entity_to_v4l2_subdev(source_pad->entity); >+ if (!source) { >+ /* This should really not happen, so is not worth a message. */ >+ return -EPIPE; >+ } >+ >+ csi->source = container_of(source->asd, struct rkisp1_sensor_async, >+ asd); >+ >+ if (csi->source->mbus_type != V4L2_MBUS_CSI2_DPHY) >+ return -EINVAL; I would set here csi->source to NULL, and shouldn't setting it be inside the lock? thanks, Dafna >+ >+ mutex_lock(&csi->ops_lock); >+ ret = rkisp1_csi_start(csi, csi->source); >+ mutex_unlock(&csi->ops_lock); >+ if (ret) >+ return ret; >+ >+ v4l2_subdev_call(csi->source->sd, video, s_stream, true); >+ >+ return 0; >+} >+ >+/* ---------------------------------------------------------------------------- >+ * Registration >+ */ >+ >+static const struct media_entity_operations rkisp1_csi_media_ops = { >+ .link_validate = v4l2_subdev_link_validate, >+}; >+ >+static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = { >+ .s_stream = rkisp1_csi_s_stream, >+}; >+ >+static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = { >+ .enum_mbus_code = rkisp1_csi_enum_mbus_code, >+ .init_cfg = rkisp1_csi_init_config, >+ .get_fmt = rkisp1_csi_get_fmt, >+ .set_fmt = rkisp1_csi_set_fmt, >+}; >+ >+static const struct v4l2_subdev_ops rkisp1_csi_ops = { >+ .video = &rkisp1_csi_video_ops, >+ .pad = &rkisp1_csi_pad_ops, >+}; >+ >+int rkisp1_csi_register(struct rkisp1_device *rkisp1) >+{ >+ struct rkisp1_csi *csi = &rkisp1->csi; >+ struct v4l2_subdev_state state = {}; >+ struct media_pad *pads; >+ struct v4l2_subdev *sd; >+ int ret; >+ >+ csi->rkisp1 = rkisp1; >+ mutex_init(&csi->ops_lock); >+ >+ sd = &csi->sd; >+ v4l2_subdev_init(sd, &rkisp1_csi_ops); >+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; >+ sd->entity.ops = &rkisp1_csi_media_ops; >+ sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; >+ sd->owner = THIS_MODULE; >+ strscpy(sd->name, RKISP1_CSI_DEV_NAME, sizeof(sd->name)); >+ >+ pads = csi->pads; >+ pads[RKISP1_CSI_PAD_SINK].flags = MEDIA_PAD_FL_SINK | >+ MEDIA_PAD_FL_MUST_CONNECT; >+ pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE | >+ MEDIA_PAD_FL_MUST_CONNECT; >+ >+ ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads); >+ if (ret) >+ goto error; >+ >+ state.pads = csi->pad_cfg; >+ rkisp1_csi_init_config(sd, &state); >+ >+ ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd); >+ if (ret) { >+ dev_err(sd->dev, "Failed to register csi receiver subdev\n"); >+ goto error; >+ } >+ >+ return 0; >+ >+error: >+ media_entity_cleanup(&sd->entity); >+ mutex_destroy(&csi->ops_lock); >+ csi->rkisp1 = NULL; >+ return ret; >+} >+ >+void rkisp1_csi_unregister(struct rkisp1_device *rkisp1) >+{ >+ struct rkisp1_csi *csi = &rkisp1->csi; >+ >+ if (!csi->rkisp1) >+ return; >+ >+ v4l2_device_unregister_subdev(&csi->sd); >+ media_entity_cleanup(&csi->sd.entity); >+ mutex_destroy(&csi->ops_lock); >+} >+ > int rkisp1_csi_init(struct rkisp1_device *rkisp1) > { > struct rkisp1_csi *csi = &rkisp1->csi; >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h >index 97ce7e7959ab..ddf8e5e08f55 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h >@@ -13,10 +13,14 @@ > > struct rkisp1_device; > struct rkisp1_sensor_async; >+struct v4l2_subdev; > > int rkisp1_csi_init(struct rkisp1_device *rkisp1); > void rkisp1_csi_cleanup(struct rkisp1_device *rkisp1); > >+int rkisp1_csi_register(struct rkisp1_device *rkisp1); >+void rkisp1_csi_unregister(struct rkisp1_device *rkisp1); >+ > int rkisp1_csi_start(struct rkisp1_csi *csi, > const struct rkisp1_sensor_async *sensor); > void rkisp1_csi_stop(struct rkisp1_csi *csi); >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index 2c441665c017..5428e19e818f 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -324,6 +324,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1) > > static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1) > { >+ rkisp1_csi_unregister(rkisp1); > rkisp1_params_unregister(rkisp1); > rkisp1_stats_unregister(rkisp1); > rkisp1_capture_devs_unregister(rkisp1); >@@ -355,6 +356,10 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1) > if (ret) > goto error; > >+ ret = rkisp1_csi_register(rkisp1); >+ if (ret) >+ goto error; >+ > ret = rkisp1_create_links(rkisp1); > if (ret) > goto error; >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:06, Laurent Pinchart wrote: >From: Paul Elder <paul.elder@ideasonboard.com> > >When registering the notifier, replace the manual while loop with >fwnode_graph_for_each_endpoint. This simplifies error handling. > >Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- > .../platform/rockchip/rkisp1/rkisp1-dev.c | 44 +++++++++---------- > 1 file changed, 20 insertions(+), 24 deletions(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index c3a7ab70bbef..0eb37ba557ce 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -168,29 +168,28 @@ static const struct v4l2_async_notifier_operations rkisp1_subdev_notifier_ops = > static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1) > { > struct v4l2_async_notifier *ntf = &rkisp1->notifier; >- unsigned int next_id = 0; >+ struct fwnode_handle *fwnode = dev_fwnode(rkisp1->dev); >+ struct fwnode_handle *ep; > unsigned int index = 0; >- int ret; >+ int ret = 0; > > v4l2_async_nf_init(ntf); > >- while (1) { >+ ntf->ops = &rkisp1_subdev_notifier_ops; >+ >+ fwnode_graph_for_each_endpoint(fwnode, ep) { > struct v4l2_fwnode_endpoint vep = { > .bus_type = V4L2_MBUS_CSI2_DPHY > }; > struct rkisp1_sensor_async *rk_asd; >- struct fwnode_handle *source = NULL; >- struct fwnode_handle *ep; >- >- ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(rkisp1->dev), >- 0, next_id, >- FWNODE_GRAPH_ENDPOINT_NEXT); >- if (!ep) >- break; >+ struct fwnode_handle *source; > > ret = v4l2_fwnode_endpoint_parse(ep, &vep); >- if (ret) >- goto err_parse; >+ if (ret) { >+ dev_err(rkisp1->dev, "failed to parse endpoint %pfw\n", >+ ep); >+ break; >+ } > > source = fwnode_graph_get_remote_endpoint(ep); > if (!source) { >@@ -198,14 +197,15 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1) > "endpoint %pfw has no remote endpoint\n", > ep); > ret = -ENODEV; >- goto err_parse; >+ break; > } > > rk_asd = v4l2_async_nf_add_fwnode(ntf, source, > struct rkisp1_sensor_async); > if (IS_ERR(rk_asd)) { >+ fwnode_handle_put(source); > ret = PTR_ERR(rk_asd); >- goto err_parse; >+ break; > } > > rk_asd->index = index++; >@@ -216,27 +216,23 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1) > > dev_dbg(rkisp1->dev, "registered ep id %d with %d lanes\n", > vep.base.id, rk_asd->lanes); >+ } > >- next_id = vep.base.id + 1; >- >- fwnode_handle_put(ep); >- >- continue; >-err_parse: >+ if (ret) { > fwnode_handle_put(ep); >- fwnode_handle_put(source); > v4l2_async_nf_cleanup(ntf); > return ret; > } > >- if (next_id == 0) >+ if (!index) > dev_dbg(rkisp1->dev, "no remote subdevice found\n"); >- ntf->ops = &rkisp1_subdev_notifier_ops; >+ > ret = v4l2_async_nf_register(&rkisp1->v4l2_dev, ntf); > if (ret) { > v4l2_async_nf_cleanup(ntf); > return ret; > } >+ > return 0; > } > >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:07, Laurent Pinchart wrote: >Different ISP versions implement different sets of features. The driver >already takes the version into account in several places, but this >approach won't scale well for features that are found in different >versions. Introduce a new mechanism using a features bitmask in the >rkisp1_info structure to indicate which features the ISP support. > >The first feature bit tells if the ISP has an internal CSI-2 receiver, >which is not available in all ISP versions. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- > .../platform/rockchip/rkisp1/rkisp1-common.h | 15 +++++++++++++++ > .../media/platform/rockchip/rkisp1/rkisp1-dev.c | 2 ++ > 2 files changed, 17 insertions(+) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >index e436f1572566..dedfcf3466c8 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >@@ -98,6 +98,19 @@ enum rkisp1_isp_pad { > RKISP1_ISP_PAD_MAX > }; > >+/* >+ * enum rkisp1_feature - ISP features >+ * >+ * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver >+ * >+ * The ISP features are stored in a bitmask in &rkisp1_info.features and allow >+ * the driver to implement support for features present in some ISP versions >+ * only. >+ */ >+enum rkisp1_feature { >+ RKISP1_FEATURE_MIPI_CSI2 = BIT(0), >+}; >+ > /* > * struct rkisp1_info - Model-specific ISP Information > * >@@ -106,6 +119,7 @@ enum rkisp1_isp_pad { > * @isrs: array of ISP interrupt descriptors > * @isr_size: number of entries in the @isrs array > * @isp_ver: ISP version >+ * @features: bitmatk of rkisp1_feature features implemented by the ISP > * > * This structure contains information about the ISP specific to a particular > * ISP model, version, or integration in a particular SoC. >@@ -116,6 +130,7 @@ struct rkisp1_info { > const struct rkisp1_isr_data *isrs; > unsigned int isr_size; > enum rkisp1_cif_isp_version isp_ver; >+ unsigned int features; > }; > > /* >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index 1dcade2fd2a7..bc278b49fefc 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -460,6 +460,7 @@ static const struct rkisp1_info px30_isp_info = { > .isrs = px30_isp_isrs, > .isr_size = ARRAY_SIZE(px30_isp_isrs), > .isp_ver = RKISP1_V12, >+ .features = RKISP1_FEATURE_MIPI_CSI2, > }; > > static const char * const rk3399_isp_clks[] = { >@@ -478,6 +479,7 @@ static const struct rkisp1_info rk3399_isp_info = { > .isrs = rk3399_isp_isrs, > .isr_size = ARRAY_SIZE(rk3399_isp_isrs), > .isp_ver = RKISP1_V10, >+ .features = RKISP1_FEATURE_MIPI_CSI2, > }; > > static const struct of_device_id rkisp1_of_match[] = { >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:07, Laurent Pinchart wrote: >Not all ISP versions integrate a MIPI CSI-2 receiver. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- > .../platform/rockchip/rkisp1/rkisp1-dev.c | 50 +++++++++++++------ > 1 file changed, 34 insertions(+), 16 deletions(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index bc278b49fefc..f2475c6235ea 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -205,6 +205,14 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1) > > switch (reg) { > case 0: >+ /* MIPI CSI-2 port */ >+ if (!(rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)) { >+ dev_err(rkisp1->dev, >+ "internal CSI must be available for port 0\n"); >+ ret = -EINVAL; >+ break; >+ } >+ > vep.bus_type = V4L2_MBUS_CSI2_DPHY; > break; > >@@ -330,13 +338,16 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1) > unsigned int i; > int ret; > >- /* Link the CSI receiver to the ISP. */ >- ret = media_create_pad_link(&rkisp1->csi.sd.entity, RKISP1_CSI_PAD_SRC, >- &rkisp1->isp.sd.entity, >- RKISP1_ISP_PAD_SINK_VIDEO, >- MEDIA_LNK_FL_ENABLED); >- if (ret) >- return ret; >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) { >+ /* Link the CSI receiver to the ISP. */ >+ ret = media_create_pad_link(&rkisp1->csi.sd.entity, >+ RKISP1_CSI_PAD_SRC, >+ &rkisp1->isp.sd.entity, >+ RKISP1_ISP_PAD_SINK_VIDEO, >+ MEDIA_LNK_FL_ENABLED); >+ if (ret) >+ return ret; >+ } > > /* create ISP->RSZ->CAP links */ > for (i = 0; i < 2; i++) { >@@ -379,7 +390,8 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1) > > static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1) > { >- rkisp1_csi_unregister(rkisp1); >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) >+ rkisp1_csi_unregister(rkisp1); > rkisp1_params_unregister(rkisp1); > rkisp1_stats_unregister(rkisp1); > rkisp1_capture_devs_unregister(rkisp1); >@@ -411,9 +423,11 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1) > if (ret) > goto error; > >- ret = rkisp1_csi_register(rkisp1); >- if (ret) >- goto error; >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) { >+ ret = rkisp1_csi_register(rkisp1); >+ if (ret) >+ goto error; >+ } > > ret = rkisp1_create_links(rkisp1); > if (ret) >@@ -576,9 +590,11 @@ static int rkisp1_probe(struct platform_device *pdev) > goto err_unreg_v4l2_dev; > } > >- ret = rkisp1_csi_init(rkisp1); >- if (ret) >- goto err_unreg_media_dev; >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) { >+ ret = rkisp1_csi_init(rkisp1); >+ if (ret) >+ goto err_unreg_media_dev; >+ } > > ret = rkisp1_entities_register(rkisp1); > if (ret) >@@ -595,7 +611,8 @@ static int rkisp1_probe(struct platform_device *pdev) > err_unreg_entities: > rkisp1_entities_unregister(rkisp1); > err_cleanup_csi: >- rkisp1_csi_cleanup(rkisp1); >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) >+ rkisp1_csi_cleanup(rkisp1); > err_unreg_media_dev: > media_device_unregister(&rkisp1->media_dev); > err_unreg_v4l2_dev: >@@ -613,7 +630,8 @@ static int rkisp1_remove(struct platform_device *pdev) > v4l2_async_nf_cleanup(&rkisp1->notifier); > > rkisp1_entities_unregister(rkisp1); >- rkisp1_csi_cleanup(rkisp1); >+ if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) >+ rkisp1_csi_cleanup(rkisp1); > rkisp1_debug_cleanup(rkisp1); > > media_device_unregister(&rkisp1->media_dev); >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:07, Laurent Pinchart wrote: >From: Paul Elder <paul.elder@ideasonboard.com> > >The ISP version in the i.MX8MP has a set of registers currently not >handled by the driver for output size configuration. Add a feature flag >to determine if the ISP requires this, and set the registers based on >that. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >--- > drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c | 8 ++++++++ > drivers/media/platform/rockchip/rkisp1/rkisp1-common.h | 1 + > drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 ++- > drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h | 9 +++++++++ > 4 files changed, 20 insertions(+), 1 deletion(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >index d5904c96ff3f..3c1ade601bf4 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c >@@ -420,6 +420,14 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap) > rkisp1_write(rkisp1, cap->config->mi.cr_size_init, > rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR)); > >+ if (rkisp1->info->features & RKISP1_FEATURE_MAIN_STRIDE) { >+ rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_LLENGTH, pixm->width); >+ rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_WIDTH, pixm->width); >+ rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_HEIGHT, pixm->height); >+ rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_SIZE, >+ pixm->width * pixm->height); >+ } >+ > rkisp1_irq_frame_end_enable(cap); > > /* set uv swapping for semiplanar formats */ >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >index 18a48ecda173..6ce92b5fd465 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >@@ -112,6 +112,7 @@ enum rkisp1_feature { > RKISP1_FEATURE_MIPI_CSI2 = BIT(0), > RKISP1_FEATURE_DUAL_CROP = BIT(1), > RKISP1_FEATURE_RSZ_CROP = BIT(2), >+ RKISP1_FEATURE_MAIN_STRIDE = BIT(3), missing doc thanks, Dafna > }; > > /* >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >index 003bd7db54b9..a9c93191020f 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >@@ -515,7 +515,8 @@ static const struct rkisp1_info imx8mp_isp_info = { > .isrs = imx8mp_isp_isrs, > .isr_size = ARRAY_SIZE(imx8mp_isp_isrs), > .isp_ver = IMX8MP_V10, >- .features = RKISP1_FEATURE_RSZ_CROP, >+ .features = RKISP1_FEATURE_RSZ_CROP >+ | RKISP1_FEATURE_MAIN_STRIDE, > }; > > static const struct of_device_id rkisp1_of_match[] = { >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >index 1fc54ab22b6d..5c2195019723 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >@@ -1013,6 +1013,15 @@ > #define RKISP1_CIF_MI_SP_CB_BASE_AD_INIT2 (RKISP1_CIF_MI_BASE + 0x00000140) > #define RKISP1_CIF_MI_SP_CR_BASE_AD_INIT2 (RKISP1_CIF_MI_BASE + 0x00000144) > #define RKISP1_CIF_MI_XTD_FORMAT_CTRL (RKISP1_CIF_MI_BASE + 0x00000148) >+#define RKISP1_CIF_MI_MP_HANDSHAKE_0 (RKISP1_CIF_MI_BASE + 0x0000014C) >+#define RKISP1_CIF_MI_MP_Y_LLENGTH (RKISP1_CIF_MI_BASE + 0x00000150) >+#define RKISP1_CIF_MI_MP_Y_SLICE_OFFSET (RKISP1_CIF_MI_BASE + 0x00000154) >+#define RKISP1_CIF_MI_MP_C_SLICE_OFFSET (RKISP1_CIF_MI_BASE + 0x00000158) >+#define RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT (RKISP1_CIF_MI_BASE + 0x0000015C) >+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE (RKISP1_CIF_MI_BASE + 0x00000160) >+#define RKISP1_CIF_MI_MP_Y_PIC_WIDTH (RKISP1_CIF_MI_BASE + 0x00000164) >+#define RKISP1_CIF_MI_MP_Y_PIC_HEIGHT (RKISP1_CIF_MI_BASE + 0x00000168) >+#define RKISP1_CIF_MI_MP_Y_PIC_SIZE (RKISP1_CIF_MI_BASE + 0x0000016C) > > #define RKISP1_CIF_SMIA_BASE 0x00001A00 > #define RKISP1_CIF_SMIA_CTRL (RKISP1_CIF_SMIA_BASE + 0x00000000) >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 01.07.2022 02:07, Laurent Pinchart wrote: >From: Paul Elder <paul.elder@ideasonboard.com> > >Add register definitions and value macros for the test pattern generator >block in the ISP. > >Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com> >--- > .../platform/rockchip/rkisp1/rkisp1-regs.h | 32 +++++++++++++++++++ > 1 file changed, 32 insertions(+) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >index dd63ae13e603..34f4fe09c88d 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h >@@ -717,6 +717,27 @@ > #define RKISP1_CIF_ISP_DPF_SPATIAL_COEFF_MAX 0x1F > #define RKISP1_CIF_ISP_DPF_NLL_COEFF_N_MAX 0x3FF > >+/* TPG */ >+#define RKISP1_CIF_ISP_TPG_CTRL_ENA BIT(0) >+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_3X3_COLOR_BLOCK (0 << 1) >+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_COLOR_BAR (1 << 1) >+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_GRAY_BAR (2 << 1) >+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_HIGHLIGHT_GRID (3 << 1) >+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_RAND (4 << 1) >+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_RGGB (0 << 4) >+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_GRBG (1 << 4) >+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_GBRB (2 << 4) >+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_BGGR (3 << 4) >+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_8 (0 << 6) >+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_10 (1 << 6) >+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_12 (2 << 6) >+#define RKISP1_CIF_ISP_TPG_CTRL_DEF_SYNC BIT(8) >+#define RKISP1_CIF_ISP_TPG_CTRL_MAX_SYNC BIT(9) >+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_1080P (0 << 10) >+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_720P (1 << 10) >+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_4K (2 << 10) >+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_USER_DEFINED (3 << 10) >+ > /* =================================================================== */ > /* CIF Registers */ > /* =================================================================== */ >@@ -912,6 +933,17 @@ > #define RKISP1_CIF_ISP_SH_DELAY (RKISP1_CIF_ISP_SH_BASE + 0x00000008) > #define RKISP1_CIF_ISP_SH_TIME (RKISP1_CIF_ISP_SH_BASE + 0x0000000C) > >+#define RKISP1_CIF_ISP_TPG_BASE 0x00000700 >+#define RKISP1_CIF_ISP_TPG_CTRL (RKISP1_CIF_ISP_TPG_BASE + 0x00000000) >+#define RKISP1_CIF_ISP_TPG_TOTAL_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000004) >+#define RKISP1_CIF_ISP_TPG_ACT_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000008) >+#define RKISP1_CIF_ISP_TPG_FP_IN (RKISP1_CIF_ISP_TPG_BASE + 0x0000000C) >+#define RKISP1_CIF_ISP_TPG_BP_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000010) >+#define RKISP1_CIF_ISP_TPG_W_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000014) >+#define RKISP1_CIF_ISP_TPG_GAP_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000018) >+#define RKISP1_CIF_ISP_TPG_GAP_STD_IN (RKISP1_CIF_ISP_TPG_BASE + 0x0000001C) >+#define RKISP1_CIF_ISP_TPG_RANDOM_SEED_IN (RKISP1_CIF_ISP_TPG_BASE + 0x00000020) >+ > #define RKISP1_CIF_C_PROC_BASE 0x00000800 > #define RKISP1_CIF_C_PROC_CTRL (RKISP1_CIF_C_PROC_BASE + 0x00000000) > #define RKISP1_CIF_C_PROC_CONTRAST (RKISP1_CIF_C_PROC_BASE + 0x00000004) >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi Dafna, On Mon, Jul 11, 2022 at 04:22:12AM +0300, Dafna Hirschfeld wrote: > On 01.07.2022 02:06, Laurent Pinchart wrote: > > From: Paul Elder <paul.elder@ideasonboard.com> > > > > The CSI receiver is a component separate from the ISP or the resizers. > > It is actually optional, not all device model include a CSI receiver. On > > some SoCs CSI-2 support can be provided through an external CSI-2 > > receiver, connected to the ISP's parallel input. > > > > To support those use cases, create a V4L2 subdev to model the CSI > > receiver. It will allow the driver to handle both internal and external > > CSI receivers the same way. > > > > The next commit will plumb the CSI subdev to the rest of the driver, > > replacing direct function calls. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Changes since v1: > > > > - Rename RKISP1_CSI_PAD_MAX to RKISP1_CSI_PAD_NUM > > - Simplify format propagation > > - Fix usage of uninitialized variables > > - White space fixes > > --- > > .../platform/rockchip/rkisp1/rkisp1-common.h | 17 ++ > > .../platform/rockchip/rkisp1/rkisp1-csi.c | 288 ++++++++++++++++++ > > .../platform/rockchip/rkisp1/rkisp1-csi.h | 4 + > > .../platform/rockchip/rkisp1/rkisp1-dev.c | 5 + > > 4 files changed, 314 insertions(+) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > index 5301461d3ea3..84832e1367ff 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > @@ -68,6 +68,13 @@ enum rkisp1_rsz_pad { > > RKISP1_RSZ_PAD_MAX > > }; > > > > +/* enum for the csi receiver pads */ > > +enum rkisp1_csi_pad { > > + RKISP1_CSI_PAD_SINK, > > + RKISP1_CSI_PAD_SRC, > > + RKISP1_CSI_PAD_NUM > > +}; > > + > > /* enum for the capture id */ > > enum rkisp1_stream_id { > > RKISP1_MAINPATH, > > @@ -141,11 +148,21 @@ struct rkisp1_sensor_async { > > * @rkisp1: pointer to the rkisp1 device > > * @dphy: a pointer to the phy > > * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt) > > + * @sd: v4l2_subdev variable > > + * @pads: media pads > > + * @pad_cfg: configurations for the pads > > + * @ops_lock: a lock for the subdev ops > > + * @source: source in-use, set when starting streaming > > */ > > struct rkisp1_csi { > > struct rkisp1_device *rkisp1; > > struct phy *dphy; > > bool is_dphy_errctrl_disabled; > > + struct v4l2_subdev sd; > > + struct media_pad pads[RKISP1_CSI_PAD_NUM]; > > + struct v4l2_subdev_pad_config pad_cfg[RKISP1_CSI_PAD_NUM]; > > + struct mutex ops_lock; /* serialize the subdevice ops */ > > + struct rkisp1_sensor_async *source; > > }; > > > > /* > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c > > index 81849c8e9c73..173a0550af5c 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c > > @@ -15,10 +15,35 @@ > > #include <linux/phy/phy-mipi-dphy.h> > > > > #include <media/v4l2-ctrls.h> > > +#include <media/v4l2-fwnode.h> > > > > #include "rkisp1-common.h" > > #include "rkisp1-csi.h" > > > > +#define RKISP1_CSI_DEV_NAME RKISP1_DRIVER_NAME "_csi" > > + > > +#define RKISP1_CSI_DEF_FMT MEDIA_BUS_FMT_SRGGB10_1X10 > > + > > +static inline struct rkisp1_csi *to_rkisp1_csi(struct v4l2_subdev *sd) > > +{ > > + return container_of(sd, struct rkisp1_csi, sd); > > +} > > + > > +static struct v4l2_mbus_framefmt * > > +rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi, > > + struct v4l2_subdev_state *sd_state, > > + unsigned int pad, u32 which) > > +{ > > + struct v4l2_subdev_state state = { > > + .pads = csi->pad_cfg > > + }; > > + > > + if (which == V4L2_SUBDEV_FORMAT_TRY) > > + return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad); > > + else > > + return v4l2_subdev_get_try_format(&csi->sd, &state, pad); > > +} > > + > > static int rkisp1_csi_config(struct rkisp1_csi *csi, > > const struct rkisp1_sensor_async *sensor) > > { > > @@ -186,6 +211,269 @@ irqreturn_t rkisp1_csi_isr(int irq, void *ctx) > > return IRQ_HANDLED; > > } > > > > +/* ---------------------------------------------------------------------------- > > + * Subdev pad operations > > + */ > > + > > +static void rkisp1_csi_set_src_fmt(struct rkisp1_csi *csi, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_mbus_framefmt *format, > > + unsigned int which) > > +{ > > + struct v4l2_mbus_framefmt *sink_fmt; > > + > > + /* We don't set the src format directly; take it from the sink format */ > > + sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK, > > + which); > > + > > + *format = *sink_fmt; > > +} > > + > > +static void rkisp1_csi_set_sink_fmt(struct rkisp1_csi *csi, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_mbus_framefmt *format, > > + unsigned int which) > > +{ > > + const struct rkisp1_mbus_info *mbus_info; > > + struct v4l2_mbus_framefmt *sink_fmt, *src_fmt; > > + > > + sink_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SINK, > > + which); > > + src_fmt = rkisp1_csi_get_pad_fmt(csi, sd_state, RKISP1_CSI_PAD_SRC, > > + which); > > + > > + sink_fmt->code = format->code; > > + > > + mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code); > > + if (!mbus_info || !(mbus_info->direction & RKISP1_ISP_SD_SINK)) { > > + sink_fmt->code = RKISP1_CSI_DEF_FMT; > > + mbus_info = rkisp1_mbus_info_get_by_code(sink_fmt->code); > > + } > > + > > + sink_fmt->width = clamp_t(u32, format->width, > > + RKISP1_ISP_MIN_WIDTH, > > + RKISP1_ISP_MAX_WIDTH); > > + sink_fmt->height = clamp_t(u32, format->height, > > + RKISP1_ISP_MIN_HEIGHT, > > + RKISP1_ISP_MAX_HEIGHT); > > + > > + /* Propagate to source pad */ > > + *src_fmt = *sink_fmt; > > + > > + *format = *sink_fmt; > > +} > > + > > +static int rkisp1_csi_enum_mbus_code(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_mbus_code_enum *code) > > +{ > > + unsigned int i; > > + int pos = 0; > > + > > + for (i = 0; ; i++) { > > + const struct rkisp1_mbus_info *fmt = > > + rkisp1_mbus_info_get_by_index(i); > > + > > + if (!fmt) > > + return -EINVAL; > > + > > + if (fmt->direction & RKISP1_ISP_SD_SINK) > > + pos++; > > + > > + if (code->index == pos - 1) { > > + code->code = fmt->mbus_code; > > + return 0; > > + } > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int rkisp1_csi_init_config(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state) > > +{ > > + struct v4l2_mbus_framefmt *sink_fmt, *src_fmt; > > + > > + sink_fmt = v4l2_subdev_get_try_format(sd, sd_state, > > + RKISP1_CSI_PAD_SINK); > > + src_fmt = v4l2_subdev_get_try_format(sd, sd_state, > > + RKISP1_CSI_PAD_SRC); > > + > > + sink_fmt->width = RKISP1_DEFAULT_WIDTH; > > + sink_fmt->height = RKISP1_DEFAULT_HEIGHT; > > + sink_fmt->field = V4L2_FIELD_NONE; > > + sink_fmt->code = RKISP1_CSI_DEF_FMT; > > + > > + *src_fmt = *sink_fmt; > > + > > + return 0; > > +} > > + > > +static int rkisp1_csi_get_fmt(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_format *fmt) > > +{ > > + struct rkisp1_csi *csi = to_rkisp1_csi(sd); > > + > > + mutex_lock(&csi->ops_lock); > > + fmt->format = *rkisp1_csi_get_pad_fmt(csi, sd_state, fmt->pad, > > + fmt->which); > > + mutex_unlock(&csi->ops_lock); > > + return 0; > > +} > > + > > +static int rkisp1_csi_set_fmt(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_format *fmt) > > +{ > > + struct rkisp1_csi *csi = to_rkisp1_csi(sd); > > + > > + mutex_lock(&csi->ops_lock); > > + if (fmt->pad == RKISP1_CSI_PAD_SINK) > > + rkisp1_csi_set_sink_fmt(csi, sd_state, &fmt->format, > > + fmt->which); > > + else > > + rkisp1_csi_set_src_fmt(csi, sd_state, &fmt->format, > > + fmt->which); > > + > > + mutex_unlock(&csi->ops_lock); > > + return 0; > > +} > > + > > +/* ---------------------------------------------------------------------------- > > + * Subdev video operations > > + */ > > + > > +static int rkisp1_csi_s_stream(struct v4l2_subdev *sd, int enable) > > +{ > > + struct rkisp1_csi *csi = to_rkisp1_csi(sd); > > + struct rkisp1_device *rkisp1 = csi->rkisp1; > > + struct media_pad *source_pad; > > + struct v4l2_subdev *source; > > + int ret; > > + > > + if (!enable) { > > + v4l2_subdev_call(csi->source->sd, video, s_stream, > > + false); > > + > > + rkisp1_csi_stop(csi); > > + > > + return 0; > > + } > > + > > + source_pad = media_entity_remote_source_pad_unique(&sd->entity); > > + if (IS_ERR(source_pad)) { > > + dev_dbg(rkisp1->dev, "Failed to get source for CSI: %d\n", > > + IS_ERR(source_pad)); > > s/IS_ERR/PTR_ERR/ Oops. Fixed. > > + return -EPIPE; > > + } > > + > > + source = media_entity_to_v4l2_subdev(source_pad->entity); > > + if (!source) { > > + /* This should really not happen, so is not worth a message. */ > > + return -EPIPE; > > + } > > + > > + csi->source = container_of(source->asd, struct rkisp1_sensor_async, > > + asd); > > + > > + if (csi->source->mbus_type != V4L2_MBUS_CSI2_DPHY) > > + return -EINVAL; > > I would set here csi->source to NULL, OK. > and shouldn't setting it be inside the lock? csi->source is only accessed by this function, so I don't think that's needed. > > + mutex_lock(&csi->ops_lock); > > + ret = rkisp1_csi_start(csi, csi->source); > > + mutex_unlock(&csi->ops_lock); > > + if (ret) > > + return ret; > > + > > + v4l2_subdev_call(csi->source->sd, video, s_stream, true); > > + > > + return 0; > > +} > > + > > +/* ---------------------------------------------------------------------------- > > + * Registration > > + */ > > + > > +static const struct media_entity_operations rkisp1_csi_media_ops = { > > + .link_validate = v4l2_subdev_link_validate, > > +}; > > + > > +static const struct v4l2_subdev_video_ops rkisp1_csi_video_ops = { > > + .s_stream = rkisp1_csi_s_stream, > > +}; > > + > > +static const struct v4l2_subdev_pad_ops rkisp1_csi_pad_ops = { > > + .enum_mbus_code = rkisp1_csi_enum_mbus_code, > > + .init_cfg = rkisp1_csi_init_config, > > + .get_fmt = rkisp1_csi_get_fmt, > > + .set_fmt = rkisp1_csi_set_fmt, > > +}; > > + > > +static const struct v4l2_subdev_ops rkisp1_csi_ops = { > > + .video = &rkisp1_csi_video_ops, > > + .pad = &rkisp1_csi_pad_ops, > > +}; > > + > > +int rkisp1_csi_register(struct rkisp1_device *rkisp1) > > +{ > > + struct rkisp1_csi *csi = &rkisp1->csi; > > + struct v4l2_subdev_state state = {}; > > + struct media_pad *pads; > > + struct v4l2_subdev *sd; > > + int ret; > > + > > + csi->rkisp1 = rkisp1; > > + mutex_init(&csi->ops_lock); > > + > > + sd = &csi->sd; > > + v4l2_subdev_init(sd, &rkisp1_csi_ops); > > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > + sd->entity.ops = &rkisp1_csi_media_ops; > > + sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; > > + sd->owner = THIS_MODULE; > > + strscpy(sd->name, RKISP1_CSI_DEV_NAME, sizeof(sd->name)); > > + > > + pads = csi->pads; > > + pads[RKISP1_CSI_PAD_SINK].flags = MEDIA_PAD_FL_SINK | > > + MEDIA_PAD_FL_MUST_CONNECT; > > + pads[RKISP1_CSI_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE | > > + MEDIA_PAD_FL_MUST_CONNECT; > > + > > + ret = media_entity_pads_init(&sd->entity, RKISP1_CSI_PAD_NUM, pads); > > + if (ret) > > + goto error; > > + > > + state.pads = csi->pad_cfg; > > + rkisp1_csi_init_config(sd, &state); > > + > > + ret = v4l2_device_register_subdev(&csi->rkisp1->v4l2_dev, sd); > > + if (ret) { > > + dev_err(sd->dev, "Failed to register csi receiver subdev\n"); > > + goto error; > > + } > > + > > + return 0; > > + > > +error: > > + media_entity_cleanup(&sd->entity); > > + mutex_destroy(&csi->ops_lock); > > + csi->rkisp1 = NULL; > > + return ret; > > +} > > + > > +void rkisp1_csi_unregister(struct rkisp1_device *rkisp1) > > +{ > > + struct rkisp1_csi *csi = &rkisp1->csi; > > + > > + if (!csi->rkisp1) > > + return; > > + > > + v4l2_device_unregister_subdev(&csi->sd); > > + media_entity_cleanup(&csi->sd.entity); > > + mutex_destroy(&csi->ops_lock); > > +} > > + > > int rkisp1_csi_init(struct rkisp1_device *rkisp1) > > { > > struct rkisp1_csi *csi = &rkisp1->csi; > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h > > index 97ce7e7959ab..ddf8e5e08f55 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h > > @@ -13,10 +13,14 @@ > > > > struct rkisp1_device; > > struct rkisp1_sensor_async; > > +struct v4l2_subdev; > > > > int rkisp1_csi_init(struct rkisp1_device *rkisp1); > > void rkisp1_csi_cleanup(struct rkisp1_device *rkisp1); > > > > +int rkisp1_csi_register(struct rkisp1_device *rkisp1); > > +void rkisp1_csi_unregister(struct rkisp1_device *rkisp1); > > + > > int rkisp1_csi_start(struct rkisp1_csi *csi, > > const struct rkisp1_sensor_async *sensor); > > void rkisp1_csi_stop(struct rkisp1_csi *csi); > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > > index 2c441665c017..5428e19e818f 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > > @@ -324,6 +324,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1) > > > > static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1) > > { > > + rkisp1_csi_unregister(rkisp1); > > rkisp1_params_unregister(rkisp1); > > rkisp1_stats_unregister(rkisp1); > > rkisp1_capture_devs_unregister(rkisp1); > > @@ -355,6 +356,10 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1) > > if (ret) > > goto error; > > > > + ret = rkisp1_csi_register(rkisp1); > > + if (ret) > > + goto error; > > + > > ret = rkisp1_create_links(rkisp1); > > if (ret) > > goto error;