| Message ID | 20190418100634.984-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/icl: Whitelist GEN9_SLICE_COMMON_ECO_CHICKEN1 | expand |
On 18/04/2019 18:06, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > WaEnableStateCacheRedirectToCS context workaround configures the L3 cache > to benefit 3d workloads but media has different requirements. > > Remove the workaround and whitelist the register to allow any userspace > configure the behaviour to their liking. > > v2: > * Remove the workaround apart from adding the whitelist. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: kevin.ma@intel.com > Cc: xiaogang.li@intel.com Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Mesa commits : commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) commit d1be67db39463b48369cb71979ed18662b2c157e (iris) > --- > drivers/gpu/drm/i915/intel_workarounds.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index b3cbed1ee1c9..baed186724d2 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -556,10 +556,6 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, > GEN11_TDL_CLOCK_GATING_FIX_DISABLE); > > - /* WaEnableStateCacheRedirectToCS:icl */ > - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN1, > - GEN11_STATE_CACHE_REDIRECT_TO_CS); > - > /* Wa_2006665173:icl (pre-prod) */ > if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0)) > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > @@ -1070,6 +1066,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > /* WaAllowUMDToModifySamplerMode:icl */ > whitelist_reg(w, GEN10_SAMPLER_MODE); > + > + /* WaEnableStateCacheRedirectToCS:icl */ > + whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1); > } > > void intel_engine_init_whitelist(struct intel_engine_cs *engine)
+ Anuj Quoting Lionel Landwerlin (2019-04-26 11:13:58) > On 18/04/2019 18:06, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > > WaEnableStateCacheRedirectToCS context workaround configures the L3 cache > > to benefit 3d workloads but media has different requirements. > > > > Remove the workaround and whitelist the register to allow any userspace > > configure the behaviour to their liking. > > > > v2: > > * Remove the workaround apart from adding the whitelist. > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Cc: kevin.ma@intel.com > > Cc: xiaogang.li@intel.com > > > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > > Mesa commits : > > commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) > > commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) > > commit d1be67db39463b48369cb71979ed18662b2c157e (iris) Could somebody confirm that applying this patch does not cause hangs in older mesa, and the performance drop (if any) is insignificant? Best Regards, Joonas
Joonas, Mesa now applies this WA on ICL and we're not seeing any regressions in CI. I tested Mesa with and without this patch applied to kernel. I don't see any performance impact to Manhattan from GfxBench5. I'm little surprised to see it's not really helping benchmark performance in Mesa. I'll dig bit more to figure out a possible explanation. I haven't tried any other benchmarks with this patch. Thanks Anuj On 04/26/2019 01:31 AM, Joonas Lahtinen wrote: > + Anuj > > Quoting Lionel Landwerlin (2019-04-26 11:13:58) >> On 18/04/2019 18:06, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> >>> WaEnableStateCacheRedirectToCS context workaround configures the L3 cache >>> to benefit 3d workloads but media has different requirements. >>> >>> Remove the workaround and whitelist the register to allow any userspace >>> configure the behaviour to their liking. >>> >>> v2: >>> * Remove the workaround apart from adding the whitelist. >>> >>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>> Cc: kevin.ma@intel.com >>> Cc: xiaogang.li@intel.com >> >> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> >> >> Mesa commits : >> >> commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) >> >> commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) >> >> commit d1be67db39463b48369cb71979ed18662b2c157e (iris) > Could somebody confirm that applying this patch does not cause hangs in > older mesa, and the performance drop (if any) is insignificant? > > Best Regards, > Joonas
On Thu, Apr 18, 2019 at 3:06 AM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > WaEnableStateCacheRedirectToCS context workaround configures the L3 cache > to benefit 3d workloads but media has different requirements. > > Remove the workaround and whitelist the register to allow any userspace > configure the behaviour to their liking. > > v2: > * Remove the workaround apart from adding the whitelist. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: kevin.ma@intel.com > Cc: xiaogang.li@intel.com > --- > drivers/gpu/drm/i915/intel_workarounds.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index b3cbed1ee1c9..baed186724d2 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -556,10 +556,6 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, > GEN11_TDL_CLOCK_GATING_FIX_DISABLE); > > - /* WaEnableStateCacheRedirectToCS:icl */ > - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN1, > - GEN11_STATE_CACHE_REDIRECT_TO_CS); > - > /* Wa_2006665173:icl (pre-prod) */ > if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0)) > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > @@ -1070,6 +1066,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > /* WaAllowUMDToModifySamplerMode:icl */ > whitelist_reg(w, GEN10_SAMPLER_MODE); > + > + /* WaEnableStateCacheRedirectToCS:icl */ > + whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1); > } > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx Acked-by: Anuj Phogat <anuj.phogat@gmail.com>
On 26/04/2019 17:58, Anuj Phogat wrote: > > Joonas, > > Mesa now applies this WA on ICL and we're not seeing any regressions in CI. > I tested Mesa with and without this patch applied to kernel. I don't see > any > performance impact to Manhattan from GfxBench5. I'm little surprised to > see it's not really helping benchmark performance in Mesa. I'll dig bit > more > to figure out a possible explanation. I haven't tried any other benchmarks > with this patch. I think the concern was, if user is running old Mesa (no WA) on new kernel (no WA) there wouldn't be any GPU hangs, just theoretical (yet unmeasured) perf drop? Regards, Tvrtko > > Thanks > Anuj > On 04/26/2019 01:31 AM, Joonas Lahtinen wrote: >> + Anuj >> >> Quoting Lionel Landwerlin (2019-04-26 11:13:58) >>> On 18/04/2019 18:06, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> WaEnableStateCacheRedirectToCS context workaround configures the L3 >>>> cache >>>> to benefit 3d workloads but media has different requirements. >>>> >>>> Remove the workaround and whitelist the register to allow any userspace >>>> configure the behaviour to their liking. >>>> >>>> v2: >>>> * Remove the workaround apart from adding the whitelist. >>>> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>>> Cc: kevin.ma@intel.com >>>> Cc: xiaogang.li@intel.com >>> >>> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>> >>> >>> Mesa commits : >>> >>> commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) >>> >>> commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) >>> >>> commit d1be67db39463b48369cb71979ed18662b2c157e (iris) >> Could somebody confirm that applying this patch does not cause hangs in >> older mesa, and the performance drop (if any) is insignificant? >> >> Best Regards, >> Joonas > > >
On Sun, Apr 28, 2019 at 10:57 PM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > > On 26/04/2019 17:58, Anuj Phogat wrote: > > > > Joonas, > > > > Mesa now applies this WA on ICL and we're not seeing any regressions in CI. > > I tested Mesa with and without this patch applied to kernel. I don't see > > any > > performance impact to Manhattan from GfxBench5. I'm little surprised to > > see it's not really helping benchmark performance in Mesa. I'll dig bit > > more > > to figure out a possible explanation. I haven't tried any other benchmarks > > with this patch. > > I think the concern was, if user is running old Mesa (no WA) on new > kernel (no WA) there wouldn't be any GPU hangs, just theoretical (yet > unmeasured) perf drop? > I also tested Manhattan with Mesa (no WA) and Kernel (no WA) and didn't see a GPU hang or any perf drop. The no change in perf might be due to currently used L3 configuration in Mesa which doesn't allocate anything to CS Command buffer section. Mesa now carries the WA in case we choose to use a different L3 config in future. > Regards, > > Tvrtko > > > > > Thanks > > Anuj > > On 04/26/2019 01:31 AM, Joonas Lahtinen wrote: > >> + Anuj > >> > >> Quoting Lionel Landwerlin (2019-04-26 11:13:58) > >>> On 18/04/2019 18:06, Tvrtko Ursulin wrote: > >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> > >>>> WaEnableStateCacheRedirectToCS context workaround configures the L3 > >>>> cache > >>>> to benefit 3d workloads but media has different requirements. > >>>> > >>>> Remove the workaround and whitelist the register to allow any userspace > >>>> configure the behaviour to their liking. > >>>> > >>>> v2: > >>>> * Remove the workaround apart from adding the whitelist. > >>>> > >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > >>>> Cc: kevin.ma@intel.com > >>>> Cc: xiaogang.li@intel.com > >>> > >>> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > >>> > >>> > >>> Mesa commits : > >>> > >>> commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) > >>> > >>> commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) > >>> > >>> commit d1be67db39463b48369cb71979ed18662b2c157e (iris) > >> Could somebody confirm that applying this patch does not cause hangs in > >> older mesa, and the performance drop (if any) is insignificant? > >> > >> Best Regards, > >> Joonas > > > > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
So we can check in this patch right now, right? -----Original Message----- From: Anuj Phogat [mailto:anuj.phogat@gmail.com] Sent: Tuesday, April 30, 2019 1:17 AM To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Phogat, Anuj <anuj.phogat@intel.com>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Intel GFX <Intel-gfx@lists.freedesktop.org>; Landwerlin, Lionel G <lionel.g.landwerlin@intel.com>; Ma, Kevin <kevin.ma@intel.com>; Li, Xiaogang <xiaogang.li@intel.com> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/icl: Whitelist GEN9_SLICE_COMMON_ECO_CHICKEN1 On Sun, Apr 28, 2019 at 10:57 PM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > > On 26/04/2019 17:58, Anuj Phogat wrote: > > > > Joonas, > > > > Mesa now applies this WA on ICL and we're not seeing any regressions in CI. > > I tested Mesa with and without this patch applied to kernel. I don't > > see any performance impact to Manhattan from GfxBench5. I'm little > > surprised to see it's not really helping benchmark performance in > > Mesa. I'll dig bit more to figure out a possible explanation. I > > haven't tried any other benchmarks with this patch. > > I think the concern was, if user is running old Mesa (no WA) on new > kernel (no WA) there wouldn't be any GPU hangs, just theoretical (yet > unmeasured) perf drop? > I also tested Manhattan with Mesa (no WA) and Kernel (no WA) and didn't see a GPU hang or any perf drop. The no change in perf might be due to currently used L3 configuration in Mesa which doesn't allocate anything to CS Command buffer section. Mesa now carries the WA in case we choose to use a different L3 config in future. > Regards, > > Tvrtko > > > > > Thanks > > Anuj > > On 04/26/2019 01:31 AM, Joonas Lahtinen wrote: > >> + Anuj > >> > >> Quoting Lionel Landwerlin (2019-04-26 11:13:58) > >>> On 18/04/2019 18:06, Tvrtko Ursulin wrote: > >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> > >>>> WaEnableStateCacheRedirectToCS context workaround configures the > >>>> L3 cache to benefit 3d workloads but media has different > >>>> requirements. > >>>> > >>>> Remove the workaround and whitelist the register to allow any > >>>> userspace configure the behaviour to their liking. > >>>> > >>>> v2: > >>>> * Remove the workaround apart from adding the whitelist. > >>>> > >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > >>>> Cc: kevin.ma@intel.com > >>>> Cc: xiaogang.li@intel.com > >>> > >>> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > >>> > >>> > >>> Mesa commits : > >>> > >>> commit db5b372bb9f5a0dfea86618f8f9832f25d9eaf71 (anv) > >>> > >>> commit eaadb62c9ea98f841d7ffc26c14341abdf84d2d6 (i965) > >>> > >>> commit d1be67db39463b48369cb71979ed18662b2c157e (iris) > >> Could somebody confirm that applying this patch does not cause > >> hangs in older mesa, and the performance drop (if any) is insignificant? > >> > >> Best Regards, > >> Joonas > > > > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Tvrtko Ursulin (2019-04-18 13:06:34) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > WaEnableStateCacheRedirectToCS context workaround configures the L3 cache > to benefit 3d workloads but media has different requirements. > > Remove the workaround and whitelist the register to allow any userspace > configure the behaviour to their liking. > > v2: > * Remove the workaround apart from adding the whitelist. Please add the information that removal of the workaround doesn't cause any ill effects with old Mesa and add Fixes:, so this gets picked up to 5.2. No need for stable as Icelake is alpha_support before 5.2 With those changes this is: Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: kevin.ma@intel.com > Cc: xiaogang.li@intel.com > --- > drivers/gpu/drm/i915/intel_workarounds.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c > index b3cbed1ee1c9..baed186724d2 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -556,10 +556,6 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) > WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, > GEN11_TDL_CLOCK_GATING_FIX_DISABLE); > > - /* WaEnableStateCacheRedirectToCS:icl */ > - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN1, > - GEN11_STATE_CACHE_REDIRECT_TO_CS); > - > /* Wa_2006665173:icl (pre-prod) */ > if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0)) > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > @@ -1070,6 +1066,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > /* WaAllowUMDToModifySamplerMode:icl */ > whitelist_reg(w, GEN10_SAMPLER_MODE); > + > + /* WaEnableStateCacheRedirectToCS:icl */ > + whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1); > } > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 30/04/2019 07:53, Joonas Lahtinen wrote: > Quoting Tvrtko Ursulin (2019-04-18 13:06:34) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> WaEnableStateCacheRedirectToCS context workaround configures the L3 cache >> to benefit 3d workloads but media has different requirements. >> >> Remove the workaround and whitelist the register to allow any userspace >> configure the behaviour to their liking. >> >> v2: >> * Remove the workaround apart from adding the whitelist. > > Please add the information that removal of the workaround doesn't cause > any ill effects with old Mesa and add Fixes:, so this gets picked up to > 5.2. No need for stable as Icelake is alpha_support before 5.2 > > With those changes this is: > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Pushed. Thanks for all the testing on both sides! Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c index b3cbed1ee1c9..baed186724d2 100644 --- a/drivers/gpu/drm/i915/intel_workarounds.c +++ b/drivers/gpu/drm/i915/intel_workarounds.c @@ -556,10 +556,6 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine) WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, GEN11_TDL_CLOCK_GATING_FIX_DISABLE); - /* WaEnableStateCacheRedirectToCS:icl */ - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN1, - GEN11_STATE_CACHE_REDIRECT_TO_CS); - /* Wa_2006665173:icl (pre-prod) */ if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0)) WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, @@ -1070,6 +1066,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) /* WaAllowUMDToModifySamplerMode:icl */ whitelist_reg(w, GEN10_SAMPLER_MODE); + + /* WaEnableStateCacheRedirectToCS:icl */ + whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1); } void intel_engine_init_whitelist(struct intel_engine_cs *engine)