* [PATCH v9 01/16] dt-bindings: display: imx: Add i.MX8qxp/qm PRG binding
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
@ 2026-11-03 6:21 ` Liu Ying
2026-11-03 6:21 ` [PATCH v9 02/16] dt-bindings: display: imx: Add i.MX8qxp/qm DPR channel binding Liu Ying
` (14 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:21 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, 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 80d9c9e9a1a9..4bd0e4ea8495 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8813,6 +8813,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 02/16] dt-bindings: display: imx: Add i.MX8qxp/qm DPR channel binding
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
2026-11-03 6:21 ` [PATCH v9 01/16] dt-bindings: display: imx: Add i.MX8qxp/qm PRG binding Liu Ying
@ 2026-11-03 6:21 ` Liu Ying
2026-11-03 6:21 ` [PATCH v9 03/16] drm/imx: dc-plane: Disable plane when it is invisible Liu Ying
` (13 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:21 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, 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 4bd0e4ea8495..e04da44e0bda 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8813,6 +8813,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 03/16] drm/imx: dc-plane: Disable plane when it is invisible
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
2026-11-03 6:21 ` [PATCH v9 01/16] dt-bindings: display: imx: Add i.MX8qxp/qm PRG binding Liu Ying
2026-11-03 6:21 ` [PATCH v9 02/16] dt-bindings: display: imx: Add i.MX8qxp/qm DPR channel binding Liu Ying
@ 2026-11-03 6:21 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback Liu Ying
` (12 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:21 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Laurentiu Palcu
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (2 preceding siblings ...)
2026-11-03 6:21 ` [PATCH v9 03/16] drm/imx: dc-plane: Disable plane when it is invisible Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:39 ` sashiko-bot
2026-11-03 6:22 ` [PATCH v9 05/16] drm/imx: dc-fu: Fix dimensions Liu Ying
` (11 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying
Currently, a CRTC can be disabled by a commit without considering
it's relevant plane's status(either enabled or disabled). A problem
is that dc_crtc_atomic_disable() puts Pixel Engine and Display Engine's
runtime PMs, so dc_plane_atomic_update() or dc_plane_atomic_disable()
may access registers with AXI clock and CFG clock disabled. To fix
this, specify .atomic_commit_tail callback(based on
drm_atomic_helper_commit_tail() without the unneeded
drm_atomic_helper_fake_vblank() function call) to add
DRM_PLANE_COMMIT_ACTIVE_ONLY and DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET
flags to drm_atomic_helper_commit_planes() function call's flags
argument so that .atomic_update and .atomic_disable callbacks in
dc_plane.c are skipped when necessary.
Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v9:
- Newly introduced. (Sashiko AI bot)
---
drivers/gpu/drm/imx/dc/dc-kms.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/imx/dc/dc-kms.c b/drivers/gpu/drm/imx/dc/dc-kms.c
index 0e2ce6a42548..9ffa15ac43a2 100644
--- a/drivers/gpu/drm/imx/dc/dc-kms.c
+++ b/drivers/gpu/drm/imx/dc/dc-kms.c
@@ -15,6 +15,7 @@
#include <drm/drm_encoder.h>
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_mode_config.h>
+#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -30,6 +31,29 @@ static const struct drm_mode_config_funcs dc_drm_mode_config_funcs = {
.atomic_commit = drm_atomic_helper_commit,
};
+static void dc_drm_atomic_commit_tail(struct drm_atomic_commit *state)
+{
+ struct drm_device *dev = state->dev;
+
+ drm_atomic_helper_commit_modeset_disables(dev, state);
+
+ drm_atomic_helper_commit_planes(dev, state,
+ DRM_PLANE_COMMIT_ACTIVE_ONLY |
+ DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET);
+
+ drm_atomic_helper_commit_modeset_enables(dev, state);
+
+ drm_atomic_helper_commit_hw_done(state);
+
+ drm_atomic_helper_wait_for_vblanks(dev, state);
+
+ drm_atomic_helper_cleanup_planes(dev, state);
+}
+
+static const struct drm_mode_config_helper_funcs dc_drm_mode_config_helpers = {
+ .atomic_commit_tail = dc_drm_atomic_commit_tail,
+};
+
static const struct drm_encoder_funcs dc_kms_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
@@ -103,6 +127,7 @@ int dc_kms_init(struct dc_drm_device *dc_drm)
drm->mode_config.max_width = 8192;
drm->mode_config.max_height = 8192;
drm->mode_config.funcs = &dc_drm_mode_config_funcs;
+ drm->mode_config.helper_private = &dc_drm_mode_config_helpers;
drm->vblank_disable_immediate = true;
drm->max_vblank_count = DC_FRAMEGEN_MAX_FRAME_INDEX;
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback
2026-11-03 6:22 ` [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback Liu Ying
@ 2026-11-03 6:39 ` sashiko-bot
2026-11-03 7:03 ` Liu Ying
0 siblings, 1 reply; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 6:39 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imx: dc: plane disable skipped on modeset in dc_drm_atomic_commit_tail()
--
commit ff022f683849ca82589cd00028c8fb7c412423dc
Author: Liu Ying <victor.liu@nxp.com>
drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback
This commit introduces a custom atomic_commit_tail callback for the i.MX8qxp
Display Controller KMS driver that passes DRM_PLANE_COMMIT_ACTIVE_ONLY and
DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET to drm_atomic_helper_commit_planes()
to avoid accessing plane registers while clocks are disabled.
> diff --git a/drivers/gpu/drm/imx/dc/dc-kms.c b/drivers/gpu/drm/imx/dc/dc-kms.c
> index 0e2ce6a425487..9ffa15ac43a25 100644
> --- a/drivers/gpu/drm/imx/dc/dc-kms.c
> +++ b/drivers/gpu/drm/imx/dc/dc-kms.c
[ ... ]
> @@ -30,6 +31,29 @@ static const struct drm_mode_config_funcs dc_drm_mode_config_funcs = {
> .atomic_commit = drm_atomic_helper_commit,
> };
>
> +static void dc_drm_atomic_commit_tail(struct drm_atomic_commit *state)
> +{
> + struct drm_device *dev = state->dev;
> +
> + drm_atomic_helper_commit_modeset_disables(dev, state);
> +
> + drm_atomic_helper_commit_planes(dev, state,
> + DRM_PLANE_COMMIT_ACTIVE_ONLY |
> + DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET);
[Severity: High]
Could passing DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET (and
DRM_PLANE_COMMIT_ACTIVE_ONLY) here leave the plane's FetchUnit source buffer
and LayerBlend enabled in hardware after the old framebuffer is cleaned up?
In drm_atomic_helper_commit_planes(), DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET
skips calling funcs->atomic_disable() when the CRTC needs a modeset:
if (disabling && funcs->atomic_disable) {
struct drm_crtc_state *crtc_state;
crtc_state = old_plane_state->crtc->state;
if (drm_atomic_crtc_needs_modeset(crtc_state) &&
no_disable)
continue;
funcs->atomic_disable(plane, state);
}
That flag is meant for drivers where the CRTC atomic_disable callback also
disables the CRTC's planes. However, in drivers/gpu/drm/imx/dc/dc-crtc.c,
dc_crtc_atomic_disable() only disables FrameGen and calls pm_runtime_put() on
the Display Engine and the shared Pixel Engine without disabling the planes:
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);
dc_fg_disable_clock(dc_crtc->fg);
/* request pixel engine power-off as plane is off too */
pm_runtime_put(dc_drm->pe->dev);
And in drivers/gpu/drm/imx/dc/dc-plane.c, __dc_plane_atomic_disable() is what
actually disables the FetchUnit source buffer and LayerBlend:
/* 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));
When dc_crtc_atomic_enable() runs with new_crtc_state->plane_mask == 0, it
switches ed_cont's source selection to cf_cont, but it does not call
fu_ops->disable_src_buf():
/* show CRTC background if no plane is enabled */
if (new_crtc_state->plane_mask == 0) {
/* constframe in content stream shows black frame */
dc_cf_constantcolor_black(dc_crtc->cf_cont);
cf_link = dc_cf_get_link_id(dc_crtc->cf_cont);
dc_ed_pec_src_sel(dc_crtc->ed_cont, cf_link);
}
If an active CRTC with an enabled primary plane undergoes a modeset while
remaining active and disables its plane in the same commit,
dc_crtc_atomic_disable() calls asynchronous pm_runtime_put(dc_drm->pe->dev)
and dc_crtc_atomic_begin() immediately calls
pm_runtime_resume_and_get(dc_drm->pe->dev) before runtime suspend runs (or the
second CRTC on the shared Pixel Engine keeps dc_drm->pe->dev active), so
dc_pe_runtime_resume() does not reset the Pixel Engine hardware. Because
drm_atomic_crtc_needs_modeset() is true, DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET
skips dc_plane_atomic_disable(), leaving SOURCEBUFFERENABLE and LayerBlend
enabled after drm_atomic_helper_cleanup_planes() cleans up the framebuffer.
Similarly, because drm_atomic_helper_swap_state() swaps crtc->state before
dc_drm_atomic_commit_tail() runs, plane_crtc_active(old_plane_state) evaluates
to false whenever a CRTC is being disabled (new_crtc_state->active == false).
With DRM_PLANE_COMMIT_ACTIVE_ONLY, disabling a CRTC and its plane while the
second CRTC keeps the shared dc_drm->pe->dev powered on, followed by
re-enabling the first CRTC with plane_mask == 0, also skips
dc_plane_atomic_disable() without ever calling fu_ops->disable_src_buf().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback
2026-11-03 6:39 ` sashiko-bot
@ 2026-11-03 7:03 ` Liu Ying
0 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 7:03 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
On Tue, Nov 03, 2026 at 06:39:15AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] drm/imx: dc: plane disable skipped on modeset in dc_drm_atomic_commit_tail()
> --
>
> commit ff022f683849ca82589cd00028c8fb7c412423dc
> Author: Liu Ying <victor.liu@nxp.com>
>
> drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback
>
> This commit introduces a custom atomic_commit_tail callback for the i.MX8qxp
> Display Controller KMS driver that passes DRM_PLANE_COMMIT_ACTIVE_ONLY and
> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET to drm_atomic_helper_commit_planes()
> to avoid accessing plane registers while clocks are disabled.
>
> > diff --git a/drivers/gpu/drm/imx/dc/dc-kms.c b/drivers/gpu/drm/imx/dc/dc-kms.c
> > index 0e2ce6a425487..9ffa15ac43a25 100644
> > --- a/drivers/gpu/drm/imx/dc/dc-kms.c
> > +++ b/drivers/gpu/drm/imx/dc/dc-kms.c
> [ ... ]
> > @@ -30,6 +31,29 @@ static const struct drm_mode_config_funcs dc_drm_mode_config_funcs = {
> > .atomic_commit = drm_atomic_helper_commit,
> > };
> >
> > +static void dc_drm_atomic_commit_tail(struct drm_atomic_commit *state)
> > +{
> > + struct drm_device *dev = state->dev;
> > +
> > + drm_atomic_helper_commit_modeset_disables(dev, state);
> > +
> > + drm_atomic_helper_commit_planes(dev, state,
> > + DRM_PLANE_COMMIT_ACTIVE_ONLY |
> > + DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET);
>
> [Severity: High]
> Could passing DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET (and
> DRM_PLANE_COMMIT_ACTIVE_ONLY) here leave the plane's FetchUnit source buffer
> and LayerBlend enabled in hardware after the old framebuffer is cleaned up?
No, because the plane's CRTC is being disabled, all hardware units backing
the plane will effectively stop working together with the disabled CRTC.
>
> In drm_atomic_helper_commit_planes(), DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET
> skips calling funcs->atomic_disable() when the CRTC needs a modeset:
>
> if (disabling && funcs->atomic_disable) {
> struct drm_crtc_state *crtc_state;
>
> crtc_state = old_plane_state->crtc->state;
>
> if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> no_disable)
> continue;
>
> funcs->atomic_disable(plane, state);
> }
>
> That flag is meant for drivers where the CRTC atomic_disable callback also
> disables the CRTC's planes. However, in drivers/gpu/drm/imx/dc/dc-crtc.c,
> dc_crtc_atomic_disable() only disables FrameGen and calls pm_runtime_put() on
> the Display Engine and the shared Pixel Engine without disabling the planes:
>
> 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);
>
> dc_fg_disable_clock(dc_crtc->fg);
>
> /* request pixel engine power-off as plane is off too */
> pm_runtime_put(dc_drm->pe->dev);
>
> And in drivers/gpu/drm/imx/dc/dc-plane.c, __dc_plane_atomic_disable() is what
> actually disables the FetchUnit source buffer and LayerBlend:
>
> /* 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));
>
> When dc_crtc_atomic_enable() runs with new_crtc_state->plane_mask == 0, it
> switches ed_cont's source selection to cf_cont, but it does not call
> fu_ops->disable_src_buf():
That's fine, because FetchUnit and LayerBlend won't work if ed_cont's source
selection is switched to cf_cont.
>
> /* show CRTC background if no plane is enabled */
> if (new_crtc_state->plane_mask == 0) {
> /* constframe in content stream shows black frame */
> dc_cf_constantcolor_black(dc_crtc->cf_cont);
>
> cf_link = dc_cf_get_link_id(dc_crtc->cf_cont);
> dc_ed_pec_src_sel(dc_crtc->ed_cont, cf_link);
> }
[...]
--
Regards,
Liu Ying
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 05/16] drm/imx: dc-fu: Fix dimensions
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (3 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 04/16] drm/imx: dc-kms: Use driver specific .atomic_commit_tail callback Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 06/16] drm/imx: dc: Flag CRTC status in GENERALPURPOSE register Liu Ying
` (10 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 06/16] drm/imx: dc: Flag CRTC status in GENERALPURPOSE register
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (4 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 05/16] drm/imx: dc-fu: Fix dimensions Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:32 ` sashiko-bot
2026-11-03 6:22 ` [PATCH v9 07/16] drm/imx: dc-crtc: Disable at boot Liu Ying
` (9 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Laurentiu Palcu
Map Display Controller's register GENERALPURPOSE 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_is_enabled() so that the CRTC driver can use it to check
if the CRTC is 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>
---
v9:
- Fix typos in patch subject and commit message. (Sashiko AI bot)
- Set .max_register_is_0 = true in dc_gp_regmap_config. (Sashiko AI bot)
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 | 76 +++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/imx/dc/dc-drv.h | 7 ++++
2 files changed, 83 insertions(+)
diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
index 13795a2ad735..4c0749e5aa09 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,22 @@
#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,
+ .max_register_is_0 = true,
};
DEFINE_DRM_GEM_DMA_FOPS(dc_drm_driver_fops);
@@ -156,6 +170,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 +183,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 +233,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 06/16] drm/imx: dc: Flag CRTC status in GENERALPURPOSE register
2026-11-03 6:22 ` [PATCH v9 06/16] drm/imx: dc: Flag CRTC status in GENERALPURPOSE register Liu Ying
@ 2026-11-03 6:32 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 6:32 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/imx: dc: commit message mismatch for dc_drv_set_crtc_{enable,disable}()
--
commit 1bad6b1819c8236cf47f95cfccc89fe8559ef3c8
Author: Liu Ying <victor.liu@nxp.com>
drm/imx: dc: Flag CRTC status in GENERALPURPOSE 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 bits.
> drm/imx: dc: Flag CRTC status in GENERALPURPOSE register
>
> Map Display Controller's register GENERALPURPOSE 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_is_enabled() so that the CRTC driver can use it to check
> if the CRTC is 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]
As reported by Sashiko on v8 [1], the commit message states that helpers
dc_drv_flag_crtc_{enable,disable}() are created, whereas
drivers/gpu/drm/imx/dc/dc-drv.c and drivers/gpu/drm/imx/dc/dc-drv.h define
and declare dc_drv_set_crtc_enable() and dc_drv_set_crtc_disable(). Should
the commit message be updated to reference
dc_drv_set_crtc_{enable,disable}()?
[1] https://lore.kernel.org/r/sashiko-outbox-190808@kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=6
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 07/16] drm/imx: dc-crtc: Disable at boot
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (5 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 06/16] drm/imx: dc: Flag CRTC status in GENERALPURPOSE register Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:38 ` sashiko-bot
2026-11-03 6:22 ` [PATCH v9 08/16] drm/imx: dc: Add PRG support Liu Ying
` (8 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li, Laurentiu Palcu
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>
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v9:
- Collect Laurentiu's R-b tag.
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 4c0749e5aa09..24b4be7be09e 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>
@@ -109,6 +110,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,
@@ -131,6 +133,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 07/16] drm/imx: dc-crtc: Disable at boot
2026-11-03 6:22 ` [PATCH v9 07/16] drm/imx: dc-crtc: Disable at boot Liu Ying
@ 2026-11-03 6:38 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 6:38 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
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 8bca21cfc221adc6d10d2019ff78d5b2dc5c6e91
Author: Liu Ying <victor.liu@nxp.com>
drm/imx: dc-crtc: Disable at boot
This commit adds a dc_crtc_disable_at_boot() helper and calls it for each
CRTC during driver bind to disable any running CRTCs and balance display
and AXI clock usage counts before registering the DRM device.
> 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]
As reported by Sashiko on v8 [1], can dc_crtc_disable_at_boot() in
drivers/gpu/drm/imx/dc/dc-crtc.c prematurely disable dc_drm->pe->clk_axi
when disabling CRTC 1 fails?
In dc_drm_bind(), drm_for_each_crtc() invokes dc_crtc_disable_at_boot()
for CRTC 0 and then CRTC 1. When CRTC 0 was active at boot, its call sets
dc_drm->pe_clk_axi_needs_disable = true and clears CRTC_ENABLED(0).
When dc_crtc_disable_at_boot() then runs for CRTC 1, if
pm_runtime_resume_and_get(dc_crtc->de->dev) fails and jumps to out1,
CRTC 1's FrameGen has not been stopped and CRTC_ENABLED(1) has not been
cleared. However, at out1, dc_drm->pe_clk_axi_needs_disable is already
true from CRTC 0 and drm_crtc_index(crtc) == 1, so
clk_disable_unprepare(dc_drm->pe->clk_axi) is called unconditionally.
If CRTC 1 was also active at boot, wouldn't this gate the Pixel Engine
AXI clock while CRTC 1 is still scanning out from the Pixel Engine, and
leave CRTC_ENABLED(1) set in the general purpose register so that a
subsequent bind retry disables dc_drm->pe->clk_axi a second time?
[1] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-6-c4f81afd4533@nxp.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=7
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 08/16] drm/imx: dc: Add PRG support
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (6 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 07/16] drm/imx: dc-crtc: Disable at boot Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:36 ` sashiko-bot
2026-11-03 6:22 ` [PATCH v9 09/16] drm/imx: dc: Add DPR channel support Liu Ying
` (7 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li, Laurentiu Palcu
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>
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v9:
- Drop "struct dc_prg *prg" argument from dc_prg_stride_supported().
(Laurentiu)
- Collect Laurentiu's R-b tag.
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 | 317 ++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/imx/dc/dc-prg.h | 34 +++++
5 files changed, 354 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 24b4be7be09e..e1c7d51db82b 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -357,6 +357,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..b80ce5e64117
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-prg.c
@@ -0,0 +1,317 @@
+// 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(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..ea11c4080927
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-prg.h
@@ -0,0 +1,34 @@
+/* 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(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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 08/16] drm/imx: dc: Add PRG support
2026-11-03 6:22 ` [PATCH v9 08/16] drm/imx: dc: Add PRG support Liu Ying
@ 2026-11-03 6:36 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 6:36 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
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 in dc_prg_configure()
--
commit d1c395ad8b2bd23da43b9ddd3c832ab32084d33e
Author: Liu Ying <victor.liu@nxp.com>
drm/imx: dc: Add PRG support
This commit adds a platform driver for the Display Prefetch Resolve Gasket
(PRG), which sits between a Display Prefetch Resolve Channel (DPRC) and a
FetchUnit as part of the prefetch engine.
> 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..b80ce5e641173
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-prg.c
> @@ -0,0 +1,317 @@
[ ... ]
> +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 v8 [1], in drivers/gpu/drm/imx/dc/dc-prg.c, what
happens if pm_runtime_resume_and_get() fails in dc_prg_configure() when start
is true?
Because dc_prg_configure() returns void, the error is not propagated to the
caller and the PRG device remains runtime-suspended with its clocks disabled
in dc_prg_runtime_resume().
Callers that proceed after dc_prg_configure() will then call dc_prg_enable(),
dc_prg_reg_update(), or dc_prg_shadow_enable(), performing MMIO writes to
PRG_CTRL and PRG_REG_UPDATE while the PRG clocks are off. Subsequent updates
with start set to false will also write to the PRG registers without clocks
enabled.
Furthermore, when the plane or CRTC is later disabled, dc_prg_disable() will
write to PRG_CTRL while unclocked and call 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 runtime PM active
state so subsequent register accesses and pm_runtime_put() are skipped when
resume fails?
[1] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-7-c4f81afd4533@nxp.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=8
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 09/16] drm/imx: dc: Add DPR channel support
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (7 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 08/16] drm/imx: dc: Add PRG support Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:53 ` sashiko-bot
2026-11-03 6:22 ` [PATCH v9 10/16] drm/imx: dc: Use TCON operation mode Liu Ying
` (6 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li, Laurentiu Palcu
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>
Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v9:
- Drop "struct dc_dprc *dprc" argument from dc_dprc_rtram_width_supported()
and dc_dprc_stride_supported(). (Laurentiu)
- Collect Laurentiu's R-b tag.
- Change dprc->lock from spinlock_t to raw_spinlock_t. (Sashiko AI bot)
- Add IRQF_NO_THREAD flag to devm_request_irq() function call. (Sashiko AI bot)
- Mask all IRQ bits with the scoped guard dprc->lock before calling
sychronize_hardirq() in dc_dprc_disable_repeat_en() so that runtime PM
callbacks are synchronous with DPRC register accesses in
dc_dprc_wrap_irq_handler(). (Sashiko AI bot)
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 | 36 +++
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, 592 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..aa6899d162e3
--- /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 IRQ_ALL_MASK GENMASK(7, 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;
+ raw_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(raw_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)
+{
+ scoped_guard(raw_spinlock, &dprc->lock)
+ regmap_write(dprc->reg, IRQ_MASK + SET, IRQ_ALL_MASK);
+
+ 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(raw_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(unsigned int width)
+{
+ return width <= DPU_DPRC_MAX_RTRAM_WIDTH;
+}
+
+bool dc_dprc_stride_supported(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(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;
+ raw_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 | IRQF_NO_THREAD, 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(raw_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..2081e69434e4
--- /dev/null
+++ b/drivers/gpu/drm/imx/dc/dc-dprc.h
@@ -0,0 +1,36 @@
+/* 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(unsigned int width);
+
+bool dc_dprc_stride_supported(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 e1c7d51db82b..9fb9ee640a4d 100644
--- a/drivers/gpu/drm/imx/dc/dc-drv.c
+++ b/drivers/gpu/drm/imx/dc/dc-drv.c
@@ -350,6 +350,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 b80ce5e64117..b6a2dc8f6df7 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);
@@ -225,6 +227,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 ea11c4080927..a26647784d00 100644
--- a/drivers/gpu/drm/imx/dc/dc-prg.h
+++ b/drivers/gpu/drm/imx/dc/dc-prg.h
@@ -31,4 +31,8 @@ bool dc_prg_stride_supported(unsigned int stride, dma_addr_t baddr);
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 09/16] drm/imx: dc: Add DPR channel support
2026-11-03 6:22 ` [PATCH v9 09/16] drm/imx: dc: Add DPR channel support Liu Ying
@ 2026-11-03 6:53 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 6:53 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/imx: dc-dprc: invalid wait context calling dc_prg_shadow_enable() in IRQ
- [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()
- [Medium] drm/imx: dc-dprc: error IRQs left masked on restart before runtime suspend
--
commit cd34e5bff983d01a81bde212b10cde47c6bbe04b
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) in the i.MX8 Display Controller and links it with the PRG driver.
[Severity: High]
As reported by Sashiko on v8 [1], in drivers/gpu/drm/imx/dc/dc-dprc.c at
line 243, can dc_dprc_configure() leak a runtime PM reference and leave PRG
runtime PM unbalanced when given an unsupported format or when PRG runtime
resume fails?
When dc_dprc_configure() is called with start set to true, it calls
pm_runtime_resume_and_get(dev) at line 207 and writes hardware registers
before checking format->format in the switch statement at line 230. If the
format hits the default branch at line 243, the function logs an error and
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 without a matching reference if dc_dprc_disable() is
later called.
Similarly, at line 259, dc_dprc_configure() calls dc_prg_configure(). In
drivers/gpu/drm/imx/dc/dc-prg.c at line 137, if start is true and
pm_runtime_resume_and_get() fails on prg->dev, dc_prg_configure() returns
void without reporting the error. dc_dprc_configure() then unconditionally
calls dc_dprc_enable() at line 262 and dc_dprc_reg_update() at line 264,
which write to prg->reg while PRG clocks are still disabled.
Could format validation be done before pm_runtime_resume_and_get(), and
could dc_prg_configure() return an error code so dc_dprc_configure() can
unwind the dprc->dev runtime PM reference on failure?
[Severity: Medium]
In drivers/gpu/drm/imx/dc/dc-dprc.c at line 276, does
dc_dprc_disable_repeat_en() leave the hardware error interrupts masked if
the channel is restarted before runtime suspend runs?
In dc_dprc_runtime_resume() at line 507, the driver resets the hardware with
dc_dprc_reset() and writes IRQ_CTRL_MASK to IRQ_MASK at line 511, which
masks the three control interrupts while unmasking the five error interrupts.
When stopping the channel, dc_dprc_disable_repeat_en() at line 276 sets
IRQ_ALL_MASK in IRQ_MASK, masking both the control and error interrupts.
Then dc_dprc_disable() at line 292 calls pm_runtime_put(dprc->dev), which
schedules an asynchronous suspend on pm_wq.
If dc_dprc_configure() is called again with start set to true before the
asynchronous runtime suspend callback executes, pm_runtime_resume_and_get()
at line 207 sees the device is already active and returns immediately
without running dc_dprc_runtime_resume(). Because dc_dprc_configure() only
unmasks IRQ_DPR_CRTL_DONE via dc_dprc_enable_ctrl_done_irq() at line 190,
won't the five error interrupts remain masked and dc_dprc_reset() be skipped
for the new stream?
[Severity: High]
As reported by Sashiko on v8 [2], in drivers/gpu/drm/imx/dc/dc-dprc.c at
line 310, does calling dc_prg_shadow_enable() from
dc_dprc_wrap_irq_handler() trigger an invalid wait context on PREEMPT_RT or
lockdep?
In dc_dprc_probe() at line 463, devm_request_irq() requests wrap_irq with
IRQF_SHARED | IRQF_NO_THREAD, so dc_dprc_wrap_irq_handler() always runs in
hardirq context. When IRQ_DPR_CRTL_DONE is handled at line 362, the call
chain dc_dprc_wrap_irq_handler() -> dc_dprc_ctrl_done_handle() ->
dc_prg_shadow_enable() writes to prg->reg using regmap_write().
Unlike dc_dprc_regmap_config, dc_prg_regmap_config in
drivers/gpu/drm/imx/dc/dc-prg.c at line 71 sets fast_io to true but does not
set use_raw_spinlock to true. As a result, prg->reg uses a standard
spinlock_t, which becomes a sleepable lock on PREEMPT_RT and violates
lockdep's raw lock nesting rules when acquired from an IRQF_NO_THREAD hardirq
handler (as well as from dc_dprc_configure() when called with local IRQs and
preemption disabled). Should dc_prg_regmap_config also set use_raw_spinlock
to true?
[Severity: High]
As reported by Sashiko on v8 [3], in drivers/gpu/drm/imx/dc/dc-dprc.c at
line 322, can the runtime PM handling in dc_dprc_wrap_irq_handler() race
with dc_dprc_disable() and dc_dprc_runtime_suspend()?
Because wrap_irq is requested with IRQF_SHARED at line 463, another DPR
channel sharing the interrupt line can invoke dc_dprc_wrap_irq_handler() at
any time, even after this channel has masked its own interrupts in
dc_dprc_disable_repeat_en(). This creates three issues with the
pm_runtime_get_noresume() / pm_runtime_active() / pm_runtime_put_noidle()
pattern at lines 322 and 366:
1. If a shared interrupt runs dc_dprc_wrap_irq_handler() and increments
usage_count via pm_runtime_get_noresume() at line 322 while
dc_dprc_disable() at line 292 calls pm_runtime_put() (or while the async
runtime suspend worker is checking usage_count), the runtime PM core sees
usage_count > 0 and aborts the idle/suspend transition. When the IRQ handler
then drops its reference with pm_runtime_put_noidle() at line 366, no new
idle or suspend request is queued, leaving dprc->dev active with its clocks
enabled indefinitely.
2. In rpm_suspend(), usage_count is checked before the status changes from
RPM_ACTIVE to RPM_SUSPENDING and before dc_dprc_runtime_suspend() is called
at line 487. If a shared interrupt fires on another CPU after the
usage_count check passes, pm_runtime_get_noresume() does not stop the
ongoing suspend and pm_runtime_active() still returns true until the status
is updated, allowing the handler to read IRQ_MASK and IRQ_MASK_STATUS while
dc_dprc_runtime_suspend() is disabling dprc->clks.
3. pm_runtime_active(dev) returns true whenever dev->power.disable_depth is
non-zero, even if the device is runtime-suspended with its clocks off (for
example, during system suspend after __device_suspend_late() disables
runtime PM and before noirq suspend disables interrupts). A shared interrupt
during that window will see pm_runtime_active(dev) as true and read
dprc->reg while clocks are disabled.
Could the driver track whether clocks are enabled under dprc->lock in the
runtime PM callbacks instead of relying on pm_runtime_get_noresume() and
pm_runtime_active() in the hardirq handler?
[1] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-8-c4f81afd4533@nxp.com
[2] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-8-c4f81afd4533@nxp.com
[3] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-8-c4f81afd4533@nxp.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=9
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 10/16] drm/imx: dc: Use TCON operation mode
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (8 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 09/16] drm/imx: dc: Add DPR channel support Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 11/16] drm/imx: dc-ed: Support getting source selection Liu Ying
` (5 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 11/16] drm/imx: dc-ed: Support getting source selection
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (9 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 10/16] drm/imx: dc: Use TCON operation mode Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 12/16] drm/imx: dc-lb: Support getting secondary input selection Liu Ying
` (4 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 12/16] drm/imx: dc-lb: Support getting secondary input selection
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (10 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 11/16] drm/imx: dc-ed: Support getting source selection Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 13/16] drm/imx: dc-ed: Drop initial source selection Liu Ying
` (3 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 13/16] drm/imx: dc-ed: Drop initial source selection
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (11 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 12/16] drm/imx: dc-lb: Support getting secondary input selection Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 14/16] drm/imx: dc-lb: Drop initial primary and secondary input selections Liu Ying
` (2 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 14/16] drm/imx: dc-lb: Drop initial primary and secondary input selections
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (12 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 13/16] drm/imx: dc-ed: Drop initial source selection Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 15/16] drm/imx: dc-fu: Get DPR channel Liu Ying
2026-11-03 6:22 ` [PATCH v9 16/16] drm/imx: dc: Use prefetch engine Liu Ying
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 15/16] drm/imx: dc-fu: Get DPR channel
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (13 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 14/16] drm/imx: dc-lb: Drop initial primary and secondary input selections Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 6:22 ` [PATCH v9 16/16] drm/imx: dc: Use prefetch engine Liu Ying
15 siblings, 0 replies; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v9 16/16] drm/imx: dc: Use prefetch engine
2026-11-03 6:21 [PATCH v9 00/16] drm/imx: dc: Use prefetch engine Liu Ying
` (14 preceding siblings ...)
2026-11-03 6:22 ` [PATCH v9 15/16] drm/imx: dc-fu: Get DPR channel Liu Ying
@ 2026-11-03 6:22 ` Liu Ying
2026-11-03 7:03 ` sashiko-bot
15 siblings, 1 reply; 24+ messages in thread
From: Liu Ying @ 2026-11-03 6:22 UTC (permalink / raw)
To: Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Dmitry Baryshkov
Cc: dri-devel, devicetree, imx, linux-arm-kernel, linux-kernel,
Marek Vasut, Alexander Stein, Liu Ying, Frank Li
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>
---
v9:
- Drop "struct dc_dprc *dprc" argument from dc_dprc_rtram_width_supported()
and dc_dprc_stride_supported() function calls. (Laurentiu)
- Check old_plane_state->visible before calling dc_dprc_disable() in
dc_crtc_atomic_disable(). (Sashiko AI bot)
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 | 46 +++++++++--
6 files changed, 229 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..55c198db9644 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 && old_plane_state->visible)
+ 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..ba20d3535515 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,26 @@ 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 drm_framebuffer *fb = state->fb;
+ dma_addr_t baseaddr = drm_fb_dma_get_gem_addr(fb, state, 0);
+ u32 src_w = drm_rect_width(&state->src) >> 16;
+
+ if (!dc_dprc_rtram_width_supported(src_w)) {
+ dc_plane_dbg(state->plane, "bad RTRAM width for DPRC\n");
+ return -EINVAL;
+ }
+
+ if (!dc_dprc_stride_supported(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 +139,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 +178,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 +210,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 +220,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v9 16/16] drm/imx: dc: Use prefetch engine
2026-11-03 6:22 ` [PATCH v9 16/16] drm/imx: dc: Use prefetch engine Liu Ying
@ 2026-11-03 7:03 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-11-03 7:03 UTC (permalink / raw)
To: Liu Ying; +Cc: dri-devel, devicetree, imx, robh, conor+dt, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/imx: dc: missing use_raw_spinlock in dc_prg_regmap_config
- [Medium] drm/imx: dc: premature source checks in dc_crtc_disable_at_boot()
- [Medium] drm/imx: dc: unbalanced DPRC and PRG runtime PM in dc_crtc_disable_at_boot()
--
commit a0700a3c30143af37a2ae48e1eba7fde342cf2a1
Author: Liu Ying <victor.liu@nxp.com>
drm/imx: dc: Use prefetch engine
This commit configures and disables prefetch engines (DPR channel and PRGs)
alongside FetchUnits during atomic commits and boot-time cleanup, adapts
FetchUnit burst size and stride fixups for PRG requirements, and enables
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..55c198db9644c 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
[ ... ]
> @@ -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]
As reported by Sashiko on v8 [1], when the display was enabled by the
bootloader rather than a previous instance of this driver
(!crtc_is_enabled), disable_dprc is never set and ed_src and lb_sec are
not used anywhere else in dc_crtc_disable_at_boot().
However, dc_ed_pec_src_sel_get() and dc_lb_pec_dynamic_sec_sel_get() are
called unconditionally before checking crtc_is_enabled. If the bootloader
enabled the display using only the safety stream (leaving ed_cont at its
reset state), using LayerBlend in primary-only mode, or using a different
LayerBlend or FetchUnit not in sec_sels[], dc_crtc_disable_at_boot()
jumps to out3 without disabling FrameGen and returns an error that aborts
driver probe in dc_drm_bind():
drivers/gpu/drm/imx/dc/dc-drv.c:dc_drm_bind()
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;
}
}
Should dc_crtc_disable_at_boot() check !crtc_is_enabled and jump to
disable before querying ed_src and lb_sec?
> +
> +disable:
> + enable_irq(dc_crtc->irq_dec_seqcomplete);
> +
> + 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);
> + }
[Severity: Medium]
When a plane is enabled, dc_dprc_configure() and dc_prg_configure() call
pm_runtime_resume_and_get() on dprc->dev and prg->dev. If the driver is
unbound or unloaded while the plane is active, dc_drm_unbind() calls
drm_dev_unplug() before drm_atomic_helper_shutdown(), so
dc_crtc_atomic_disable() exits early at drm_dev_enter() without calling
dc_dprc_disable().
Unlike de->dev and pe->dev, which are child devices created by
devm_of_platform_populate() in dc_probe() and recreated on rebind,
dprc->dev and prg->dev are top-level platform devices whose struct device
and power.usage_count persist across unbind/rebind and module reload.
When dc_crtc_disable_at_boot() subsequently runs on rebind or module
reload, dc_dprc_disable_at_boot() and dc_prg_disable_at_boot() disable the
clocks directly without balancing runtime PM:
drivers/gpu/drm/imx/dc/dc-dprc.c:
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");
}
drivers/gpu/drm/imx/dc/dc-prg.c:
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);
}
Won't this leave dprc->dev and prg->dev with an elevated runtime PM
usage_count after module reload (preventing runtime suspend on future
plane disablement), or leave them marked active with their clocks disabled
across a sysfs unbind/rebind of the master device so that the next
dc_dprc_configure() accesses unclocked registers?
> diff --git a/drivers/gpu/drm/imx/dc/dc-plane.c b/drivers/gpu/drm/imx/dc/dc-plane.c
> index 42c0a46281cb1..ba20d35355155 100644
> --- a/drivers/gpu/drm/imx/dc/dc-plane.c
> +++ b/drivers/gpu/drm/imx/dc/dc-plane.c
[ ... ]
> @@ -193,6 +220,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]
When dc_plane_atomic_update() calls dc_dprc_configure() with
prefetch_start set to true, dc_dprc_enable_ctrl_done_irq() unmasks the
control done interrupt on dprc->wrap_irq.
In dc_dprc_probe(), dprc->wrap_irq is requested with IRQF_NO_THREAD, so
dc_dprc_wrap_irq_handler() runs in hardirq context and calls into PRG
regmap:
dc_dprc_wrap_irq_handler()
-> dc_dprc_ctrl_done_handle()
-> dc_prg_shadow_enable()
-> regmap_write(prg->reg, PRG_CTRL + SET, SHADOW_EN)
However, dc_prg_regmap_config in drivers/gpu/drm/imx/dc/dc-prg.c does not
set use_raw_spinlock:
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,
};
Without .use_raw_spinlock = true in dc_prg_regmap_config, won't
regmap_write() on prg->reg acquire a sleeping spinlock_t inside hardirq
context on preempt-rt kernels and trigger an invalid wait context warning
under lockdep?
[1] https://lore.kernel.org/r/20261030-imx8-dc-prefetch-v8-15-c4f81afd4533@nxp.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-imx8-dc-prefetch-v9-0-3b985e7636e2@nxp.com?part=16
^ permalink raw reply [flat|nested] 24+ messages in thread