Message-ID: <aNLnoZsMiWNogUEB@lizhi-Precision-Tower-5810>
Date: Tue, 23 Sep 2025 14:32:01 -0400
From: Frank Li <Frank.li@....com>
To: Liu Ying <victor.liu@....com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Dmitry Baryshkov <lumag@...nel.org>,
	dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 14/14] drm/imx: dc: Use prefetch engine

On Tue, Sep 23, 2025 at 10:08:04AM +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@....com>

Reviewed-by: Frank Li <Frank.Li@....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();
> +	} 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);
>  }
>
> -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
> @@ -335,14 +388,29 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *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 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);
> +
> +	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);
> @@ -383,13 +454,53 @@ void dc_crtc_disable_at_boot(struct drm_crtc *crtc)
>  		return;
>  	}
>
> +	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 out1;
> +	}
> +
>  	if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
>  		dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
> -		goto out;
> +		goto out2;
> +	}
> +
> +	ret = dc_ed_pec_src_sel_get(dc_crtc->ed_cont, &ed_src);
> +	if (ret) {
> +		dc_crtc_err(crtc, "failed to get content ExtDst's source\n");
> +		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) {
> +		dc_crtc_err(crtc,
> +			    "failed to get primary plane LayerBlend secondary source\n");
> +		goto out2;
>  	}
>
> -	dc_crtc_dbg(crtc, "disabling at boot\n");
> -	__dc_crtc_disable_fg(crtc);
> +	disable_dprc = true;
> +
> +disable:
> +	enable_irq(dc_crtc->irq_dec_seqcomplete);
> +
> +	if (disable_dprc) {
> +		dc_crtc_dbg(crtc, "disabling along with DPRC REPEAT_EN at boot\n");
> +		__dc_crtc_disable_fg_along_with_dprc_repeat_en(crtc);
> +	} else {
> +		dc_crtc_dbg(crtc, "disabling 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 (disable_dprc)
> +		dc_dprc_disable_at_boot(dc_plane->fu->dprc);
> +
>  	dc_fg_disable_clock(dc_crtc->fg);
>
>  	if (!dc_drm->pe_clk_axi_disabled) {
> @@ -397,7 +508,13 @@ void dc_crtc_disable_at_boot(struct drm_crtc *crtc)
>  		dc_drm->pe_clk_axi_disabled = true;
>  	}
>
> -out:
> +out2:
> +	ret = pm_runtime_put(dc_drm->pe->dev);
> +	if (ret)
> +		dc_crtc_err(crtc, "failed to put DC pixel engine RPM: %d\n",
> +			    ret);
> +
> +out1:
>  	ret = pm_runtime_put(dc_crtc->de->dev);
>  	if (ret < 0)
>  		dc_crtc_err(crtc, "failed to put DC display engine RPM: %d\n",
> diff --git a/drivers/gpu/drm/imx/dc/dc-fu.c b/drivers/gpu/drm/imx/dc/dc-fu.c
> index 47d436abb65157de7cab74565e44b199be76de52..bc439c3520d45f894c0afab5b3d52f2f3309c2e2 100644
> --- a/drivers/gpu/drm/imx/dc/dc-fu.c
> +++ b/drivers/gpu/drm/imx/dc/dc-fu.c
> @@ -124,13 +124,28 @@ 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;
>
>  	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,
> @@ -150,8 +165,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 44b9497e53580589a05bcb180eb2312ea4449da4..09570955a5b92105ef7010f71fa615a1cefc9b7e 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 a25d47eebd28792e4b53b4ecc89907ce00430c2c..8b45b21a6f8a7e6e6ed2563499753200bdd42ebc 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 d8b946fb90de638da2bf4667307f11b06f4e77f5..8ef754492b2dcb5d986a63f516328f8d2512c7b6 100644
> --- a/drivers/gpu/drm/imx/dc/dc-plane.c
> +++ b/drivers/gpu/drm/imx/dc/dc-plane.c
> @@ -17,6 +17,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_print.h>
>
> +#include "dc-dprc.h"
>  #include "dc-drv.h"
>  #include "dc-fu.h"
>  #include "dc-kms.h"
> @@ -44,11 +45,6 @@ static const struct drm_plane_funcs dc_plane_funcs = {
>  	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
>  };
>
> -static inline struct dc_plane *to_dc_plane(struct drm_plane *plane)
> -{
> -	return container_of(plane, struct dc_plane, base);
> -}
> -
>  static int dc_plane_check_max_source_resolution(struct drm_plane_state *state)
>  {
>  	int src_h = drm_rect_height(&state->src) >> 16;
> @@ -88,6 +84,28 @@ static int dc_plane_check_fb(struct drm_plane_state *state)
>  	return 0;
>  }
>
> +static int dc_plane_check_dprc(struct drm_plane_state *state)
> +{
> +	struct dc_plane *dplane = to_dc_plane(state->plane);
> +	struct drm_framebuffer *fb = state->fb;
> +	dma_addr_t baseaddr = drm_fb_dma_get_gem_addr(fb, state, 0);
> +	struct dc_dprc *dprc = dplane->fu->dprc;
> +	u32 src_w = drm_rect_width(&state->src) >> 16;
> +
> +	if (!dc_dprc_rtram_width_supported(dprc, src_w)) {
> +		dc_plane_dbg(state->plane, "bad RTRAM width for DPRC\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!dc_dprc_stride_supported(dprc, fb->pitches[0], src_w, fb->format,
> +				      baseaddr)) {
> +		dc_plane_dbg(state->plane, "fb bad pitches[0] for DPRC\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  dc_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_state *state)
>  {
> @@ -123,7 +141,11 @@ dc_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_state *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);
>  }
>
>  static void
> @@ -131,6 +153,12 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *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);
> +	bool prefetch_start = needs_modeset || !old_state->fb;
>  	struct dc_plane *dplane = to_dc_plane(plane);
>  	struct drm_framebuffer *fb = new_state->fb;
>  	const struct dc_fu_ops *fu_ops;
> @@ -152,7 +180,8 @@ dc_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *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);
> @@ -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
>

Powered by blists - more mailing lists