[PATCH v8 00/15] drm/imx: dc: Use prefetch engine

[PATCH v8 00/15] drm/imx: dc: Use prefetch engine

[Liu Ying]

Hi,

This patch series aims to use prefetch engine in imx8-dc KMS driver.

One prefetch engine consists of one DPR Channel(DPRC) and one or two
Display Prefetch Resolve Gaskets(PRGs).  It prefetches data from
DDR, resolves data in tile formats if needed and outputs the result
data to FetchUnit(s).  When one PRG is used, a prefetch engine supports
processing pixel formats with one planar, e.g., RGB formats.  When two
are used, pixel formats with two planars are supported, e.g., NV12.

For now, this patch series only supports prefetch engines with one PRG
because it is enough for XRGB8888 which is the only pixel format
supported by imx8-dc plane driver.  Two PRGs would be supported by
future patches.

Patch 1 and 2 add dt-bindings for DPRC and PRG.  They were reviewed
by Rob[1][2].  I slightly modified the patches' commit message to use
imperative mood.

Patch 3 fixes invisible plane issue reported by Sashiko AI bot in v6.

Patch 4 fixes FetchUnit dimensions as related to prefetch engine function.

Patch 5 flags CRTC status in GERNERALPURPOSE register as needed by patch 6.

Patch 6 disables CRTC at boot if needed to support prefetch engine better.

Patch 7 and 8 add PRG and DPRC drivers.

Patch 9 to 14 do preparations before using prefetch engine in imx8-dc KMS
driver.

Patch 15 uses prefetch engine in imx8-dc KMS driver.

[1] https://lore.kernel.org/lkml/20201207165945.GA430214@robh.at.kernel.org/
[2] https://lore.kernel.org/lkml/20201207170206.GA434964@robh.at.kernel.org/

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
Changes in v8:
- Collect Laurentiu's R-b tags.
- Collect Alexander's R-b tag.
- Fix Sashiko AI bot's comments.  See detail changelog in each patch.
- Fix Alexander's comments.  See detail changelog in each patch.
- Fix Laurentiu's comments.  See detail changelog in each patch.
- Trivial tweaks.
- Link to v7: https://patch.msgid.link/20261026-imx8-dc-prefetch-v7-0-a0b50cc4fc65@nxp.com

Changes in v7:
- Collect Maxime's A-b tags.
- Fix Sashiko AI bot's comments.  See detail changelog in each patch.
- Trivial tweaks.
- Cc Alexander.
- Link to v6: https://patch.msgid.link/20260904-imx8-dc-prefetch-v6-0-26643e3f6cb7@nxp.com

Changes in v6:
- Rebase onto the latest drm-misc-next and resolve conflicts in patch 4&7&13.
- Include "dc-drv.h" in patch 5&6 to avoid build warnings with C=1.
- Fix PRG BYPASS programming in patch 5.
- Link to v5: https://lore.kernel.org/r/20251027-imx8-dc-prefetch-v5-0-4ecb6c6d4941@nxp.com

Changes in v5:
- Squash MAINTAINERS file update into patch 1&2.  (Krzysztof)
- Link to v4: https://lore.kernel.org/r/20251016-imx8-dc-prefetch-v4-0-dfda347cb3c5@nxp.com

Changes in v4:
- Rebase onto the latest drm-misc-next and resolve conflicts in patch 4&10.
- Sort bits and bitfields in descending order for register PRG_CTRL in patch 6.
- Cc Marek as stakeholder of the driver due to recent i.MX95 DC patches posted.
- Link to v3: https://lore.kernel.org/r/20250929-imx8-dc-prefetch-v3-0-c01d0608add2@nxp.com

Changes in v3:
- Call dc_dprc_enable() only when start == true in patch 7.  (Frank)
- Collect Frank's R-b tags.
- Link to v2: https://lore.kernel.org/r/20250923-imx8-dc-prefetch-v2-0-5d69dc9ac8b5@nxp.com

Changes in v2:
- Rebase the patch series onto next-20250922.
- Collect Alexander's and Frank's R-b tags.
- Manage PRG's and DPRC's clocks with bulk interfaces in patch 6&7.  (Frank)
- Sort variables in probe function in reverse Christmas tree fashion in patch 7.  (Frank)
- Link to v1: https://lore.kernel.org/r/20250704-imx8-dc-prefetch-v1-0-784c03fd645f@nxp.com

---
Liu Ying (15):
      dt-bindings: display: imx: Add i.MX8qxp/qm PRG binding
      dt-bindings: display: imx: Add i.MX8qxp/qm DPR channel binding
      drm/imx: dc-plane: Disable plane when it is invisible
      drm/imx: dc-fu: Fix dimensions
      drm/imx: dc: Flag CRTC status in GERNERALPURPOSE register
      drm/imx: dc-crtc: Disable at boot
      drm/imx: dc: Add PRG support
      drm/imx: dc: Add DPR channel support
      drm/imx: dc: Use TCON operation mode
      drm/imx: dc-ed: Support getting source selection
      drm/imx: dc-lb: Support getting secondary input selection
      drm/imx: dc-ed: Drop initial source selection
      drm/imx: dc-lb: Drop initial primary and secondary input selections
      drm/imx: dc-fu: Get DPR channel
      drm/imx: dc: Use prefetch engine

 .../bindings/display/imx/fsl,imx8qxp-dprc.yaml     | 100 ++++
 .../bindings/display/imx/fsl,imx8qxp-prg.yaml      |  60 +++
 MAINTAINERS                                        |   2 +
 drivers/gpu/drm/imx/dc/Kconfig                     |   1 +
 drivers/gpu/drm/imx/dc/Makefile                    |   6 +-
 drivers/gpu/drm/imx/dc/dc-crtc.c                   | 244 +++++++++-
 drivers/gpu/drm/imx/dc/dc-de.h                     |   2 +
 drivers/gpu/drm/imx/dc/dc-dprc.c                   | 534 +++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-dprc.h                   |  37 ++
 drivers/gpu/drm/imx/dc/dc-drv.c                    |  88 ++++
 drivers/gpu/drm/imx/dc/dc-drv.h                    |  12 +
 drivers/gpu/drm/imx/dc/dc-ed.c                     |  27 +-
 drivers/gpu/drm/imx/dc/dc-fg.c                     |  11 +-
 drivers/gpu/drm/imx/dc/dc-fl.c                     |   7 +-
 drivers/gpu/drm/imx/dc/dc-fu.c                     |  59 ++-
 drivers/gpu/drm/imx/dc/dc-fu.h                     |  11 +-
 drivers/gpu/drm/imx/dc/dc-fw.c                     |   7 +-
 drivers/gpu/drm/imx/dc/dc-kms.h                    |   7 +
 drivers/gpu/drm/imx/dc/dc-lb.c                     |  27 +-
 drivers/gpu/drm/imx/dc/dc-pe.h                     |   2 +
 drivers/gpu/drm/imx/dc/dc-plane.c                  | 102 ++--
 drivers/gpu/drm/imx/dc/dc-prg.c                    | 330 +++++++++++++
 drivers/gpu/drm/imx/dc/dc-prg.h                    |  39 ++
 drivers/gpu/drm/imx/dc/dc-tc.c                     | 119 ++++-
 24 files changed, 1771 insertions(+), 63 deletions(-)
---
base-commit: cf61bef3801c3c9d0d8c39026b7b2fb242898a3a
change-id: 20250703-imx8-dc-prefetch-028e306591c6

Best regards,
--  
Regards,
Liu Ying

[PATCH v8 01/15] dt-bindings: display: imx: Add i.MX8qxp/qm PRG binding

[Liu Ying]

Add bindings for i.MX8qxp/qm Display Prefetch Resolve Gasket.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
Changed commit message to use imperative mood.

v5:
- Add the DT binding file in MAINTAINERS.  (Krzysztof)
---
 .../bindings/display/imx/fsl,imx8qxp-prg.yaml      | 60 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-prg.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-prg.yaml
new file mode 100644
index 000000000000..3ff46e0d4e73
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-prg.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,imx8qxp-prg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8qm/qxp Display Prefetch Resolve Gasket
+
+maintainers:
+  - Liu Ying <victor.liu@nxp.com>
+
+description: |
+  The i.MX8qm/qxp Prefetch Resolve Gasket (PRG) is a gasket interface between
+  RTRAM controller and Display Controller.  The main function is to convert
+  the AXI interface to the RTRAM interface, which includes re-mapping the
+  ARADDR to a RTRAM address.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx8qxp-prg
+      - fsl,imx8qm-prg
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: rtram clock
+      - description: apb clock
+
+  clock-names:
+    items:
+      - const: rtram
+      - const: apb
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8-lpcg.h>
+    #include <dt-bindings/firmware/imx/rsrc.h>
+    prg@56040000 {
+        compatible = "fsl,imx8qxp-prg";
+        reg = <0x56040000 0x10000>;
+        clocks = <&dc0_prg0_lpcg IMX_LPCG_CLK_0>,
+                 <&dc0_prg0_lpcg IMX_LPCG_CLK_4>;
+        clock-names = "rtram", "apb";
+        power-domains = <&pd IMX_SC_R_DC_0>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a08087fb6e67..d9a6bdc65826 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8806,6 +8806,7 @@ L:	dri-devel@lists.freedesktop.org
 S:	Maintained
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc*.yaml
+F:	Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-prg.yaml
 F:	drivers/gpu/drm/imx/dc/
 
 DRM DRIVERS FOR FREESCALE IMX BRIDGE

-- 
2.43.0

[PATCH v8 02/15] dt-bindings: display: imx: Add i.MX8qxp/qm DPR channel binding

[Liu Ying]

Add bindings for i.MX8qxp/qm Display Prefetch Resolve Channel.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
Changed commit message to use imperative mood.

v5:
- Add the DT binding file in MAINTAINERS.  (Krzysztof)
---
 .../bindings/display/imx/fsl,imx8qxp-dprc.yaml     | 100 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 101 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dprc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dprc.yaml
new file mode 100644
index 000000000000..bd94254c1288
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dprc.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,imx8qxp-dprc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8qm/qxp Display Prefetch Resolve Channel
+
+maintainers:
+  - Liu Ying <victor.liu@nxp.com>
+
+description: |
+  The i.MX8qm/qxp Display Prefetch Resolve Channel(DPRC) is an engine which
+  fetches display data before the display pipeline needs the data to drive
+  pixels in the active display region.  This data is transformed, or resolved,
+  from a variety of tiled buffer formats into linear format, if needed.
+  The DPR works with a double bank memory structure.  This memory structure is
+  implemented in the Resolve Tile Memory(RTRAM) and the banks are referred to
+  as A and B.  Each bank is either 4 or 8 lines high depending on the source
+  frame buffer format.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx8qxp-dpr-channel
+      - fsl,imx8qm-dpr-channel
+
+  reg:
+    maxItems: 1
+
+  interrupts-extended:
+    items:
+      - description: DPR wrap interrupt
+      - description: |
+          'r_rtram_stall' interrupt which indicates relevant i.MX8qm/qxp
+          Prefetch Resolve Gasket(PRG) or PRGs are forcing an underflow
+          condition in the RTRAM.
+
+  interrupt-names:
+    items:
+      - const: dpr_wrap
+      - const: r_rtram_stall
+
+  clocks:
+    items:
+      - description: apb clock
+      - description: b clock
+      - description: rtram clock
+
+  clock-names:
+    items:
+      - const: apb
+      - const: b
+      - const: rtram
+
+  fsl,sc-resource:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The SCU resource ID associated with this DPRC instance.
+
+  fsl,prgs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      List of phandle which points to PRG or PRGs associated with
+      this DPRC instance.
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts-extended
+  - interrupt-names
+  - clocks
+  - clock-names
+  - fsl,sc-resource
+  - fsl,prgs
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8-lpcg.h>
+    #include <dt-bindings/firmware/imx/rsrc.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    dpr-channel@56100000 {
+        compatible = "fsl,imx8qxp-dpr-channel";
+        reg = <0x56100000 0x10000>;
+        interrupts-extended = <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
+                              <&dc0_irqsteer 324>;
+        interrupt-names = "dpr_wrap", "r_rtram_stall";
+        clocks = <&dc0_dpr1_lpcg IMX_LPCG_CLK_4>,
+                 <&dc0_dpr1_lpcg IMX_LPCG_CLK_5>,
+                 <&dc0_rtram1_lpcg IMX_LPCG_CLK_0>;
+        clock-names = "apb", "b", "rtram";
+        fsl,sc-resource = <IMX_SC_R_DC_0_VIDEO0>;
+        fsl,prgs = <&dc0_prg4>, <&dc0_prg5>;
+        power-domains = <&pd IMX_SC_R_DC_0>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index d9a6bdc65826..cbb8aef958e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8806,6 +8806,7 @@ L:	dri-devel@lists.freedesktop.org
 S:	Maintained
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc*.yaml
+F:	Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dprc.yaml
 F:	Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-prg.yaml
 F:	drivers/gpu/drm/imx/dc/
 

-- 
2.43.0

[PATCH v8 03/15] drm/imx: dc-plane: Disable plane when it is invisible

[Liu Ying]

Fetch unit backing plane doesn't support invisible plane because
SOURCEBUFFERDIMENSION and FRAMEDIMENSIONS registers only accept
dimension for visible frame.  Disable plane when it is invisible
to avoid wrongly programming the two registers with invisible
frame.

Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Collect Laurentiu's R-b tag.

v7:
- Newly introduced to fix invisible planes.  (Sashiko AI bot)
---
 drivers/gpu/drm/imx/dc/dc-plane.c | 54 ++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-plane.c b/drivers/gpu/drm/imx/dc/dc-plane.c
index 62dd0576fadc..42c0a46281cb 100644
--- a/drivers/gpu/drm/imx/dc/dc-plane.c
+++ b/drivers/gpu/drm/imx/dc/dc-plane.c
@@ -126,6 +126,33 @@ dc_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_commit *state)
 	return dc_plane_check_fb(plane_state);
 }
 
+/* A prerequisite is that drm_dev_enter() has been called beforehand. */
+static void __dc_plane_atomic_disable(struct drm_plane *plane)
+{
+	struct dc_plane *dplane = to_dc_plane(plane);
+	const struct dc_fu_ops *fu_ops;
+
+	/* disable fetchunit in shadow */
+	fu_ops = dc_fu_get_ops(dplane->fu);
+	fu_ops->disable_src_buf(dplane->fu, DC_FETCHUNIT_FRAC0);
+
+	/* set ExtDst's source to ConstFrame */
+	dc_ed_pec_src_sel(dplane->ed, dc_cf_get_link_id(dplane->cf));
+}
+
+static void dc_plane_atomic_disable(struct drm_plane *plane,
+				    struct drm_atomic_commit *state)
+{
+	int idx;
+
+	if (!drm_dev_enter(plane->dev, &idx))
+		return;
+
+	__dc_plane_atomic_disable(plane);
+
+	drm_dev_exit(idx);
+}
+
 static void
 dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 {
@@ -143,6 +170,11 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 	if (!drm_dev_enter(plane->dev, &idx))
 		return;
 
+	if (!new_state->visible) {
+		__dc_plane_atomic_disable(plane);
+		goto out;
+	}
+
 	src_w = drm_rect_width(&new_state->src) >> 16;
 	src_h = drm_rect_height(&new_state->src) >> 16;
 
@@ -171,27 +203,7 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 
 	/* set ExtDst's source to LayerBlend */
 	dc_ed_pec_src_sel(dplane->ed, dc_lb_get_link_id(lb));
-
-	drm_dev_exit(idx);
-}
-
-static void dc_plane_atomic_disable(struct drm_plane *plane,
-				    struct drm_atomic_commit *state)
-{
-	struct dc_plane *dplane = to_dc_plane(plane);
-	const struct dc_fu_ops *fu_ops;
-	int idx;
-
-	if (!drm_dev_enter(plane->dev, &idx))
-		return;
-
-	/* disable fetchunit in shadow */
-	fu_ops = dc_fu_get_ops(dplane->fu);
-	fu_ops->disable_src_buf(dplane->fu, DC_FETCHUNIT_FRAC0);
-
-	/* set ExtDst's source to ConstFrame */
-	dc_ed_pec_src_sel(dplane->ed, dc_cf_get_link_id(dplane->cf));
-
+out:
 	drm_dev_exit(idx);
 }
 

-- 
2.43.0

[PATCH v8 04/15] drm/imx: dc-fu: Fix dimensions

[Liu Ying]

Fix off-by-one issue in LINEWIDTH, LINECOUNT, FRAMEWIDTH and FRAMEHEIGHT
macro definitions.  The first two macros are used to set a fetchunit's
source buffer dimension and the other two are used to set a fetchunit's
frame dimension.  It appears that display controller itself works ok
without this fix, however, it enters panic mode when prefetch engine(DPRC
and PRGs) attaches to it without this fix.

Fixes: 0e177d5ce01c ("drm/imx: Add i.MX8qxp Display Controller pixel engine")
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v4:
- Rebase onto the latest drm-misc-next and resolve conflicts.

v2:
- Collect Alexander's and Frank's R-b tags.
---
 drivers/gpu/drm/imx/dc/dc-fu.c | 4 ++--
 drivers/gpu/drm/imx/dc/dc-fu.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-fu.c b/drivers/gpu/drm/imx/dc/dc-fu.c
index 1d8f74babef8..2e197b3344bd 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.c
+++ b/drivers/gpu/drm/imx/dc/dc-fu.c
@@ -31,8 +31,8 @@
 #define STRIDE(x)			FIELD_PREP(STRIDE_MASK, (x) - 1)
 
 /* SOURCEBUFFERDIMENSION */
-#define LINECOUNT(x)			FIELD_PREP(GENMASK(29, 16), (x))
-#define LINEWIDTH(x)			FIELD_PREP(GENMASK(13, 0), (x))
+#define LINECOUNT(x)			FIELD_PREP(GENMASK(29, 16), (x) - 1)
+#define LINEWIDTH(x)			FIELD_PREP(GENMASK(13, 0), (x) - 1)
 
 /* LAYEROFFSET */
 #define LAYERYOFFSET(x)			FIELD_PREP(GENMASK(30, 16), (x))
diff --git a/drivers/gpu/drm/imx/dc/dc-fu.h b/drivers/gpu/drm/imx/dc/dc-fu.h
index f678de3ca8c0..cc93317e3001 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.h
+++ b/drivers/gpu/drm/imx/dc/dc-fu.h
@@ -38,8 +38,8 @@
 #define YUVCONVERSIONMODE(x)		FIELD_PREP(YUVCONVERSIONMODE_MASK, (x))
 
 /* FRAMEDIMENSIONS */
-#define FRAMEHEIGHT(x)			FIELD_PREP(GENMASK(29, 16), (x))
-#define FRAMEWIDTH(x)			FIELD_PREP(GENMASK(13, 0), (x))
+#define FRAMEHEIGHT(x)			FIELD_PREP(GENMASK(29, 16), (x) - 1)
+#define FRAMEWIDTH(x)			FIELD_PREP(GENMASK(13, 0), (x) - 1)
 
 /* CONTROL */
 #define INPUTSELECT_MASK		GENMASK(4, 3)

-- 
2.43.0

[PATCH v8 05/15] drm/imx: dc: Flag CRTC status in GERNERALPURPOSE register

[Liu Ying]

Map Display Controller's register GERNERALPURPOSE at offset 0x100 and
create an MMIO regmap to access the register.  Create helpers
dc_drv_flag_crtc_{enable,disable}() with the 2 LSB bits in the register
to flag the CRTC enablement/disablement status.  Also, create a helper
dc_drv_crtc_was_enabled() so that the CRTC driver can use it to check
if the CRTC was enabled by a previous boot or not.  This is a preparation
for an upcoming patch to disable display controller at boot if the CRTC
was enabled by the driver during a previous boot instead of a bootloader.

Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Rename dc_drv_crtc_was_enabled() to dc_drv_crtc_is_enabled().  (Laurentiu)
- Use dev_err_probe() to handle errors returned from several function calls.
  (Laurentiu)
- Collect Laurentiu's R-b tag.

v7:
- Newly introduced in v7 as a preparation for fixing Sashiko AI bot's
  comment on an upcoming patch.  (Sashiko AI bot)
---
 drivers/gpu/drm/imx/dc/dc-drv.c | 75 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-drv.h |  7 ++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
index 13795a2ad735..9629beaed43c 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic_helper.h>
@@ -28,9 +29,21 @@
 #include "dc-drv.h"
 #include "dc-pe.h"
 
+/* General Purpose register offset used to flag driver usage. */
+#define GENERALPURPOSE		0x100
+#define  CRTC_ENABLED(x)	BIT(x)
+
 struct dc_priv {
 	struct drm_device *drm;
 	struct clk *clk_cfg;
+	struct regmap *gp_regmap;
+};
+
+static const struct regmap_config dc_gp_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0,
 };
 
 DEFINE_DRM_GEM_DMA_FOPS(dc_drm_driver_fops);
@@ -156,6 +169,8 @@ static int dc_probe(struct platform_device *pdev)
 {
 	struct component_match *match = NULL;
 	struct dc_priv *priv;
+	struct resource *r;
+	void __iomem *base;
 	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -167,6 +182,27 @@ static int dc_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clk_cfg),
 				     "failed to get cfg clock\n");
 
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "failed to get MEM IO resource\n");
+
+	if (!devm_request_mem_region(&pdev->dev, r->start + GENERALPURPOSE, 4,
+				     dev_name(&pdev->dev)))
+		return dev_err_probe(&pdev->dev, -EBUSY,
+				     "failed to request memory region\n");
+
+	base = devm_ioremap(&pdev->dev, r->start + GENERALPURPOSE, 4);
+	if (!base)
+		return dev_err_probe(&pdev->dev, -ENOMEM,
+				     "failed to do ioremap\n");
+
+	priv->gp_regmap = devm_regmap_init_mmio(&pdev->dev, base,
+						&dc_gp_regmap_config);
+	if (IS_ERR(priv->gp_regmap))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->gp_regmap),
+				     "failed to init General Purpose regmap\n");
+
 	dev_set_drvdata(&pdev->dev, priv);
 
 	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
@@ -196,6 +232,45 @@ static void dc_remove(struct platform_device *pdev)
 	component_master_del(&pdev->dev, &dc_drm_ops);
 }
 
+void dc_drv_set_crtc_enable(struct dc_drm_device *dc_drm,
+			    unsigned int crtc_index)
+{
+	struct dc_priv *priv = dev_get_drvdata(dc_drm->base.dev);
+
+	if (WARN_ON(crtc_index > 1))
+		return;
+
+	regmap_set_bits(priv->gp_regmap, 0, CRTC_ENABLED(crtc_index));
+}
+
+void dc_drv_set_crtc_disable(struct dc_drm_device *dc_drm,
+			     unsigned int crtc_index)
+{
+	struct dc_priv *priv = dev_get_drvdata(dc_drm->base.dev);
+
+	if (WARN_ON(crtc_index > 1))
+		return;
+
+	regmap_clear_bits(priv->gp_regmap, 0, CRTC_ENABLED(crtc_index));
+}
+
+bool dc_drv_crtc_is_enabled(struct dc_drm_device *dc_drm,
+			    unsigned int crtc_index)
+{
+	struct dc_priv *priv = dev_get_drvdata(dc_drm->base.dev);
+	unsigned int val;
+	int ret;
+
+	if (WARN_ON(crtc_index > 1))
+		return false;
+
+	ret = regmap_read(priv->gp_regmap, 0, &val);
+	if (ret)
+		return false;
+
+	return !!(val & CRTC_ENABLED(crtc_index));
+}
+
 static int dc_runtime_suspend(struct device *dev)
 {
 	struct dc_priv *priv = dev_get_drvdata(dev);
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
index eb61b8c76269..8283795f3c76 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.h
+++ b/drivers/gpu/drm/imx/dc/dc-drv.h
@@ -99,4 +99,11 @@ static inline int dc_subdev_get_id(const struct dc_subdev_info *info,
 void dc_de_post_bind(struct dc_drm_device *dc_drm);
 void dc_pe_post_bind(struct dc_drm_device *dc_drm);
 
+void dc_drv_set_crtc_enable(struct dc_drm_device *dc_drm,
+			    unsigned int crtc_index);
+void dc_drv_set_crtc_disable(struct dc_drm_device *dc_drm,
+			     unsigned int crtc_index);
+bool dc_drv_crtc_is_enabled(struct dc_drm_device *dc_drm,
+			    unsigned int crtc_index);
+
 #endif /* __DC_DRV_H__ */

-- 
2.43.0

[PATCH v8 06/15] drm/imx: dc-crtc: Disable at boot

[Liu Ying]

CRTC(s) could still be running after the DRM device is unplugged by
calling drm_dev_unplug(), because the CRTC disablement logic is
protected and bypassed by the drm_dev_enter()/drm_dev_exit() pair.
Hence, Pixel Engine's AXI clock use count(managed by Pixel Engine
driver's runtime PM) and pixel clock use count could be inbalanced
after removing and re-installing the driver module.  To fix this,
add a helper dc_crtc_disable_at_boot() and call it to properly
disable all CRTCs before advertising DRM device to user-space by
calling drm_dev_register().

Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Call dc_drv_set_crtc_disable() in dc_crtc_disable_at_boot() after CRTC
  is disabled.  (Sashiko AI bot)
- Make dc_crtc_disable_at_boot() return an integer instead of void.
  (Laurentiu)
- Postpone Pixel Engine AXI clock disablement at boot to the time point
  where CRTC1(with index == 1) is being disabled at boot.

v7:
- Check CRTC status to ensure that clocks were enabled by a previous boot
  before disabling them in dc_crtc_disable_at_boot().  (Sashiko AI bot)
- Modify debug message in dc_crtc_disable_at_boot().

v6:
- Rebase onto the latest drm-misc-next and resolve conflicts.

v3:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-crtc.c | 72 ++++++++++++++++++++++++++++++++++------
 drivers/gpu/drm/imx/dc/dc-drv.c  | 11 ++++++
 drivers/gpu/drm/imx/dc/dc-drv.h  |  3 ++
 3 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
index 0d64186a9901..5bc952d6883e 100644
--- a/drivers/gpu/drm/imx/dc/dc-crtc.c
+++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
@@ -227,6 +227,7 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	struct drm_crtc_state *new_crtc_state =
 				drm_atomic_get_new_crtc_state(state, crtc);
 	struct drm_display_mode *adj = &new_crtc_state->adjusted_mode;
+	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
 	enum dc_link_id cf_link;
 	int idx, ret;
@@ -287,29 +288,37 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 
 	DC_CRTC_CHECK_FRAMEGEN_FIFO(dc_crtc->fg);
 
+	dc_drv_set_crtc_enable(dc_drm, drm_crtc_index(crtc));
+
 	drm_dev_exit(idx);
 
 out:
 	dc_crtc_queue_state_event(new_crtc_state);
 }
 
-static void
-dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
+static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
 {
-	struct drm_crtc_state *new_crtc_state =
-				drm_atomic_get_new_crtc_state(state, crtc);
-	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
-	int idx;
-
-	if (!drm_dev_enter(crtc->dev, &idx))
-		goto out;
 
 	enable_irq(dc_crtc->irq_dec_seqcomplete);
 	dc_fg_disable(dc_crtc->fg);
 	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
 	disable_irq(dc_crtc->irq_dec_seqcomplete);
+}
 
+static void
+dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
+{
+	struct drm_crtc_state *new_crtc_state =
+				drm_atomic_get_new_crtc_state(state, crtc);
+	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
+	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
+	int idx;
+
+	if (!drm_dev_enter(crtc->dev, &idx))
+		goto out;
+
+	__dc_crtc_disable_fg(crtc);
 	dc_fg_disable_clock(dc_crtc->fg);
 
 	/* request pixel engine power-off as plane is off too */
@@ -318,8 +327,9 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	/* request display engine power-off when CRTC is disabled */
 	pm_runtime_put(dc_crtc->de->dev);
 
-	drm_dev_exit(idx);
+	dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
 
+	drm_dev_exit(idx);
 out:
 	drm_crtc_vblank_off(crtc);
 
@@ -331,6 +341,48 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	spin_unlock_irq(&crtc->dev->event_lock);
 }
 
+int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
+{
+	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
+	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
+	if (ret < 0) {
+		dc_crtc_err(crtc, "failed to get DC display engine RPM: %d\n",
+			    ret);
+		goto out1;
+	}
+
+	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
+		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
+		goto out2;
+	}
+
+	dc_crtc_dbg(crtc, "disabling FrameGen at boot\n");
+	__dc_crtc_disable_fg(crtc);
+
+	if (!dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc)))
+		goto out2;
+
+	dc_crtc_dbg(crtc, "disabling clocks at boot\n");
+
+	dc_fg_disable_clock(dc_crtc->fg);
+
+	dc_drm->pe_clk_axi_needs_disable = true;
+
+	dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
+
+out2:
+	pm_runtime_put(dc_crtc->de->dev);
+out1:
+	/* Let CRTC(with index == 1) disablement path disable AXI clock. */
+	if (dc_drm->pe_clk_axi_needs_disable && drm_crtc_index(crtc) == 1)
+		clk_disable_unprepare(dc_drm->pe->clk_axi);
+
+	return ret;
+}
+
 static bool dc_crtc_get_scanout_position(struct drm_crtc *crtc,
 					 bool in_vblank_irq,
 					 int *vpos, int *hpos,
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
index 9629beaed43c..393f7e07df7b 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -17,6 +17,7 @@
 
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_dma.h>
 #include <drm/drm_fourcc.h>
@@ -108,6 +109,7 @@ static int dc_drm_bind(struct device *dev)
 	struct dc_priv *priv = dev_get_drvdata(dev);
 	struct dc_drm_device *dc_drm;
 	struct drm_device *drm;
+	struct drm_crtc *crtc;
 	int ret;
 
 	dc_drm = devm_drm_dev_alloc(dev, &dc_drm_driver, struct dc_drm_device,
@@ -130,6 +132,15 @@ static int dc_drm_bind(struct device *dev)
 	if (ret)
 		return ret;
 
+	drm_for_each_crtc(crtc, drm) {
+		ret = dc_crtc_disable_at_boot(crtc);
+		if (ret) {
+			dev_err(dev, "failed to disable CRTC%u at boot: %d\n",
+				drm_crtc_index(crtc), ret);
+			goto err;
+		}
+	}
+
 	ret = drm_dev_register(drm, 0);
 	if (ret) {
 		dev_err(dev, "failed to register drm device: %d\n", ret);
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
index 8283795f3c76..1f0af7af5749 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.h
+++ b/drivers/gpu/drm/imx/dc/dc-drv.h
@@ -50,6 +50,8 @@ struct dc_drm_device {
 	struct dc_pe *pe;
 	/** @tc: tcon list */
 	struct dc_tc *tc[DC_DISPLAYS];
+	/** @pe_clk_axi_needs_disable: disablement flag at boot */
+	bool pe_clk_axi_needs_disable;
 };
 
 struct dc_subdev_info {
@@ -96,6 +98,7 @@ static inline int dc_subdev_get_id(const struct dc_subdev_info *info,
 	return -EINVAL;
 }
 
+int dc_crtc_disable_at_boot(struct drm_crtc *crtc);
 void dc_de_post_bind(struct dc_drm_device *dc_drm);
 void dc_pe_post_bind(struct dc_drm_device *dc_drm);
 

-- 
2.43.0

[PATCH v8 07/15] drm/imx: dc: Add PRG support

[Liu Ying]

Display Prefetch Resolve Gasket(PRG) is a part of a FetchUnit's
prefetch engine.  It sits between a Display Prefetch Resolve
Channel(DPRC) and a FetchUnit.  Add a platform driver to support
the PRG.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Fix dc_prg_burst_size_fixup() to avoid overflowing signed integer 1.
  (Sashiko AI bot)
- Check device_link_add()'s return value and bail out if needed.
  (Sashiko AI bot)
- Drop unused header files.  (Laurentiu)

v7:
- Collect Maxime's A-b tag.
- Return 128B burst size from dc_prg_burst_size_fixup() when baddr == 0.
  This avoids undefined behavior of __ffs(0).  (Sashiko AI bot)
- Initialize and declare prg_node at the same time.  (Sashiko AI bot)
- Add device link between PRG's consumer and PRG.  (Sashiko AI bot)

v6:
- Include "dc-drv.h" to avoid build warning with C=1.
- Fix PRG BYPASS programming.

v4:
- Sort bits and bitfields in descending order for register PRG_CTRL.

v2:
- Manage clocks with bulk interfaces.  (Frank)
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/Makefile |   2 +-
 drivers/gpu/drm/imx/dc/dc-drv.c |   1 +
 drivers/gpu/drm/imx/dc/dc-drv.h |   1 +
 drivers/gpu/drm/imx/dc/dc-prg.c | 318 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-prg.h |  35 +++++
 5 files changed, 356 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/dc/Makefile b/drivers/gpu/drm/imx/dc/Makefile
index b9d33c074984..e3a06ee3ce1a 100644
--- a/drivers/gpu/drm/imx/dc/Makefile
+++ b/drivers/gpu/drm/imx/dc/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 imx8-dc-drm-objs := dc-cf.o dc-crtc.o dc-de.o dc-drv.o dc-ed.o dc-fg.o dc-fl.o \
-		    dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o \
+		    dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o dc-prg.o  \
 		    dc-plane.o dc-tc.o
 
 obj-$(CONFIG_DRM_IMX8_DC) += imx8-dc-drm.o
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
index 393f7e07df7b..8e1e495f1992 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -356,6 +356,7 @@ static struct platform_driver * const dc_drivers[] = {
 	&dc_ic_driver,
 	&dc_lb_driver,
 	&dc_pe_driver,
+	&dc_prg_driver,
 	&dc_tc_driver,
 	&dc_driver,
 };
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
index 1f0af7af5749..7873cd421942 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.h
+++ b/drivers/gpu/drm/imx/dc/dc-drv.h
@@ -81,6 +81,7 @@ extern struct platform_driver dc_fw_driver;
 extern struct platform_driver dc_ic_driver;
 extern struct platform_driver dc_lb_driver;
 extern struct platform_driver dc_pe_driver;
+extern struct platform_driver dc_prg_driver;
 extern struct platform_driver dc_tc_driver;
 
 static inline int dc_subdev_get_id(const struct dc_subdev_info *info,
diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c
new file mode 100644
index 000000000000..46108f7e09af
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-prg.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2025 NXP
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include "dc-drv.h"
+#include "dc-prg.h"
+
+#define SET			0x4
+#define CLR			0x8
+#define TOG			0xc
+
+#define PRG_CTRL		0x00
+#define  SHADOW_EN		BIT(31)
+#define  SOFTRST		BIT(30)
+#define  DES_DATA_TYPE_MASK	GENMASK(17, 16)
+#define  DES_DATA_TYPE_32BPP	FIELD_PREP(DES_DATA_TYPE_MASK, 0)
+#define  DES_DATA_TYPE_24BPP	FIELD_PREP(DES_DATA_TYPE_MASK, 1)
+#define  DES_DATA_TYPE_16BPP	FIELD_PREP(DES_DATA_TYPE_MASK, 2)
+#define  DES_DATA_TYPE_8BPP	FIELD_PREP(DES_DATA_TYPE_MASK, 3)
+#define  SHADOW_LOAD_MODE	BIT(5)
+#define  HANDSHAKE_MODE_4LINES	0
+#define  SC_DATA_TYPE_8BIT	0
+#define  BYPASS			BIT(0)
+
+#define PRG_STATUS		0x10
+
+#define PRG_REG_UPDATE		0x20
+#define  REG_UPDATE		BIT(0)
+
+#define PRG_STRIDE		0x30
+#define  STRIDE(n)		FIELD_PREP(GENMASK(15, 0), (n) - 1)
+
+#define PRG_HEIGHT		0x40
+#define  HEIGHT(n)		FIELD_PREP(GENMASK(15, 0), (n) - 1)
+
+#define PRG_BADDR		0x50
+#define PRG_OFFSET		0x60
+
+#define PRG_WIDTH		0x70
+#define  WIDTH(n)		FIELD_PREP(GENMASK(15, 0), (n) - 1)
+
+#define DPU_PRG_MAX_STRIDE	0x10000
+
+struct dc_prg {
+	struct device *dev;
+	struct regmap *reg;
+	struct list_head list;
+	struct clk_bulk_data *clks;
+	int num_clks;
+};
+
+static DEFINE_MUTEX(dc_prg_list_mutex);
+static LIST_HEAD(dc_prg_list);
+
+static const struct regmap_config dc_prg_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	.max_register = PRG_WIDTH + TOG,
+};
+
+static void dc_prg_reset(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_CTRL + SET, SOFTRST);
+	fsleep(10);
+	regmap_write(prg->reg, PRG_CTRL + CLR, SOFTRST);
+	fsleep(10);
+}
+
+void dc_prg_enable(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_CTRL + CLR, BYPASS);
+}
+
+void dc_prg_disable(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_CTRL, BYPASS);
+
+	pm_runtime_put(prg->dev);
+}
+
+void dc_prg_disable_at_boot(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_CTRL, BYPASS);
+
+	clk_bulk_disable_unprepare(prg->num_clks, prg->clks);
+}
+
+static unsigned int dc_prg_burst_size_fixup(dma_addr_t baddr)
+{
+	unsigned int burst_size;
+
+	if (baddr == 0)
+		return 128U;
+
+	burst_size = 1U << __ffs(baddr);
+	burst_size = round_up(burst_size, 8);
+	burst_size = min(burst_size, 128U);
+
+	return burst_size;
+}
+
+static unsigned int
+dc_prg_stride_fixup(unsigned int stride, unsigned int burst_size)
+{
+	return round_up(stride, burst_size);
+}
+
+void dc_prg_configure(struct dc_prg *prg,
+		      unsigned int width, unsigned int height,
+		      unsigned int stride, unsigned int bits_per_pixel,
+		      dma_addr_t baddr, bool start)
+{
+	struct device *dev = prg->dev;
+	unsigned int burst_size;
+	u32 val;
+	int ret;
+
+	if (start) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to get RPM: %d\n", ret);
+			return;
+		}
+	}
+
+	burst_size = dc_prg_burst_size_fixup(baddr);
+
+	stride = dc_prg_stride_fixup(stride, burst_size);
+
+	regmap_write(prg->reg, PRG_STRIDE, STRIDE(stride));
+	regmap_write(prg->reg, PRG_WIDTH, WIDTH(width));
+	regmap_write(prg->reg, PRG_HEIGHT, HEIGHT(height));
+	regmap_write(prg->reg, PRG_OFFSET, 0);
+	regmap_write(prg->reg, PRG_BADDR, baddr);
+
+	val = SHADOW_LOAD_MODE | SC_DATA_TYPE_8BIT | HANDSHAKE_MODE_4LINES;
+
+	switch (bits_per_pixel) {
+	case 32:
+		val |= DES_DATA_TYPE_32BPP;
+		break;
+	case 24:
+		val |= DES_DATA_TYPE_24BPP;
+		break;
+	case 16:
+		val |= DES_DATA_TYPE_16BPP;
+		break;
+	case 8:
+		val |= DES_DATA_TYPE_8BPP;
+		break;
+	}
+
+	/* no shadow for the first frame */
+	val |= start ? BYPASS : SHADOW_EN;
+
+	regmap_write(prg->reg, PRG_CTRL, val);
+}
+
+void dc_prg_reg_update(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_REG_UPDATE, REG_UPDATE);
+}
+
+void dc_prg_shadow_enable(struct dc_prg *prg)
+{
+	regmap_write(prg->reg, PRG_CTRL + SET, SHADOW_EN);
+}
+
+bool dc_prg_stride_supported(struct dc_prg *prg,
+			     unsigned int stride, dma_addr_t baddr)
+{
+	unsigned int burst_size;
+
+	burst_size = dc_prg_burst_size_fixup(baddr);
+
+	stride = dc_prg_stride_fixup(stride, burst_size);
+
+	if (stride > DPU_PRG_MAX_STRIDE)
+		return false;
+
+	return true;
+}
+
+struct dc_prg *
+dc_prg_lookup_by_phandle(struct device *dev, const char *name, int index)
+{
+	struct device_node *prg_node __free(device_node) =
+				of_parse_phandle(dev->of_node, name, index);
+	struct device_link *link;
+	struct dc_prg *prg;
+
+	if (!prg_node)
+		return NULL;
+
+	guard(mutex)(&dc_prg_list_mutex);
+	list_for_each_entry(prg, &dc_prg_list, list) {
+		if (prg_node == prg->dev->of_node) {
+			link = device_link_add(dev, prg->dev,
+					       DL_FLAG_AUTOREMOVE_CONSUMER);
+			if (!link) {
+				dev_err(dev, "failed to add device link to PRG\n");
+				return NULL;
+			}
+
+			return prg;
+		}
+	}
+
+	return NULL;
+}
+
+static int dc_prg_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *base;
+	struct dc_prg *prg;
+	int ret;
+
+	prg = devm_kzalloc(dev, sizeof(*prg), GFP_KERNEL);
+	if (!prg)
+		return -ENOMEM;
+
+	base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	prg->reg = devm_regmap_init_mmio(dev, base, &dc_prg_regmap_config);
+	if (IS_ERR(prg->reg))
+		return PTR_ERR(prg->reg);
+
+	prg->num_clks = devm_clk_bulk_get_all(dev, &prg->clks);
+	if (prg->num_clks < 0)
+		return dev_err_probe(dev, prg->num_clks, "failed to get clocks\n");
+
+	dev_set_drvdata(dev, prg);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable PM runtime\n");
+
+	prg->dev = dev;
+
+	guard(mutex)(&dc_prg_list_mutex);
+	list_add(&prg->list, &dc_prg_list);
+
+	return 0;
+}
+
+static void dc_prg_remove(struct platform_device *pdev)
+{
+	struct dc_prg *prg = dev_get_drvdata(&pdev->dev);
+
+	guard(mutex)(&dc_prg_list_mutex);
+	list_del(&prg->list);
+}
+
+static int dc_prg_runtime_suspend(struct device *dev)
+{
+	struct dc_prg *prg = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(prg->num_clks, prg->clks);
+
+	return 0;
+}
+
+static int dc_prg_runtime_resume(struct device *dev)
+{
+	struct dc_prg *prg = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(prg->num_clks, prg->clks);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks: %d\n", ret);
+		return ret;
+	}
+
+	dc_prg_reset(prg);
+
+	return 0;
+}
+
+static const struct dev_pm_ops dc_prg_pm_ops = {
+	RUNTIME_PM_OPS(dc_prg_runtime_suspend, dc_prg_runtime_resume, NULL)
+};
+
+static const struct of_device_id dc_prg_dt_ids[] = {
+	{ .compatible = "fsl,imx8qxp-prg", },
+	{ /* sentinel */ }
+};
+
+struct platform_driver dc_prg_driver = {
+	.probe = dc_prg_probe,
+	.remove = dc_prg_remove,
+	.driver = {
+		.name = "imx8-dc-prg",
+		.suppress_bind_attrs = true,
+		.of_match_table = dc_prg_dt_ids,
+		.pm = pm_ptr(&dc_prg_pm_ops),
+	},
+};
diff --git a/drivers/gpu/drm/imx/dc/dc-prg.h b/drivers/gpu/drm/imx/dc/dc-prg.h
new file mode 100644
index 000000000000..6fd9b050bfa1
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-prg.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef __DC_PRG_H__
+#define __DC_PRG_H__
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+struct dc_prg;
+
+void dc_prg_enable(struct dc_prg *prg);
+
+void dc_prg_disable(struct dc_prg *prg);
+
+void dc_prg_disable_at_boot(struct dc_prg *prg);
+
+void dc_prg_configure(struct dc_prg *prg,
+		      unsigned int width, unsigned int height,
+		      unsigned int stride, unsigned int bits_per_pixel,
+		      dma_addr_t baddr, bool start);
+
+void dc_prg_reg_update(struct dc_prg *prg);
+
+void dc_prg_shadow_enable(struct dc_prg *prg);
+
+bool dc_prg_stride_supported(struct dc_prg *prg,
+			     unsigned int stride, dma_addr_t baddr);
+
+struct dc_prg *
+dc_prg_lookup_by_phandle(struct device *dev, const char *name, int index);
+
+#endif

-- 
2.43.0

[PATCH v8 08/15] drm/imx: dc: Add DPR channel support

[Liu Ying]

Display Prefetch Resolve Channel(DPRC) is a part of a prefetch engine.
It fetches display data, transforms it to linear format and stores it
to DPRC's RTRAM.  PRG, as the other part of a prefetch engine, acts as
a gasket between the RTRAM controller and a FetchUnit.  Add a platform
driver to support the DPRC.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Check device_link_add()'s return value and bail out if needed.
  (Sashiko AI bot)
- Mask IRQ_DPR_CRTL_DONE irq and synchronize wrap_irq in
  dc_dprc_disable_repeat_en() to avoid a race condition.  (Sashiko AI bot)
- Call devm_pm_runtime_enable() before devm_request_irq() to avoid reading
  dprc->reg in dc_dprc_wrap_irq_handler while dprc->clks are disabled, since
  another device sharing wrap_irq could raise an interrupt during probe.
  (Sashiko AI bot)
- Drop an unused header file.  (Laurentiu)
- Use dev_err_probe() to handle errors returned from several function calls.
  (Laurentiu)

v7:
- Collect Maxime's A-b tag.
- Fix dc_dprc_regmap_write_ranges[].  (Sashiko AI bot)
- Fix dc_dprc_wrap_irq_handler() return value.  (Sashiko AI bot)
- Check runtime PM status in dc_dprc_wrap_irq_handler().  (Sashiko AI bot)
- Fix DPU_DRPC_MAX_STRIDE macro.  (Sashiko AI bot)
- Support deferral probe if platform_get_irq_byname() fails.  (Sashiko AI bot)
- Avoid dangling prg->dprc by calling dc_prg_set_dprc() properly.
  (Sashiko AI bot)
- Add dc_dprc_get() helper for consumer device to get DPRC properly.
- Set use_raw_spinlock flag in regmap configuration to avoid a potential
  hard lockup because dc_dprc_disable_repeat_en() is supposed to be called
  with local irq and preempt disabled.

v6:
- Include "dc-drv.h" to avoid build warning with C=1.

v3:
- Call dc_dprc_enable() only when start == true.  (Frank)
- Collect Frank's R-b tag.

v2:
- Manage clocks with bulk interfaces.  (Frank)
- Sort variables in probe function in reverse Christmas tree fashion.  (Frank)
---
 drivers/gpu/drm/imx/dc/Kconfig   |   1 +
 drivers/gpu/drm/imx/dc/Makefile  |   6 +-
 drivers/gpu/drm/imx/dc/dc-dprc.c | 534 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-dprc.h |  37 +++
 drivers/gpu/drm/imx/dc/dc-drv.c  |   1 +
 drivers/gpu/drm/imx/dc/dc-drv.h  |   1 +
 drivers/gpu/drm/imx/dc/dc-prg.c  |  12 +
 drivers/gpu/drm/imx/dc/dc-prg.h  |   4 +
 8 files changed, 593 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/Kconfig b/drivers/gpu/drm/imx/dc/Kconfig
index 415993207f2e..507dc9a92d96 100644
--- a/drivers/gpu/drm/imx/dc/Kconfig
+++ b/drivers/gpu/drm/imx/dc/Kconfig
@@ -1,6 +1,7 @@
 config DRM_IMX8_DC
 	tristate "Freescale i.MX8 Display Controller Graphics"
 	depends on DRM && COMMON_CLK && OF && (ARCH_MXC || COMPILE_TEST)
+	depends on IMX_SCU
 	select DRM_CLIENT_SELECTION
 	select DRM_GEM_DMA_HELPER
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/imx/dc/Makefile b/drivers/gpu/drm/imx/dc/Makefile
index e3a06ee3ce1a..fd5d62783971 100644
--- a/drivers/gpu/drm/imx/dc/Makefile
+++ b/drivers/gpu/drm/imx/dc/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
-imx8-dc-drm-objs := dc-cf.o dc-crtc.o dc-de.o dc-drv.o dc-ed.o dc-fg.o dc-fl.o \
-		    dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o dc-prg.o  \
-		    dc-plane.o dc-tc.o
+imx8-dc-drm-objs := dc-cf.o dc-crtc.o dc-de.o dc-dprc.o dc-drv.o dc-ed.o \
+		    dc-fg.o dc-fl.o dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o \
+		    dc-pe.o dc-prg.o dc-plane.o dc-tc.o
 
 obj-$(CONFIG_DRM_IMX8_DC) += imx8-dc-drm.o
diff --git a/drivers/gpu/drm/imx/dc/dc-dprc.c b/drivers/gpu/drm/imx/dc/dc-dprc.c
new file mode 100644
index 000000000000..29188fa72a32
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-dprc.c
@@ -0,0 +1,534 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2025 NXP
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/firmware/imx/svc/misc.h>
+#include <linux/hardirq.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/firmware/imx/rsrc.h>
+
+#include "dc-dprc.h"
+#include "dc-drv.h"
+#include "dc-prg.h"
+
+#define SET					0x4
+#define CLR					0x8
+#define TOG					0xc
+
+#define SYSTEM_CTRL0				0x00
+#define  SW_SHADOW_LOAD_SEL			BIT(4)
+#define  SHADOW_LOAD_EN				BIT(3)
+#define  REPEAT_EN				BIT(2)
+#define  SOFT_RESET				BIT(1)
+#define  RUN_EN					BIT(0)	/* self-clearing */
+
+#define IRQ_MASK				0x20
+#define IRQ_MASK_STATUS				0x30
+#define IRQ_NONMASK_STATUS			0x40
+#define  DPR2RTR_FIFO_LOAD_BUF_RDY_UV_ERROR	BIT(7)
+#define  DPR2RTR_FIFO_LOAD_BUF_RDY_YRGB_ERROR	BIT(6)
+#define  DPR2RTR_UV_FIFO_OVFL			BIT(5)
+#define  DPR2RTR_YRGB_FIFO_OVFL			BIT(4)
+#define  IRQ_AXI_READ_ERROR			BIT(3)
+#define  IRQ_DPR_SHADOW_LOADED_MASK		BIT(2)
+#define  IRQ_DPR_RUN				BIT(1)
+#define  IRQ_DPR_CRTL_DONE			BIT(0)
+#define  IRQ_CTRL_MASK				GENMASK(2, 0)
+
+#define MODE_CTRL0				0x50
+#define  A_COMP_SEL(byte)			FIELD_PREP(GENMASK(17, 16), (byte))
+#define  R_COMP_SEL(byte)			FIELD_PREP(GENMASK(15, 14), (byte))
+#define  G_COMP_SEL(byte)			FIELD_PREP(GENMASK(13, 12), (byte))
+#define  B_COMP_SEL(byte)			FIELD_PREP(GENMASK(11, 10), (byte))
+#define  PIX_SIZE_32BIT				FIELD_PREP(GENMASK(7, 6), 0x2)
+#define  LINE4					BIT(1)
+#define  BUF2					0
+
+#define FRAME_CTRL0				0x70
+#define  PITCH(n)				FIELD_PREP(GENMASK(31, 16), (n))
+
+#define FRAME_1P_CTRL0				0x90
+#define FRAME_2P_CTRL0				0xe0
+#define  MAX_BYTES_PREQ_MASK			GENMASK(2, 0)
+#define  BYTE_1K				FIELD_PREP(MAX_BYTES_PREQ_MASK, 0x4)
+
+#define FRAME_1P_PIX_X_CTRL			0xa0
+#define  NUM_X_PIX_WIDE(n)			FIELD_PREP(GENMASK(15, 0), (n))
+
+#define FRAME_1P_PIX_Y_CTRL			0xb0
+#define  NUM_Y_PIX_HIGH(n)			FIELD_PREP(GENMASK(15, 0), (n))
+
+#define FRAME_1P_BASE_ADDR_CTRL0		0xc0
+
+#define FRAME_PIX_X_ULC_CTRL			0xf0
+#define  CROP_ULC_X(n)				FIELD_PREP(GENMASK(15, 0), (n))
+
+#define FRAME_PIX_Y_ULC_CTRL			0x100
+#define  CROP_ULC_Y(n)				FIELD_PREP(GENMASK(15, 0), (n))
+
+#define FRAME_2P_BASE_ADDR_CTRL0		0x110
+
+#define STATUS_CTRL0				0x130
+#define STATUS_CTRL1				0x140
+
+#define RTRAM_CTRL0				0x200
+#define  THRES_LOW(n)				FIELD_PREP(GENMASK(6, 4), (n))
+#define  THRES_HIGH(n)				FIELD_PREP(GENMASK(3, 1), (n))
+
+#define DPU_DRPC_MAX_STRIDE			0xffff
+#define DPU_DPRC_MAX_RTRAM_WIDTH		2880
+
+struct dc_dprc {
+	struct device *dev;
+	struct regmap *reg;
+	struct list_head list;
+	struct clk_bulk_data *clks;
+	int num_clks;
+	int wrap_irq;
+	struct imx_sc_ipc *ipc_handle;
+	spinlock_t lock;	/* protect IRQ registers */
+	u32 sc_resource;
+	struct dc_prg *prg;
+};
+
+static DEFINE_MUTEX(dc_dprc_list_mutex);
+static LIST_HEAD(dc_dprc_list);
+
+static const struct regmap_range dc_dprc_regmap_write_ranges[] = {
+	regmap_reg_range(SYSTEM_CTRL0, SYSTEM_CTRL0 + TOG),
+	regmap_reg_range(IRQ_MASK, MODE_CTRL0 + TOG),
+	regmap_reg_range(FRAME_CTRL0, FRAME_CTRL0 + TOG),
+	regmap_reg_range(FRAME_1P_CTRL0, FRAME_1P_BASE_ADDR_CTRL0 + TOG),
+	regmap_reg_range(FRAME_PIX_X_ULC_CTRL, FRAME_2P_BASE_ADDR_CTRL0 + TOG),
+	regmap_reg_range(STATUS_CTRL0, STATUS_CTRL0 + TOG),
+	regmap_reg_range(RTRAM_CTRL0, RTRAM_CTRL0 + TOG),
+};
+
+static const struct regmap_range dc_dprc_regmap_read_ranges[] = {
+	regmap_reg_range(SYSTEM_CTRL0, SYSTEM_CTRL0 + TOG),
+	regmap_reg_range(IRQ_MASK, IRQ_MASK_STATUS + TOG),
+	regmap_reg_range(MODE_CTRL0, MODE_CTRL0 + TOG),
+	regmap_reg_range(FRAME_CTRL0, FRAME_CTRL0 + TOG),
+	regmap_reg_range(FRAME_1P_CTRL0, FRAME_1P_BASE_ADDR_CTRL0 + TOG),
+	regmap_reg_range(FRAME_PIX_X_ULC_CTRL, FRAME_2P_BASE_ADDR_CTRL0 + TOG),
+	regmap_reg_range(STATUS_CTRL0, STATUS_CTRL1 + TOG),
+	regmap_reg_range(RTRAM_CTRL0, RTRAM_CTRL0 + TOG),
+};
+
+static const struct regmap_access_table dc_dprc_regmap_write_table = {
+	.yes_ranges = dc_dprc_regmap_write_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dc_dprc_regmap_write_ranges),
+};
+
+static const struct regmap_access_table dc_dprc_regmap_read_table = {
+	.yes_ranges = dc_dprc_regmap_read_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dc_dprc_regmap_read_ranges),
+};
+
+static const struct regmap_config dc_dprc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	/*
+	 * Registers could be accessed with local irq and preempt disabled,
+	 * so use a raw spinlock to avoid sleeping on PREEMPT_RT.
+	 */
+	.use_raw_spinlock = true,
+	.wr_table = &dc_dprc_regmap_write_table,
+	.rd_table = &dc_dprc_regmap_read_table,
+	.max_register = RTRAM_CTRL0 + TOG,
+};
+
+static void dc_dprc_set_stream_id(struct dc_dprc *dprc, unsigned int stream_id)
+{
+	int ret;
+
+	ret = imx_sc_misc_set_control(dprc->ipc_handle, dprc->sc_resource,
+				      IMX_SC_C_KACHUNK_SEL, stream_id);
+	if (ret)
+		dev_err(dprc->dev, "failed to set KACHUNK_SEL: %d\n", ret);
+}
+
+static void dc_dprc_reset(struct dc_dprc *dprc)
+{
+	regmap_write(dprc->reg, SYSTEM_CTRL0 + SET, SOFT_RESET);
+	fsleep(20);
+	regmap_write(dprc->reg, SYSTEM_CTRL0 + CLR, SOFT_RESET);
+	fsleep(20);
+}
+
+static void dc_dprc_enable(struct dc_dprc *dprc)
+{
+	dc_prg_enable(dprc->prg);
+}
+
+static void dc_dprc_reg_update(struct dc_dprc *dprc)
+{
+	dc_prg_reg_update(dprc->prg);
+}
+
+static void dc_dprc_enable_ctrl_done_irq(struct dc_dprc *dprc)
+{
+	guard(spinlock_irqsave)(&dprc->lock);
+	regmap_write(dprc->reg, IRQ_MASK + CLR, IRQ_DPR_CRTL_DONE);
+}
+
+void dc_dprc_configure(struct dc_dprc *dprc, unsigned int stream_id,
+		       unsigned int width, unsigned int height,
+		       unsigned int stride,
+		       const struct drm_format_info *format,
+		       dma_addr_t baddr, bool start)
+{
+	unsigned int prg_stride = width * format->cpp[0];
+	unsigned int bpp = format->cpp[0] * 8;
+	struct device *dev = dprc->dev;
+	unsigned int p1_w, p1_h;
+	u32 val;
+	int ret;
+
+	if (start) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to get RPM: %d\n", ret);
+			return;
+		}
+
+		dc_dprc_set_stream_id(dprc, stream_id);
+	}
+
+	p1_w = round_up(width, format->cpp[0] == 2 ? 32 : 16);
+	p1_h = round_up(height, 4);
+
+	regmap_write(dprc->reg, FRAME_CTRL0, PITCH(stride));
+	regmap_write(dprc->reg, FRAME_1P_CTRL0, BYTE_1K);
+	regmap_write(dprc->reg, FRAME_1P_PIX_X_CTRL, NUM_X_PIX_WIDE(p1_w));
+	regmap_write(dprc->reg, FRAME_1P_PIX_Y_CTRL, NUM_Y_PIX_HIGH(p1_h));
+	regmap_write(dprc->reg, FRAME_1P_BASE_ADDR_CTRL0, baddr);
+	regmap_write(dprc->reg, FRAME_PIX_X_ULC_CTRL, CROP_ULC_X(0));
+	regmap_write(dprc->reg, FRAME_PIX_Y_ULC_CTRL, CROP_ULC_Y(0));
+
+	regmap_write(dprc->reg, RTRAM_CTRL0, THRES_LOW(3) | THRES_HIGH(7));
+
+	val = LINE4 | BUF2;
+	switch (format->format) {
+	case DRM_FORMAT_XRGB8888:
+		/*
+		 * It turns out pixel components are mapped directly
+		 * without position change via DPR processing with
+		 * the following color component configurations.
+		 * Leave the pixel format to be handled by the
+		 * display controllers.
+		 */
+		val |= A_COMP_SEL(3) | R_COMP_SEL(2) |
+		       G_COMP_SEL(1) | B_COMP_SEL(0);
+		val |= PIX_SIZE_32BIT;
+		break;
+	default:
+		dev_err(dev, "unsupported format 0x%08x\n", format->format);
+		return;
+	}
+	regmap_write(dprc->reg, MODE_CTRL0, val);
+
+	if (start) {
+		/* software shadow load for the first frame */
+		val = SW_SHADOW_LOAD_SEL | SHADOW_LOAD_EN;
+		regmap_write(dprc->reg, SYSTEM_CTRL0, val);
+
+		/* and then, run... */
+		val |= RUN_EN | REPEAT_EN;
+		regmap_write(dprc->reg, SYSTEM_CTRL0, val);
+	}
+
+	dc_prg_configure(dprc->prg, width, height, prg_stride, bpp, baddr, start);
+
+	if (start)
+		dc_dprc_enable(dprc);
+
+	dc_dprc_reg_update(dprc);
+
+	if (start)
+		dc_dprc_enable_ctrl_done_irq(dprc);
+
+	dev_dbg(dev, "w: %u, h: %u, s: %u, fmt: 0x%08x\n",
+		width, height, stride, format->format);
+}
+
+void dc_dprc_disable_repeat_en(struct dc_dprc *dprc)
+{
+	guard(spinlock)(&dprc->lock);
+	regmap_write(dprc->reg, IRQ_MASK + SET, IRQ_DPR_CRTL_DONE);
+
+	synchronize_hardirq(dprc->wrap_irq);
+
+	regmap_write(dprc->reg, SYSTEM_CTRL0 + CLR, REPEAT_EN);
+	dev_dbg(dprc->dev, "disable REPEAT_EN\n");
+}
+
+/*
+ * dc_dprc_disable_repeat_en() must be called beforehand to mask
+ * IRQ_DPR_CRTL_DONE and synchronize wrap_irq.
+ */
+void dc_dprc_disable(struct dc_dprc *dprc)
+{
+	dc_prg_disable(dprc->prg);
+
+	pm_runtime_put(dprc->dev);
+
+	dev_dbg(dprc->dev, "disable\n");
+}
+
+void dc_dprc_disable_at_boot(struct dc_dprc *dprc)
+{
+	dc_prg_disable_at_boot(dprc->prg);
+
+	clk_bulk_disable_unprepare(dprc->num_clks, dprc->clks);
+
+	dev_dbg(dprc->dev, "disable at boot\n");
+}
+
+static void dc_dprc_ctrl_done_handle(struct dc_dprc *dprc)
+{
+	regmap_write(dprc->reg, SYSTEM_CTRL0, REPEAT_EN);
+
+	dc_prg_shadow_enable(dprc->prg);
+
+	dev_dbg(dprc->dev, "CTRL done handle\n");
+}
+
+static irqreturn_t dc_dprc_wrap_irq_handler(int irq, void *data)
+{
+	struct dc_dprc *dprc = data;
+	struct device *dev = dprc->dev;
+	u32 mask = 0, status = 0;
+	irqreturn_t result;
+
+	pm_runtime_get_noresume(dev);
+	if (!pm_runtime_active(dev)) {
+		result = IRQ_NONE;
+		goto out;
+	}
+
+	scoped_guard(spinlock, &dprc->lock) {
+		/* cache valid IRQ status */
+		regmap_read(dprc->reg, IRQ_MASK, &mask);
+		regmap_read(dprc->reg, IRQ_MASK_STATUS, &status);
+		status &= ~mask;
+
+		/* mask the IRQ(s) being handled */
+		regmap_write(dprc->reg, IRQ_MASK + SET, status);
+
+		/* clear status register */
+		regmap_write(dprc->reg, IRQ_MASK_STATUS, status);
+	}
+
+	if (status == 0) {
+		result = IRQ_NONE;
+		goto out;
+	}
+
+	if (status & DPR2RTR_FIFO_LOAD_BUF_RDY_UV_ERROR)
+		dev_err(dev, "DPR to RTRAM FIFO load UV buffer ready error\n");
+
+	if (status & DPR2RTR_FIFO_LOAD_BUF_RDY_YRGB_ERROR)
+		dev_err(dev, "DPR to RTRAM FIFO load YRGB buffer ready error\n");
+
+	if (status & DPR2RTR_UV_FIFO_OVFL)
+		dev_err(dev, "DPR to RTRAM FIFO UV FIFO overflow\n");
+
+	if (status & DPR2RTR_YRGB_FIFO_OVFL)
+		dev_err(dev, "DPR to RTRAM FIFO YRGB FIFO overflow\n");
+
+	if (status & IRQ_AXI_READ_ERROR)
+		dev_err(dev, "AXI read error\n");
+
+	if (status & IRQ_DPR_CRTL_DONE)
+		dc_dprc_ctrl_done_handle(dprc);
+
+	result = IRQ_HANDLED;
+out:
+	pm_runtime_put_noidle(dev);
+	return result;
+}
+
+bool dc_dprc_rtram_width_supported(struct dc_dprc *dprc, unsigned int width)
+{
+	return width <= DPU_DPRC_MAX_RTRAM_WIDTH;
+}
+
+bool dc_dprc_stride_supported(struct dc_dprc *dprc,
+			      unsigned int stride, unsigned int width,
+			      const struct drm_format_info *format,
+			      dma_addr_t baddr)
+{
+	unsigned int prg_stride = width * format->cpp[0];
+
+	if (stride > DPU_DRPC_MAX_STRIDE)
+		return false;
+
+	if (!dc_prg_stride_supported(dprc->prg, prg_stride, baddr))
+		return false;
+
+	return true;
+}
+
+struct dc_dprc *dc_dprc_get(struct device *dev, struct dc_dprc *dprc)
+{
+	struct device_link *link;
+	struct dc_dprc *iter;
+
+	guard(mutex)(&dc_dprc_list_mutex);
+	list_for_each_entry(iter, &dc_dprc_list, list) {
+		if (iter == dprc) {
+			link = device_link_add(dev, dprc->dev,
+					       DL_FLAG_AUTOREMOVE_CONSUMER);
+			if (!link) {
+				dev_err(dev, "failed to add device link to DPRC\n");
+				return NULL;
+			}
+
+			return dprc;
+		}
+	}
+
+	return NULL;
+}
+
+static int dc_dprc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct resource *res;
+	struct dc_dprc *dprc;
+	void __iomem *base;
+	int ret;
+
+	dprc = devm_kzalloc(dev, sizeof(*dprc), GFP_KERNEL);
+	if (!dprc)
+		return -ENOMEM;
+
+	ret = imx_scu_get_handle(&dprc->ipc_handle);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get SCU ipc handle\n");
+
+	base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	dprc->reg = devm_regmap_init_mmio(dev, base, &dc_dprc_regmap_config);
+	if (IS_ERR(dprc->reg))
+		return PTR_ERR(dprc->reg);
+
+	dprc->wrap_irq = platform_get_irq_byname(pdev, "dpr_wrap");
+	if (dprc->wrap_irq < 0)
+		return dev_err_probe(dev, dprc->wrap_irq, "failed to get dpr_wrap IRQ\n");
+
+	dprc->num_clks = devm_clk_bulk_get_all(dev, &dprc->clks);
+	if (dprc->num_clks < 0)
+		return dev_err_probe(dev, dprc->num_clks, "failed to get clocks\n");
+
+	ret = of_property_read_u32(np, "fsl,sc-resource", &dprc->sc_resource);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get SC resource\n");
+
+	dprc->prg = dc_prg_lookup_by_phandle(dev, "fsl,prgs", 0);
+	if (!dprc->prg)
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "failed to lookup PRG\n");
+
+	dprc->dev = dev;
+	spin_lock_init(&dprc->lock);
+
+	dev_set_drvdata(dev, dprc);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable PM runtime\n");
+
+	ret = devm_request_irq(dev, dprc->wrap_irq, dc_dprc_wrap_irq_handler,
+			       IRQF_SHARED, dev_name(dev), dprc);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to request dpr_wrap IRQ(%d)\n",
+				     dprc->wrap_irq);
+
+	guard(mutex)(&dc_dprc_list_mutex);
+	list_add(&dprc->list, &dc_dprc_list);
+
+	dc_prg_set_dprc(dprc->prg, dprc);
+
+	return 0;
+}
+
+static void dc_dprc_remove(struct platform_device *pdev)
+{
+	struct dc_dprc *dprc = dev_get_drvdata(&pdev->dev);
+
+	dc_prg_set_dprc(dprc->prg, NULL);
+
+	guard(mutex)(&dc_dprc_list_mutex);
+	list_del(&dprc->list);
+}
+
+static int dc_dprc_runtime_suspend(struct device *dev)
+{
+	struct dc_dprc *dprc = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(dprc->num_clks, dprc->clks);
+
+	return 0;
+}
+
+static int dc_dprc_runtime_resume(struct device *dev)
+{
+	struct dc_dprc *dprc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(dprc->num_clks, dprc->clks);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks: %d\n", ret);
+		return ret;
+	}
+
+	dc_dprc_reset(dprc);
+
+	/* disable all control IRQs and enable all error IRQs */
+	guard(spinlock_irqsave)(&dprc->lock);
+	regmap_write(dprc->reg, IRQ_MASK, IRQ_CTRL_MASK);
+
+	return 0;
+}
+
+static const struct dev_pm_ops dc_dprc_pm_ops = {
+	RUNTIME_PM_OPS(dc_dprc_runtime_suspend, dc_dprc_runtime_resume, NULL)
+};
+
+static const struct of_device_id dc_dprc_dt_ids[] = {
+	{ .compatible = "fsl,imx8qxp-dpr-channel", },
+	{ /* sentinel */ }
+};
+
+struct platform_driver dc_dprc_driver = {
+	.probe = dc_dprc_probe,
+	.remove = dc_dprc_remove,
+	.driver = {
+		.name = "imx8-dc-dpr-channel",
+		.suppress_bind_attrs = true,
+		.of_match_table = dc_dprc_dt_ids,
+		.pm = pm_ptr(&dc_dprc_pm_ops),
+	},
+};
diff --git a/drivers/gpu/drm/imx/dc/dc-dprc.h b/drivers/gpu/drm/imx/dc/dc-dprc.h
new file mode 100644
index 000000000000..45b0f1d3dd6e
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-dprc.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef __DC_DPRC_H__
+#define __DC_DPRC_H__
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+#include <drm/drm_fourcc.h>
+
+struct dc_dprc;
+
+void dc_dprc_configure(struct dc_dprc *dprc, unsigned int stream_id,
+		       unsigned int width, unsigned int height,
+		       unsigned int stride,
+		       const struct drm_format_info *format,
+		       dma_addr_t baddr, bool start);
+
+void dc_dprc_disable_repeat_en(struct dc_dprc *dprc);
+
+void dc_dprc_disable(struct dc_dprc *dprc);
+
+void dc_dprc_disable_at_boot(struct dc_dprc *dprc);
+
+bool dc_dprc_rtram_width_supported(struct dc_dprc *dprc, unsigned int width);
+
+bool dc_dprc_stride_supported(struct dc_dprc *dprc,
+			      unsigned int stride, unsigned int width,
+			      const struct drm_format_info *format,
+			      dma_addr_t baddr);
+
+struct dc_dprc *dc_dprc_get(struct device *dev, struct dc_dprc *dprc);
+
+#endif
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
index 8e1e495f1992..9fbc588cd09d 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -349,6 +349,7 @@ static struct platform_driver dc_driver = {
 static struct platform_driver * const dc_drivers[] = {
 	&dc_cf_driver,
 	&dc_de_driver,
+	&dc_dprc_driver,
 	&dc_ed_driver,
 	&dc_fg_driver,
 	&dc_fl_driver,
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
index 7873cd421942..ba6ed7b06bfc 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.h
+++ b/drivers/gpu/drm/imx/dc/dc-drv.h
@@ -74,6 +74,7 @@ int dc_plane_init(struct dc_drm_device *dc_drm, struct dc_plane *dc_plane);
 
 extern struct platform_driver dc_cf_driver;
 extern struct platform_driver dc_de_driver;
+extern struct platform_driver dc_dprc_driver;
 extern struct platform_driver dc_ed_driver;
 extern struct platform_driver dc_fg_driver;
 extern struct platform_driver dc_fl_driver;
diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c
index 46108f7e09af..82cf8170eea3 100644
--- a/drivers/gpu/drm/imx/dc/dc-prg.c
+++ b/drivers/gpu/drm/imx/dc/dc-prg.c
@@ -17,6 +17,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
+#include "dc-dprc.h"
 #include "dc-drv.h"
 #include "dc-prg.h"
 
@@ -62,6 +63,7 @@ struct dc_prg {
 	struct list_head list;
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct dc_dprc *dprc;
 };
 
 static DEFINE_MUTEX(dc_prg_list_mutex);
@@ -226,6 +228,16 @@ dc_prg_lookup_by_phandle(struct device *dev, const char *name, int index)
 	return NULL;
 }
 
+void dc_prg_set_dprc(struct dc_prg *prg, struct dc_dprc *dprc)
+{
+	prg->dprc = dprc;
+}
+
+struct dc_dprc *dc_prg_get_dprc(struct dc_prg *prg)
+{
+	return prg->dprc;
+}
+
 static int dc_prg_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/drivers/gpu/drm/imx/dc/dc-prg.h b/drivers/gpu/drm/imx/dc/dc-prg.h
index 6fd9b050bfa1..f29d154f7de5 100644
--- a/drivers/gpu/drm/imx/dc/dc-prg.h
+++ b/drivers/gpu/drm/imx/dc/dc-prg.h
@@ -32,4 +32,8 @@ bool dc_prg_stride_supported(struct dc_prg *prg,
 struct dc_prg *
 dc_prg_lookup_by_phandle(struct device *dev, const char *name, int index);
 
+void dc_prg_set_dprc(struct dc_prg *prg, struct dc_dprc *dprc);
+
+struct dc_dprc *dc_prg_get_dprc(struct dc_prg *prg);
+
 #endif

-- 
2.43.0

[PATCH v8 09/15] drm/imx: dc: Use TCON operation mode

[Liu Ying]

In TCON operation mode, sync signals from FrameGen are ignored, but
a much more customized output timing can be generated by the TCON
module.  By using TCON operaton mode, generate KACHUNK signal along
with HSYNC/VSYNC/data enable signals.  The KACHUNK signal is used as
a synchronization signal inside the prefetch engine(DPRC + PRG(s),
attached to FetchUnit(s)).  Carefully switch TCON bypass mode to TCON
operation mode when CRTC is being enabled so that the prefetch engine
may evade the first dumb frame generated by the display controller.
The switching is done with local irq and preempt disabled to ensure
it is done right after FrameGen frame counter moves as instructed by
design team.  Set use_raw_spinlock flag in flag in FrameGen and Tcon
regmap configurations to avoid sleeping on PREEMPT_RT when switching.
Also, replace jiffies with timekeeping API ktime_add_ms(), ktime_get()
and ktime_before() in dc_fg_wait_for_frame_index_moving() to avoid a
potential hard lockup when switching.

Since TCON BYPASS bit is controlled by KMS driver when doing atomic
commits, drop the bit setting when initializing TCON.  This also
avoids accidentally initializing TCON BYPASS bit to 1 when driver
module removing and re-installing where an upcoming patch would
disable a CRTC at boot in TCON operation mode if needed.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v7:
- Set use_raw_spinlock flag in FrameGen and Tcon regmap configurations to
  avoid sleeping on PREEMPT_RT.  (Sashiko AI bot)
- Replace jiffies with timekeeping API ktime_add_ms(), ktime_get() and
  ktime_before() in dc_fg_wait_for_frame_index_moving() to avoid a
  potential hard lockup.  (Sashiko AI bot)
- Use adjusted timings(e.g., m->crtc_hdisplay) in dc_tc_cfg_videomode()
  instead of logical mode timings (e.g., m->hdisplay).  (Sashiko AI bot)
- Update commit message.

v6:
- Rebase onto the latest drm-misc-next and resolve conflicts.

v3:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-crtc.c |  28 +++++++++
 drivers/gpu/drm/imx/dc/dc-de.h   |   2 +
 drivers/gpu/drm/imx/dc/dc-fg.c   |  11 +++-
 drivers/gpu/drm/imx/dc/dc-kms.h  |   2 +
 drivers/gpu/drm/imx/dc/dc-tc.c   | 119 +++++++++++++++++++++++++++++++++++++--
 5 files changed, 155 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
index 5bc952d6883e..d03246036d4a 100644
--- a/drivers/gpu/drm/imx/dc/dc-crtc.c
+++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
@@ -6,8 +6,10 @@
 #include <linux/completion.h>
 #include <linux/container_of.h>
 #include <linux/interrupt.h>
+#include <linux/irqflags.h>
 #include <linux/irqreturn.h>
 #include <linux/pm_runtime.h>
+#include <linux/preempt.h>
 #include <linux/spinlock.h>
 
 #include <drm/drm_atomic.h>
@@ -68,6 +70,14 @@ do {									\
 							__func__);	\
 } while (0)
 
+#define DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(fg)		\
+do {									\
+	if (!dc_fg_wait_for_frame_index_moving(fg))			\
+		dc_crtc_err(crtc,					\
+			"%s: FrameGen frame index isn't moving\n",	\
+							__func__);	\
+} while (0)
+
 static inline struct dc_crtc *to_dc_crtc(struct drm_crtc *crtc)
 {
 	return container_of(crtc, struct dc_crtc, base);
@@ -230,6 +240,7 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
 	enum dc_link_id cf_link;
+	unsigned long flags;
 	int idx, ret;
 
 	dc_crtc_dbg(crtc, "mode " DRM_MODE_FMT "\n", DRM_MODE_ARG(adj));
@@ -250,6 +261,7 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	enable_irq(dc_crtc->irq_ed_safe_shdload);
 
 	dc_fg_cfg_videomode(dc_crtc->fg, adj);
+	dc_tc_cfg_videomode(dc_crtc->tc, adj);
 
 	dc_cf_framedimensions(dc_crtc->cf_cont,
 			      adj->crtc_hdisplay, adj->crtc_vdisplay);
@@ -274,7 +286,22 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
 	dc_ed_pec_sync_trigger(dc_crtc->ed_safe);
 	dc_fg_shdtokgen(dc_crtc->fg);
+
+	/* Don't relinquish CPU until TCON is set to operation mode. */
+	local_irq_save(flags);
+	preempt_disable();
+
 	dc_fg_enable(dc_crtc->fg);
+	/*
+	 * Turn TCON into operation mode as soon as the first dumb
+	 * frame is generated by DC(we don't relinquish CPU to ensure
+	 * this).  This makes DPR/PRG be able to evade the frame.
+	 */
+	DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
+	dc_tc_set_operation_mode(dc_crtc->tc);
+
+	local_irq_restore(flags);
+	preempt_enable();
 
 	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(ed_safe_shdload_done);
 	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(ed_cont_shdload_done);
@@ -567,6 +594,7 @@ int dc_crtc_init(struct dc_drm_device *dc_drm, int crtc_index)
 	dc_crtc->ed_cont = pe->ed_cont[crtc_index];
 	dc_crtc->ed_safe = pe->ed_safe[crtc_index];
 	dc_crtc->fg = de->fg;
+	dc_crtc->tc = de->tc;
 
 	dc_crtc->irq_dec_framecomplete = de->irq_framecomplete;
 	dc_crtc->irq_dec_seqcomplete = de->irq_seqcomplete;
diff --git a/drivers/gpu/drm/imx/dc/dc-de.h b/drivers/gpu/drm/imx/dc/dc-de.h
index 211f3fcc1a9a..c39f2ef5eea9 100644
--- a/drivers/gpu/drm/imx/dc/dc-de.h
+++ b/drivers/gpu/drm/imx/dc/dc-de.h
@@ -54,6 +54,8 @@ enum drm_mode_status dc_fg_check_clock(struct dc_fg *fg, int clk_khz);
 void dc_fg_init(struct dc_fg *fg);
 
 /* Timing Controller Unit */
+void dc_tc_set_operation_mode(struct dc_tc *tc);
+void dc_tc_cfg_videomode(struct dc_tc *tc, struct drm_display_mode *m);
 void dc_tc_init(struct dc_tc *tc);
 
 #endif /* __DC_DISPLAY_ENGINE_H__ */
diff --git a/drivers/gpu/drm/imx/dc/dc-fg.c b/drivers/gpu/drm/imx/dc/dc-fg.c
index 3741dc66c0d9..26b786e146d4 100644
--- a/drivers/gpu/drm/imx/dc/dc-fg.c
+++ b/drivers/gpu/drm/imx/dc/dc-fg.c
@@ -8,7 +8,7 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/device.h>
-#include <linux/jiffies.h>
+#include <linux/ktime.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -122,6 +122,11 @@ static const struct regmap_config dc_fg_regmap_config = {
 	.reg_stride = 4,
 	.val_bits = 32,
 	.fast_io = true,
+	/*
+	 * Registers could be accessed with local irq and preempt disabled,
+	 * so use a raw spinlock to avoid sleeping on PREEMPT_RT.
+	 */
+	.use_raw_spinlock = true,
 	.wr_table = &dc_fg_regmap_write_table,
 	.rd_table = &dc_fg_regmap_read_table,
 	.max_register = FGCHSTATCLR,
@@ -230,7 +235,7 @@ u32 dc_fg_get_line_index(struct dc_fg *fg)
 
 bool dc_fg_wait_for_frame_index_moving(struct dc_fg *fg)
 {
-	unsigned long timeout = jiffies + msecs_to_jiffies(100);
+	ktime_t timeout = ktime_add_ms(ktime_get(), 100);
 	u32 frame_index, last_frame_index;
 
 	frame_index = dc_fg_get_frame_index(fg);
@@ -238,7 +243,7 @@ bool dc_fg_wait_for_frame_index_moving(struct dc_fg *fg)
 		last_frame_index = frame_index;
 		frame_index = dc_fg_get_frame_index(fg);
 	} while (last_frame_index == frame_index &&
-		 time_before(jiffies, timeout));
+		 ktime_before(ktime_get(), timeout));
 
 	return last_frame_index != frame_index;
 }
diff --git a/drivers/gpu/drm/imx/dc/dc-kms.h b/drivers/gpu/drm/imx/dc/dc-kms.h
index cd7860eff986..a25d47eebd28 100644
--- a/drivers/gpu/drm/imx/dc/dc-kms.h
+++ b/drivers/gpu/drm/imx/dc/dc-kms.h
@@ -50,6 +50,8 @@ struct dc_crtc {
 	struct dc_ed *ed_safe;
 	/** @fg: framegen */
 	struct dc_fg *fg;
+	/** @tc: tcon */
+	struct dc_tc *tc;
 	/**
 	 * @irq_dec_framecomplete:
 	 *
diff --git a/drivers/gpu/drm/imx/dc/dc-tc.c b/drivers/gpu/drm/imx/dc/dc-tc.c
index d0d4faba790e..626758904f77 100644
--- a/drivers/gpu/drm/imx/dc/dc-tc.c
+++ b/drivers/gpu/drm/imx/dc/dc-tc.c
@@ -8,11 +8,30 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
+#include <drm/drm_modes.h>
+
 #include "dc-drv.h"
 #include "dc-de.h"
 
 #define TCON_CTRL	0x410
-#define  CTRL_RST_VAL	0x01401408
+#define  SPLITPOSITION_MASK	GENMASK(29, 16)
+#define  SPLITPOSITION(n)	FIELD_PREP(SPLITPOSITION_MASK, (n))
+#define  DUAL_SWAP	BIT(15)
+#define  MINILVDS_OPCODE_MASK	GENMASK(14, 12)
+#define  MODE_4PAIRS	FIELD_PREP(MINILVDS_OPCODE_MASK, 0x1)
+#define  LVDS_CLOCK_INV	BIT(11)
+#define  LVDS_BALANCE	BIT(10)
+#define  LVDSMODE	BIT(9)
+#define  ENLVDS		BIT(8)
+#define  INV_CTRL_MASK	GENMASK(7, 4)
+#define  BYPASS		BIT(3)
+#define  TCON_SYNC	BIT(2)
+#define  CHANNELMODE_MASK	GENMASK(1, 0)
+#define  CTRL_RST_MASK	\
+		(SPLITPOSITION_MASK | DUAL_SWAP | MINILVDS_OPCODE_MASK | \
+		 LVDS_CLOCK_INV | LVDS_BALANCE | LVDSMODE | ENLVDS | \
+		 INV_CTRL_MASK | TCON_SYNC | CHANNELMODE_MASK)
+#define  CTRL_RST_VAL  (SPLITPOSITION(0x140) | MODE_4PAIRS | LVDS_BALANCE)
 
 /* red: MAPBIT 29-20, green: MAPBIT 19-10, blue: MAPBIT 9-0 */
 #define MAPBIT3_0	0x418
@@ -24,6 +43,16 @@
 #define MAPBIT27_24	0x430
 #define MAPBIT31_28	0x434
 
+#define SPGPOSON(n)	(0x460 + (n) * 16)
+#define SPGMASKON(n)	(0x464 + (n) * 16)
+#define SPGPOSOFF(n)	(0x468 + (n) * 16)
+#define SPGMASKOFF(n)	(0x46c + (n) * 16)
+#define  X(n)		FIELD_PREP(GENMASK(30, 16), (n))
+#define  Y(n)		FIELD_PREP(GENMASK(14, 0), (n))
+
+#define SMXSIGS(n)	(0x520 + (n) * 8)
+#define SMXFCTTABLE(n)	(0x524 + (n) * 8)
+
 static const struct dc_subdev_info dc_tc_info[] = {
 	{ .reg_start = 0x5618c800, .id = 0, },
 	{ .reg_start = 0x5618e400, .id = 1, },
@@ -32,6 +61,8 @@ static const struct dc_subdev_info dc_tc_info[] = {
 static const struct regmap_range dc_tc_regmap_ranges[] = {
 	regmap_reg_range(TCON_CTRL, TCON_CTRL),
 	regmap_reg_range(MAPBIT3_0, MAPBIT31_28),
+	regmap_reg_range(SPGPOSON(0), SPGMASKOFF(4)),
+	regmap_reg_range(SMXSIGS(0), SMXFCTTABLE(3)),
 };
 
 static const struct regmap_access_table dc_tc_regmap_access_table = {
@@ -44,9 +75,14 @@ static const struct regmap_config dc_tc_regmap_config = {
 	.reg_stride = 4,
 	.val_bits = 32,
 	.fast_io = true,
+	/*
+	 * Registers could be accessed with local irq and preempt disabled,
+	 * so use a raw spinlock to avoid sleeping on PREEMPT_RT.
+	 */
+	.use_raw_spinlock = true,
 	.wr_table = &dc_tc_regmap_access_table,
 	.rd_table = &dc_tc_regmap_access_table,
-	.max_register = MAPBIT31_28,
+	.max_register = SMXFCTTABLE(3),
 };
 
 /*
@@ -59,10 +95,85 @@ static const u32 dc_tc_mapbit[] = {
 	0x13121110, 0x03020100, 0x07060504, 0x00000908,
 };
 
+void dc_tc_set_operation_mode(struct dc_tc *tc)
+{
+	regmap_write_bits(tc->reg, TCON_CTRL, BYPASS, 0);
+}
+
+void dc_tc_cfg_videomode(struct dc_tc *tc, struct drm_display_mode *m)
+{
+	int hdisplay, hsync_start, hsync_end;
+	int vdisplay, vsync_start, vsync_end;
+	int y;
+
+	hdisplay = m->crtc_hdisplay;
+	vdisplay = m->crtc_vdisplay;
+	hsync_start = m->crtc_hsync_start;
+	vsync_start = m->crtc_vsync_start;
+	hsync_end = m->crtc_hsync_end;
+	vsync_end = m->crtc_vsync_end;
+
+	/*
+	 * Turn TCON into operation mode later after the first dumb frame is
+	 * generated by DC.  This makes DPR/PRG be able to evade the frame.
+	 */
+	regmap_write_bits(tc->reg, TCON_CTRL, BYPASS, BYPASS);
+
+	/* dsp_control[0]: HSYNC */
+	regmap_write(tc->reg, SPGPOSON(0), X(hsync_start));
+	regmap_write(tc->reg, SPGMASKON(0), 0xffff);
+
+	regmap_write(tc->reg, SPGPOSOFF(0), X(hsync_end));
+	regmap_write(tc->reg, SPGMASKOFF(0), 0xffff);
+
+	regmap_write(tc->reg, SMXSIGS(0), 0x2);
+	regmap_write(tc->reg, SMXFCTTABLE(0), 0x1);
+
+	/* dsp_control[1]: VSYNC */
+	regmap_write(tc->reg, SPGPOSON(1), X(hsync_start) | Y(vsync_start - 1));
+	regmap_write(tc->reg, SPGMASKON(1), 0x0);
+
+	regmap_write(tc->reg, SPGPOSOFF(1), X(hsync_start) | Y(vsync_end - 1));
+	regmap_write(tc->reg, SPGMASKOFF(1), 0x0);
+
+	regmap_write(tc->reg, SMXSIGS(1), 0x3);
+	regmap_write(tc->reg, SMXFCTTABLE(1), 0x1);
+
+	/* dsp_control[2]: data enable */
+	/* horizontal */
+	regmap_write(tc->reg, SPGPOSON(2), 0x0);
+	regmap_write(tc->reg, SPGMASKON(2), 0xffff);
+
+	regmap_write(tc->reg, SPGPOSOFF(2), X(hdisplay));
+	regmap_write(tc->reg, SPGMASKOFF(2), 0xffff);
+
+	/* vertical */
+	regmap_write(tc->reg, SPGPOSON(3), 0x0);
+	regmap_write(tc->reg, SPGMASKON(3), 0x7fff0000);
+
+	regmap_write(tc->reg, SPGPOSOFF(3), Y(vdisplay));
+	regmap_write(tc->reg, SPGMASKOFF(3), 0x7fff0000);
+
+	regmap_write(tc->reg, SMXSIGS(2), 0x2c);
+	regmap_write(tc->reg, SMXFCTTABLE(2), 0x8);
+
+	/* dsp_control[3]: KACHUNK */
+	y = vdisplay + 1;
+
+	regmap_write(tc->reg, SPGPOSON(4), X(0x0) | Y(y));
+	regmap_write(tc->reg, SPGMASKON(4), 0x0);
+
+	regmap_write(tc->reg, SPGPOSOFF(4), X(0x20) | Y(y));
+	regmap_write(tc->reg, SPGMASKOFF(4), 0x0);
+
+	regmap_write(tc->reg, SMXSIGS(3), 0x6);
+	regmap_write(tc->reg, SMXFCTTABLE(3), 0x2);
+}
+
 void dc_tc_init(struct dc_tc *tc)
 {
-	/* reset TCON_CTRL to POR default so that TCON works in bypass mode */
-	regmap_write(tc->reg, TCON_CTRL, CTRL_RST_VAL);
+	/* reset TCON_CTRL to POR default except for touching BYPASS bit */
+	regmap_write_bits(tc->reg, TCON_CTRL, CTRL_RST_MASK, CTRL_RST_VAL);
 
 	/* set format */
 	regmap_bulk_write(tc->reg, MAPBIT3_0, dc_tc_mapbit,

-- 
2.43.0

[PATCH v8 10/15] drm/imx: dc-ed: Support getting source selection

[Liu Ying]

Add a helper to get ExtDst source selection.  This is needed by
disabling CRTC at boot in an upcoming commit which tries to get
the source selection.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Collect Alexander's R-b tag.
- Fix dc_ed_pec_src_sel_get()'s return value when regmap_read() fails.
  (Alexander)
- Iterate over src_sels[] to find the source selection.  (Sashiko AI bot)

v7:
- Collect Maxime's A-b tag.
- Check regmap_read()'s return valure.  (Sashiko AI bot)

v2:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-ed.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-pe.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/imx/dc/dc-ed.c b/drivers/gpu/drm/imx/dc/dc-ed.c
index de1b71315eab..a4a21d0affb6 100644
--- a/drivers/gpu/drm/imx/dc/dc-ed.c
+++ b/drivers/gpu/drm/imx/dc/dc-ed.c
@@ -149,6 +149,27 @@ void dc_ed_pec_src_sel(struct dc_ed *ed, enum dc_link_id src)
 	}
 }
 
+int dc_ed_pec_src_sel_get(struct dc_ed *ed, enum dc_link_id *src)
+{
+	u32 val;
+	int ret;
+	int i;
+
+	ret = regmap_read(ed->reg_pec, PIXENGCFG_DYNAMIC, &val);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(src_sels); i++) {
+		if (src_sels[i] == val) {
+			dev_dbg(ed->dev, "get source selection: 0x%02x\n", val);
+			*src = val;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 void dc_ed_pec_sync_trigger(struct dc_ed *ed)
 {
 	regmap_write(ed->reg_pec, PIXENGCFG_TRIGGER, SYNC_TRIGGER);
diff --git a/drivers/gpu/drm/imx/dc/dc-pe.h b/drivers/gpu/drm/imx/dc/dc-pe.h
index f5e01a6eb9e9..0b81dc6b251c 100644
--- a/drivers/gpu/drm/imx/dc/dc-pe.h
+++ b/drivers/gpu/drm/imx/dc/dc-pe.h
@@ -85,6 +85,7 @@ void dc_cf_init(struct dc_cf *cf);
 
 /* External Destination Unit */
 void dc_ed_pec_src_sel(struct dc_ed *ed, enum dc_link_id src);
+int dc_ed_pec_src_sel_get(struct dc_ed *ed, enum dc_link_id *src);
 void dc_ed_pec_sync_trigger(struct dc_ed *ed);
 void dc_ed_init(struct dc_ed *ed);
 

-- 
2.43.0

[PATCH v8 11/15] drm/imx: dc-lb: Support getting secondary input selection

[Liu Ying]

Add a helper to get LayerBlend secondary input selection.  This is
needed by disabling CRTC at boot in an upcoming commit which tries
to get the selection.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Fix dc_lb_pec_dynamic_sec_sel_get()'s return value when regmap_read() fails.
- Iterate over sec_sels[] to find the secondary input selection.

v7:
- Collect Maxime's A-b tag.
- Check regmap_read()'s return value.

v4:
- Rebase onto the latest drm-misc-next and resolve conflict.

v2:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-lb.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-pe.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/imx/dc/dc-lb.c b/drivers/gpu/drm/imx/dc/dc-lb.c
index cb614f3c2f69..924a0de4b909 100644
--- a/drivers/gpu/drm/imx/dc/dc-lb.c
+++ b/drivers/gpu/drm/imx/dc/dc-lb.c
@@ -19,6 +19,8 @@
 #define  PIXENGCFG_DYNAMIC_SEC_SEL_MASK		GENMASK(13, 8)
 #define  PIXENGCFG_DYNAMIC_SEC_SEL(x)		\
 		FIELD_PREP(PIXENGCFG_DYNAMIC_SEC_SEL_MASK, (x))
+#define  PIXENGCFG_DYNAMIC_SEC_SEL_GET(x)	\
+		FIELD_GET(PIXENGCFG_DYNAMIC_SEC_SEL_MASK, (x))
 #define  PIXENGCFG_DYNAMIC_PRIM_SEL_MASK	GENMASK(5, 0)
 #define  PIXENGCFG_DYNAMIC_PRIM_SEL(x)		\
 		FIELD_PREP(PIXENGCFG_DYNAMIC_PRIM_SEL_MASK, (x))
@@ -181,6 +183,29 @@ void dc_lb_pec_dynamic_sec_sel(struct dc_lb *lb, enum dc_link_id sec)
 	dev_warn(lb->dev, "invalid secondary input selection:%d\n", sec);
 }
 
+int dc_lb_pec_dynamic_sec_sel_get(struct dc_lb *lb, enum dc_link_id *sec)
+{
+	u32 val;
+	int ret;
+	int i;
+
+	ret = regmap_read(lb->reg_pec, PIXENGCFG_DYNAMIC, &val);
+	if (ret)
+		return ret;
+
+	val = PIXENGCFG_DYNAMIC_SEC_SEL_GET(val);
+
+	for (i = 0; i < ARRAY_SIZE(sec_sels); i++) {
+		if (sec_sels[i] == val) {
+			dev_dbg(lb->dev, "get secondary input selection: 0x%02x\n", val);
+			*sec = val;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 void dc_lb_pec_clken(struct dc_lb *lb, enum dc_pec_clken clken)
 {
 	regmap_write_bits(lb->reg_pec, PIXENGCFG_DYNAMIC, CLKEN_MASK,
diff --git a/drivers/gpu/drm/imx/dc/dc-pe.h b/drivers/gpu/drm/imx/dc/dc-pe.h
index 0b81dc6b251c..4e38aead69bc 100644
--- a/drivers/gpu/drm/imx/dc/dc-pe.h
+++ b/drivers/gpu/drm/imx/dc/dc-pe.h
@@ -93,6 +93,7 @@ void dc_ed_init(struct dc_ed *ed);
 enum dc_link_id dc_lb_get_link_id(struct dc_lb *lb);
 void dc_lb_pec_dynamic_prim_sel(struct dc_lb *lb, enum dc_link_id prim);
 void dc_lb_pec_dynamic_sec_sel(struct dc_lb *lb, enum dc_link_id sec);
+int dc_lb_pec_dynamic_sec_sel_get(struct dc_lb *lb, enum dc_link_id *sec);
 void dc_lb_pec_clken(struct dc_lb *lb, enum dc_pec_clken clken);
 void dc_lb_mode(struct dc_lb *lb, enum dc_lb_mode mode);
 void dc_lb_position(struct dc_lb *lb, int x, int y);

-- 
2.43.0

[PATCH v8 12/15] drm/imx: dc-ed: Drop initial source selection

[Liu Ying]

It's unnecessary to set initial ExtDst source selection because KMS
driver would do that when doing atomic commits.  Drop the selection.
This is needed as a preparation for reading ExtDst source selection
when trying to disable CRTC at boot in an upcoming commit.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v7:
- Collect Maxime's A-b tag.

v2:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-ed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-ed.c b/drivers/gpu/drm/imx/dc/dc-ed.c
index a4a21d0affb6..1370b968168b 100644
--- a/drivers/gpu/drm/imx/dc/dc-ed.c
+++ b/drivers/gpu/drm/imx/dc/dc-ed.c
@@ -197,7 +197,6 @@ static inline void dc_ed_disable_gamma_apply(struct dc_ed *ed)
 
 void dc_ed_init(struct dc_ed *ed)
 {
-	dc_ed_pec_src_sel(ed, LINK_ID_NONE);
 	dc_ed_pec_enable_shden(ed);
 	dc_ed_pec_poweron(ed);
 	dc_ed_pec_sync_mode_single(ed);

-- 
2.43.0

[PATCH v8 13/15] drm/imx: dc-lb: Drop initial primary and secondary input selections

[Liu Ying]

It's unnecessary to set initial primary and secondary input selections
because KMS driver would do that when doing atomic commits.  Drop the
selections.  This is needed as a preparation for reading LayerBlend
secondary input selection when trying to disable CRTC at boot in an
upcoming commit.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v7:
- Collect Maxime's A-b tag.

v2:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-lb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-lb.c b/drivers/gpu/drm/imx/dc/dc-lb.c
index 924a0de4b909..36c6b37002cf 100644
--- a/drivers/gpu/drm/imx/dc/dc-lb.c
+++ b/drivers/gpu/drm/imx/dc/dc-lb.c
@@ -257,8 +257,6 @@ int dc_lb_get_id(struct dc_lb *lb)
 
 void dc_lb_init(struct dc_lb *lb)
 {
-	dc_lb_pec_dynamic_prim_sel(lb, LINK_ID_NONE);
-	dc_lb_pec_dynamic_sec_sel(lb, LINK_ID_NONE);
 	dc_lb_pec_clken(lb, CLKEN_DISABLE);
 	dc_lb_shdldsel(lb, BOTH);
 	dc_lb_shdtoksel(lb, BOTH);

-- 
2.43.0

[PATCH v8 14/15] drm/imx: dc-fu: Get DPR channel

[Liu Ying]

Add a helper dc_fu_get_dprc() to get DPR channel for FetchUnit.
And, call it from dc_{fl,fw)_bind() to get DPR channels for
FetchLayer and FetchWarp.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v7:
- Collect Maxime's A-b tag.
- Call dc_dprc_get() in dc_fu_get_dprc() to create a device link between
  consumer FetchUnit and supplier DPRC to avoid potential UAF when
  accessing fu->dprc.  (Sashiko AI bot)

v2:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-fl.c |  7 ++++++-
 drivers/gpu/drm/imx/dc/dc-fu.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-fu.h |  5 +++++
 drivers/gpu/drm/imx/dc/dc-fw.c |  7 ++++++-
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-fl.c b/drivers/gpu/drm/imx/dc/dc-fl.c
index 9f03df44a63a..f7b8c4909c80 100644
--- a/drivers/gpu/drm/imx/dc/dc-fl.c
+++ b/drivers/gpu/drm/imx/dc/dc-fl.c
@@ -100,13 +100,18 @@ static int dc_fl_bind(struct device *dev, struct device *master, void *data)
 	void __iomem *base_cfg;
 	struct dc_fl *fl;
 	struct dc_fu *fu;
-	int i, id;
+	int ret, i, id;
 
 	fl = devm_kzalloc(dev, sizeof(*fl), GFP_KERNEL);
 	if (!fl)
 		return -ENOMEM;
 
 	fu = &fl->fu;
+	fu->dev = dev;
+
+	ret = dc_fu_get_dprc(fu);
+	if (ret)
+		return ret;
 
 	res_pec = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
diff --git a/drivers/gpu/drm/imx/dc/dc-fu.c b/drivers/gpu/drm/imx/dc/dc-fu.c
index 2e197b3344bd..d9933218069c 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.c
+++ b/drivers/gpu/drm/imx/dc/dc-fu.c
@@ -8,8 +8,10 @@
 #include <linux/bits.h>
 #include <linux/math.h>
 
+#include "dc-dprc.h"
 #include "dc-fu.h"
 #include "dc-pe.h"
+#include "dc-prg.h"
 
 /* STATICCONTROL */
 #define SHDLDREQSTICKY_MASK		GENMASK(31, 24)
@@ -256,3 +258,26 @@ void dc_fu_common_hw_init(struct dc_fu *fu)
 		dc_fu_set_pixel_blend_mode(fu, i);
 	}
 }
+
+int dc_fu_get_dprc(struct dc_fu *fu)
+{
+	struct dc_dprc *dprc;
+	struct dc_prg *prg;
+
+	prg = dc_prg_lookup_by_phandle(fu->dev, "fsl,prg", 0);
+	if (!prg)
+		return dev_err_probe(fu->dev, -EPROBE_DEFER,
+				     "failed to lookup PRG\n");
+
+	dprc = dc_prg_get_dprc(prg);
+	if (!dprc)
+		return dev_err_probe(fu->dev, -EPROBE_DEFER,
+				     "failed to get DPR channel from PRG\n");
+
+	fu->dprc = dc_dprc_get(fu->dev, dprc);
+	if (!fu->dprc)
+		return dev_err_probe(fu->dev, -EPROBE_DEFER,
+				     "failed to get DPR channel\n");
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/imx/dc/dc-fu.h b/drivers/gpu/drm/imx/dc/dc-fu.h
index cc93317e3001..9fbd7aadd264 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.h
+++ b/drivers/gpu/drm/imx/dc/dc-fu.h
@@ -8,11 +8,13 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/device.h>
 #include <linux/regmap.h>
 #include <linux/types.h>
 
 #include <drm/drm_fourcc.h>
 
+#include "dc-dprc.h"
 #include "dc-pe.h"
 
 #define FRAC_OFFSET			0x28
@@ -99,6 +101,7 @@ struct dc_fu_ops {
 };
 
 struct dc_fu {
+	struct device *dev;
 	struct regmap *reg_pec;
 	struct regmap *reg_cfg;
 	char name[21];
@@ -114,6 +117,7 @@ struct dc_fu {
 	enum dc_link_id link_id;
 	struct dc_fu_ops ops;
 	struct dc_lb *lb;
+	struct dc_dprc *dprc;
 };
 
 extern const struct dc_fu_ops dc_fu_common_ops;
@@ -123,6 +127,7 @@ void dc_fu_get_pixel_format_shifts(struct dc_fu *fu, u32 format, u32 *shifts);
 void dc_fu_shdldreq_sticky(struct dc_fu *fu, u8 layer_mask);
 void dc_fu_set_src_bpp(struct dc_fu *fu, enum dc_fu_frac frac, unsigned int bpp);
 void dc_fu_common_hw_init(struct dc_fu *fu);
+int dc_fu_get_dprc(struct dc_fu *fu);
 
 const struct dc_fu_ops *dc_fu_get_ops(struct dc_fu *fu);
 
diff --git a/drivers/gpu/drm/imx/dc/dc-fw.c b/drivers/gpu/drm/imx/dc/dc-fw.c
index 14512c01ea78..030970d4f987 100644
--- a/drivers/gpu/drm/imx/dc/dc-fw.c
+++ b/drivers/gpu/drm/imx/dc/dc-fw.c
@@ -130,13 +130,18 @@ static int dc_fw_bind(struct device *dev, struct device *master, void *data)
 	void __iomem *base_cfg;
 	struct dc_fw *fw;
 	struct dc_fu *fu;
-	int i, id;
+	int ret, i, id;
 
 	fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
 	if (!fw)
 		return -ENOMEM;
 
 	fu = &fw->fu;
+	fu->dev = dev;
+
+	ret = dc_fu_get_dprc(fu);
+	if (ret)
+		return ret;
 
 	base_pec = devm_platform_get_and_ioremap_resource(pdev, 0, &res_pec);
 	if (IS_ERR(base_pec))

-- 
2.43.0

[PATCH v8 15/15] drm/imx: dc: Use prefetch engine

[Liu Ying]

One prefetch engine consists of one DPR channel and one or two PRGs.
Each PRG handles one planar in a pixel format.  Every FetchUnit used
by KMS may attach to a PRG and hence use a prefetch engine.  So, to
simplify driver code, always use prefetch engines for FetchUnits in
KMS driver and avoid supporting bypassing them.  Aside from configuring
and disabling a prefetch engine along with a FetchUnit for atomic
commits, adapt burst size/stride fixup requirements from PRG in
FetchUnit driver and set use_raw_spinlock flag in ExtDst PIXENGCFG
regmap configuration to avoid sleeping on PREEMPT_RT because
dc_ed_pec_sync_trigger() can be called with local irq and preempt
disabled.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v8:
- Inspect lb_sec read by dc_lb_pec_dynamic_sec_sel_get() in
  dc_crtc_disable_at_boot().  (Sashiko AI bot)
- Inspect ed_src read by dc_ed_pec_src_sel_get() in
  dc_crtc_disable_at_boot() and check it against LINK_ID_NONE.
  (Sashiko AI bot)
- Avoid disabling DPRC in dc_crtc_disable_at_boot() if CRTC was enabled
  by bootloader.  (Sashiko AI bot)
- Enable/disable DPRC properly for cases where plane transitions to
  visible/invisible status.  (Sashiko AI bot)

v7:
- Check {old,new}_plane_state against NULL in dc_crtc_atomic_flush().
  (Sashiko AI bot)
- Return 128B burst size from dc_fu_burst_size_fixup() when baddr == 0.
  This avoids undefined behavior of __ffs(0).  (Sashiko AI bot)
- Set use_raw_spinlock flag in ExtDst PIXENGCFG regmap configuration to
  avoid sleeping on PREEMPT_RT because dc_ed_pec_sync_trigger() is can be
  called with local irq and preempt disabled.
- Modify debug message in dc_crtc_disable_at_boot().
- Adapt dc_drv_crtc_was_enabled() in dc_crtc_disable_at_boot().
- Rename __dc_crtc_disable_fg_along_with_dprc_repeat_en() to
  dc_crtc_disable_fg_along_with_dprc_repeat_en().
- Update commit message.

v6:
- Rebase onto the latest drm-misc-next and resolve conflicts.

v3:
- Collect Frank's R-b tag.
---
 drivers/gpu/drm/imx/dc/dc-crtc.c  | 164 +++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/imx/dc/dc-ed.c    |   5 ++
 drivers/gpu/drm/imx/dc/dc-fu.c    |  30 ++++++-
 drivers/gpu/drm/imx/dc/dc-fu.h    |   2 +-
 drivers/gpu/drm/imx/dc/dc-kms.h   |   5 ++
 drivers/gpu/drm/imx/dc/dc-plane.c |  48 +++++++++--
 6 files changed, 231 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
index d03246036d4a..9d8fbcef79fe 100644
--- a/drivers/gpu/drm/imx/dc/dc-crtc.c
+++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
@@ -25,6 +25,7 @@
 #include <drm/drm_vblank.h>
 
 #include "dc-de.h"
+#include "dc-dprc.h"
 #include "dc-drv.h"
 #include "dc-kms.h"
 #include "dc-pe.h"
@@ -204,7 +205,14 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 				drm_atomic_get_old_crtc_state(state, crtc);
 	struct drm_crtc_state *new_crtc_state =
 				drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_plane_state *old_plane_state =
+			drm_atomic_get_old_plane_state(state, crtc->primary);
+	struct drm_plane_state *new_plane_state =
+			drm_atomic_get_new_plane_state(state, crtc->primary);
+	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
+	bool plane_becoming_invisible = false;
+	bool disabling_visible_plane = false;
 	int idx;
 
 	if (drm_atomic_crtc_needs_modeset(new_crtc_state) ||
@@ -216,13 +224,48 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 
 	enable_irq(dc_crtc->irq_ed_cont_shdload);
 
-	/* flush plane update out to display */
-	dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
+	if (old_plane_state && new_plane_state) {
+		disabling_visible_plane = old_plane_state->visible &&
+					  drm_atomic_plane_disabling(old_plane_state,
+								     new_plane_state);
+
+		plane_becoming_invisible = old_plane_state->fb &&
+					   new_plane_state->fb &&
+					   old_plane_state->visible &&
+					   !new_plane_state->visible;
+	}
+
+	if (disabling_visible_plane || plane_becoming_invisible) {
+		unsigned long flags;
+
+		dc_crtc_dbg(crtc, "disabling FetchUnit along with DPRC\n");
+
+		/*
+		 * Don't relinquish CPU until DPRC REPEAT_EN is disabled and
+		 * sync is triggered.
+		 */
+		local_irq_save(flags);
+		preempt_disable();
+
+		DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
+		dc_dprc_disable_repeat_en(dc_plane->fu->dprc);
+		/* flush plane update out to display */
+		dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
+
+		local_irq_restore(flags);
+		preempt_enable();
+	} else {
+		/* flush plane update out to display */
+		dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
+	}
 
 	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(ed_cont_shdload_done);
 
 	disable_irq(dc_crtc->irq_ed_cont_shdload);
 
+	if (disabling_visible_plane || plane_becoming_invisible)
+		dc_dprc_disable(dc_plane->fu->dprc);
+
 	DC_CRTC_CHECK_FRAMEGEN_FIFO(dc_crtc->fg);
 
 	drm_dev_exit(idx);
@@ -323,14 +366,33 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 	dc_crtc_queue_state_event(new_crtc_state);
 }
 
-static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
+static inline void
+dc_crtc_disable_fg_along_with_dprc_repeat_en(struct drm_crtc *crtc)
 {
+	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
+	unsigned long flags;
 
-	enable_irq(dc_crtc->irq_dec_seqcomplete);
+	/* Don't relinquish CPU until DPRC REPEAT_EN is disabled. */
+	local_irq_save(flags);
+	preempt_disable();
+
+	/*
+	 * Sync to FrameGen frame index moving so that
+	 * FrameGen can be disabled in the next frame.
+	 */
+	DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
 	dc_fg_disable(dc_crtc->fg);
-	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
-	disable_irq(dc_crtc->irq_dec_seqcomplete);
+	/*
+	 * There is one frame leftover after FrameGen disablement.
+	 * Sync to FrameGen frame index moving so that DPRC REPEAT_EN
+	 * can be disabled in the next frame.
+	 */
+	DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
+	dc_dprc_disable_repeat_en(dc_plane->fu->dprc);
+
+	local_irq_restore(flags);
+	preempt_enable();
 }
 
 static void
@@ -338,14 +400,31 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 {
 	struct drm_crtc_state *new_crtc_state =
 				drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_crtc_state *old_crtc_state =
+				drm_atomic_get_old_crtc_state(state, crtc);
+	struct drm_plane_state *old_plane_state =
+			drm_atomic_get_old_plane_state(state, crtc->primary);
 	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
+	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
 	int idx;
 
 	if (!drm_dev_enter(crtc->dev, &idx))
 		goto out;
 
-	__dc_crtc_disable_fg(crtc);
+	enable_irq(dc_crtc->irq_dec_seqcomplete);
+
+	if (old_crtc_state->plane_mask && old_plane_state->visible)
+		dc_crtc_disable_fg_along_with_dprc_repeat_en(crtc);
+	else
+		dc_fg_disable(dc_crtc->fg);
+
+	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
+	disable_irq(dc_crtc->irq_dec_seqcomplete);
+
+	if (old_crtc_state->plane_mask)
+		dc_dprc_disable(dc_plane->fu->dprc);
+
 	dc_fg_disable_clock(dc_crtc->fg);
 
 	/* request pixel engine power-off as plane is off too */
@@ -371,7 +450,11 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
 int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
 {
 	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
+	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
 	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
+	enum dc_link_id ed_src, lb_sec;
+	bool disable_dprc = false;
+	bool crtc_is_enabled;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
@@ -381,16 +464,71 @@ int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
 		goto out1;
 	}
 
+	crtc_is_enabled = dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc));
+
+	ret = pm_runtime_resume_and_get(dc_drm->pe->dev);
+	if (ret) {
+		dc_crtc_err(crtc, "failed to get DC pixel engine RPM: %d\n",
+			    ret);
+		goto out2;
+	}
+
 	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
 		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
-		goto out2;
+		goto out3;
 	}
 
-	dc_crtc_dbg(crtc, "disabling FrameGen at boot\n");
-	__dc_crtc_disable_fg(crtc);
+	ret = dc_ed_pec_src_sel_get(dc_crtc->ed_cont, &ed_src);
+	if (ret || ed_src == LINK_ID_NONE) {
+		dc_crtc_err(crtc, "failed to get content ExtDst's source\n");
+		if (ret == 0)
+			ret = -EINVAL;
+		goto out3;
+	}
+
+	if (ed_src == LINK_ID_CONSTFRAME0 || ed_src == LINK_ID_CONSTFRAME1)
+		goto disable;
+
+	ret = dc_lb_pec_dynamic_sec_sel_get(dc_plane->lb, &lb_sec);
+	if (ret || lb_sec == LINK_ID_NONE) {
+		dc_crtc_err(crtc,
+			    "failed to get primary plane LayerBlend secondary source\n");
+		if (ret == 0)
+			ret = -EINVAL;
+		goto out3;
+	}
+
+	/*
+	 * If CRTC was enabled by bootloader instead of this driver, we don't
+	 * know if DPRC was enabled or not.  In this case, enforcing DPRC
+	 * disablement potentially causes imbalanced clock enable count for
+	 * DPRC and PRG clocks.  So, we presume bootloader doesn't bother to
+	 * use prefetch engines.
+	 */
+	if (crtc_is_enabled)
+		disable_dprc = true;
+
+disable:
+	enable_irq(dc_crtc->irq_dec_seqcomplete);
 
-	if (!dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc)))
-		goto out2;
+	if (disable_dprc) {
+		dc_crtc_dbg(crtc, "disabling FrameGen along with DPRC REPEAT_EN at boot\n");
+		dc_crtc_disable_fg_along_with_dprc_repeat_en(crtc);
+	} else {
+		dc_crtc_dbg(crtc, "disabling FrameGen at boot\n");
+		dc_fg_disable(dc_crtc->fg);
+	}
+
+	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
+	disable_irq(dc_crtc->irq_dec_seqcomplete);
+
+	if (!crtc_is_enabled)
+		goto out3;
+
+	if (disable_dprc) {
+		dc_crtc_dbg(crtc, "disabling DPRC at boot\n");
+		dc_dprc_disable_at_boot(dc_plane->fu->dprc);
+	}
 
 	dc_crtc_dbg(crtc, "disabling clocks at boot\n");
 
@@ -400,6 +538,8 @@ int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
 
 	dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
 
+out3:
+	pm_runtime_put(dc_drm->pe->dev);
 out2:
 	pm_runtime_put(dc_crtc->de->dev);
 out1:
diff --git a/drivers/gpu/drm/imx/dc/dc-ed.c b/drivers/gpu/drm/imx/dc/dc-ed.c
index 1370b968168b..271d0745b21f 100644
--- a/drivers/gpu/drm/imx/dc/dc-ed.c
+++ b/drivers/gpu/drm/imx/dc/dc-ed.c
@@ -77,6 +77,11 @@ static const struct regmap_config dc_ed_pec_regmap_config = {
 	.reg_stride = 4,
 	.val_bits = 32,
 	.fast_io = true,
+	/*
+	 * Registers could be accessed with local irq and preempt disabled,
+	 * so use a raw spinlock to avoid sleeping on PREEMPT_RT.
+	 */
+	.use_raw_spinlock = true,
 	.wr_table = &dc_ed_pec_regmap_write_table,
 	.rd_table = &dc_ed_pec_regmap_read_table,
 	.volatile_table = &dc_ed_pec_regmap_volatile_table,
diff --git a/drivers/gpu/drm/imx/dc/dc-fu.c b/drivers/gpu/drm/imx/dc/dc-fu.c
index d9933218069c..fd98d02e4d6b 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.c
+++ b/drivers/gpu/drm/imx/dc/dc-fu.c
@@ -125,13 +125,31 @@ static inline void dc_fu_set_numbuffers(struct dc_fu *fu, unsigned int num)
 			  SETNUMBUFFERS_MASK, SETNUMBUFFERS(num));
 }
 
-static void dc_fu_set_burstlength(struct dc_fu *fu, dma_addr_t baddr)
+static unsigned int dc_fu_burst_size_fixup(dma_addr_t baddr)
 {
-	unsigned int burst_size, burst_length;
+	unsigned int burst_size;
+
+	if (baddr == 0)
+		return 128U;
 
 	burst_size = 1 << __ffs(baddr);
 	burst_size = round_up(burst_size, 8);
 	burst_size = min(burst_size, 128U);
+
+	return burst_size;
+}
+
+static unsigned int
+dc_fu_stride_fixup(unsigned int stride, unsigned int burst_size)
+{
+	return round_up(stride, burst_size);
+}
+
+static void dc_fu_set_burstlength(struct dc_fu *fu, dma_addr_t baddr)
+{
+	unsigned int burst_size, burst_length;
+
+	burst_size = dc_fu_burst_size_fixup(baddr);
 	burst_length = burst_size / 8;
 
 	regmap_write_bits(fu->reg_cfg, BURSTBUFFERMANAGEMENT,
@@ -151,8 +169,14 @@ void dc_fu_set_src_bpp(struct dc_fu *fu, enum dc_fu_frac frac, unsigned int bpp)
 }
 
 static void dc_fu_set_src_stride(struct dc_fu *fu, enum dc_fu_frac frac,
-				 unsigned int stride)
+				 unsigned int width, int bpp, dma_addr_t baddr)
 {
+	unsigned int burst_size = dc_fu_burst_size_fixup(baddr);
+	unsigned int stride;
+
+	stride = width * (bpp / 8);
+	stride = dc_fu_stride_fixup(stride, burst_size);
+
 	regmap_write_bits(fu->reg_cfg, fu->reg_sourcebufferattributes[frac],
 			  STRIDE_MASK, STRIDE(stride));
 }
diff --git a/drivers/gpu/drm/imx/dc/dc-fu.h b/drivers/gpu/drm/imx/dc/dc-fu.h
index 9fbd7aadd264..0527a7b2221d 100644
--- a/drivers/gpu/drm/imx/dc/dc-fu.h
+++ b/drivers/gpu/drm/imx/dc/dc-fu.h
@@ -87,7 +87,7 @@ struct dc_fu_ops {
 	void (*set_baseaddress)(struct dc_fu *fu, enum dc_fu_frac frac,
 				dma_addr_t baddr);
 	void (*set_src_stride)(struct dc_fu *fu, enum dc_fu_frac frac,
-			       unsigned int stride);
+			       unsigned int width, int bpp, dma_addr_t baddr);
 	void (*set_src_buf_dimensions)(struct dc_fu *fu, enum dc_fu_frac frac,
 				       int w, int h);
 	void (*set_fmt)(struct dc_fu *fu, enum dc_fu_frac frac,
diff --git a/drivers/gpu/drm/imx/dc/dc-kms.h b/drivers/gpu/drm/imx/dc/dc-kms.h
index a25d47eebd28..8b45b21a6f8a 100644
--- a/drivers/gpu/drm/imx/dc/dc-kms.h
+++ b/drivers/gpu/drm/imx/dc/dc-kms.h
@@ -130,4 +130,9 @@ struct dc_plane {
 	struct dc_ed *ed;
 };
 
+static inline struct dc_plane *to_dc_plane(struct drm_plane *plane)
+{
+	return container_of(plane, struct dc_plane, base);
+}
+
 #endif /* __DC_KMS_H__ */
diff --git a/drivers/gpu/drm/imx/dc/dc-plane.c b/drivers/gpu/drm/imx/dc/dc-plane.c
index 42c0a46281cb..2b58c4e2b7e4 100644
--- a/drivers/gpu/drm/imx/dc/dc-plane.c
+++ b/drivers/gpu/drm/imx/dc/dc-plane.c
@@ -17,6 +17,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_print.h>
 
+#include "dc-dprc.h"
 #include "dc-drv.h"
 #include "dc-fu.h"
 #include "dc-kms.h"
@@ -44,11 +45,6 @@ static const struct drm_plane_funcs dc_plane_funcs = {
 	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
 };
 
-static inline struct dc_plane *to_dc_plane(struct drm_plane *plane)
-{
-	return container_of(plane, struct dc_plane, base);
-}
-
 static int dc_plane_check_max_source_resolution(struct drm_plane_state *state)
 {
 	int src_h = drm_rect_height(&state->src) >> 16;
@@ -88,6 +84,28 @@ static int dc_plane_check_fb(struct drm_plane_state *state)
 	return 0;
 }
 
+static int dc_plane_check_dprc(struct drm_plane_state *state)
+{
+	struct dc_plane *dplane = to_dc_plane(state->plane);
+	struct drm_framebuffer *fb = state->fb;
+	dma_addr_t baseaddr = drm_fb_dma_get_gem_addr(fb, state, 0);
+	struct dc_dprc *dprc = dplane->fu->dprc;
+	u32 src_w = drm_rect_width(&state->src) >> 16;
+
+	if (!dc_dprc_rtram_width_supported(dprc, src_w)) {
+		dc_plane_dbg(state->plane, "bad RTRAM width for DPRC\n");
+		return -EINVAL;
+	}
+
+	if (!dc_dprc_stride_supported(dprc, fb->pitches[0], src_w, fb->format,
+				      baseaddr)) {
+		dc_plane_dbg(state->plane, "fb bad pitches[0] for DPRC\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 dc_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_commit *state)
 {
@@ -123,7 +141,11 @@ dc_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_commit *state)
 	if (ret)
 		return ret;
 
-	return dc_plane_check_fb(plane_state);
+	ret = dc_plane_check_fb(plane_state);
+	if (ret)
+		return ret;
+
+	return dc_plane_check_dprc(plane_state);
 }
 
 /* A prerequisite is that drm_dev_enter() has been called beforehand. */
@@ -158,12 +180,18 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 {
 	struct drm_plane_state *new_state =
 				drm_atomic_get_new_plane_state(state, plane);
+	struct drm_plane_state *old_state =
+				drm_atomic_get_old_plane_state(state, plane);
+	struct drm_crtc_state *new_crtc_state =
+				drm_atomic_get_new_crtc_state(state, new_state->crtc);
+	bool needs_modeset = drm_atomic_crtc_needs_modeset(new_crtc_state);
 	struct dc_plane *dplane = to_dc_plane(plane);
 	struct drm_framebuffer *fb = new_state->fb;
 	const struct dc_fu_ops *fu_ops;
 	struct dc_lb *lb = dplane->lb;
 	struct dc_fu *fu = dplane->fu;
 	dma_addr_t baseaddr;
+	bool prefetch_start;
 	int src_w, src_h;
 	int idx;
 
@@ -184,7 +212,8 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 
 	fu_ops->set_layerblend(fu, lb);
 	fu_ops->set_burstlength(fu, baseaddr);
-	fu_ops->set_src_stride(fu, DC_FETCHUNIT_FRAC0, fb->pitches[0]);
+	fu_ops->set_src_stride(fu, DC_FETCHUNIT_FRAC0, src_w,
+			       fb->format->cpp[0] * 8, baseaddr);
 	fu_ops->set_src_buf_dimensions(fu, DC_FETCHUNIT_FRAC0, src_w, src_h);
 	fu_ops->set_fmt(fu, DC_FETCHUNIT_FRAC0, fb->format);
 	fu_ops->set_framedimensions(fu, src_w, src_h);
@@ -193,6 +222,11 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
 
 	dc_plane_dbg(plane, "uses %s\n", fu_ops->get_name(fu));
 
+	prefetch_start = needs_modeset || !old_state->fb ||
+			 (!old_state->visible && new_state->visible);
+	dc_dprc_configure(fu->dprc, new_state->crtc->index, src_w, src_h,
+			  fb->pitches[0], fb->format, baseaddr, prefetch_start);
+
 	dc_lb_pec_dynamic_prim_sel(lb, dc_cf_get_link_id(dplane->cf));
 	dc_lb_pec_dynamic_sec_sel(lb, fu_ops->get_link_id(fu));
 	dc_lb_mode(lb, LB_BLEND);

-- 
2.43.0

Re: [PATCH v8 07/15] drm/imx: dc: Add PRG support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imx: dc-prg: unclocked MMIO and RPM underflow on dc_prg_configure() error
--

commit dc9803b183fdde7606d3c14739f864581b99c2a3
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc: Add PRG support

This commit adds a platform driver to support the Display Prefetch Resolve
Gasket (PRG), which sits between a Display Prefetch Resolve Channel (DPRC)
and a FetchUnit.

> diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c
> new file mode 100644
> index 0000000000000..46108f7e09af3
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-prg.c
[ ... ]
> +void dc_prg_enable(struct dc_prg *prg)
> +{
> +	regmap_write(prg->reg, PRG_CTRL + CLR, BYPASS);
> +}
> +
> +void dc_prg_disable(struct dc_prg *prg)
> +{
> +	regmap_write(prg->reg, PRG_CTRL, BYPASS);
> +
> +	pm_runtime_put(prg->dev);
> +}
[ ... ]
> +void dc_prg_configure(struct dc_prg *prg,
> +		      unsigned int width, unsigned int height,
> +		      unsigned int stride, unsigned int bits_per_pixel,
> +		      dma_addr_t baddr, bool start)
> +{
> +	struct device *dev = prg->dev;
> +	unsigned int burst_size;
> +	u32 val;
> +	int ret;
> +
> +	if (start) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to get RPM: %d\n", ret);
> +			return;
> +		}
> +	}

[Severity: High]
As reported by Sashiko on v7 [1], what happens if pm_runtime_resume_and_get()
fails in dc_prg_configure() in drivers/gpu/drm/imx/dc/dc-prg.c when start is
true?

Because dc_prg_configure() returns void, the caller is not notified that the
PRG device remains runtime-suspended with its clocks disabled. Subsequent
calls to dc_prg_enable(), dc_prg_reg_update(), dc_prg_shadow_enable(), or
later dc_prg_configure() updates with start set to false will perform MMIO
writes to PRG registers while the hardware is unclocked and unpowered, which
can trigger a bus abort on i.MX8 SoCs.

Additionally, when the plane or CRTC is later disabled, dc_prg_disable()
writes to PRG_CTRL while unclocked and unconditionally calls
pm_runtime_put(prg->dev), underflowing the runtime PM usage counter on
prg->dev.

Should dc_prg_configure() return an error code or track the active state so
callers do not access unclocked registers or drop an unacquired runtime PM
reference?

[1] https://lore.kernel.org/r/20261026-imx8-dc-prefetch-v7-7-a0b50cc4fc65@nxp.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-imx8-dc-prefetch-v8-0-c4f81afd4533@nxp.com?part=7

Re: [PATCH v8 05/15] drm/imx: dc: Flag CRTC status in GERNERALPURPOSE register

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] drm/imx: dc: missing max_register_is_0 in dc_gp_regmap_config
- [Low] drm/imx: dc: commit message uses stale helper names and register typo
--

commit 376ae6ee1d9ffb7f4790013466f30007db36afe6
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc: Flag CRTC status in GERNERALPURPOSE register

This commit maps the Display Controller's GENERALPURPOSE register via an MMIO
regmap and adds helpers to set, clear, and query the CRTC enablement status in
its 2 LSB bits.

>     drm/imx: dc: Flag CRTC status in GERNERALPURPOSE register
>     
>     Map Display Controller's register GERNERALPURPOSE at offset 0x100 and
>     create an MMIO regmap to access the register.  Create helpers
>     dc_drv_flag_crtc_{enable,disable}() with the 2 LSB bits in the register
>     to flag the CRTC enablement/disablement status.  Also, create a helper
>     dc_drv_crtc_was_enabled() so that the CRTC driver can use it to check
>     if the CRTC was enabled by a previous boot or not.  This is a preparation
>     for an upcoming patch to disable display controller at boot if the CRTC
>     was enabled by the driver during a previous boot instead of a bootloader.
>     
>     Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
>     Signed-off-by: Liu Ying <victor.liu@nxp.com>

[Severity: Low]
This isn't a bug in the code, but should the commit message be updated to
match the current symbol names?

In drivers/gpu/drm/imx/dc/dc-drv.c and drivers/gpu/drm/imx/dc/dc-drv.h, the
helpers are named dc_drv_set_crtc_enable(), dc_drv_set_crtc_disable(), and
dc_drv_crtc_is_enabled(), rather than dc_drv_flag_crtc_{enable,disable}() and
dc_drv_crtc_was_enabled().

Also, the commit subject and body spell the register name as GERNERALPURPOSE,
whereas the macro in drivers/gpu/drm/imx/dc/dc-drv.c is defined as
GENERALPURPOSE.

> diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
> index 13795a2ad7350..9629beaed43c1 100644
> --- a/drivers/gpu/drm/imx/dc/dc-drv.c
> +++ b/drivers/gpu/drm/imx/dc/dc-drv.c
> @@ -13,6 +13,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>  
>  #include <drm/clients/drm_client_setup.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -28,9 +29,21 @@
>  #include "dc-drv.h"
>  #include "dc-pe.h"
>  
> +/* General Purpose register offset used to flag driver usage. */
> +#define GENERALPURPOSE		0x100
> +#define  CRTC_ENABLED(x)	BIT(x)
> +
>  struct dc_priv {
>  	struct drm_device *drm;
>  	struct clk *clk_cfg;
> +	struct regmap *gp_regmap;
> +};
> +
> +static const struct regmap_config dc_gp_regmap_config = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.max_register = 0,
> +};

[Severity: Low]
Should dc_gp_regmap_config also set .max_register_is_0 = true here?

In dc_probe(), only a 4-byte MMIO region at r->start + GENERALPURPOSE is
mapped and passed to devm_regmap_init_mmio() for priv->gp_regmap, so offset 0
is the only valid register.

In __regmap_init(), map->max_register_is_set is initialized as:

    map->max_register_is_set = map->max_register ?: config->max_register_is_0;

Without .max_register_is_0 = true, regmap treats a zero .max_register as
unspecified and leaves map->max_register_is_set false, so regmap_writeable()
and regmap_readable() do not enforce the offset 0 bound.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-imx8-dc-prefetch-v8-0-c4f81afd4533@nxp.com?part=5

Re: [PATCH v8 06/15] drm/imx: dc-crtc: Disable at boot

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/imx: dc-crtc: premature clk_axi disable in dc_crtc_disable_at_boot()
--

commit e47c892a97b7a909d6fe7c57ba9a8454d386aea8
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc-crtc: Disable at boot

This commit adds dc_crtc_disable_at_boot() and calls it for each CRTC in
dc_drm_bind() prior to drm_dev_register() to ensure CRTCs and their
associated clocks left running across an unplug are disabled at boot.

> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index 0d64186a99018..5bc952d6883e1 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c

[ ... ]

> @@ -331,6 +341,48 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  	spin_unlock_irq(&crtc->dev->event_lock);
>  }
>  
> +int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
> +{
> +	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
> +	if (ret < 0) {
> +		dc_crtc_err(crtc, "failed to get DC display engine RPM: %d\n",
> +			    ret);
> +		goto out1;
> +	}
> +
> +	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
> +		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
> +		goto out2;
> +	}
> +
> +	dc_crtc_dbg(crtc, "disabling FrameGen at boot\n");
> +	__dc_crtc_disable_fg(crtc);
> +
> +	if (!dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc)))
> +		goto out2;
> +
> +	dc_crtc_dbg(crtc, "disabling clocks at boot\n");
> +
> +	dc_fg_disable_clock(dc_crtc->fg);
> +
> +	dc_drm->pe_clk_axi_needs_disable = true;
> +
> +	dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
> +
> +out2:
> +	pm_runtime_put(dc_crtc->de->dev);
> +out1:
> +	/* Let CRTC(with index == 1) disablement path disable AXI clock. */
> +	if (dc_drm->pe_clk_axi_needs_disable && drm_crtc_index(crtc) == 1)
> +		clk_disable_unprepare(dc_drm->pe->clk_axi);
> +
> +	return ret;
> +}

[Severity: Medium]
In dc_crtc_disable_at_boot() in drivers/gpu/drm/imx/dc/dc-crtc.c, can jumping
to out1 when pm_runtime_resume_and_get() fails on CRTC1 prematurely disable
dc_drm->pe->clk_axi?

When dc_drm_bind() iterates over CRTCs with drm_for_each_crtc():

1. If CRTC0 was left enabled before unplug, dc_crtc_disable_at_boot() disables
   CRTC0's FrameGen and pixel clock, sets dc_drm->pe_clk_axi_needs_disable to
   true, and clears CRTC_ENABLED(0).

2. Next, dc_crtc_disable_at_boot() is called for CRTC1 (drm_crtc_index(crtc) == 1).
   If pm_runtime_resume_and_get(dc_crtc->de->dev) fails, it jumps to out1
   before stopping CRTC1's FrameGen or clearing CRTC_ENABLED(1).

3. At out1, both dc_drm->pe_clk_axi_needs_disable and drm_crtc_index(crtc) == 1
   are true, so clk_disable_unprepare(dc_drm->pe->clk_axi) is called even
   though CRTC1 failed to disable.

If CRTC1 was also active, this cuts the Pixel Engine AXI clock while CRTC1 is
still scanning out through the Pixel Engine, and leaves CRTC_ENABLED(1) set so
that a subsequent bind attempt would disable dc_drm->pe->clk_axi a second time.

Should the clk_disable_unprepare(dc_drm->pe->clk_axi) call happen before out1
(under out2), or only when ret == 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-imx8-dc-prefetch-v8-0-c4f81afd4533@nxp.com?part=6

Re: [PATCH v8 08/15] drm/imx: dc: Add DPR channel support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/imx: dc-dprc: deadlock and invalid lock type in dc_dprc_disable_repeat_en()
- [High] drm/imx: dc-dprc: runtime PM race and leak in dc_dprc_wrap_irq_handler()
- [Medium] drm/imx: dc-dprc: RPM reference leak and PRG imbalance in dc_dprc_configure()
--

commit 0da98ec378c5f86195b2badbfc2867b3e806cb34
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc: Add DPR channel support

This commit adds a platform driver for the Display Prefetch Resolve
Channel (DPRC), which fetches display data, transforms it to linear
format, and stores it in RTRAM alongside its associated PRG.

> diff --git a/drivers/gpu/drm/imx/dc/dc-dprc.c b/drivers/gpu/drm/imx/dc/dc-dprc.c
> new file mode 100644
> index 0000000000000..29188fa72a327
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-dprc.c
[ ... ]
> +void dc_dprc_configure(struct dc_dprc *dprc, unsigned int stream_id,
> +		       unsigned int width, unsigned int height,
> +		       unsigned int stride,
> +		       const struct drm_format_info *format,
> +		       dma_addr_t baddr, bool start)
> +{
> +	unsigned int prg_stride = width * format->cpp[0];
> +	unsigned int bpp = format->cpp[0] * 8;
> +	struct device *dev = dprc->dev;
> +	unsigned int p1_w, p1_h;
> +	u32 val;
> +	int ret;
> +
> +	if (start) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to get RPM: %d\n", ret);
> +			return;
> +		}
> +
> +		dc_dprc_set_stream_id(dprc, stream_id);
> +	}
> +
> +	p1_w = round_up(width, format->cpp[0] == 2 ? 32 : 16);
> +	p1_h = round_up(height, 4);
> +
> +	regmap_write(dprc->reg, FRAME_CTRL0, PITCH(stride));
> +	regmap_write(dprc->reg, FRAME_1P_CTRL0, BYTE_1K);
> +	regmap_write(dprc->reg, FRAME_1P_PIX_X_CTRL, NUM_X_PIX_WIDE(p1_w));
> +	regmap_write(dprc->reg, FRAME_1P_PIX_Y_CTRL, NUM_Y_PIX_HIGH(p1_h));
> +	regmap_write(dprc->reg, FRAME_1P_BASE_ADDR_CTRL0, baddr);
> +	regmap_write(dprc->reg, FRAME_PIX_X_ULC_CTRL, CROP_ULC_X(0));
> +	regmap_write(dprc->reg, FRAME_PIX_Y_ULC_CTRL, CROP_ULC_Y(0));
> +
> +	regmap_write(dprc->reg, RTRAM_CTRL0, THRES_LOW(3) | THRES_HIGH(7));
> +
> +	val = LINE4 | BUF2;
> +	switch (format->format) {
> +	case DRM_FORMAT_XRGB8888:
> +		/*
> +		 * It turns out pixel components are mapped directly
> +		 * without position change via DPR processing with
> +		 * the following color component configurations.
> +		 * Leave the pixel format to be handled by the
> +		 * display controllers.
> +		 */
> +		val |= A_COMP_SEL(3) | R_COMP_SEL(2) |
> +		       G_COMP_SEL(1) | B_COMP_SEL(0);
> +		val |= PIX_SIZE_32BIT;
> +		break;
> +	default:
> +		dev_err(dev, "unsupported format 0x%08x\n", format->format);
> +		return;
> +	}
> +	regmap_write(dprc->reg, MODE_CTRL0, val);
> +
> +	if (start) {
> +		/* software shadow load for the first frame */
> +		val = SW_SHADOW_LOAD_SEL | SHADOW_LOAD_EN;
> +		regmap_write(dprc->reg, SYSTEM_CTRL0, val);
> +
> +		/* and then, run... */
> +		val |= RUN_EN | REPEAT_EN;
> +		regmap_write(dprc->reg, SYSTEM_CTRL0, val);
> +	}
> +
> +	dc_prg_configure(dprc->prg, width, height, prg_stride, bpp, baddr, start);
> +
> +	if (start)
> +		dc_dprc_enable(dprc);
> +
> +	dc_dprc_reg_update(dprc);

[Severity: Medium]
As reported by Sashiko on v7 [1], can dc_dprc_configure() leak the runtime
PM reference on dprc->dev and leave dprc->prg->dev unbalanced or unclocked
when start is true?

In dc_dprc_configure(), pm_runtime_resume_and_get(dev) is called when start
is true before checking format->format. If format->format hits the default
case, the function returns early without calling pm_runtime_put(dev) and
before calling dc_prg_configure(). That leaks the runtime PM reference on
dprc->dev and leaves dprc->prg->dev runtime-suspended; a subsequent call to
dc_dprc_disable() -> dc_prg_disable() will then write to prg->reg while
unclocked and underflow prg->dev's runtime PM usage count.

Similarly, when dc_dprc_configure() calls dc_prg_configure() with start
set to true:

drivers/gpu/drm/imx/dc/dc-prg.c:dc_prg_configure() {
	if (start) {
		ret = pm_runtime_resume_and_get(dev);
		if (ret < 0) {
			dev_err(dev, "failed to get RPM: %d\n", ret);
			return;
		}
	}
}

Because dc_prg_configure() returns void on pm_runtime_resume_and_get()
failure, dc_dprc_configure() proceeds unconditionally to dc_dprc_enable()
-> dc_prg_enable() and dc_dprc_reg_update() -> dc_prg_reg_update(), writing
to prg->reg while prg->clks are disabled and leaving the runtime PM
references between dprc->dev and prg->dev unbalanced.

> +
> +	if (start)
> +		dc_dprc_enable_ctrl_done_irq(dprc);
> +
> +	dev_dbg(dev, "w: %u, h: %u, s: %u, fmt: 0x%08x\n",
> +		width, height, stride, format->format);
> +}
> +
> +void dc_dprc_disable_repeat_en(struct dc_dprc *dprc)
> +{
> +	guard(spinlock)(&dprc->lock);
> +	regmap_write(dprc->reg, IRQ_MASK + SET, IRQ_DPR_CRTL_DONE);
> +
> +	synchronize_hardirq(dprc->wrap_irq);
> +
> +	regmap_write(dprc->reg, SYSTEM_CTRL0 + CLR, REPEAT_EN);
> +	dev_dbg(dprc->dev, "disable REPEAT_EN\n");
> +}

[Severity: High]
As reported by Sashiko on v7 [1], won't holding dprc->lock across
synchronize_hardirq(dprc->wrap_irq) in dc_dprc_disable_repeat_en()
deadlock if dc_dprc_wrap_irq_handler() is concurrently running?

In dc_dprc_disable_repeat_en(), guard(spinlock)(&dprc->lock) holds
dprc->lock for the entire function scope, including while waiting in
synchronize_hardirq(dprc->wrap_irq). If wrap_irq fires on another CPU (either
from DPRC before IRQ_MASK + SET takes effect, or from another device sharing
the IRQF_SHARED interrupt line) and enters dc_dprc_wrap_irq_handler(),
IRQD_IRQ_INPROGRESS is set before dc_dprc_wrap_irq_handler() attempts to
acquire dprc->lock:

CPU 0                                  CPU 1 (hardirq wrap_irq)
dc_dprc_disable_repeat_en()            handle_irq_event()
  guard(spinlock)(&dprc->lock)           /* sets IRQD_IRQ_INPROGRESS */
  ...                                    dc_dprc_wrap_irq_handler()
  synchronize_hardirq(dprc->wrap_irq)      scoped_guard(spinlock, &dprc->lock)
    /* waits for IRQD_IRQ_INPROGRESS */      /* spins waiting for dprc->lock */

Also, if dc_dprc_disable_repeat_en() is ever called with local IRQs enabled,
guard(spinlock) does not disable local interrupts, so wrap_irq on the same
CPU can preempt and self-deadlock on dprc->lock.

In addition, as noted in the comment on dc_dprc_regmap_config,
dc_dprc_disable_repeat_en() is called with local IRQs and preemption
disabled. However, dprc->lock is declared as spinlock_t rather than
raw_spinlock_t, and dc_dprc_wrap_irq_handler() -> dc_dprc_ctrl_done_handle()
-> dc_prg_shadow_enable() writes to prg->reg (whose dc_prg_regmap_config
does not set use_raw_spinlock = true) from hardirq context. On PREEMPT_RT or
with CONFIG_PROVE_RAW_LOCK_NESTING enabled, acquiring a non-raw spinlock_t
in hardirq context or with local IRQs/preemption disabled triggers an
invalid wait-context splat.

Finally, because devm_request_irq() is called without IRQF_NO_THREAD,
dc_dprc_wrap_irq_handler() is force-threaded under PREEMPT_RT or threadirqs.
In that configuration, synchronize_hardirq() does not wait for the IRQ
thread to complete, so dc_dprc_ctrl_done_handle() can still run concurrently
and write REPEAT_EN back to SYSTEM_CTRL0 and SHADOW_EN to prg->reg after
dc_dprc_disable_repeat_en() and dc_dprc_disable().

> +
> +/*
> + * dc_dprc_disable_repeat_en() must be called beforehand to mask
> + * IRQ_DPR_CRTL_DONE and synchronize wrap_irq.
> + */
> +void dc_dprc_disable(struct dc_dprc *dprc)
> +{
> +	dc_prg_disable(dprc->prg);
> +
> +	pm_runtime_put(dprc->dev);
> +
> +	dev_dbg(dprc->dev, "disable\n");
> +}
[ ... ]
> +static irqreturn_t dc_dprc_wrap_irq_handler(int irq, void *data)
> +{
> +	struct dc_dprc *dprc = data;
> +	struct device *dev = dprc->dev;
> +	u32 mask = 0, status = 0;
> +	irqreturn_t result;
> +
> +	pm_runtime_get_noresume(dev);
> +	if (!pm_runtime_active(dev)) {
> +		result = IRQ_NONE;
> +		goto out;
> +	}
> +
> +	scoped_guard(spinlock, &dprc->lock) {
> +		/* cache valid IRQ status */
> +		regmap_read(dprc->reg, IRQ_MASK, &mask);
> +		regmap_read(dprc->reg, IRQ_MASK_STATUS, &status);
> +		status &= ~mask;
> +
> +		/* mask the IRQ(s) being handled */
> +		regmap_write(dprc->reg, IRQ_MASK + SET, status);
> +
> +		/* clear status register */
> +		regmap_write(dprc->reg, IRQ_MASK_STATUS, status);
> +	}
> +
> +	if (status == 0) {
> +		result = IRQ_NONE;
> +		goto out;
> +	}
> +
> +	if (status & DPR2RTR_FIFO_LOAD_BUF_RDY_UV_ERROR)
> +		dev_err(dev, "DPR to RTRAM FIFO load UV buffer ready error\n");
> +
> +	if (status & DPR2RTR_FIFO_LOAD_BUF_RDY_YRGB_ERROR)
> +		dev_err(dev, "DPR to RTRAM FIFO load YRGB buffer ready error\n");
> +
> +	if (status & DPR2RTR_UV_FIFO_OVFL)
> +		dev_err(dev, "DPR to RTRAM FIFO UV FIFO overflow\n");
> +
> +	if (status & DPR2RTR_YRGB_FIFO_OVFL)
> +		dev_err(dev, "DPR to RTRAM FIFO YRGB FIFO overflow\n");
> +
> +	if (status & IRQ_AXI_READ_ERROR)
> +		dev_err(dev, "AXI read error\n");
> +
> +	if (status & IRQ_DPR_CRTL_DONE)
> +		dc_dprc_ctrl_done_handle(dprc);
> +
> +	result = IRQ_HANDLED;
> +out:
> +	pm_runtime_put_noidle(dev);
> +	return result;
> +}

[Severity: High]
As reported by Sashiko on v7 [1], can dc_dprc_wrap_irq_handler() still race
with runtime suspend or leak the active runtime PM state when wrap_irq is
shared?

Because wrap_irq is requested with IRQF_SHARED, another DPRC instance can
trigger dc_dprc_wrap_irq_handler() at any time:

1. In rpm_suspend(), after rpm_check_suspend_allowed() verifies usage_count
   is 0, it sets runtime_status to RPM_SUSPENDING and drops dev->power.lock
   to invoke dc_dprc_runtime_suspend(), which disables dprc->clks. Because
   pm_runtime_get_noresume(dev) and pm_runtime_active(dev) in
   dc_dprc_wrap_irq_handler() do not synchronize with dev->power.lock or
   in-flight suspend, if dc_dprc_wrap_irq_handler() runs right before
   RPM_SUSPENDING is set, it passes the pm_runtime_active(dev) check and
   reads IRQ_MASK and IRQ_MASK_STATUS from dprc->reg while
   dc_dprc_runtime_suspend() concurrently disables dprc->clks.

2. pm_runtime_active(dev) returns true whenever dev->power.disable_depth > 0,
   even if the device is runtime-suspended with its clocks off (for example,
   during late system suspend after __device_suspend_late() disables runtime
   PM, during devres teardown on unbind, or when CONFIG_PM is disabled). Any
   shared IRQ during those windows will read dprc->reg with clocks disabled.

3. If dc_dprc_wrap_irq_handler() increments usage_count from 1 to 2 via
   pm_runtime_get_noresume(dev) while dc_dprc_disable() concurrently calls
   pm_runtime_put(dprc->dev), dc_dprc_disable() drops usage_count from 2 to
   1 without scheduling idle/suspend. When dc_dprc_wrap_irq_handler() then
   calls pm_runtime_put_noidle(dev), usage_count drops from 1 to 0 without
   triggering rpm_idle() or rpm_suspend(), leaving dprc->dev permanently
   active with its clocks enabled.

[1] https://lore.kernel.org/r/20261026-imx8-dc-prefetch-v7-8-a0b50cc4fc65@nxp.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-imx8-dc-prefetch-v8-0-c4f81afd4533@nxp.com?part=8

Re: [PATCH v8 15/15] drm/imx: dc: Use prefetch engine

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/imx: dc: missing plane visibility check in dc_crtc_atomic_disable()
- [High] drm/imx: dc: dc_plane_atomic_update() starts DPRC on inactive CRTC
- [High] drm/imx: dc: deadlock and atomic sleep in dc_dprc_disable_repeat_en()
- [Medium] drm/imx: dc: premature source checks in dc_crtc_disable_at_boot()
--

commit 4fa9cf8e39f1e67c71f284dcbe5efb2eaf9658a7
Author: Liu Ying <victor.liu@nxp.com>

drm/imx: dc: Use prefetch engine

This commit configures and disables prefetch engines (DPRC and PRG) along
with FetchUnits during atomic commits and boot disablement. It also adds
burst size and stride fixups in the FetchUnit driver and sets
use_raw_spinlock in the ExtDst PIXENGCFG regmap configuration.

> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index d03246036d4a7..9d8fbcef79fe4 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
> @@ -25,6 +25,7 @@
>  #include <drm/drm_vblank.h>
>  
>  #include "dc-de.h"
> +#include "dc-dprc.h"
>  #include "dc-drv.h"
>  #include "dc-kms.h"
>  #include "dc-pe.h"
> @@ -204,7 +205,14 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  				drm_atomic_get_old_crtc_state(state, crtc);
>  	struct drm_crtc_state *new_crtc_state =
>  				drm_atomic_get_new_crtc_state(state, crtc);
> +	struct drm_plane_state *old_plane_state =
> +			drm_atomic_get_old_plane_state(state, crtc->primary);
> +	struct drm_plane_state *new_plane_state =
> +			drm_atomic_get_new_plane_state(state, crtc->primary);
> +	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	bool plane_becoming_invisible = false;
> +	bool disabling_visible_plane = false;
>  	int idx;
>  
>  	if (drm_atomic_crtc_needs_modeset(new_crtc_state) ||
> @@ -216,13 +224,48 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  
>  	enable_irq(dc_crtc->irq_ed_cont_shdload);
>  
> -	/* flush plane update out to display */
> -	dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
> +	if (old_plane_state && new_plane_state) {
> +		disabling_visible_plane = old_plane_state->visible &&
> +					  drm_atomic_plane_disabling(old_plane_state,
> +								     new_plane_state);
> +
> +		plane_becoming_invisible = old_plane_state->fb &&
> +					   new_plane_state->fb &&
> +					   old_plane_state->visible &&
> +					   !new_plane_state->visible;
> +	}
> +
> +	if (disabling_visible_plane || plane_becoming_invisible) {
> +		unsigned long flags;
> +
> +		dc_crtc_dbg(crtc, "disabling FetchUnit along with DPRC\n");
> +
> +		/*
> +		 * Don't relinquish CPU until DPRC REPEAT_EN is disabled and
> +		 * sync is triggered.
> +		 */
> +		local_irq_save(flags);
> +		preempt_disable();
> +
> +		DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
> +		dc_dprc_disable_repeat_en(dc_plane->fu->dprc);
> +		/* flush plane update out to display */
> +		dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
> +
> +		local_irq_restore(flags);
> +		preempt_enable();

[ ... ]

> @@ -323,14 +366,33 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  	dc_crtc_queue_state_event(new_crtc_state);
>  }
>  
> -static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
> +static inline void
> +dc_crtc_disable_fg_along_with_dprc_repeat_en(struct drm_crtc *crtc)
>  {
> +	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	unsigned long flags;
>  
> -	enable_irq(dc_crtc->irq_dec_seqcomplete);
> +	/* Don't relinquish CPU until DPRC REPEAT_EN is disabled. */
> +	local_irq_save(flags);
> +	preempt_disable();
> +
> +	/*
> +	 * Sync to FrameGen frame index moving so that
> +	 * FrameGen can be disabled in the next frame.
> +	 */
> +	DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
>  	dc_fg_disable(dc_crtc->fg);
> -	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
> -	disable_irq(dc_crtc->irq_dec_seqcomplete);
> +	/*
> +	 * There is one frame leftover after FrameGen disablement.
> +	 * Sync to FrameGen frame index moving so that DPRC REPEAT_EN
> +	 * can be disabled in the next frame.
> +	 */
> +	DC_CRTC_WAIT_FOR_FRAMEGEN_FRAME_INDEX_MOVING(dc_crtc->fg);
> +	dc_dprc_disable_repeat_en(dc_plane->fu->dprc);
> +
> +	local_irq_restore(flags);
> +	preempt_enable();
>  }

[Severity: High]
Can calling dc_dprc_disable_repeat_en() from dc_crtc_atomic_flush() and
dc_crtc_disable_fg_along_with_dprc_repeat_en() deadlock with the interrupt
handler or sleep in atomic context on PREEMPT_RT?

Both callers invoke dc_dprc_disable_repeat_en() with local IRQs and
preemption disabled:

    local_irq_save(flags);
    preempt_disable();
    ...
    dc_dprc_disable_repeat_en(dc_plane->fu->dprc);

Inside drivers/gpu/drm/imx/dc/dc-dprc.c, dc_dprc_disable_repeat_en() takes
dprc->lock for the entire function scope, including across
synchronize_hardirq(dprc->wrap_irq):

    void dc_dprc_disable_repeat_en(struct dc_dprc *dprc)
    {
        guard(spinlock)(&dprc->lock);
        regmap_write(dprc->reg, IRQ_MASK + SET, IRQ_DPR_CRTL_DONE);

        synchronize_hardirq(dprc->wrap_irq);

        regmap_write(dprc->reg, SYSTEM_CTRL0 + CLR, REPEAT_EN);
        dev_dbg(dprc->dev, "disable REPEAT_EN\n");
    }

Meanwhile, the interrupt handler dc_dprc_wrap_irq_handler() for
dprc->wrap_irq also acquires dprc->lock:

    scoped_guard(spinlock, &dprc->lock) {
        /* cache valid IRQ status */
        regmap_read(dprc->reg, IRQ_MASK, &mask);

If dprc->wrap_irq fires on another CPU just before
dc_dprc_disable_repeat_en() acquires dprc->lock, dc_dprc_disable_repeat_en()
will spin in synchronize_hardirq() while holding dprc->lock, waiting for
dc_dprc_wrap_irq_handler() to finish, while dc_dprc_wrap_irq_handler() spins
waiting for dprc->lock.

In addition, although this patch sets use_raw_spinlock in
dc_ed_pec_regmap_config for PREEMPT_RT, dprc->lock is a non-raw spinlock_t
(and dev_dbg() is called while interrupts and preemption are disabled). On
PREEMPT_RT, acquiring dprc->lock inside local_irq_save() and
preempt_disable() sleeps in atomic context and triggers a lockdep splat, and
devm_request_irq() without IRQF_NO_THREAD threads dc_dprc_wrap_irq_handler()
so synchronize_hardirq() will not synchronize with it.

>  
>  static void
> @@ -338,14 +400,31 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  {
>  	struct drm_crtc_state *new_crtc_state =
>  				drm_atomic_get_new_crtc_state(state, crtc);
> +	struct drm_crtc_state *old_crtc_state =
> +				drm_atomic_get_old_crtc_state(state, crtc);
> +	struct drm_plane_state *old_plane_state =
> +			drm_atomic_get_old_plane_state(state, crtc->primary);
>  	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> +	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
>  	int idx;
>  
>  	if (!drm_dev_enter(crtc->dev, &idx))
>  		goto out;
>  
> -	__dc_crtc_disable_fg(crtc);
> +	enable_irq(dc_crtc->irq_dec_seqcomplete);
> +
> +	if (old_crtc_state->plane_mask && old_plane_state->visible)
> +		dc_crtc_disable_fg_along_with_dprc_repeat_en(crtc);
> +	else
> +		dc_fg_disable(dc_crtc->fg);
> +
> +	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
> +	disable_irq(dc_crtc->irq_dec_seqcomplete);
> +
> +	if (old_crtc_state->plane_mask)
> +		dc_dprc_disable(dc_plane->fu->dprc);

[Severity: High]
As reported by Sashiko on v7 [1], what happens in dc_crtc_atomic_disable()
when the primary plane is attached to the CRTC (so
old_crtc_state->plane_mask is non-zero) but positioned completely off-screen
(so old_plane_state->visible is false)?

In dc_plane_atomic_check(), drm_atomic_helper_check_plane_state() is called
with can_position set to true, allowing a plane with a valid framebuffer and
CRTC to have plane_state->visible == false. When that happens, DPRC and PRG
are either never started in dc_plane_atomic_update() or were already stopped
by dc_dprc_disable() in a previous commit's dc_crtc_atomic_flush() when
plane_becoming_invisible was true.

In dc_crtc_atomic_disable(), the first check tests both
old_crtc_state->plane_mask and old_plane_state->visible, but the second check
only tests old_crtc_state->plane_mask before calling dc_dprc_disable():

    if (old_crtc_state->plane_mask && old_plane_state->visible)
        dc_crtc_disable_fg_along_with_dprc_repeat_en(crtc);
    else
        dc_fg_disable(dc_crtc->fg);

    DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
    disable_irq(dc_crtc->irq_dec_seqcomplete);

    if (old_crtc_state->plane_mask)
        dc_dprc_disable(dc_plane->fu->dprc);

Won't calling dc_dprc_disable() when old_plane_state->visible is false write
to unclocked PRG registers via dc_prg_disable() while runtime-suspended and
underflow the runtime PM usage counts on prg->dev and dprc->dev? Should the
second check also require old_plane_state->visible?

> +
>  	dc_fg_disable_clock(dc_crtc->fg);
>  
>  	/* request pixel engine power-off as plane is off too */
> @@ -371,7 +450,11 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
>  {
>  	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> +	struct dc_plane *dc_plane = to_dc_plane(crtc->primary);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	enum dc_link_id ed_src, lb_sec;
> +	bool disable_dprc = false;
> +	bool crtc_is_enabled;
>  	int ret;
>  
>  	ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
> @@ -381,16 +464,71 @@ int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
>  		goto out1;
>  	}
>  
> +	crtc_is_enabled = dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc));
> +
> +	ret = pm_runtime_resume_and_get(dc_drm->pe->dev);
> +	if (ret) {
> +		dc_crtc_err(crtc, "failed to get DC pixel engine RPM: %d\n",
> +			    ret);
> +		goto out2;
> +	}
> +
>  	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
>  		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
> -		goto out2;
> +		goto out3;
>  	}
>  
> -	dc_crtc_dbg(crtc, "disabling FrameGen at boot\n");
> -	__dc_crtc_disable_fg(crtc);
> +	ret = dc_ed_pec_src_sel_get(dc_crtc->ed_cont, &ed_src);
> +	if (ret || ed_src == LINK_ID_NONE) {
> +		dc_crtc_err(crtc, "failed to get content ExtDst's source\n");
> +		if (ret == 0)
> +			ret = -EINVAL;
> +		goto out3;
> +	}
>  
> -	if (!dc_drv_crtc_is_enabled(dc_drm, drm_crtc_index(crtc)))
> -		goto out2;
> +	if (ed_src == LINK_ID_CONSTFRAME0 || ed_src == LINK_ID_CONSTFRAME1)
> +		goto disable;
> +
> +	ret = dc_lb_pec_dynamic_sec_sel_get(dc_plane->lb, &lb_sec);
> +	if (ret || lb_sec == LINK_ID_NONE) {
> +		dc_crtc_err(crtc,
> +			    "failed to get primary plane LayerBlend secondary source\n");
> +		if (ret == 0)
> +			ret = -EINVAL;
> +		goto out3;
> +	}
> +
> +	/*
> +	 * If CRTC was enabled by bootloader instead of this driver, we don't
> +	 * know if DPRC was enabled or not.  In this case, enforcing DPRC
> +	 * disablement potentially causes imbalanced clock enable count for
> +	 * DPRC and PRG clocks.  So, we presume bootloader doesn't bother to
> +	 * use prefetch engines.
> +	 */
> +	if (crtc_is_enabled)
> +		disable_dprc = true;

[Severity: Medium]
In dc_crtc_disable_at_boot(), if the CRTC was enabled by the bootloader
rather than this driver (so crtc_is_enabled is false), disable_dprc is never
set to true. However, dc_ed_pec_src_sel_get() and
dc_lb_pec_dynamic_sec_sel_get() are still evaluated beforehand and abort
with -EINVAL (skipping FrameGen disablement and failing driver bind in
dc_drm_bind()) if ed_src or lb_sec is LINK_ID_NONE or an unrecognized link
ID.

If the bootloader enabled the display using only the safety stream (leaving
ed_cont at LINK_ID_NONE), using LayerBlend in neutral mode (leaving lb_sec at
LINK_ID_NONE), or using a FetchDecode unit not listed in sec_sels[], won't
dc_crtc_disable_at_boot() fail to disable FrameGen and cause driver probe to
fail? Should we jump straight to the disable label when !crtc_is_enabled
before querying ed_src and lb_sec?

[ ... ]

> diff --git a/drivers/gpu/drm/imx/dc/dc-plane.c b/drivers/gpu/drm/imx/dc/dc-plane.c
> index 42c0a46281cb1..2b58c4e2b7e45 100644
> --- a/drivers/gpu/drm/imx/dc/dc-plane.c
> +++ b/drivers/gpu/drm/imx/dc/dc-plane.c

[ ... ]

> @@ -158,12 +180,18 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
>  {
>  	struct drm_plane_state *new_state =
>  				drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_plane_state *old_state =
> +				drm_atomic_get_old_plane_state(state, plane);
> +	struct drm_crtc_state *new_crtc_state =
> +				drm_atomic_get_new_crtc_state(state, new_state->crtc);
> +	bool needs_modeset = drm_atomic_crtc_needs_modeset(new_crtc_state);
>  	struct dc_plane *dplane = to_dc_plane(plane);
>  	struct drm_framebuffer *fb = new_state->fb;
>  	const struct dc_fu_ops *fu_ops;
>  	struct dc_lb *lb = dplane->lb;
>  	struct dc_fu *fu = dplane->fu;
>  	dma_addr_t baseaddr;
> +	bool prefetch_start;
>  	int src_w, src_h;
>  	int idx;
>  
> @@ -184,7 +212,8 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
>  
>  	fu_ops->set_layerblend(fu, lb);
>  	fu_ops->set_burstlength(fu, baseaddr);
> -	fu_ops->set_src_stride(fu, DC_FETCHUNIT_FRAC0, fb->pitches[0]);
> +	fu_ops->set_src_stride(fu, DC_FETCHUNIT_FRAC0, src_w,
> +			       fb->format->cpp[0] * 8, baseaddr);
>  	fu_ops->set_src_buf_dimensions(fu, DC_FETCHUNIT_FRAC0, src_w, src_h);
>  	fu_ops->set_fmt(fu, DC_FETCHUNIT_FRAC0, fb->format);
>  	fu_ops->set_framedimensions(fu, src_w, src_h);
> @@ -193,6 +222,11 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_commit *state)
>  
>  	dc_plane_dbg(plane, "uses %s\n", fu_ops->get_name(fu));
>  
> +	prefetch_start = needs_modeset || !old_state->fb ||
> +			 (!old_state->visible && new_state->visible);
> +	dc_dprc_configure(fu->dprc, new_state->crtc->index, src_w, src_h,
> +			  fb->pitches[0], fb->format, baseaddr, prefetch_start);

[Severity: High]
As reported by Sashiko on v7 [2], when a CRTC with a visible primary plane
transitions to inactive (new_crtc_state->active == false while
new_crtc_state->enable == true, such as during DPMS OFF or system suspend via
dc_suspend() -> drm_mode_config_helper_suspend()), won't
dc_plane_atomic_update() re-enable DPRC and PRG after
dc_crtc_atomic_disable() has already disabled them?

Because dc_kms_init() uses the default drm_atomic_helper_commit_tail()
without DRM_PLANE_COMMIT_ACTIVE_ONLY:

1. drm_atomic_helper_commit_modeset_disables() runs dc_crtc_atomic_disable(),
   which calls dc_dprc_disable(dc_plane->fu->dprc) and drops the runtime PM
   reference on dc_drm->pe->dev.
2. drm_atomic_helper_commit_planes() then runs dc_crtc_atomic_begin() (which
   returns early because !new_crtc_state->active, leaving dc_drm->pe->dev
   suspended) followed by dc_plane_atomic_update().
3. In dc_plane_atomic_update(), new_state->visible is still true and
   needs_modeset is true (because new_crtc_state->active_changed is true), so
   prefetch_start is set to true. This accesses FetchUnit, LayerBlend, and
   ExtDst registers while pe->dev is runtime-suspended and calls
   dc_dprc_configure() with start = true, which calls
   pm_runtime_resume_and_get() on dprc->dev and prg->dev and starts DPRC and
   PRG with RUN_EN | REPEAT_EN while the CRTC is off, leaking those runtime
   PM references on subsequent commits.

Should dc_plane_atomic_update() check new_crtc_state->active (or should the
driver set DRM_PLANE_COMMIT_ACTIVE_ONLY in mode_config.helper_private)?

[1] https://lore.kernel.org/r/20261026-imx8-dc-prefetch-v7-15-a0b50cc4fc65@nxp.com
[2] https://lore.kernel.org/r/20261026-imx8-dc-prefetch-v7-15-a0b50cc4fc65@nxp.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-imx8-dc-prefetch-v8-0-c4f81afd4533@nxp.com?part=15

Re: [PATCH v8 06/15] drm/imx: dc-crtc: Disable at boot

[Laurentiu Palcu]

On Fri, Oct 30, 2026 at 05:54:30PM +0800, Liu Ying wrote:
> CRTC(s) could still be running after the DRM device is unplugged by
> calling drm_dev_unplug(), because the CRTC disablement logic is
> protected and bypassed by the drm_dev_enter()/drm_dev_exit() pair.
> Hence, Pixel Engine's AXI clock use count(managed by Pixel Engine
> driver's runtime PM) and pixel clock use count could be inbalanced
> after removing and re-installing the driver module.  To fix this,
> add a helper dc_crtc_disable_at_boot() and call it to properly
> disable all CRTCs before advertising DRM device to user-space by
> calling drm_dev_register().
> 
> Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v8:
> - Call dc_drv_set_crtc_disable() in dc_crtc_disable_at_boot() after CRTC
>   is disabled.  (Sashiko AI bot)
> - Make dc_crtc_disable_at_boot() return an integer instead of void.
>   (Laurentiu)
> - Postpone Pixel Engine AXI clock disablement at boot to the time point
>   where CRTC1(with index == 1) is being disabled at boot.
> 
> v7:
> - Check CRTC status to ensure that clocks were enabled by a previous boot
>   before disabling them in dc_crtc_disable_at_boot().  (Sashiko AI bot)
> - Modify debug message in dc_crtc_disable_at_boot().
> 
> v6:
> - Rebase onto the latest drm-misc-next and resolve conflicts.
> 
> v3:
> - Collect Frank's R-b tag.
> ---
>  drivers/gpu/drm/imx/dc/dc-crtc.c | 72 ++++++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/imx/dc/dc-drv.c  | 11 ++++++
>  drivers/gpu/drm/imx/dc/dc-drv.h  |  3 ++
>  3 files changed, 76 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index 0d64186a9901..5bc952d6883e 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
> @@ -227,6 +227,7 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  	struct drm_crtc_state *new_crtc_state =
>  				drm_atomic_get_new_crtc_state(state, crtc);
>  	struct drm_display_mode *adj = &new_crtc_state->adjusted_mode;
> +	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
>  	enum dc_link_id cf_link;
>  	int idx, ret;
> @@ -287,29 +288,37 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  
>  	DC_CRTC_CHECK_FRAMEGEN_FIFO(dc_crtc->fg);
>  
> +	dc_drv_set_crtc_enable(dc_drm, drm_crtc_index(crtc));
> +
>  	drm_dev_exit(idx);
>  
>  out:
>  	dc_crtc_queue_state_event(new_crtc_state);
>  }
>  
> -static void
> -dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
> +static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
>  {
> -	struct drm_crtc_state *new_crtc_state =
> -				drm_atomic_get_new_crtc_state(state, crtc);
> -	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
>  	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> -	int idx;
> -
> -	if (!drm_dev_enter(crtc->dev, &idx))
> -		goto out;
>  
>  	enable_irq(dc_crtc->irq_dec_seqcomplete);
>  	dc_fg_disable(dc_crtc->fg);
>  	DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
>  	disable_irq(dc_crtc->irq_dec_seqcomplete);
> +}
>  
> +static void
> +dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
> +{
> +	struct drm_crtc_state *new_crtc_state =
> +				drm_atomic_get_new_crtc_state(state, crtc);
> +	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	int idx;
> +
> +	if (!drm_dev_enter(crtc->dev, &idx))
> +		goto out;
> +
> +	__dc_crtc_disable_fg(crtc);
>  	dc_fg_disable_clock(dc_crtc->fg);
>  
>  	/* request pixel engine power-off as plane is off too */
> @@ -318,8 +327,9 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  	/* request display engine power-off when CRTC is disabled */
>  	pm_runtime_put(dc_crtc->de->dev);
>  
> -	drm_dev_exit(idx);
> +	dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
>  
> +	drm_dev_exit(idx);
>  out:
>  	drm_crtc_vblank_off(crtc);
>  
> @@ -331,6 +341,48 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit *state)
>  	spin_unlock_irq(&crtc->dev->event_lock);
>  }
>  
> +int dc_crtc_disable_at_boot(struct drm_crtc *crtc)
> +{
> +	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
> +	if (ret < 0) {
> +		dc_crtc_err(crtc, "failed to get DC display engine RPM: %d\n",
> +			    ret);
> +		goto out1;
> +	}
> +
> +	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
> +		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");

Shouldn't you set 'ret' to something here? Otherwise you'd return the previous
ret value, which would be 0.

-- 
Thanks,
Laurentiu

Re: [PATCH v8 07/15] drm/imx: dc: Add PRG support

[Laurentiu Palcu]

On Fri, Oct 30, 2026 at 05:54:31PM +0800, Liu Ying wrote:
> Display Prefetch Resolve Gasket(PRG) is a part of a FetchUnit's
> prefetch engine.  It sits between a Display Prefetch Resolve
> Channel(DPRC) and a FetchUnit.  Add a platform driver to support
> the PRG.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v8:
> - Fix dc_prg_burst_size_fixup() to avoid overflowing signed integer 1.
>   (Sashiko AI bot)
> - Check device_link_add()'s return value and bail out if needed.
>   (Sashiko AI bot)
> - Drop unused header files.  (Laurentiu)
> 
> v7:
> - Collect Maxime's A-b tag.
> - Return 128B burst size from dc_prg_burst_size_fixup() when baddr == 0.
>   This avoids undefined behavior of __ffs(0).  (Sashiko AI bot)
> - Initialize and declare prg_node at the same time.  (Sashiko AI bot)
> - Add device link between PRG's consumer and PRG.  (Sashiko AI bot)
> 
> v6:
> - Include "dc-drv.h" to avoid build warning with C=1.
> - Fix PRG BYPASS programming.
> 
> v4:
> - Sort bits and bitfields in descending order for register PRG_CTRL.
> 
> v2:
> - Manage clocks with bulk interfaces.  (Frank)
> - Collect Frank's R-b tag.
> ---
>  drivers/gpu/drm/imx/dc/Makefile |   2 +-
>  drivers/gpu/drm/imx/dc/dc-drv.c |   1 +
>  drivers/gpu/drm/imx/dc/dc-drv.h |   1 +
>  drivers/gpu/drm/imx/dc/dc-prg.c | 318 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/imx/dc/dc-prg.h |  35 +++++
>  5 files changed, 356 insertions(+), 1 deletion(-)
>

[...]

> diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c
> new file mode 100644
> index 000000000000..46108f7e09af
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-prg.c

[...]

> +
> +bool dc_prg_stride_supported(struct dc_prg *prg,

I searched all commits and this function is never touched again. So, you could
safely get rid of 'prg' argument since it's never used.

Otherwise:
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

> +			     unsigned int stride, dma_addr_t baddr)
> +{
> +	unsigned int burst_size;
> +
> +	burst_size = dc_prg_burst_size_fixup(baddr);
> +
> +	stride = dc_prg_stride_fixup(stride, burst_size);
> +
> +	if (stride > DPU_PRG_MAX_STRIDE)
> +		return false;
> +
> +	return true;
> +}

[...]

-- 
Thanks,
Laurentiu

Re: [PATCH v8 08/15] drm/imx: dc: Add DPR channel support

[Laurentiu Palcu]

On Fri, Oct 30, 2026 at 05:54:32PM +0800, Liu Ying wrote:
> Display Prefetch Resolve Channel(DPRC) is a part of a prefetch engine.
> It fetches display data, transforms it to linear format and stores it
> to DPRC's RTRAM.  PRG, as the other part of a prefetch engine, acts as
> a gasket between the RTRAM controller and a FetchUnit.  Add a platform
> driver to support the DPRC.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v8:
> - Check device_link_add()'s return value and bail out if needed.
>   (Sashiko AI bot)
> - Mask IRQ_DPR_CRTL_DONE irq and synchronize wrap_irq in
>   dc_dprc_disable_repeat_en() to avoid a race condition.  (Sashiko AI bot)
> - Call devm_pm_runtime_enable() before devm_request_irq() to avoid reading
>   dprc->reg in dc_dprc_wrap_irq_handler while dprc->clks are disabled, since
>   another device sharing wrap_irq could raise an interrupt during probe.
>   (Sashiko AI bot)
> - Drop an unused header file.  (Laurentiu)
> - Use dev_err_probe() to handle errors returned from several function calls.
>   (Laurentiu)
> 
> v7:
> - Collect Maxime's A-b tag.
> - Fix dc_dprc_regmap_write_ranges[].  (Sashiko AI bot)
> - Fix dc_dprc_wrap_irq_handler() return value.  (Sashiko AI bot)
> - Check runtime PM status in dc_dprc_wrap_irq_handler().  (Sashiko AI bot)
> - Fix DPU_DRPC_MAX_STRIDE macro.  (Sashiko AI bot)
> - Support deferral probe if platform_get_irq_byname() fails.  (Sashiko AI bot)
> - Avoid dangling prg->dprc by calling dc_prg_set_dprc() properly.
>   (Sashiko AI bot)
> - Add dc_dprc_get() helper for consumer device to get DPRC properly.
> - Set use_raw_spinlock flag in regmap configuration to avoid a potential
>   hard lockup because dc_dprc_disable_repeat_en() is supposed to be called
>   with local irq and preempt disabled.
> 
> v6:
> - Include "dc-drv.h" to avoid build warning with C=1.
> 
> v3:
> - Call dc_dprc_enable() only when start == true.  (Frank)
> - Collect Frank's R-b tag.
> 
> v2:
> - Manage clocks with bulk interfaces.  (Frank)
> - Sort variables in probe function in reverse Christmas tree fashion.  (Frank)
> ---
>  drivers/gpu/drm/imx/dc/Kconfig   |   1 +
>  drivers/gpu/drm/imx/dc/Makefile  |   6 +-
>  drivers/gpu/drm/imx/dc/dc-dprc.c | 534 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/imx/dc/dc-dprc.h |  37 +++
>  drivers/gpu/drm/imx/dc/dc-drv.c  |   1 +
>  drivers/gpu/drm/imx/dc/dc-drv.h  |   1 +
>  drivers/gpu/drm/imx/dc/dc-prg.c  |  12 +
>  drivers/gpu/drm/imx/dc/dc-prg.h  |   4 +
>  8 files changed, 593 insertions(+), 3 deletions(-)
> 

[...]

> diff --git a/drivers/gpu/drm/imx/dc/dc-dprc.c b/drivers/gpu/drm/imx/dc/dc-dprc.c
> new file mode 100644
> index 000000000000..29188fa72a32
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-dprc.c

[...]

> +bool dc_dprc_rtram_width_supported(struct dc_dprc *dprc, unsigned int width)

In your reply to my v7 comment, you also said that this function uses the
'dprc' argument in later commits. FWIW, I couldn't find those commits in this
series that do.  So, you could get rid of it too.

Otherwise:
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

> +{
> +	return width <= DPU_DPRC_MAX_RTRAM_WIDTH;
> +}

[...]

-- 
Thanks,
Laurentiu