[Intel-gfx] [RFC PATCH] drm/i915: don't treat small BAR as an error with CSS

[Das, Nirmoy]

On 5/11/2022 1:31 PM, Matthew Auld wrote:
> On Wed, 11 May 2022 at 11:25, Nirmoy Das <nirmoy.das at intel.com> wrote:
>> Determine lmem_size using ADDR_RANGE register so that module
>> load on platfrom with small bar with css  works.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>> ---
>> I once reserved a dg2 machine with small bar and module load failed on
>> it. I can't find that machine anymore hence sending this as RFC.
> AFAIK we currently don't want to load the driver on such dg2
> configurations, until we first have all the uapi bits landed.


Ok, sounds good.

>   The last
> patch in that series then turns this on, or at least that's what I
> have locally.
>
>>   drivers/gpu/drm/i915/gt/intel_region_lmem.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
>> index f5111c0a0060..a55eecac104e 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
>> @@ -100,10 +100,19 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt)
>>          if (!IS_DGFX(i915))
>>                  return ERR_PTR(-ENODEV);
>>
>> +       if (IS_DG1(uncore->i915)) {
>> +               lmem_size = pci_resource_len(pdev, 2);
> We can drop this, since this is set below also.
>
>> +       } else {
>> +               resource_size_t lmem_range;
>> +
>> +               lmem_range = intel_gt_read_register(&i915->gt0, XEHPSDV_TILE0_ADDR_RANGE) & 0xFFFF;
>> +               lmem_size = lmem_range >> XEHPSDV_TILE_LMEM_RANGE_SHIFT;
>> +               lmem_size *= SZ_1G;
> We can move this under HAS_FLAT_CCS.
>
> I think we need another patch that then just gracefully returns
> -EINVAL if this is a small-bar configuration, along with maybe some
> helpful drm_err() or so, which can be removed once we properly support
> it?

I will resend with this suggestion.


>   Also it looks like we are returning ENODEV in some places here,
> which looks iffy.


We do

         io_start = pci_resource_start(pdev, 2);
         io_size = min(pci_resource_len(pdev, 2), lmem_size);
         if (!io_size)
                 return ERR_PTR(-ENODEV);

Is this return looks iffy?


Thanks,

Nirmoy

>
>> +       }
>> +
>>          if (HAS_FLAT_CCS(i915)) {
>>                  u64 tile_stolen, flat_ccs_base;
>>
>> -               lmem_size = pci_resource_len(pdev, 2);
>>                  flat_ccs_base = intel_gt_read_register(gt, XEHPSDV_FLAT_CCS_BASE_ADDR);
>>                  flat_ccs_base = (flat_ccs_base >> XEHPSDV_CCS_BASE_SHIFT) * SZ_64K;
>>
>> --
>> 2.35.1
>>