mbox series

[v2,00/55] media: rkisp1: Cleanups and add support for i.MX8MP

Message ID 20220630230713.10580-1-laurent.pinchart@ideasonboard.com
Headers show
Series media: rkisp1: Cleanups and add support for i.MX8MP | expand

Message

Laurent Pinchart June 30, 2022, 11:06 p.m. UTC
Hello,

This series cleans up, reworks and extends the rkisp1 driver to support
the ISP found in the NXP i.MX8MP SoC.

The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
and in the NXP i.MX8MP have the same origin, and have slightly diverged
over time as they are now independently developed (as far as I
understand) by Rockchip and VeriSilicon. The latter is marketed under
the name "ISP8000Nano", and is close enough to the RK3399 ISP that it
can easily be supported by the same driver.

This series starts with a few changes to the V4L2 async framework
(01/55) and MC core helpers (02/55 to 04/55) to support the rest of the
changes. It continues with various cleanups and reworks in order to
support CSI-2 receivers external to the ISP as found in the i.MX8MP
(05/55 to 45/55). Patch 46/55 is a small debugging improvement, and
patches 47/55 to 55/55 then add i.MX8MP support.

Compared to v1, patches have been shuffled around a bit so that patches
01/55 to 45/55 should be ready in time for integration in v5.20, while
the remaining patches may take one more kernel release. See individual
patches for detailed changelogs.

Review comments from v1 have been taken into account, except for patches
46/55, 48/55, 50/55 and 55/55 as discussion are still ongoing there.

This series depends on v4 of "media: rkisp1: Misc bug fixes and
cleanups" ([1]), which has been accepted in the Linux media tree for
v5.20.

[1] https://lore.kernel.org/linux-media/Ymbxs2p9Tuf331qM@pendragon.ideasonboard.com/T/

Laurent Pinchart (39):
  media: v4l2-async: Add notifier operation to destroy asd instances
  media: mc-entity: Rename media_entity_remote_pad() to
    media_pad_remote_pad_first()
  media: mc-entity: Add a new helper function to get a remote pad
  media: mc-entity: Add a new helper function to get a remote pad for a
    pad
  media: rkisp1: Enable compilation on ARCH_MXC
  media: rkisp1: Disable runtime PM in probe error path
  media: rkisp1: Read the ID register at probe time instead of streamon
  media: rkisp1: Rename rkisp1_match_data to rkisp1_info
  media: rkisp1: Access ISP version from info pointer
  media: rkisp1: cap: Print debug message on failed link validation
  media: rkisp1: Move sensor .s_stream() call to ISP
  media: rkisp1: Reject sensors without pixel rate control at bound time
  media: rkisp1: Create link from sensor to ISP at notifier bound time
  media: rkisp1: Create internal links at probe time
  media: rkisp1: Rename rkisp1_subdev_notifier() to
    rkisp1_subdev_notifier_register()
  media: rkisp1: Fix sensor source pad retrieval at bound time
  media: rkisp1: isp: Start CSI-2 receiver before ISP
  media: rkisp1: csi: Handle CSI-2 RX configuration fully in
    rkisp1-csi.c
  media: rkisp1: csi: Rename CSI functions with a common rkisp1_csi
    prefix
  media: rkisp1: csi: Move start delay to rkisp1_csi_start()
  media: rkisp1: csi: Pass sensor pointer to rkisp1_csi_config()
  media: rkisp1: csi: Constify argument to rkisp1_csi_start()
  media: rkisp1: isp: Don't initialize ret to 0 in rkisp1_isp_s_stream()
  media: rkisp1: isp: Pass mbus type and flags to rkisp1_config_cif()
  media: rkisp1: isp: Rename rkisp1_device.active_sensor to source
  media: rkisp1: isp: Add container_of wrapper to cast subdev to
    rkisp1_isp
  media: rkisp1: isp: Add rkisp1_device backpointer to rkisp1_isp
  media: rkisp1: isp: Pass rkisp1_isp pointer to internal ISP functions
  media: rkisp1: isp: Move input configuration to rkisp1_config_isp()
  media: rkisp1: isp: Merge ISP_ACQ_PROP configuration in single
    variable
  media: rkisp1: isp: Initialize some variables at declaration time
  media: rkisp1: isp: Fix whitespace issues
  media: rkisp1: isp: Constify various local variables
  media: rkisp1: isp: Rename rkisp1_get_remote_source()
  media: rkisp1: isp: Disallow multiple active sources
  media: rkisp1: csi: Plumb the CSI RX subdev
  media: rkisp1: Add infrastructure to support ISP features
  media: rkisp1: Make the internal CSI-2 receiver optional
  media: rkisp1: Configure gasket on i.MX8MP

Paul Elder (16):
  media: rkisp1: Save info pointer in rkisp1_device
  media: rkisp1: Make rkisp1_isp_mbus_info common
  media: rkisp1: Split CSI handling to separate file
  media: rkisp1: csi: Implement a V4L2 subdev for the CSI receiver
  media: rkisp1: Use fwnode_graph_for_each_endpoint
  dt-bindings: media: rkisp1: Add port for parallel interface
  media: rkisp1: Support the ISP parallel input
  media: rkisp1: debug: Add dump file in debugfs for MI buffer registers
  dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
  media: rkisp1: Add match data for i.MX8MP ISP
  media: rkisp1: Add and set registers for crop for i.MX8MP
  media: rkisp1: Add and set registers for output size config on i.MX8MP
  media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
  media: rkisp1: Shift DMA buffer addresses on i.MX8MP
  media: rkisp1: Add register definitions for the test pattern generator
  media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP

 .../bindings/media/rockchip-isp1.yaml         |  30 +-
 Documentation/driver-api/media/mc-core.rst    |   5 +-
 .../driver-api/media/v4l2-subdev.rst          |   6 +
 drivers/media/i2c/adv748x/adv748x.h           |   2 +-
 drivers/media/i2c/tvp5150.c                   |   2 +-
 drivers/media/mc/mc-entity.c                  |  75 +-
 drivers/media/pci/intel/ipu3/ipu3-cio2-main.c |   2 +-
 .../media/platform/qcom/camss/camss-csid.c    |   6 +-
 .../media/platform/qcom/camss/camss-csiphy.c  |   2 +-
 .../media/platform/qcom/camss/camss-ispif.c   |   4 +-
 drivers/media/platform/qcom/camss/camss-vfe.c |   2 +-
 .../media/platform/qcom/camss/camss-video.c   |   6 +-
 drivers/media/platform/qcom/camss/camss.c     |   2 +-
 .../platform/renesas/rcar-vin/rcar-core.c     |   2 +-
 .../platform/renesas/rcar-vin/rcar-csi2.c     |   2 +-
 .../platform/renesas/rcar-vin/rcar-dma.c      |   2 +-
 .../platform/renesas/rcar-vin/rcar-v4l2.c     |   2 +-
 .../media/platform/renesas/vsp1/vsp1_entity.c |   4 +-
 .../media/platform/renesas/vsp1/vsp1_video.c  |   2 +-
 .../media/platform/rockchip/rkisp1/Kconfig    |   2 +-
 .../media/platform/rockchip/rkisp1/Makefile   |   1 +
 .../platform/rockchip/rkisp1/rkisp1-capture.c |  49 +-
 .../platform/rockchip/rkisp1/rkisp1-common.c  | 143 ++++
 .../platform/rockchip/rkisp1/rkisp1-common.h  | 116 ++-
 .../platform/rockchip/rkisp1/rkisp1-csi.c     | 526 ++++++++++++++
 .../platform/rockchip/rkisp1/rkisp1-csi.h     |  28 +
 .../platform/rockchip/rkisp1/rkisp1-debug.c   |  35 +-
 .../platform/rockchip/rkisp1/rkisp1-dev.c     | 474 +++++++-----
 .../platform/rockchip/rkisp1/rkisp1-isp.c     | 687 +++++++-----------
 .../platform/rockchip/rkisp1/rkisp1-params.c  |   2 +-
 .../platform/rockchip/rkisp1/rkisp1-regs.h    |  87 +++
 .../platform/rockchip/rkisp1/rkisp1-resizer.c |  43 +-
 .../platform/rockchip/rkisp1/rkisp1-stats.c   |   4 +-
 .../platform/samsung/exynos4-is/common.c      |   2 +-
 .../samsung/exynos4-is/fimc-capture.c         |   6 +-
 .../samsung/exynos4-is/fimc-isp-video.c       |   2 +-
 .../platform/samsung/exynos4-is/fimc-lite.c   |   2 +-
 .../platform/samsung/exynos4-is/media-dev.c   |   2 +-
 .../samsung/s3c-camif/camif-capture.c         |   2 +-
 drivers/media/platform/st/stm32/stm32-dcmi.c  |   6 +-
 .../platform/sunxi/sun6i-csi/sun6i_video.c    |   4 +-
 drivers/media/platform/ti/cal/cal-camerarx.c  |   2 +-
 drivers/media/platform/ti/cal/cal-video.c     |   2 +-
 drivers/media/platform/ti/omap3isp/isp.c      |   6 +-
 drivers/media/platform/ti/omap3isp/ispccdc.c  |   2 +-
 drivers/media/platform/ti/omap3isp/ispccp2.c  |   2 +-
 drivers/media/platform/ti/omap3isp/ispcsi2.c  |   2 +-
 drivers/media/platform/ti/omap3isp/ispvideo.c |   4 +-
 drivers/media/platform/video-mux.c            |   2 +-
 .../media/platform/xilinx/xilinx-csi2rxss.c   |   2 +-
 drivers/media/platform/xilinx/xilinx-dma.c    |   4 +-
 .../media/test-drivers/vimc/vimc-streamer.c   |   2 +-
 drivers/media/v4l2-core/v4l2-async.c          |  10 +
 .../staging/media/imx/imx-media-dev-common.c  |   2 +-
 drivers/staging/media/imx/imx-media-utils.c   |   2 +-
 drivers/staging/media/omap4iss/iss.c          |   6 +-
 drivers/staging/media/omap4iss/iss_csi2.c     |   2 +-
 drivers/staging/media/omap4iss/iss_video.c    |   2 +-
 drivers/staging/media/tegra-video/vi.c        |   4 +-
 include/media/media-entity.h                  |  68 +-
 include/media/v4l2-async.h                    |   2 +
 include/uapi/linux/rkisp1-config.h            |   3 +
 62 files changed, 1790 insertions(+), 722 deletions(-)
 create mode 100644 drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
 create mode 100644 drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h

Comments

Rob Herring (Arm) July 1, 2022, 8:18 p.m. UTC | #1
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>
Paul Elder July 7, 2022, 1:39 p.m. UTC | #2
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;
Paul Elder July 7, 2022, 1:40 p.m. UTC | #3
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
Paul Elder July 7, 2022, 2:54 p.m. UTC | #4
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);
Dafna Hirschfeld July 10, 2022, 7:40 p.m. UTC | #5
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
>
Dafna Hirschfeld July 11, 2022, 12:40 a.m. UTC | #6
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
>
Dafna Hirschfeld July 11, 2022, 12:48 a.m. UTC | #7
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
Dafna Hirschfeld July 11, 2022, 1:22 a.m. UTC | #8
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
Dafna Hirschfeld July 11, 2022, 1:38 a.m. UTC | #9
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
Dafna Hirschfeld July 11, 2022, 2:29 a.m. UTC | #10
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
Dafna Hirschfeld July 11, 2022, 2:39 a.m. UTC | #11
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
Dafna Hirschfeld July 11, 2022, 2:59 a.m. UTC | #12
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
Dafna Hirschfeld July 11, 2022, 3:42 a.m. UTC | #13
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
Laurent Pinchart July 11, 2022, 8:20 a.m. UTC | #14
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;