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

Frank Li
2025-09-19T15:42:28Z

On Fri, Jul 04, 2025 at 05:04:01PM +0800, Liu Ying wrote:
> 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, properly disable the prefetch engine at boot and adapt burst
> size/stride fixup requirements from PRG in FetchUnit driver.
>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> drivers/gpu/drm/imx/dc/dc-crtc.c | 139 +++++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/imx/dc/dc-fu.c | 27 +++++++-
> 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 +++++++++++--
> 5 files changed, 197 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index 9e9e86cd5202bcb0bb4d5627dbcefcc3f4e2ead0..4c7aab360616cb1c84c31c83f16df703b1c2c6d7 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,13 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *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 disabling_plane;
> int idx;
>
> if (drm_atomic_crtc_needs_modeset(new_crtc_state) ||
> @@ -216,13 +223,40 @@ dc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *state)
>
> enable_irq(dc_crtc->irq_ed_cont_shdload);
>
> - /* flush plane update out to display */
> - dc_ed_pec_sync_trigger(dc_crtc->ed_cont);
> + disabling_plane = drm_atomic_plane_disabling(old_plane_state,
> + new_plane_state);
> +
> + if (disabling_plane) {
> + unsigned long flags;
> +
> + dc_crtc_dbg(crtc, "disabling plane\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();

preempt_enable();
local_irq_restore(flags);

look symmetry() with enter this sections.

> + } 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_plane)
> + dc_dprc_disable(dc_plane->fu->dprc);
> +
> DC_CRTC_CHECK_FRAMEGEN_FIFO(dc_crtc->fg);
>
> drm_dev_exit(idx);
> @@ -320,14 +354,33 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> dc_crtc_queue_state_event(new_crtc_state);
> }
>

...

> + struct drm_crtc_state *old_crtc_state =
> + drm_atomic_get_old_crtc_state(state, 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);
> int idx, ret;
>
> 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)
> + __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);

Are you sure irq is disabled when call this function?

Frank
> +
> + if (old_crtc_state->plane_mask)
> + dc_dprc_disable(dc_plane->fu->dprc);
> +
> dc_fg_disable_clock(dc_crtc->fg);
>
> /* request pixel engine power-off as plane is off too */
> @@ -373,7 +441,10 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> void 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;
> int ret;
>
> ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
...

> 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);
> @@ -161,6 +190,9 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state)
>
> dc_plane_dbg(plane, "uses %s\n", fu_ops->get_name(fu));
>
> + 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.34.1
>