Re: [Intel-gfx] [PATCH] drm/i915/display: Add smem fallback allocation for dpt

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 17.3.2022 13.55, Matthew Auld wrote:
> On Wed, 16 Mar 2022 at 22:23, Juha-Pekka Heikkila
> <juhapekka.heikkila@gmail.com> wrote:
>>
>> Add fallback smem allocation for dpt if stolen memory
>> allocation failed.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dpt.c | 18 ++++++++++++++----
>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
>> index fb0e7e79e0cd..c8b66433d4db 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
>> @@ -10,6 +10,7 @@
>>   #include "intel_display_types.h"
>>   #include "intel_dpt.h"
>>   #include "intel_fb.h"
>> +#include "gem/i915_gem_internal.h"
> 
> Nit: these should be kept sorted
> 
>>
>>   struct i915_dpt {
>>          struct i915_address_space vm;
>> @@ -128,6 +129,10 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm)
>>          void __iomem *iomem;
>>          struct i915_gem_ww_ctx ww;
>>          int err;
>> +       u64 pin_flags = 0;
>> +
>> +       if (i915_gem_object_is_stolen(dpt->obj))
>> +               pin_flags |= PIN_MAPPABLE; /* for i915_vma_pin_iomap(stolen) */
>>
>>          wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>>          atomic_inc(&i915->gpu_error.pending_fb_pin);
>> @@ -138,7 +143,7 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm)
>>                          continue;
>>
>>                  vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096,
>> -                                                 HAS_LMEM(i915) ? 0 : PIN_MAPPABLE);
>> +                                                 pin_flags);
>>                  if (IS_ERR(vma)) {
>>                          err = PTR_ERR(vma);
>>                          continue;
>> @@ -248,10 +253,15 @@ intel_dpt_create(struct intel_framebuffer *fb)
>>
>>          size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);
>>
>> -       if (HAS_LMEM(i915))
>> -               dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
>> -       else
>> +       dpt_obj = i915_gem_object_create_lmem(i915, size, I915_BO_ALLOC_CONTIGUOUS);
>> +       if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
>>                  dpt_obj = i915_gem_object_create_stolen(i915, size);
>> +       if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
>> +               drm_dbg_kms(&i915->drm, "fb: [FB:%d] Allocating dpt from smem\n",
>> +                           fb->base.base.id);
>> +
>> +               dpt_obj = i915_gem_object_create_internal(i915, size);
> 
> Looks like we are missing some prerequisite patch to be able to
> directly map such memory in vma_pin_iomap?

For these functions I'm more like a consumer, I was following 
suggestions from Chris on this. Is there something extra that should be 
considered in this regard when use it like this?

> 
>> +       }
>>          if (IS_ERR(dpt_obj))
>>                  return ERR_CAST(dpt_obj);
>>
>> --
>> 2.28.0
>>