Message ID | z6xndjwwwnck67qcv2355v5qejq64qldziqg7saint3eqe6fo2@6sx7xyh5juvc (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [RFC] Revert "drm/i915: Disable compression tricks on JSL" | expand |
Hi The bug appeared recently once, and it is possible that it will pop up from time to times, so it might be better to get rid of this workaround from the kernel, especially since it's already in Mesa. I would like to know, what you think about it ?
On Wed, Mar 05, 2025 at 02:49:46PM +0000, Sebastian Brzezinka wrote: > This reverts commit 0ddae025ab6cefa9aba757da3cd1d27908d70b0e. > > According to bspec 14181, CACHE_MODE_0 is a register that's under userspace > control, and DISABLE_REPACKING_FOR_COMPRESSION workaround should be already > in all recent Mesa releases. So, there is no need to include it in kernel. igt doesn't have it. > > Also, this workaround·sporadically fails to load: > ``` > ERROR GT0: engine workaround lost on application! (reg[7000]=0x0, > relevant bits were 0x0 vs expected 0x8000) > ``` If it somehow fails to load from the kernel why would it work from userspace? Hmm, apparently CACHE_MODE_0 needs the mcr steering stuff. Does that fix the verification fail? > > Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 - > drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 --------- > 2 files changed, 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > index 7421ed18d8d1..52ddfa9f3ad3 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > @@ -440,7 +440,6 @@ > #define XEHPG_INSTDONE_GEOM_SVG MCR_REG(0x666c) > > #define CACHE_MODE_0_GEN7 _MMIO(0x7000) /* IVB+ */ > -#define DISABLE_REPACKING_FOR_COMPRESSION REG_BIT(15) /* jsl+ */ > #define RC_OP_FLUSH_ENABLE (1 << 0) > #define HIZ_RAW_STALL_OPT_DISABLE (1 << 2) > #define CACHE_MODE_1 _MMIO(0x7004) /* IVB+ */ > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 116683ebe074..51839f270d57 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2306,15 +2306,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) > GEN8_RC_SEMA_IDLE_MSG_DISABLE); > } > > - if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) { > - /* > - * "Disable Repacking for Compression (masked R/W access) > - * before rendering compressed surfaces for display." > - */ > - wa_masked_en(wal, CACHE_MODE_0_GEN7, > - DISABLE_REPACKING_FOR_COMPRESSION); > - } > - > if (GRAPHICS_VER(i915) == 11) { > /* This is not an Wa. Enable for better image quality */ > wa_masked_en(wal, > -- > 2.34.1
Hi Ville On Wed Mar 5, 2025 at 3:26 PM UTC, Ville Syrjälä wrote: > On Wed, Mar 05, 2025 at 02:49:46PM +0000, Sebastian Brzezinka wrote: >> This reverts commit 0ddae025ab6cefa9aba757da3cd1d27908d70b0e. >> >> According to bspec 14181, CACHE_MODE_0 is a register that's under userspace >> control, and DISABLE_REPACKING_FOR_COMPRESSION workaround should be already >> in all recent Mesa releases. So, there is no need to include it in kernel. > > igt doesn't have it. > >> >> Also, this workaround·sporadically fails to load: >> ``` >> ERROR GT0: engine workaround lost on application! (reg[7000]=0x0, >> relevant bits were 0x0 vs expected 0x8000) >> ``` > > If it somehow fails to load from the kernel why would it > work from userspace? > > Hmm, apparently CACHE_MODE_0 needs the mcr steering stuff. > Does that fix the verification fail? Thanks for sugestion. Right now I think that I try to move this wa to icl_ctx_workarounds_init as both Mat and Chriss notice that register is a part of the context.
On Wed, Mar 05, 2025 at 05:00:26PM +0000, Sebastian Brzezinka wrote: > Hi Ville > > On Wed Mar 5, 2025 at 3:26 PM UTC, Ville Syrjälä wrote: > > On Wed, Mar 05, 2025 at 02:49:46PM +0000, Sebastian Brzezinka wrote: > >> This reverts commit 0ddae025ab6cefa9aba757da3cd1d27908d70b0e. > >> > >> According to bspec 14181, CACHE_MODE_0 is a register that's under userspace > >> control, and DISABLE_REPACKING_FOR_COMPRESSION workaround should be already > >> in all recent Mesa releases. So, there is no need to include it in kernel. > > > > igt doesn't have it. > > > >> > >> Also, this workaround·sporadically fails to load: > >> ``` > >> ERROR GT0: engine workaround lost on application! (reg[7000]=0x0, > >> relevant bits were 0x0 vs expected 0x8000) > >> ``` > > > > If it somehow fails to load from the kernel why would it > > work from userspace? > > > > Hmm, apparently CACHE_MODE_0 needs the mcr steering stuff. > > Does that fix the verification fail? > > Thanks for sugestion. Right now I think that I try to move this wa to > icl_ctx_workarounds_init as both Mat and Chriss notice that register > is a part of the context. Hmm, didn't realize there was a separate list for that. It looks to me like there are a bunch of context saved registers handled in the enging_ctx() stuff currently. I think someone needs to go through all this stuff and relocate all the registers to their correct spots.
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 7421ed18d8d1..52ddfa9f3ad3 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -440,7 +440,6 @@ #define XEHPG_INSTDONE_GEOM_SVG MCR_REG(0x666c) #define CACHE_MODE_0_GEN7 _MMIO(0x7000) /* IVB+ */ -#define DISABLE_REPACKING_FOR_COMPRESSION REG_BIT(15) /* jsl+ */ #define RC_OP_FLUSH_ENABLE (1 << 0) #define HIZ_RAW_STALL_OPT_DISABLE (1 << 2) #define CACHE_MODE_1 _MMIO(0x7004) /* IVB+ */ diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 116683ebe074..51839f270d57 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -2306,15 +2306,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) GEN8_RC_SEMA_IDLE_MSG_DISABLE); } - if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) { - /* - * "Disable Repacking for Compression (masked R/W access) - * before rendering compressed surfaces for display." - */ - wa_masked_en(wal, CACHE_MODE_0_GEN7, - DISABLE_REPACKING_FOR_COMPRESSION); - } - if (GRAPHICS_VER(i915) == 11) { /* This is not an Wa. Enable for better image quality */ wa_masked_en(wal,
This reverts commit 0ddae025ab6cefa9aba757da3cd1d27908d70b0e. According to bspec 14181, CACHE_MODE_0 is a register that's under userspace control, and DISABLE_REPACKING_FOR_COMPRESSION workaround should be already in all recent Mesa releases. So, there is no need to include it in kernel. Also, this workaround·sporadically fails to load: ``` ERROR GT0: engine workaround lost on application! (reg[7000]=0x0, relevant bits were 0x0 vs expected 0x8000) ``` Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 - drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 --------- 2 files changed, 10 deletions(-)