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

On 27/11/2020 13:52, Chris Wilson wrote:

Quoting Matthew Auld (2020-11-27 12:06:34)

...

From: Imre Deak imre.deak@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@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