Re: [PATCH v2 5/6] drm/xe/display: Remove duplicated code

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