Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] xe/lrc: Mismatched platform condition applies ADL workaround to all Gen12 - [High] xe/lrc: Hardware defaults corrupted by hardcoded RCS offset for XCS engines --
commit 6eb083cb06a370d43050c04bc2d40604dc4f058f Author: Helen Koike <[email protected]> drm/xe/lrc: Restore CTX_CS_INDIRECT_CTX_OFFSET programming for ADL CTX_CS_INDIRECT_CTX_OFFSET default value is not retrieved from the GPU by inhibit+context save mechanism. Thus, at restore, 0x0 is programed back to the GPU, which causes sporadic hangs on Alder Lake. Fix it by partially reverting commit c9dfd66cb91e and re-adding the programming of CTX_CS_INDIRECT_CTX_OFFSET for Alder Lake. > diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c > index bf5c2351aed3d..95285ae069290 100644 > --- a/drivers/gpu/drm/xe/xe_lrc.c > +++ b/drivers/gpu/drm/xe/xe_lrc.c [ ... ] > @@ -1459,13 +1459,18 @@ setup_indirect_ctx(struct xe_lrc *lrc, struct > xe_hw_engine *hwe) > > /* > * Enable INDIRECT_CTX leaving INDIRECT_CTX_OFFSET at its default: it > - * varies per engine class, but the default is good enough > + * varies per engine class, but the default is good enough, except on > + * Alder Lake. > */ > xe_lrc_write_ctx_reg(lrc, > CTX_CS_INDIRECT_CTX, > (xe_bo_ggtt_addr(lrc->bo) + state.offset) | > /* Size in CLs. */ > (state.written * sizeof(u32) / 64)); > + if (GRAPHICS_VER(lrc_to_xe(lrc)) < 20) [Severity: High] Does this condition apply the ADL-specific fix to all platforms with graphics version < 20? The commit message and comments explicitly state the intention to limit this workaround exclusively to Alder Lake. However, checking for graphics version < 20 evaluates to true for all Gen12 platforms, including DG2, MTL, and PVC, during LRC context setup in setup_indirect_ctx(). Could this cause hardware misconfiguration on non-ADL platforms that may not expect or support the CTX_CS_INDIRECT_CTX_OFFSET rewrite, potentially leading to sporadic hangs or context corruption? > + xe_lrc_write_ctx_reg(lrc, > + CTX_CS_INDIRECT_CTX_OFFSET, > + CTX_INDIRECT_CTX_OFFSET_DEFAULT); [Severity: High] Does this code write a hardcoded, RCS-specific default value to all engines that have INDIRECT_CTX enabled, including non-RCS engines which require a different offset? If a non-RCS/CCS engine has indirect context enabled (for example, via userspace configfs enabling context restore ctx_restore_mid_bb for XCS engines), setup_indirect_ctx() executes and unconditionally writes CTX_INDIRECT_CTX_OFFSET_DEFAULT (0xD) without checking the engine class. As noted in commit c9dfd66cb91e, the correct offset varies per engine class. Could forcing the RCS-specific 0xD on Video or Copy engines overwrite their native hardware defaults and cause GPU hangs or context restoration failures? > > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
