Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The patch removes the validation from the integrated graphics path, rendering the `has_lmembar()` logic completely dead and meaningless. --
commit b290f7e32abe43d35ac4f5def3b10e9bcf641a88 Author: Andrew Zhou <[email protected]> drm/xe/display: Remove invalid initial plane PTE check for iGPU This commit removes the initial plane PTE local memory validation for integrated graphics, as Pantherlake can have the XE_GGTT_PTEDM bit set. The check was causing a failure to create the initial framebuffer. > diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c > b/drivers/gpu/drm/xe/display/xe_initial_plane.c > index 0f86b73036d03..8c95dc4e6f2ef 100644 > --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c > +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c [ ... ] > @@ -93,13 +92,6 @@ initial_plane_bo(struct xe_device *xe, > return NULL; > } > > - 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; > - } > - [Severity: Low] Does this leave need_pte_local() with dead logic? By removing this validation from the integrated graphics path (!IS_DGFX(xe)), the only remaining caller of need_pte_local() is inside the earlier if (IS_DGFX(xe)) branch in initial_plane_bo(). Because need_pte_local() is defined as: static bool need_pte_local(struct xe_device *xe) { return IS_DGFX(xe) || has_lmembar(xe); } It will now unconditionally evaluate to true when called, rendering the has_lmembar(xe) check completely meaningless. Should need_pte_local() and its underlying logic be simplified or removed as well to prevent confusion? > phys_base = base; > flags |= XE_BO_FLAG_STOLEN; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
