Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
On Fri, Aug 25, 2017 at 09:13:18AM -0700, Oscar Mateo wrote: > > > On 08/25/2017 12:41 AM, Michał Winiarski wrote: > > On Thu, Aug 24, 2017 at 03:42:05PM -0700, Oscar Mateo wrote: > > > > > > On 08/23/2017 03:02 PM, Rodrigo Vivi wrote: > > > > Must Force Non-Coherent whenever executing a 3D context. > > > > This is a workaround for a possible hang in the unlikely event > > > >a TLB invalidation occurs during a PSD flush. > > > > Set masked bit 4 in 0x7300 during boot. > > > This bug should not be present in HW anymore. A different reason to keep > > > doing this is performance, though. > > Doesn't userspace already has a choice between coherent and non-coherent > > access. > > Why would we want to cheat, and force non-coherent when it clearly wants to > > use > > coherent one? > > But does it? I cannot find anything in i915 to allow userspace to flip this > chicken bit (the register is not whitelisted and we don't have any execbuf > flag/hint). Or am I missing something? It's a hardware interface, not i915 interface. See: IHD-OS-SKL-Vol7-05.16 Shared Functions -> Overview of memory accesses (Cache behavior) (page 144) https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol07-3d_media_gpgpu.pdf The workaround is removing the ability to choose coherency model - even if userspace is opting in for coherent path, the HW will use non-coherent one. -Michał > > > (well, because that's what we've been always doing is one reason, the other > > one > > may be that "clearly" can sometimes turn out to be "by accident") > > > > -Michał > > > > > > Cc: Mika Kuoppala > > > > Cc: Oscar Mateo > > > > Signed-off-by: Rodrigo Vivi > > > > --- > > > >drivers/gpu/drm/i915/intel_engine_cs.c | 5 - > > > >1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > index a6ac9d0a4156..7dfc78b038c4 100644 > > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct > > > > intel_engine_cs *engine) > > > > int ret; > > > > /* WaForceContextSaveRestoreNonCoherent:cnl */ > > > > + /* WaForceEnableNonCoherent:cnl */ > > > > WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, > > > > - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); > > > > + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | > > > > + HDC_FORCE_NON_COHERENT); > > > > + > > > > /* WaDisableReplayBufferBankArbitrationOptimization:cnl */ > > > > WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, > > > ___ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
On 08/25/2017 12:41 AM, Michał Winiarski wrote: On Thu, Aug 24, 2017 at 03:42:05PM -0700, Oscar Mateo wrote: On 08/23/2017 03:02 PM, Rodrigo Vivi wrote: Must Force Non-Coherent whenever executing a 3D context. This is a workaround for a possible hang in the unlikely event a TLB invalidation occurs during a PSD flush. Set masked bit 4 in 0x7300 during boot. This bug should not be present in HW anymore. A different reason to keep doing this is performance, though. Doesn't userspace already has a choice between coherent and non-coherent access. Why would we want to cheat, and force non-coherent when it clearly wants to use coherent one? But does it? I cannot find anything in i915 to allow userspace to flip this chicken bit (the register is not whitelisted and we don't have any execbuf flag/hint). Or am I missing something? (well, because that's what we've been always doing is one reason, the other one may be that "clearly" can sometimes turn out to be "by accident") -Michał Cc: Mika Kuoppala Cc: Oscar Mateo Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a6ac9d0a4156..7dfc78b038c4 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaForceContextSaveRestoreNonCoherent:cnl */ + /* WaForceEnableNonCoherent:cnl */ WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | + HDC_FORCE_NON_COHERENT); + /* WaDisableReplayBufferBankArbitrationOptimization:cnl */ WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
Michał Winiarski writes: > On Thu, Aug 24, 2017 at 03:42:05PM -0700, Oscar Mateo wrote: >> >> >> On 08/23/2017 03:02 PM, Rodrigo Vivi wrote: >> > Must Force Non-Coherent whenever executing a 3D context. >> > This is a workaround for a possible hang in the unlikely event >> > a TLB invalidation occurs during a PSD flush. >> > Set masked bit 4 in 0x7300 during boot. >> >> This bug should not be present in HW anymore. A different reason to keep >> doing this is performance, though. > > Doesn't userspace already has a choice between coherent and non-coherent > access. > Why would we want to cheat, and force non-coherent when it clearly wants to > use > coherent one? > (well, because that's what we've been always doing is one reason, the other > one > may be that "clearly" can sometimes turn out to be "by accident") > Caveat for below is that I haven't investigated the cnl particulars. But we did end up having system hangs on skl in the past by mismatching this, see commit e238659ddd889a5d3fbdfa1a2ab120f90404bf41 -Mika > -Michał > >> >> > Cc: Mika Kuoppala >> > Cc: Oscar Mateo >> > Signed-off-by: Rodrigo Vivi >> > --- >> > drivers/gpu/drm/i915/intel_engine_cs.c | 5 - >> > 1 file changed, 4 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >> > b/drivers/gpu/drm/i915/intel_engine_cs.c >> > index a6ac9d0a4156..7dfc78b038c4 100644 >> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> > @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct >> > intel_engine_cs *engine) >> >int ret; >> >/* WaForceContextSaveRestoreNonCoherent:cnl */ >> > + /* WaForceEnableNonCoherent:cnl */ >> >WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, >> > -HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); >> > +HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | >> > +HDC_FORCE_NON_COHERENT); >> > + >> >/* WaDisableReplayBufferBankArbitrationOptimization:cnl */ >> >WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
On Thu, Aug 24, 2017 at 03:42:05PM -0700, Oscar Mateo wrote: > > > On 08/23/2017 03:02 PM, Rodrigo Vivi wrote: > > Must Force Non-Coherent whenever executing a 3D context. > > This is a workaround for a possible hang in the unlikely event > > a TLB invalidation occurs during a PSD flush. > > Set masked bit 4 in 0x7300 during boot. > > This bug should not be present in HW anymore. A different reason to keep > doing this is performance, though. Doesn't userspace already has a choice between coherent and non-coherent access. Why would we want to cheat, and force non-coherent when it clearly wants to use coherent one? (well, because that's what we've been always doing is one reason, the other one may be that "clearly" can sometimes turn out to be "by accident") -Michał > > > Cc: Mika Kuoppala > > Cc: Oscar Mateo > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_engine_cs.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > index a6ac9d0a4156..7dfc78b038c4 100644 > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct > > intel_engine_cs *engine) > > int ret; > > /* WaForceContextSaveRestoreNonCoherent:cnl */ > > + /* WaForceEnableNonCoherent:cnl */ > > WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, > > - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); > > + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | > > + HDC_FORCE_NON_COHERENT); > > + > > /* WaDisableReplayBufferBankArbitrationOptimization:cnl */ > > WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
On 08/23/2017 03:02 PM, Rodrigo Vivi wrote: Must Force Non-Coherent whenever executing a 3D context. This is a workaround for a possible hang in the unlikely event a TLB invalidation occurs during a PSD flush. Set masked bit 4 in 0x7300 during boot. This bug should not be present in HW anymore. A different reason to keep doing this is performance, though. Cc: Mika Kuoppala Cc: Oscar Mateo Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a6ac9d0a4156..7dfc78b038c4 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaForceContextSaveRestoreNonCoherent:cnl */ + /* WaForceEnableNonCoherent:cnl */ WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | + HDC_FORCE_NON_COHERENT); + /* WaDisableReplayBufferBankArbitrationOptimization:cnl */ WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/cnl: WaForceEnableNonCoherent
Must Force Non-Coherent whenever executing a 3D context. This is a workaround for a possible hang in the unlikely event a TLB invalidation occurs during a PSD flush. Set masked bit 4 in 0x7300 during boot. Cc: Mika Kuoppala Cc: Oscar Mateo Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a6ac9d0a4156..7dfc78b038c4 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1071,8 +1071,11 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaForceContextSaveRestoreNonCoherent:cnl */ + /* WaForceEnableNonCoherent:cnl */ WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0, - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | + HDC_FORCE_NON_COHERENT); + /* WaDisableReplayBufferBankArbitrationOptimization:cnl */ WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
