[PATCH 1/2] drm/xe/display: Move dpt allocation to helper

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Juha-Pekka Heikkila
Sent: Tuesday, January 14, 2025 10:04 AM
To: intel-xe at lists.freedesktop.org
Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
Subject: [PATCH 1/2] drm/xe/display: Move dpt allocation to helper
> 
> Simplify __xe_pin_fb_vma_dpt() by moving dpt allocation into helper.
> This also fixes bug where dpt could have been allocated from system
> memory when on dgfx.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  drivers/gpu/drm/xe/display/xe_fb_pin.c | 67 +++++++++++++++++---------
>  1 file changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index 9fa51b84737c..c28885316986 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -77,6 +77,47 @@ write_dpt_remapped(struct xe_bo *bo, struct iosys_map *map, u32 *dpt_ofs,
>  	*dpt_ofs = ALIGN(*dpt_ofs, 4096);
>  }
>  
> +static struct xe_bo *xe_alloc_dpt_bo(struct xe_device *xe,
> +				     struct xe_tile *tile0, u64 size,
> +				     u64 physical_alignment)
> +{
> +	struct xe_bo *dpt;
> +
> +	/*
> +	 * If DGFX: try VRAM0 only
> +	 */
> +	if (IS_DGFX(xe)) {
> +		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> +						      size, ~0ull,
> +						      ttm_bo_type_kernel,
> +						      XE_BO_FLAG_VRAM0 |
> +						      XE_BO_FLAG_GGTT |
> +						      XE_BO_FLAG_PAGETABLE,
> +						      physical_alignment);
> +	} else {
> +		/*
> +		 * For IGFX: first try STOLEN. on fail try SYSTEM.
> +		 */
> +		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> +						      size, ~0ull,
> +						      ttm_bo_type_kernel,
> +						      XE_BO_FLAG_STOLEN |
> +						      XE_BO_FLAG_GGTT |
> +						      XE_BO_FLAG_PAGETABLE,
> +						      physical_alignment);
> +		if (IS_ERR(dpt)) {
> +			dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> +							      size, ~0ull,
> +							      ttm_bo_type_kernel,
> +							      XE_BO_FLAG_SYSTEM |
> +							      XE_BO_FLAG_GGTT |
> +							      XE_BO_FLAG_PAGETABLE,
> +							      physical_alignment);
> +		}
> +	}
> +	return dpt;

We might be able to collapse some of this logic by storing the flags separately:

"""
static struct xe_bo *xe_alloc_dpt_bo(struct xe_device *xe,
				     struct xe_tile *tile0, u64 size,
				     u64 physical_alignment)
{
	struct xe_bo *dpt;
	u32 base_flags = XE_BO_FLAG_GGTT | XE_BO_FLAG_PAGETABLE;
	u32 flags = base_flags;

	/*
	 * If DGFX: try VRAM0.
	 * If IGFX: try STOLEN.
	 */
	flags |= IS_DGFX(xe) ? XE_BO_FLAG_VRAM0 : XE_BO_FLAG_STOLEN;

	dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL, size,
					      ~0ull, ttm_bo_type_kernel,
					      flags, physical_alignment);

	/*
	 * For IGFX, we first try STOLEN, and on a failure we try SYSTEM.
	 * DGFX should only attempt VRAM0
	 */
	if (IS_DGFX(xe) && IS_ERR(dpt))
		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
						      size, ~0ull,
						      ttm_bo_type_kernel,
						      base_flags |
						      XE_BO_FLAG_SYSTEM,
						      physical_alignment);
	return dpt;
}
"""
This isn't a particularly necessary compression, but it might be worth considering.
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> +}
> +
>  static int __xe_pin_fb_vma_dpt(const struct intel_framebuffer *fb,
>  			       const struct i915_gtt_view *view,
>  			       struct i915_vma *vma,
> @@ -99,30 +140,8 @@ static int __xe_pin_fb_vma_dpt(const struct intel_framebuffer *fb,
>  		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
>  				 XE_PAGE_SIZE);
>  
> -	if (IS_DGFX(xe))
> -		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> -						      dpt_size, ~0ull,
> -						      ttm_bo_type_kernel,
> -						      XE_BO_FLAG_VRAM0 |
> -						      XE_BO_FLAG_GGTT |
> -						      XE_BO_FLAG_PAGETABLE,
> -						      physical_alignment);
> -	else
> -		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> -						      dpt_size,  ~0ull,
> -						      ttm_bo_type_kernel,
> -						      XE_BO_FLAG_STOLEN |
> -						      XE_BO_FLAG_GGTT |
> -						      XE_BO_FLAG_PAGETABLE,
> -						      physical_alignment);
> -	if (IS_ERR(dpt))
> -		dpt = xe_bo_create_pin_map_at_aligned(xe, tile0, NULL,
> -						      dpt_size,  ~0ull,
> -						      ttm_bo_type_kernel,
> -						      XE_BO_FLAG_SYSTEM |
> -						      XE_BO_FLAG_GGTT |
> -						      XE_BO_FLAG_PAGETABLE,
> -						      physical_alignment);
> +	dpt = xe_alloc_dpt_bo(xe, tile0, dpt_size, physical_alignment);
> +
>  	if (IS_ERR(dpt))
>  		return PTR_ERR(dpt);
>  
> -- 
> 2.45.2
> 
>