[PATCH] drm/i915: Pre-populate the cursor physical dma address

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Monday, March 25, 2024 11:28 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: stable at vger.kernel.org; Borislav Petkov <bp at alien8.de>
> Subject: [PATCH] drm/i915: Pre-populate the cursor physical dma address
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Calling i915_gem_object_get_dma_address() from the vblank evade critical
> section triggers might_sleep().
> 
> While we know that we've already pinned the framebuffer and thus
> i915_gem_object_get_dma_address() will in fact not sleep in this case, it
> seems reasonable to keep the unconditional might_sleep() for maximum
> coverage.
> 
> So let's instead pre-populate the dma address during fb pinning, which all
> happens before we enter the vblank evade critical section.
> 
> We can use u32 for the dma address as this class of hardware doesn't
> support >32bit addresses.
> 
> Cc: stable at vger.kernel.org
> Fixes: 0225a90981c8 ("drm/i915: Make cursor plane registers unlocked")
> Link: https://lore.kernel.org/intel-
> gfx/20240227100342.GAZd2zfmYcPS_SndtO at fat_crate.local/

Nit. This could be changed to Closes and moved after Reported-by to keep checkpatch happy but otherwise, LGTM.

Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>

> Reported-by: Borislav Petkov <bp at alien8.de>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c        |  4 +---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  1 +
>  drivers/gpu/drm/i915/display/intel_fb_pin.c        | 10 ++++++++++
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index f8b33999d43f..0d3da55e1c24 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -36,12 +36,10 @@ static u32 intel_cursor_base(const struct
> intel_plane_state *plane_state)  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(plane_state->uapi.plane->dev);
> -	const struct drm_framebuffer *fb = plane_state->hw.fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	u32 base;
> 
>  	if (DISPLAY_INFO(dev_priv)->cursor_needs_physical)
> -		base = i915_gem_object_get_dma_address(obj, 0);
> +		base = plane_state->phys_dma_addr;
>  	else
>  		base = intel_plane_ggtt_offset(plane_state);
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8a35fb6b2ade..68f26a33870b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -728,6 +728,7 @@ struct intel_plane_state {  #define PLANE_HAS_FENCE
> BIT(0)
> 
>  	struct intel_fb_view view;
> +	u32 phys_dma_addr; /* for cursor_needs_physical */
> 
>  	/* Plane pxp decryption state */
>  	bool decrypt;
> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c
> b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> index 7b42aef37d2f..b6df9baf481b 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> @@ -255,6 +255,16 @@ int intel_plane_pin_fb(struct intel_plane_state
> *plane_state)
>  			return PTR_ERR(vma);
> 
>  		plane_state->ggtt_vma = vma;
> +
> +		/*
> +		 * Pre-populate the dma address before we enter the vblank
> +		 * evade critical section as
> i915_gem_object_get_dma_address()
> +		 * will trigger might_sleep() even if it won't actually sleep,
> +		 * which is the case when the fb has already been pinned.
> +		 */
> +		if (phys_cursor)
> +			plane_state->phys_dma_addr =
> +
> 	i915_gem_object_get_dma_address(intel_fb_obj(fb), 0);
>  	} else {
>  		struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> 
> --
> 2.43.2