Re: [PATCH 1/2] drm/xe: Skip over non leaf pte for PRL generation

["Summers, Stuart" <stuart.summers@intel.com>]

On Thu, 2026-01-29 at 08:27 +0000, Brian Nguyen wrote:
> The check using xe_child->base.children was insufficient in
> determining
> if a pte was a leaf node. So explicitly check for if a pte is a leaf
> through the bit checks.
> 
> Fixes: b912138df299 ("drm/xe: Create page reclaim list on unbind")
> 
> v2:
>  - Remove old assert. (Matt R)
> 
> Signed-off-by: Brian Nguyen <brian3.nguyen@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 6703a7049227..b73a356d0fa1 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1655,7 +1655,7 @@ static int xe_pt_stage_unbind_entry(struct
> xe_ptw *parent, pgoff_t offset,
>  
>         XE_WARN_ON(!*child);
>         XE_WARN_ON(!level);
> -       /* Check for leaf node */
> +       /* Optimistically check for leaf node, may not be guaranteed

I would keep the comments the same - we're still trying to check a leaf
node here and we aren't really doing anything special. If we have
questions, we can look at the commit history to determine what changed
from your prior implementation.

Or if you want documentation here, it's more interesting to me *why* we
can't see this from the child alone than just the fact that we can't
(which we can observe by the if condition).

Also applies to the comment below too.

Thanks,
Stuart

> from children alone */
>         if (xe_walk->prl && xe_page_reclaim_list_valid(xe_walk->prl)
> &&
>             (!xe_child->base.children || !xe_child-
> >base.children[first])) {
>                 struct iosys_map *leaf_map = &xe_child->bo->vmap;
> @@ -1675,10 +1675,13 @@ static int xe_pt_stage_unbind_entry(struct
> xe_ptw *parent, pgoff_t offset,
>                                 break;
>                         }
>  
> -                       /* Ensure it is a defined page */
> -                       xe_tile_assert(xe_walk->tile,
> -                                      xe_child->level == 0 ||
> -                                      (pte & (XE_PTE_PS64 |
> XE_PDE_PS_2M | XE_PDPE_PS_1G)));
> +                       /*
> +                        * The check for xe_pt's children is
> insufficient to determine leaf.
> +                        * If not leaf, break out and continue in
> next page walk level.
> +                        */
> +                       if (xe_child->level > 0 &&
> +                           !(pte & (XE_PTE_PS64 | XE_PDE_PS_2M |
> XE_PDPE_PS_1G)))
> +                               break;
>  
>                         /* An entry should be added for 64KB but
> contigious 4K have XE_PTE_PS64 */
>                         if (pte & XE_PTE_PS64)