[Intel-xe] [PATCH v6 1/2] drm/xe/display: Don't try to use vram if not available

[Ruhl, Michael J]

>-----Original Message-----
>From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>Sent: Tuesday, October 17, 2023 7:13 AM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>; intel-xe at lists.freedesktop.org
>Subject: Re: [PATCH v6 1/2] drm/xe/display: Don't try to use vram if not
>available
>
>On 16.10.2023 18.03, Ruhl, Michael J wrote:
>>> -----Original Message-----
>>> From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>>> Sent: Monday, October 16, 2023 10:54 AM
>>> To: Ruhl, Michael J <michael.j.ruhl at intel.com>
>>> Subject: Fwd: [PATCH v6 1/2] drm/xe/display: Don't try to use vram if not
>>> available
>>>
>>> Trying to get bo from vram when vram not available will cause
>>> WARN_ON() hence avoid touching vram if not available.
>>>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>>> ---
>>>   drivers/gpu/drm/xe/display/xe_fb_pin.c | 11 ++++++-----
>>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> index 2c36e5032433..89cf65c58191 100644
>>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> @@ -62,11 +62,12 @@ static int __xe_pin_fb_vma_dpt(struct
>>> intel_framebuffer *fb,
>>>   		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
>>>   				 XE_PAGE_SIZE);
>>>   -	dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> -				  ttm_bo_type_kernel,
>>> -				  XE_BO_CREATE_VRAM0_BIT |
>>> -				  XE_BO_CREATE_GGTT_BIT);
>>> -	if (IS_ERR(dpt))
>>> +	if (IS_DGFX(xe))
>>> +		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> +					   ttm_bo_type_kernel,
>>> +					   XE_BO_CREATE_VRAM0_BIT |
>>> +					   XE_BO_CREATE_GGTT_BIT);
>>> +	else
>>>   		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>>   					   ttm_bo_type_kernel,
>>>   					   XE_BO_CREATE_STOLEN_BIT |
>>
>> Hi Juha,
>>
>> You have changed this code from:
>>
>> "Best effort allocation" (try vram, and then fallback to stolen), to "Discrete
>gets vram, !discrete gets
>> stolen".
>
>Hi Michael,
>
>this is correct. vram is for discrete and stolen is found on integrated.
>Earlier pattern was copied from i915 way of setting up display page
>table but it had to be changed because asking for vram on Xe while on
>discrete will cause WARN_ON() hence exact same pattern from i915
>couldn't be used.

Ok, that makes sense.  If there is still time, I would suggest updating the commit
message to say something like:

--
Discrete should us vram only.  Current allocation allows a fallback.
Update display allocation to require vram for discrete.
--

Or something like that so the commit message tells us why the change is happening.

Thanks,

M

>/Juha-Pekka
>
>>
>> If this is correct:
>>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
>>
>> Is this new sematic the "correct" allocation method?
>>
>> Thanks,
>>
>> Mike
>>
>>
>>> --
>>> 2.25.1
>>