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

[Lucas De Marchi]

On Tue, Feb 04, 2025 at 12:37:07PM +0200, Jani Nikula wrote:
>On Mon, 03 Feb 2025, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Mon, Feb 03, 2025 at 03:07:08PM +0200, Jani Nikula wrote:
>>>On Tue, 14 Jan 2025, Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com> wrote:
>>>> 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.
>>
>> I'd also get the fix in place that will also be needed in stable for
>> platforms like BMG. Then the second patch to move it to this helper.
>
>Let me clarify, are you asking for changes before merging?

the reply I gave was very confusing, sorry. What I meant is that there
are 2 things here:

1) A fix, so it doesn't try to to allocate dpt from system memory on
dgfx
2) Move the code to a separate function.

For backporting to stable we'd only need (1), which is trivial, rather
than also needing (2) which may have more conflicts.

I'll leave that up to you to decide though.

>
>
>>
>>>
>>>Cc: Rodrigo, Lucas, Maarten
>>>
>>>Should this be merged via drm-intel or drm-xe?
>>
>> I'd say.... if it's possible, through drm-intel since it's under
>> display/. However this only uses xe stuff and may have conflict if
>> drm-intel is not in sync, so it may need a backmerge from drm-next.
>>
>> Since it only deals with xe-stuff, landing it through drm-xe should be
>> good too.
>
>I'd generally err towards merging all of display through drm-intel, but
>just asking as a courtesy here because it only touches xe side of
>things.

yeah, I think it makes sense to prefer drm-intel. Ack for that.


Acked-by: Lucas De Marchi <lucas.demarchi at intel.com>


Lucas De Marchi

>
>BR,
>Jani.
>
>
>
>>
>> Lucas De Marchi
>>
>>>
>>>>
>>>> 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;
>>>> +}
>>>> +
>>>>  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);
>>>
>>>--
>>>Jani Nikula, Intel
>
>-- 
>Jani Nikula, Intel