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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 04 Feb 2025, Lucas De Marchi <lucas.demarchi@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@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@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

J.



>
>>
>>
>>>
>>>>
>>>>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@intel.com>
>
>
> Lucas De Marchi
>
>>
>>BR,
>>Jani.
>>
>>
>>
>>>
>>> Lucas De Marchi
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@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

-- 
Jani Nikula, Intel