On Wed, Nov 01, 2017 at 02:11:05PM -0700, Rodrigo Vivi wrote: > 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.
I would say we need at least the GEN10_READ_HIT_WRITEONLY_DISABLE bit, and then we can decide if we are adding the other one or not. Within the same program (I think sometimes even within a single draw call), it's possible that we have some surfaces that are multisampled while others are not, so in that case we would probably have to set both anyway. However, I don't have a test case that proves that the workaround for the multisampled case is required... > > 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. Oh, I haven't noticed that! Will fix it in the next version... > > #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
