On Wed, Nov 01, 2017 at 04:32:35PM +0000, Rafael Antognolli wrote: > The workaround for this is described as: > > "if RenderSurfaceState.Num_Multisamples > 1, disable RCC clock gating if > RenderSurfaceState.Num_Multisamples == 1, set 0x7010[14] = 1" > > So it looks like the userspace should be responsible for setting these, > based on the number of multisamples dependency. However, the register > that controls RCC clock gating is not a context register, and cannot be > set by userspace. > > Since we would end up setting one or another based on the number of > multisamples anyway, it seems harmless to just set both all the time. > > This change (specially the GEN10_READ_HIT_WRITEONLY_DISABLE bit)
I wonder if we shouldn't stay only with this bit. For me it looks like one or another. > improves CNL stability by avoiding some of the hangs seen in the > platform. But this is what matters. If this is the safest option for us let's do it. > > Signed-off-by: Rafael Antognolli <[email protected]> > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8c775e96b4e4..d9a65cebefaa 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3837,6 +3837,7 @@ enum { > */ > #define SLICE_UNIT_LEVEL_CLKGATE _MMIO(0x94d4) > #define SARBUNIT_CLKGATE_DIS (1 << 5) > +#define RCCUNIT_CLKGATE_DIS (1 << 7) > > /* > * Display engine regs > @@ -7016,6 +7017,7 @@ enum { > #define GEN7_COMMON_SLICE_CHICKEN1 _MMIO(0x7010) > # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26)) > # define GEN9_RHWO_OPTIMIZATION_DISABLE (1<<14) > +# define GEN10_READ_HIT_WRITEONLY_DISABLE (1<<14) I don't believe you need to redefine this. It is same as GEN9_RHWO_OPTIMIZATION_DISABLE. RCC Read Hit Write Only Optimization Disabled, SKL+ o spec. > #define COMMON_SLICE_CHICKEN2 _MMIO(0x7014) > # define GEN9_PBE_COMPRESSED_HASH_SELECTION (1<<13) > # define GEN9_DISABLE_GATHER_AT_SET_SHADER_COMMON_SLICE (1<<12) > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c > index f31f2d6384c3..0d8e25a4623a 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1320,6 +1320,11 @@ static int cnl_init_workarounds(struct intel_engine_cs > *engine) > WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, GEN9_PREEMPT_GPGPU_LEVEL_MASK, > GEN9_PREEMPT_GPGPU_COMMAND_LEVEL); > > + /* ReadHitWriteOnlyDisable: cnl */ I was going to complain about the name but I saw on bspec it is really ReadHitWriteOnlyDisable while on wa_database it is WaReadHitWriteOnlyDisable I would tend to prefer the second one, but with the first one the search on Bspec works and search on wa_database also works... while second one it would be found on BSpec. So let it be: ReadHitWriteOnlyDisable Thanks, Rodrigo. > + WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1, > + GEN10_READ_HIT_WRITEONLY_DISABLE); > + WA_SET_BIT_MASKED(SLICE_UNIT_LEVEL_CLKGATE, RCCUNIT_CLKGATE_DIS); > + > /* WaEnablePreemptionGranularityControlByUMD:cnl */ > I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1, > _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL)); > -- > 2.13.6 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
