On Wed, Jul 15, 2026 at 01:34:28PM +0200, Maarten Lankhorst wrote: > > > On 7/15/26 13:05, Maarten Lankhorst wrote: > > The order of pte vs checks isn't important, so read the pte > > outside the if block. This makes it slightly more readable. > > > > Signed-off-by: Maarten Lankhorst <[email protected]> > > --- > > drivers/gpu/drm/xe/display/xe_initial_plane.c | 35 ++++++------------- > > 1 file changed, 11 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c > > b/drivers/gpu/drm/xe/display/xe_initial_plane.c > > index 5540b0fca392a..e16a6a1e6288a 100644 > > --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c > > +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c > > @@ -64,7 +64,7 @@ initial_plane_bo(struct xe_device *xe, > > struct xe_bo *bo; > > resource_size_t phys_base; > > u32 base, size, flags; > > - u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : > > SZ_4K; > > + u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : > > SZ_4K, pte; > > struct xe_ggtt_node *original_ggtt_node; > > > > if (plane_config->size == 0) > > @@ -77,16 +77,14 @@ initial_plane_bo(struct xe_device *xe, > > page_size); > > size -= base; > > > > - if (IS_DGFX(xe)) { > > - u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base); > > - > > - if (is_pte_local(pte) != need_pte_local(xe)) { > > - drm_err(&xe->drm, "Initial plane PTE has bad local > > memory bit\n"); > > - return NULL; > > - } > > - > > - phys_base = pte & ~(page_size - 1); > > + pte = xe_ggtt_read_pte(tile0->mem.ggtt, base); > > + phys_base = pte & ~(page_size - 1); > > + if (is_pte_local(pte) != need_pte_local(xe)) { > > + drm_err(&xe->drm, "Initial plane PTE has bad local memory > > bit\n"); > > + return NULL; > > + } > > > > + if (IS_DGFX(xe)) { > > flags |= XE_BO_FLAG_VRAM0; > > > > /* > > @@ -104,25 +102,14 @@ initial_plane_bo(struct xe_device *xe, > > "Using phys_base=%pa, based on initial plane > > programming\n", > > &phys_base); > > } else { > > - struct ttm_resource_manager *stolen; > > - u64 pte; > > + flags |= XE_BO_FLAG_STOLEN; > > + phys_base -= xe_ttm_stolen_gpu_offset(xe); > > > > - stolen = ttm_manager_type(&xe->ttm, XE_PL_STOLEN); > > - if (!stolen) { > > + if (!ttm_manager_type(&xe->ttm, XE_PL_STOLEN)) { > > drm_dbg_kms(&xe->drm, "No stolen for initial FB\n"); > > return NULL; > > } > > > > Woops, phys_base adjustment should be after !stolen check, > result should be the same though.
I would still like to see the dma_addr/phys_base/ggtt offset (old and new) debug stuff from https://patchwork.freedesktop.org/patch/724207/?series=166362&rev=1 sucked in as well, as a separate patch. It's useful to see all of that when debugging this stuff... But this patch seems fine, with the reordering to avoid oopsing if stolen isn't there Reviewed-by: Ville Syrjälä <[email protected]> > > > - pte = xe_ggtt_read_pte(tile0->mem.ggtt, base); > > - > > - if (is_pte_local(pte) != need_pte_local(xe)) { > > - drm_err(&xe->drm, "Initial plane PTE has bad local > > memory bit\n"); > > - return NULL; > > - } > > - > > - phys_base = (pte & ~(page_size - 1)) - > > xe_ttm_stolen_gpu_offset(xe); > > - flags |= XE_BO_FLAG_STOLEN; > > - > > if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) && > > IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) && > > !xe_display_bo_fbdev_prefer_stolen(xe, plane_config->size)) > > { -- Ville Syrjälä Intel
