Re: [Intel-gfx] [PATCH 2/2] drm/i915/display: Do not use stolen on MTL

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 10/07/2023 10:00, Nirmoy Das wrote:
> Hi Tvrkto,
> 
> On 7/6/2023 3:43 PM, Tvrtko Ursulin wrote:
>>
>> On 06/07/2023 14:35, Nirmoy Das wrote:
>>>
>>> On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote:
>>>>
>>>> On 30/06/2023 18:01, Nirmoy Das wrote:
>>>>> Use smem on MTL due to a HW bug in MTL that prevents
>>>>> reading from stolen memory using LMEM BAR.
>>>>
>>>> Does anything remain in stolen or could the memory region just not 
>>>> be created?
>>>
>>>
>>> GSC requires DSM which can't use smem for another bug.
>>
>> Okay, thanks.
>>
>> As a related comment, these if-if-if object creation ladders were 
>> always a bit ugly and some years ago I was suggesting we create a 
>> helper with some "intent/usage" flags. Which could then dtrt ie. 
>> create the right object for that intent/usage and platform. I *think* 
>> I possibly even had a RFC... need to try and find it.
> 
> 
> Did you find it :) Would be nice to have a better way to detect and 
> apply memory region as per platfrom/usecase.

Nope. Basically the idea boiled down to figuring out if it is possible 
to express the "requirements" via intent flags. Like do we need CPU 
access, is it mostly GPU, can it be volatile etc. And then combine the 
intent with the platform to figure out what kind of object to create. 
But it was many years ago and I am not sure if the idea would still 
apply so easily, without looking at the all call sites. Could easily end 
up complicated so I cannot dare to say it is worth spending time looking 
at this.

Regards,

Tvrtko

> 
> 
> Regards,
> 
> Nirmoy
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> Regards,
>>>
>>> Nirmoy
>>>
>>>
>>>>
>>>> Regards,
>>>>
>>>> Tvrtko
>>>>
>>>>> Cc: Oak Zeng <oak.zeng@intel.com>
>>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/display/intel_fbdev.c   | 2 ++
>>>>>   drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++---
>>>>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c 
>>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> index 1cc0ddc6a310..10e38d60f9ef 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper 
>>>>> *helper,
>>>>>           obj = i915_gem_object_create_lmem(dev_priv, size,
>>>>>                             I915_BO_ALLOC_CONTIGUOUS |
>>>>>                             I915_BO_ALLOC_USER);
>>>>> +    } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */
>>>>> +        obj = i915_gem_object_create_shmem(dev_priv, size);
>>>>>       } else {
>>>>>           /*
>>>>>            * If the FB is too big, just don't use it since fbdev is 
>>>>> not very
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c 
>>>>> b/drivers/gpu/drm/i915/display/intel_overlay.c
>>>>> index d6fe2bbabe55..05ae446c8a56 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
>>>>> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct 
>>>>> drm_device *dev, void *data,
>>>>>   static int get_registers(struct intel_overlay *overlay, bool 
>>>>> use_phys)
>>>>>   {
>>>>>       struct drm_i915_private *i915 = overlay->i915;
>>>>> -    struct drm_i915_gem_object *obj;
>>>>> +    struct drm_i915_gem_object *obj = NULL;
>>>>>       struct i915_vma *vma;
>>>>>       int err;
>>>>>   -    obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
>>>>> -    if (IS_ERR(obj))
>>>>> +    if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
>>>>> +        obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
>>>>> +    if (IS_ERR_OR_NULL(obj))
>>>>>           obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
>>>>>       if (IS_ERR(obj))
>>>>>           return PTR_ERR(obj);