[v2] drm/i915/display: Check GGTT to determine phys_base
Commit Message
There was an assumption that for iGPU there should be a 1:1 mapping
of GGTT to physical address pointing to the framebuffer.
This assumption is not strictly true effective generation 8 or newer.
Fix that by checking GGTT to determine the phys address on gen8+.
The following algorithm for phys_base should be valid for all platforms:
1. Find pte
2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915->mm.stolen_region
3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
- On older platforms, stolen_region points to system memory, starting at 0
- on DG2, it uses lmem region which starts at 0 as well
- on MTL, stolen_region points to stolen-local which starts at 0x800000
Changes from v1:
- Add an if statement for gen7-, where there is a 1:1 mapping
Signed-off-by: Paz Zcharya <pazz@chromium.org>
---
.../drm/i915/display/intel_plane_initial.c | 64 +++++++++++--------
1 file changed, 39 insertions(+), 25 deletions(-)
Comments
Thank You for the patch. We noticed a break in the customer board with
the latest GOP + this patch.
Thank You
Khaled
On Wed, 2023-12-06 at 18:46 +0000, Paz Zcharya wrote:
> There was an assumption that for iGPU there should be a 1:1 mapping
> of GGTT to physical address pointing to the framebuffer.
> This assumption is not strictly true effective generation 8 or newer.
> Fix that by checking GGTT to determine the phys address on gen8+.
>
> The following algorithm for phys_base should be valid for all
> platforms:
> 1. Find pte
> 2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
> i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915-
> >mm.stolen_region
> 3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
>
> - On older platforms, stolen_region points to system memory, starting
> at 0
> - on DG2, it uses lmem region which starts at 0 as well
> - on MTL, stolen_region points to stolen-local which starts at
> 0x800000
>
> Changes from v1:
> - Add an if statement for gen7-, where there is a 1:1 mapping
>
> Signed-off-by: Paz Zcharya <pazz@chromium.org>
> ---
>
> .../drm/i915/display/intel_plane_initial.c | 64 +++++++++++----
> ----
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index a55c09cbd0e4..7d9bb631b93b 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -59,44 +59,58 @@ initial_plane_vma(struct drm_i915_private *i915,
> return NULL;
>
> base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
> - if (IS_DGFX(i915)) {
> +
> + if (GRAPHICS_VER(i915) < 8) {
> + /*
> + * In gen7-, there is a 1:1 mapping
> + * between GSM and physical address.
> + */
> + phys_base = base;
> + mem = i915->mm.stolen_region;
> + } else {
> + /*
> + * In gen8+, there is no 1:1 mapping between
> + * GSM and physical address, so we need to
> + * check GGTT to determine the physical address.
> + */
> gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
> gen8_pte_t pte;
>
> gte += base / I915_GTT_PAGE_SIZE;
> -
> pte = ioread64(gte);
> - if (!(pte & GEN12_GGTT_PTE_LM)) {
> - drm_err(&i915->drm,
> - "Initial plane programming missing
> PTE_LM bit\n");
> - return NULL;
> - }
> -
> - phys_base = pte & I915_GTT_PAGE_MASK;
> - mem = i915->mm.regions[INTEL_REGION_LMEM_0];
>
> - /*
> - * We don't currently expect this to ever be placed in
> the
> - * stolen portion.
> - */
> - if (phys_base >= resource_size(&mem->region)) {
> - drm_err(&i915->drm,
> - "Initial plane programming using
> invalid range, phys_base=%pa\n",
> - &phys_base);
> - return NULL;
> + if (IS_DGFX(i915)) {
> + if (!(pte & GEN12_GGTT_PTE_LM)) {
> + drm_err(&i915->drm,
> + "Initial plane programming
> missing PTE_LM bit\n");
> + return NULL;
> + }
> + mem = i915->mm.regions[INTEL_REGION_LMEM_0];
> + } else {
> + mem = i915->mm.stolen_region;
> }
>
> - drm_dbg(&i915->drm,
> - "Using phys_base=%pa, based on initial plane
> programming\n",
> - &phys_base);
> - } else {
> - phys_base = base;
> - mem = i915->mm.stolen_region;
> + phys_base = (pte & I915_GTT_PAGE_MASK) - mem-
> >region.start;
> }
>
> if (!mem)
> return NULL;
>
> + /*
> + * We don't currently expect this to ever be placed in the
> + * stolen portion.
> + */
> + if (phys_base >= resource_size(&mem->region)) {
> + drm_err(&i915->drm,
> + "Initial plane programming using invalid range,
> phys_base=%pa\n",
> + &phys_base);
> + return NULL;
> + }
> +
> + drm_dbg(&i915->drm,
> + "Using phys_base=%pa, based on initial plane
> programming\n",
> + &phys_base);
> +
> size = round_up(plane_config->base + plane_config->size,
> mem->min_page_size);
> size -= base;
@@ -59,44 +59,58 @@ initial_plane_vma(struct drm_i915_private *i915,
return NULL;
base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
- if (IS_DGFX(i915)) {
+
+ if (GRAPHICS_VER(i915) < 8) {
+ /*
+ * In gen7-, there is a 1:1 mapping
+ * between GSM and physical address.
+ */
+ phys_base = base;
+ mem = i915->mm.stolen_region;
+ } else {
+ /*
+ * In gen8+, there is no 1:1 mapping between
+ * GSM and physical address, so we need to
+ * check GGTT to determine the physical address.
+ */
gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
gen8_pte_t pte;
gte += base / I915_GTT_PAGE_SIZE;
-
pte = ioread64(gte);
- if (!(pte & GEN12_GGTT_PTE_LM)) {
- drm_err(&i915->drm,
- "Initial plane programming missing PTE_LM bit\n");
- return NULL;
- }
-
- phys_base = pte & I915_GTT_PAGE_MASK;
- mem = i915->mm.regions[INTEL_REGION_LMEM_0];
- /*
- * We don't currently expect this to ever be placed in the
- * stolen portion.
- */
- if (phys_base >= resource_size(&mem->region)) {
- drm_err(&i915->drm,
- "Initial plane programming using invalid range, phys_base=%pa\n",
- &phys_base);
- return NULL;
+ if (IS_DGFX(i915)) {
+ if (!(pte & GEN12_GGTT_PTE_LM)) {
+ drm_err(&i915->drm,
+ "Initial plane programming missing PTE_LM bit\n");
+ return NULL;
+ }
+ mem = i915->mm.regions[INTEL_REGION_LMEM_0];
+ } else {
+ mem = i915->mm.stolen_region;
}
- drm_dbg(&i915->drm,
- "Using phys_base=%pa, based on initial plane programming\n",
- &phys_base);
- } else {
- phys_base = base;
- mem = i915->mm.stolen_region;
+ phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
}
if (!mem)
return NULL;
+ /*
+ * We don't currently expect this to ever be placed in the
+ * stolen portion.
+ */
+ if (phys_base >= resource_size(&mem->region)) {
+ drm_err(&i915->drm,
+ "Initial plane programming using invalid range, phys_base=%pa\n",
+ &phys_base);
+ return NULL;
+ }
+
+ drm_dbg(&i915->drm,
+ "Using phys_base=%pa, based on initial plane programming\n",
+ &phys_base);
+
size = round_up(plane_config->base + plane_config->size,
mem->min_page_size);
size -= base;