Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
On Tue, Jan 08, 2019 at 12:32:05PM -0800, Kenneth Graunke wrote: > On Tuesday, January 8, 2019 7:53:05 AM PST Joonas Lahtinen wrote: > > + Ken/Jason for Mesa > > Quoting Matt Roper (2019-01-07 21:19:31) > > > On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote: > > > > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > > > > > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > > > > > According to Workaround database ICL also needs > > > > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > > > > > fine-granularity preemptions per-context. > > > > > > > > > > I must wonder where is the userspace component that needs this, and > > > > > why > > > > > it hasn't been noticed earlier? > > > > > > > > > > Or is this one more of the cases when no userspace actually uses the > > > > > register? > > > > > > > > It's used: > > > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 > > > > > > > > -Michał > > > > > > Wasn't this just an artificial i915-only workaround that was added to > > > prevent breakage of pre-preemption UMD's? Initial gen9 driver releases > > > didn't support preemption, so when preemption support did get added to > > > i915, the kernel had to force object-level off by default at context > > > creation to avoid breaking old userspace that didn't build batch buffers > > > with all the necessary preemption workarounds. This CS_CHICKEN1 > > > register was then exposed to userspace so that newer, preemption-aware > > > userspace could opt back in if it properly supported preemption. It's not only that userspace didn't build proper batch buffers with the necessary workarounds, but that most of the workarounds required disabling preemption depending on the type of primitive being drawn. So userspace needed access to CS_CHICKEN1 to be able to enable/disable preemption for those. > > > For gen11, there shouldn't be any "old" userspace around that doesn't > > > support preemption, so shouldn't the kernel just leave object-level > > > preemption enabled by default (meaning there's no need to expose this > > > register to userspace to allow it to explicitly opt-in)? > > > > Makes sense to me. We should have known by know if somebody expects to > > control the register, because they would be failing to do so. > > > > Mesa could also drop the register load for Gen11+ > > > > Regards, Joonas > > + Rafael, as he's done all the preemption work in Mesa. > > That seems reasonable to me. It looks like i965 always enables > mid-object preemption (sets CS_CHICKEN1 bit 0) on Gen10+, and never > disables it. You can probably safely turn it on by default, and we > can stop writing the register altogether. Yeah, I noticed this after re-reading some other thread, right after we got the preemption patches merged. On gen11, we have some workarounds but they don't require us to disable preemption through CS_CHICKEN1, so it should be safe for the kernel to not whitelist or disable it. Thanks, Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
Quoting Sripada, Radhakrishna (2019-01-09 22:38:36) > Looks good to me. There is already conclusion in the other thread that this should NOT be merged. Regards, Joonas > > > -Original Message- > > From: Souza, Jose > > Sent: Friday, January 4, 2019 9:37 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Oscar Mateo ; Sripada, Radhakrishna > > ; Souza, Jose > > Subject: [PATCH] drm/i915/icl: Apply > > WaEnablePreemptionGranularityControlByUMD > > > > According to Workaround database ICL also needs > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > fine-granularity preemptions per-context. > > > > BSpec: 11348 > > Cc: Oscar Mateo > > Cc: Radhakrishna Sripada > > Signed-off-by: José Roberto de Souza > > Reviewed-by: Radhakrishna Sripada > > --- > > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > > b/drivers/gpu/drm/i915/intel_workarounds.c > > index 480c53a2ecb5..bbc5a66faa07 100644 > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct i915_wa_list > > *w) > > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > > > - /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > > + /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] > > +*/ > > whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ @@ - > > 1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > > > /* WaAllowUMDToModifySamplerMode:icl */ > > whitelist_reg(w, GEN10_SAMPLER_MODE); > > + > > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > > + whitelist_reg(w, GEN8_CS_CHICKEN1); > > } > > > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) @@ -1186,8 > > +1189,8 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine) > > GEN7_DISABLE_SAMPLER_PREFETCH); > > } > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > - /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > + /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl > > +*/ > > wa_masked_en(wal, > >GEN7_FF_SLICE_CS_CHICKEN1, > >GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > -- > > 2.20.1 > > ___ > 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/icl: Apply WaEnablePreemptionGranularityControlByUMD
Looks good to me. > -Original Message- > From: Souza, Jose > Sent: Friday, January 4, 2019 9:37 AM > To: intel-gfx@lists.freedesktop.org > Cc: Oscar Mateo ; Sripada, Radhakrishna > ; Souza, Jose > Subject: [PATCH] drm/i915/icl: Apply > WaEnablePreemptionGranularityControlByUMD > > According to Workaround database ICL also needs > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > fine-granularity preemptions per-context. > > BSpec: 11348 > Cc: Oscar Mateo > Cc: Radhakrishna Sripada > Signed-off-by: José Roberto de Souza Reviewed-by: Radhakrishna Sripada > --- > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > b/drivers/gpu/drm/i915/intel_workarounds.c > index 480c53a2ecb5..bbc5a66faa07 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct i915_wa_list > *w) > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > - /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > + /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] > +*/ > whitelist_reg(w, GEN8_CS_CHICKEN1); > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ @@ - > 1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > /* WaAllowUMDToModifySamplerMode:icl */ > whitelist_reg(w, GEN10_SAMPLER_MODE); > + > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > + whitelist_reg(w, GEN8_CS_CHICKEN1); > } > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) @@ -1186,8 > +1189,8 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine) > GEN7_DISABLE_SAMPLER_PREFETCH); > } > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > - /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > + if (IS_GEN_RANGE(i915, 9, 11)) { > + /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl > +*/ > wa_masked_en(wal, >GEN7_FF_SLICE_CS_CHICKEN1, >GEN9_FFSC_PERCTX_PREEMPT_CTRL); > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
On Mon, Jan 07, 2019 at 11:19:31AM -0800, Matt Roper wrote: > On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote: > > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > > > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > > > According to Workaround database ICL also needs > > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > > > fine-granularity preemptions per-context. > > > > > > I must wonder where is the userspace component that needs this, and why > > > it hasn't been noticed earlier? > > > > > > Or is this one more of the cases when no userspace actually uses the > > > register? > > > > It's used: > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 > > > > -Michał > > Wasn't this just an artificial i915-only workaround that was added to > prevent breakage of pre-preemption UMD's? Initial gen9 driver releases > didn't support preemption, so when preemption support did get added to > i915, the kernel had to force object-level off by default at context > creation to avoid breaking old userspace that didn't build batch buffers > with all the necessary preemption workarounds. This CS_CHICKEN1 > register was then exposed to userspace so that newer, preemption-aware > userspace could opt back in if it properly supported preemption. It wasn't just old userspace vs preeemption-aware userspace. Even the preemption-aware userspace needs to downgrade its preemption granularity as a WA, see: https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_draw.c#L876 > For gen11, there shouldn't be any "old" userspace around that doesn't > support preemption, so shouldn't the kernel just leave object-level > preemption enabled by default (meaning there's no need to expose this > register to userspace to allow it to explicitly opt-in)? Agreed, as a bonus we don't allow anyone to explicity opt-out - which is great, as long as everything works reliably. -Michał > > Matt > > > > > > Regards, Joonas > > > > > > > > > > > BSpec: 11348 > > > > Cc: Oscar Mateo > > > > Cc: Radhakrishna Sripada > > > > Signed-off-by: José Roberto de Souza > > > > --- > > > > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > > > > b/drivers/gpu/drm/i915/intel_workarounds.c > > > > index 480c53a2ecb5..bbc5a66faa07 100644 > > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct > > > > i915_wa_list *w) > > > > /* > > > > WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > > > > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > > > > > > > - /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > > > > + /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */ > > > > whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > > > > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ > > > > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct > > > > i915_wa_list *w) > > > > > > > > /* WaAllowUMDToModifySamplerMode:icl */ > > > > whitelist_reg(w, GEN10_SAMPLER_MODE); > > > > + > > > > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > > > > + whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > } > > > > > > > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > > > > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct > > > > intel_engine_cs *engine) > > > > GEN7_DISABLE_SAMPLER_PREFETCH); > > > > } > > > > > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > > > - /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > > > + /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */ > > > > wa_masked_en(wal, > > > > GEN7_FF_SLICE_CS_CHICKEN1, > > > > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > > > -- > > > > 2.20.1 > > > > > > > > ___ > > > > 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 > > -- > Matt Roper > Graphics Software Engineer > IoTG Plat
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
On Tuesday, January 8, 2019 7:53:05 AM PST Joonas Lahtinen wrote: > + Ken/Jason for Mesa > Quoting Matt Roper (2019-01-07 21:19:31) > > On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote: > > > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > > > > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > > > > According to Workaround database ICL also needs > > > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > > > > fine-granularity preemptions per-context. > > > > > > > > I must wonder where is the userspace component that needs this, and why > > > > it hasn't been noticed earlier? > > > > > > > > Or is this one more of the cases when no userspace actually uses the > > > > register? > > > > > > It's used: > > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 > > > > > > -Michał > > > > Wasn't this just an artificial i915-only workaround that was added to > > prevent breakage of pre-preemption UMD's? Initial gen9 driver releases > > didn't support preemption, so when preemption support did get added to > > i915, the kernel had to force object-level off by default at context > > creation to avoid breaking old userspace that didn't build batch buffers > > with all the necessary preemption workarounds. This CS_CHICKEN1 > > register was then exposed to userspace so that newer, preemption-aware > > userspace could opt back in if it properly supported preemption. > > > > For gen11, there shouldn't be any "old" userspace around that doesn't > > support preemption, so shouldn't the kernel just leave object-level > > preemption enabled by default (meaning there's no need to expose this > > register to userspace to allow it to explicitly opt-in)? > > Makes sense to me. We should have known by know if somebody expects to > control the register, because they would be failing to do so. > > Mesa could also drop the register load for Gen11+ > > Regards, Joonas + Rafael, as he's done all the preemption work in Mesa. That seems reasonable to me. It looks like i965 always enables mid-object preemption (sets CS_CHICKEN1 bit 0) on Gen10+, and never disables it. You can probably safely turn it on by default, and we can stop writing the register altogether. Thanks for the heads up! --Ken signature.asc Description: This is a digitally signed message part. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
+ Ken/Jason for Mesa Quoting Matt Roper (2019-01-07 21:19:31) > On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote: > > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > > > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > > > According to Workaround database ICL also needs > > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > > > fine-granularity preemptions per-context. > > > > > > I must wonder where is the userspace component that needs this, and why > > > it hasn't been noticed earlier? > > > > > > Or is this one more of the cases when no userspace actually uses the > > > register? > > > > It's used: > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 > > > > -Michał > > Wasn't this just an artificial i915-only workaround that was added to > prevent breakage of pre-preemption UMD's? Initial gen9 driver releases > didn't support preemption, so when preemption support did get added to > i915, the kernel had to force object-level off by default at context > creation to avoid breaking old userspace that didn't build batch buffers > with all the necessary preemption workarounds. This CS_CHICKEN1 > register was then exposed to userspace so that newer, preemption-aware > userspace could opt back in if it properly supported preemption. > > For gen11, there shouldn't be any "old" userspace around that doesn't > support preemption, so shouldn't the kernel just leave object-level > preemption enabled by default (meaning there's no need to expose this > register to userspace to allow it to explicitly opt-in)? Makes sense to me. We should have known by know if somebody expects to control the register, because they would be failing to do so. Mesa could also drop the register load for Gen11+ Regards, Joonas > > Matt > > > > > > Regards, Joonas > > > > > > > > > > > BSpec: 11348 > > > > Cc: Oscar Mateo > > > > Cc: Radhakrishna Sripada > > > > Signed-off-by: José Roberto de Souza > > > > --- > > > > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > > > > b/drivers/gpu/drm/i915/intel_workarounds.c > > > > index 480c53a2ecb5..bbc5a66faa07 100644 > > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct > > > > i915_wa_list *w) > > > > /* > > > > WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > > > > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > > > > > > > - /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > > > > + /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */ > > > > whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > > > > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ > > > > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct > > > > i915_wa_list *w) > > > > > > > > /* WaAllowUMDToModifySamplerMode:icl */ > > > > whitelist_reg(w, GEN10_SAMPLER_MODE); > > > > + > > > > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > > > > + whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > } > > > > > > > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > > > > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct > > > > intel_engine_cs *engine) > > > > GEN7_DISABLE_SAMPLER_PREFETCH); > > > > } > > > > > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > > > - /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > > > + /* > > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */ > > > > wa_masked_en(wal, > > > > GEN7_FF_SLICE_CS_CHICKEN1, > > > > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > > > -- > > > > 2.20.1 > > > > > > > > ___ > > > > 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 > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https:
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote: > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > > According to Workaround database ICL also needs > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > > fine-granularity preemptions per-context. > > > > I must wonder where is the userspace component that needs this, and why > > it hasn't been noticed earlier? > > > > Or is this one more of the cases when no userspace actually uses the > > register? > > It's used: > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 > > -Michał Wasn't this just an artificial i915-only workaround that was added to prevent breakage of pre-preemption UMD's? Initial gen9 driver releases didn't support preemption, so when preemption support did get added to i915, the kernel had to force object-level off by default at context creation to avoid breaking old userspace that didn't build batch buffers with all the necessary preemption workarounds. This CS_CHICKEN1 register was then exposed to userspace so that newer, preemption-aware userspace could opt back in if it properly supported preemption. For gen11, there shouldn't be any "old" userspace around that doesn't support preemption, so shouldn't the kernel just leave object-level preemption enabled by default (meaning there's no need to expose this register to userspace to allow it to explicitly opt-in)? Matt > > > Regards, Joonas > > > > > > > > BSpec: 11348 > > > Cc: Oscar Mateo > > > Cc: Radhakrishna Sripada > > > Signed-off-by: José Roberto de Souza > > > --- > > > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > > > b/drivers/gpu/drm/i915/intel_workarounds.c > > > index 480c53a2ecb5..bbc5a66faa07 100644 > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct > > > i915_wa_list *w) > > > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl > > > */ > > > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > > > > > - /* > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > > > + /* > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */ > > > whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ > > > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list > > > *w) > > > > > > /* WaAllowUMDToModifySamplerMode:icl */ > > > whitelist_reg(w, GEN10_SAMPLER_MODE); > > > + > > > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > > > + whitelist_reg(w, GEN8_CS_CHICKEN1); > > > } > > > > > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > > > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct > > > intel_engine_cs *engine) > > > GEN7_DISABLE_SAMPLER_PREFETCH); > > > } > > > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > > - /* > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > > + /* > > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */ > > > wa_masked_en(wal, > > > GEN7_FF_SLICE_CS_CHICKEN1, > > > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > > -- > > > 2.20.1 > > > > > > ___ > > > 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 -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD
On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote: > Quoting José Roberto de Souza (2019-01-04 19:37:00) > > According to Workaround database ICL also needs > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > > fine-granularity preemptions per-context. > > I must wonder where is the userspace component that needs this, and why > it hasn't been noticed earlier? > > Or is this one more of the cases when no userspace actually uses the > register? It's used: https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64 -Michał > Regards, Joonas > > > > > BSpec: 11348 > > Cc: Oscar Mateo > > Cc: Radhakrishna Sripada > > Signed-off-by: José Roberto de Souza > > --- > > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > > b/drivers/gpu/drm/i915/intel_workarounds.c > > index 480c53a2ecb5..bbc5a66faa07 100644 > > --- a/drivers/gpu/drm/i915/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct i915_wa_list > > *w) > > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > > > - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] > > */ > > + /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */ > > whitelist_reg(w, GEN8_CS_CHICKEN1); > > > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ > > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list > > *w) > > > > /* WaAllowUMDToModifySamplerMode:icl */ > > whitelist_reg(w, GEN10_SAMPLER_MODE); > > + > > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > > + whitelist_reg(w, GEN8_CS_CHICKEN1); > > } > > > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct intel_engine_cs > > *engine) > > GEN7_DISABLE_SAMPLER_PREFETCH); > > } > > > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > > - /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > > + if (IS_GEN_RANGE(i915, 9, 11)) { > > + /* > > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */ > > wa_masked_en(wal, > > GEN7_FF_SLICE_CS_CHICKEN1, > > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > > -- > > 2.20.1 > > > > ___ > > 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/icl: Apply WaEnablePreemptionGranularityControlByUMD
Quoting José Roberto de Souza (2019-01-04 19:37:00) > According to Workaround database ICL also needs > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do > fine-granularity preemptions per-context. I must wonder where is the userspace component that needs this, and why it hasn't been noticed earlier? Or is this one more of the cases when no userspace actually uses the register? Regards, Joonas > > BSpec: 11348 > Cc: Oscar Mateo > Cc: Radhakrishna Sripada > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/intel_workarounds.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > b/drivers/gpu/drm/i915/intel_workarounds.c > index 480c53a2ecb5..bbc5a66faa07 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct i915_wa_list *w) > /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ > whitelist_reg(w, GEN9_CTX_PREEMPT_REG); > > - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */ > + /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */ > whitelist_reg(w, GEN8_CS_CHICKEN1); > > /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list *w) > > /* WaAllowUMDToModifySamplerMode:icl */ > whitelist_reg(w, GEN10_SAMPLER_MODE); > + > + /* WaEnablePreemptionGranularityControlByUMD:icl */ > + whitelist_reg(w, GEN8_CS_CHICKEN1); > } > > void intel_engine_init_whitelist(struct intel_engine_cs *engine) > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct intel_engine_cs > *engine) > GEN7_DISABLE_SAMPLER_PREFETCH); > } > > - if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) { > - /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */ > + if (IS_GEN_RANGE(i915, 9, 11)) { > + /* > WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */ > wa_masked_en(wal, > GEN7_FF_SLICE_CS_CHICKEN1, > GEN9_FFSC_PERCTX_PREEMPT_CTRL); > -- > 2.20.1 > > ___ > 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