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

[Tvrtko Ursulin]

On 04/02/2025 15:11, Jani Nikula wrote:
> On Tue, 04 Feb 2025, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> 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.
> 
> Now there's also the conversation with Tvrtko, and I've lost track
> whether this should be merged at all. ;D

I don't mind 1/2, only 2/2 would be a bit in the way of adding AuxCSS 
support for ADL-P.

Regards,

Tvrtko

>>>>>
>>>>> 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
>