On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Declutter initial_plane_vma() a bit by pulling the lmem and smem
readout paths into their own functions.
TODO: the smem path should still be fixed to get and validate
the dma address from the pte as well
Cc: Paz Zcharya <pazz@xxxxxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I am not sure about this split, split suggests the paths significantly
differs, but they differ just by 2 things:
- mem region,
- assumption about 1:1 mapping for older platforms.
Btw I was wondering if wouldn't be good to abstract out pte retrieval,
as the pattern "ggtt->gsm + offset / I915_GTT_PAGE_SIZE" is present in
multiple places and depends on hw gen, but maybe it is another patch.
No strong feelings.
Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
Regards
Andrzej
Regards
Andrzej
---
.../drm/i915/display/intel_display_types.h | 2 +
.../drm/i915/display/intel_plane_initial.c | 145 +++++++++++-------
2 files changed, 95 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 341d80c2b9de..d2b0cc754667 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -782,6 +782,8 @@ struct intel_plane_state {
struct intel_initial_plane_config {
struct intel_framebuffer *fb;
+ struct intel_memory_region *mem;
+ resource_size_t phys_base;
struct i915_vma *vma;
unsigned int tiling;
int size;
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index 48b74319f45c..78bff6181ceb 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -44,6 +44,93 @@ intel_reuse_initial_plane_obj(struct drm_i915_private *i915,
return false;
}
+static bool
+initial_plane_phys_lmem(struct drm_i915_private *i915,
+ struct intel_initial_plane_config *plane_config)
+{
+ gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
+ struct intel_memory_region *mem;
+ dma_addr_t dma_addr;
+ gen8_pte_t pte;
+ u32 base;
+
+ base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
+
+ 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 false;
+ }
+
+ dma_addr = pte & GEN12_GGTT_PTE_ADDR_MASK;
+
+ if (IS_DGFX(i915))
+ mem = i915->mm.regions[INTEL_REGION_LMEM_0];
+ else
+ mem = i915->mm.stolen_region;
+ if (!mem) {
+ drm_dbg_kms(&i915->drm,
+ "Initial plane memory region not initialized\n");
+ return false;
+ }
+
+ /*
+ * On lmem we don't currently expect this to
+ * ever be placed in the stolen portion.
+ */
+ if (dma_addr < mem->region.start || dma_addr > mem->region.end) {
+ drm_err(&i915->drm,
+ "Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n",
+ &dma_addr, mem->region.name, &mem->region.start, &mem->region.end);
+ return false;
+ }
+
+ drm_dbg(&i915->drm,
+ "Using dma_addr=%pa, based on initial plane programming\n",
+ &dma_addr);
+
+ plane_config->phys_base = dma_addr - mem->region.start;
+ plane_config->mem = mem;
+
+ return true;
+}
+
+static bool
+initial_plane_phys_smem(struct drm_i915_private *i915,
+ struct intel_initial_plane_config *plane_config)
+{
+ struct intel_memory_region *mem;
+ u32 base;
+
+ base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
+
+ mem = i915->mm.stolen_region;
+ if (!mem) {
+ drm_dbg_kms(&i915->drm,
+ "Initial plane memory region not initialized\n");
+ return false;
+ }
+
+ /* FIXME get and validate the dma_addr from the PTE */
+ plane_config->phys_base = base;
+ plane_config->mem = mem;
+
+ return true;
+}
+
+static bool
+initial_plane_phys(struct drm_i915_private *i915,
+ struct intel_initial_plane_config *plane_config)
+{
+ if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915))
+ return initial_plane_phys_lmem(i915, plane_config);
+ else
+ return initial_plane_phys_smem(i915, plane_config);
+}
+
static struct i915_vma *
initial_plane_vma(struct drm_i915_private *i915,
struct intel_initial_plane_config *plane_config)
@@ -58,59 +145,13 @@ initial_plane_vma(struct drm_i915_private *i915,
if (plane_config->size == 0)
return NULL;
+ if (!initial_plane_phys(i915, plane_config))
+ return NULL;
+
+ phys_base = plane_config->phys_base;
+ mem = plane_config->mem;
+
base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
- if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915)) {
- gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
- dma_addr_t dma_addr;
- 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;
- }
-
- dma_addr = pte & GEN12_GGTT_PTE_ADDR_MASK;
-
- if (IS_DGFX(i915))
- mem = i915->mm.regions[INTEL_REGION_LMEM_0];
- else
- mem = i915->mm.stolen_region;
- if (!mem) {
- drm_dbg_kms(&i915->drm,
- "Initial plane memory region not initialized\n");
- return NULL;
- }
-
- /*
- * On lmem we don't currently expect this to
- * ever be placed in the stolen portion.
- */
- if (dma_addr < mem->region.start || dma_addr > mem->region.end) {
- drm_err(&i915->drm,
- "Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n",
- &dma_addr, mem->region.name, &mem->region.start, &mem->region.end);
- return NULL;
- }
-
- drm_dbg(&i915->drm,
- "Using dma_addr=%pa, based on initial plane programming\n",
- &dma_addr);
-
- phys_base = dma_addr - mem->region.start;
- } else {
- phys_base = base;
- mem = i915->mm.stolen_region;
- if (!mem) {
- drm_dbg_kms(&i915->drm,
- "Initial plane memory region not initialized\n");
- return NULL;
- }
- }
-
size = round_up(plane_config->base + plane_config->size,
mem->min_page_size);
size -= base;
