[Intel-gfx] [RFC PATCH 118/162] drm/i915/dg1: Reserve first 1MB of local memory

[Matthew Auld]

On 27/11/2020 13:52, Chris Wilson wrote:
> Quoting Matthew Auld (2020-11-27 12:06:34)
>> From: Imre Deak <imre.deak at intel.com>
>>
>> On DG1 A0/B0 steppings the first 1MB of local memory must be reserved.
>> One reason for this is that the 0xA0000-0xB0000 range is not accessible
>> by the display, probably since this region is redirected to another
>> memory location for legacy VGA compatibility.
>>
>> BSpec: 50586
>> Testcase: igt/kms_big_fb/linear-64bpp-rotate-0
>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_region_lmem.c | 52 ++++++++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_region_lmem.c b/drivers/gpu/drm/i915/intel_region_lmem.c
>> index 939cf0d195a5..eafef7034680 100644
>> --- a/drivers/gpu/drm/i915/intel_region_lmem.c
>> +++ b/drivers/gpu/drm/i915/intel_region_lmem.c
>> @@ -137,6 +137,48 @@ intel_setup_fake_lmem(struct drm_i915_private *i915)
>>          return mem;
>>   }
>>   
>> +static void get_legacy_lowmem_region(struct intel_uncore *uncore,
>> +                                    u64 *start, u32 *size)
>> +{
>> +       *start = 0;
>> +       *size = 0;
>> +
>> +       if (!IS_DG1_REVID(uncore->i915, DG1_REVID_A0, DG1_REVID_B0))
>> +               return;
>> +
>> +       *size = SZ_1M;
>> +
>> +       DRM_DEBUG_DRIVER("LMEM: reserved legacy low-memory [0x%llx-0x%llx]\n",
>> +                        *start, *start + *size);
>> +}
>> +
>> +static int reserve_lowmem_region(struct intel_uncore *uncore,
>> +                                struct intel_memory_region *mem)
>> +{
>> +       u64 reserve_start;
>> +       u64 reserve_end;
>> +       u64 region_start;
>> +       u32 region_size;
>> +       int ret;
>> +
>> +       get_legacy_lowmem_region(uncore, &region_start, &region_size);
>> +       reserve_start = region_start;
>> +       reserve_end = region_start + region_size;
>> +
>> +       if (!reserve_end)
>> +               return 0;
>> +
>> +       DRM_INFO("LMEM: reserving low-memory region [0x%llx-0x%llx]\n",
>> +                reserve_start, reserve_end);
>> +       ret = i915_buddy_alloc_range(&mem->mm, &mem->reserved,
>> +                                    reserve_start,
>> +                                    reserve_end - reserve_start);
> 
> Isn't this now relative to the stolen offset? Should this be reserved,
> or excluded like stolen?

AFAIK stolen is just snipped off at the end of lmem, so I don't think it 
really matters if we exclude or reserve. But for this if we exclude then 
the region.start might have "strange" alignment, which is annoying since 
alloc(some_power_of_two) might not give us the expected alignment, 
whereas if we reserve then the allocator is aware, and so we should get 
the proper alignment. Maybe you have better ideas with how to handle 
this, but I think keeping the alignment property is nice.

> -Chris
>