[Intel-xe] [PATCH v3 3/3] 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: Friday, October 6, 2023 9:43 AM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>; intel-xe at lists.freedesktop.org
>Subject: Re: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if
>not available
>
>On 5.10.2023 21.56, Ruhl, Michael J wrote:
>>> -----Original Message-----
>>> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Juha-
>>> Pekka Heikkila
>>> Sent: Thursday, October 5, 2023 2:34 PM
>>> To: intel-xe at lists.freedesktop.org
>>> Subject: [Intel-xe] [PATCH v3 3/3] 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 e8e38091c8e6..a79c2416579f 100644
>>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> @@ -83,7 +83,7 @@ static int __xe_pin_fb_vma_dpt(struct
>intel_framebuffer
>>> *fb,
>>> 	struct xe_device *xe = to_xe_device(fb->base.dev);
>>> 	struct xe_tile *tile0 = xe_device_get_root_tile(xe);
>>> 	struct xe_ggtt *ggtt = tile0->mem.ggtt;
>>> -	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt;
>>> +	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt = ERR_PTR(-EINVAL);
>>> 	u32 dpt_size, size = bo->ttm.base.size;
>>>
>>> 	if (view->type == I915_GTT_VIEW_NORMAL)
>>> @@ -96,10 +96,11 @@ 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 (tile0->mem.vram.usable_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))
>>
>> Hmm, seems like just 'else' here would be sufficient (if _size) vram0 else
>sysmem) ?
>
>That's good idea, I'll send new version.
>
>>
>> It also seems like maybe this check should be in the xe_bo_create_pin_map
>function?
>> I.e. validate that the requested region bit is valid before creating the bo?
>>
>> That would let you keep the pattern for the IS_ERR without the init..
>
>This I didn't quite understand, I'm still new to how things work on Xe.

I was suggesting that xe_bo_create_pin_map() should validate that the VRAM0/1 bit was valid
for the tile and return  an error if not.

So you would have no change here, but eh xe_bo_create_pin_map() would get updated...

No worries. 😊

I will take a look at your refresh...

M

>/Juha-Pekka
>
>>
>> Just some thoughts.
>>
>> M
>>
>>> 		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> 					   ttm_bo_type_kernel,
>>> --
>>> 2.25.1
>>