Message ID | 20180316121456.11577-5-mika.kuoppala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Mika Kuoppala <mika.kuoppala@linux.intel.com> writes: > From: Michel Thierry <michel.thierry@intel.com> > > The bits used to reset the different engines/domains have changed in > GEN11, this patch maps the reset engine mask bits with the new bits > in the reset control register. > > v2: Use shift-left instead of BIT macro to match the file style (Paulo). > v3: Reuse gen8_reset_engines (Daniele). > v4: Do not call intel_uncore_forcewake_reset after reset, we may be > using the forcewake to read protected registers elsewhere and those > results may be clobbered by the concurrent dropping of forcewake. > > bspec: 19212 > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++ > drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++-- > 2 files changed, 62 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 9eaaa96287ec..f3cc77690124 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define GEN6_GRDOM_VECS (1 << 4) > #define GEN9_GRDOM_GUC (1 << 5) > #define GEN8_GRDOM_MEDIA2 (1 << 7) > +/* GEN11 changed all bit defs except for FULL & RENDER */ > +#define GEN11_GRDOM_FULL GEN6_GRDOM_FULL > +#define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER > +#define GEN11_GRDOM_BLT (1 << 2) > +#define GEN11_GRDOM_GUC (1 << 3) > +#define GEN11_GRDOM_MEDIA (1 << 5) > +#define GEN11_GRDOM_MEDIA2 (1 << 6) > +#define GEN11_GRDOM_MEDIA3 (1 << 7) > +#define GEN11_GRDOM_MEDIA4 (1 << 8) I would like these to be named like they are in bspec. First being MEDIA0. > +#define GEN11_GRDOM_VECS (1 << 13) > +#define GEN11_GRDOM_VECS2 (1 << 14) And same to these, VECS0 and VECS1. -Mika > > #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base+0x228) > #define RING_PP_DIR_BASE_READ(engine) _MMIO((engine)->mmio_base+0x518) > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 4c616d074a97..cabbf0e682e7 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, > return gen6_hw_domain_reset(dev_priv, hw_mask); > } > > +/** > + * gen11_reset_engines - reset individual engines > + * @dev_priv: i915 device > + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset > + * > + * This function will reset the individual engines that are set in engine_mask. > + * If you provide ALL_ENGINES as mask, full global domain reset will be issued. > + * > + * Note: It is responsibility of the caller to handle the difference between > + * asking full domain reset versus reset for all available individual engines. > + * > + * Returns 0 on success, nonzero on error. > + */ > +static int gen11_reset_engines(struct drm_i915_private *dev_priv, > + unsigned engine_mask) > +{ > + struct intel_engine_cs *engine; > + const u32 hw_engine_mask[I915_NUM_ENGINES] = { > + [RCS] = GEN11_GRDOM_RENDER, > + [BCS] = GEN11_GRDOM_BLT, > + [VCS] = GEN11_GRDOM_MEDIA, > + [VCS2] = GEN11_GRDOM_MEDIA2, > + [VCS3] = GEN11_GRDOM_MEDIA3, > + [VCS4] = GEN11_GRDOM_MEDIA4, > + [VECS] = GEN11_GRDOM_VECS, > + [VECS2] = GEN11_GRDOM_VECS2, > + }; > + u32 hw_mask; > + > + BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES); > + > + if (engine_mask == ALL_ENGINES) { > + hw_mask = GEN11_GRDOM_FULL; > + } else { > + unsigned int tmp; > + > + hw_mask = 0; > + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > + hw_mask |= hw_engine_mask[engine->id]; > + } > + > + return gen6_hw_domain_reset(dev_priv, hw_mask); > +} > + > /** > * __intel_wait_for_register_fw - wait until register matches expected state > * @dev_priv: the i915 device > @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv, > if (gen8_reset_engine_start(engine)) > goto not_ready; > > - return gen6_reset_engines(dev_priv, engine_mask); > + if (INTEL_GEN(dev_priv) >= 11) > + return gen11_reset_engines(dev_priv, engine_mask); > + else > + return gen6_reset_engines(dev_priv, engine_mask); > > not_ready: > for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv) > > int intel_reset_guc(struct drm_i915_private *dev_priv) > { > + u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC : > + GEN9_GRDOM_GUC; > int ret; > > GEM_BUG_ON(!HAS_GUC(dev_priv)); > > intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); > - ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC); > + ret = gen6_hw_domain_reset(dev_priv, guc_domain); > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > > return ret; > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2018-03-16 12:14:53) > +static int gen11_reset_engines(struct drm_i915_private *dev_priv, > + unsigned engine_mask) > +{ > + struct intel_engine_cs *engine; > + const u32 hw_engine_mask[I915_NUM_ENGINES] = { > + [RCS] = GEN11_GRDOM_RENDER, > + [BCS] = GEN11_GRDOM_BLT, > + [VCS] = GEN11_GRDOM_MEDIA, > + [VCS2] = GEN11_GRDOM_MEDIA2, > + [VCS3] = GEN11_GRDOM_MEDIA3, > + [VCS4] = GEN11_GRDOM_MEDIA4, > + [VECS] = GEN11_GRDOM_VECS, > + [VECS2] = GEN11_GRDOM_VECS2, > + }; No gratuitously decorating Christmas trees. > + u32 hw_mask; > + > + BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES); > + > + if (engine_mask == ALL_ENGINES) { > + hw_mask = GEN11_GRDOM_FULL; > + } else { Plonk struct intel_engine_cs *engine; here instead. > + unsigned int tmp; > + > + hw_mask = 0; > + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > + hw_mask |= hw_engine_mask[engine->id]; > + }
On 16/03/18 05:14, Mika Kuoppala wrote: > From: Michel Thierry <michel.thierry@intel.com> > > The bits used to reset the different engines/domains have changed in > GEN11, this patch maps the reset engine mask bits with the new bits > in the reset control register. > > v2: Use shift-left instead of BIT macro to match the file style (Paulo). > v3: Reuse gen8_reset_engines (Daniele). > v4: Do not call intel_uncore_forcewake_reset after reset, we may be > using the forcewake to read protected registers elsewhere and those > results may be clobbered by the concurrent dropping of forcewake. > > bspec: 19212 > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++ > drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++-- > 2 files changed, 62 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 9eaaa96287ec..f3cc77690124 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define GEN6_GRDOM_VECS (1 << 4) > #define GEN9_GRDOM_GUC (1 << 5) > #define GEN8_GRDOM_MEDIA2 (1 << 7) > +/* GEN11 changed all bit defs except for FULL & RENDER */ > +#define GEN11_GRDOM_FULL GEN6_GRDOM_FULL > +#define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER > +#define GEN11_GRDOM_BLT (1 << 2) > +#define GEN11_GRDOM_GUC (1 << 3) > +#define GEN11_GRDOM_MEDIA (1 << 5) > +#define GEN11_GRDOM_MEDIA2 (1 << 6) > +#define GEN11_GRDOM_MEDIA3 (1 << 7) > +#define GEN11_GRDOM_MEDIA4 (1 << 8) > +#define GEN11_GRDOM_VECS (1 << 13) > +#define GEN11_GRDOM_VECS2 (1 << 14) > > #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base+0x228) > #define RING_PP_DIR_BASE_READ(engine) _MMIO((engine)->mmio_base+0x518) > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 4c616d074a97..cabbf0e682e7 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, > return gen6_hw_domain_reset(dev_priv, hw_mask); > } > > +/** > + * gen11_reset_engines - reset individual engines > + * @dev_priv: i915 device > + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset > + * > + * This function will reset the individual engines that are set in engine_mask. > + * If you provide ALL_ENGINES as mask, full global domain reset will be issued. > + * > + * Note: It is responsibility of the caller to handle the difference between > + * asking full domain reset versus reset for all available individual engines. > + * > + * Returns 0 on success, nonzero on error. > + */ > +static int gen11_reset_engines(struct drm_i915_private *dev_priv, > + unsigned engine_mask) > +{ > + struct intel_engine_cs *engine; > + const u32 hw_engine_mask[I915_NUM_ENGINES] = { > + [RCS] = GEN11_GRDOM_RENDER, > + [BCS] = GEN11_GRDOM_BLT, > + [VCS] = GEN11_GRDOM_MEDIA, > + [VCS2] = GEN11_GRDOM_MEDIA2, > + [VCS3] = GEN11_GRDOM_MEDIA3, > + [VCS4] = GEN11_GRDOM_MEDIA4, > + [VECS] = GEN11_GRDOM_VECS, > + [VECS2] = GEN11_GRDOM_VECS2, > + }; Just a thought, but since this function is a copy of gen6_reset_engines with the only difference being the array (GEN11_GRDOM_FULL is also the same as GEN6_GRDOM_FULL), would it make sense to just add the array to the gen6 function? e.g.: const u32 gen6_hw_engine_mask[] = { .... } const u32 gen11_hw_engine_mask[] = { .... } const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ? gen11_hw_engine_mask : gen6_hw_engine_mask; My Ack still stands regardless and I also agree with renaming the defines to be closer to the specs. Daniele > + u32 hw_mask; > + > + BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES); > + > + if (engine_mask == ALL_ENGINES) { > + hw_mask = GEN11_GRDOM_FULL; > + } else { > + unsigned int tmp; > + > + hw_mask = 0; > + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > + hw_mask |= hw_engine_mask[engine->id]; > + } > + > + return gen6_hw_domain_reset(dev_priv, hw_mask); > +} > + > /** > * __intel_wait_for_register_fw - wait until register matches expected state > * @dev_priv: the i915 device > @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv, > if (gen8_reset_engine_start(engine)) > goto not_ready; > > - return gen6_reset_engines(dev_priv, engine_mask); > + if (INTEL_GEN(dev_priv) >= 11) > + return gen11_reset_engines(dev_priv, engine_mask); > + else > + return gen6_reset_engines(dev_priv, engine_mask); > > not_ready: > for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv) > > int intel_reset_guc(struct drm_i915_private *dev_priv) > { > + u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC : > + GEN9_GRDOM_GUC; > int ret; > > GEM_BUG_ON(!HAS_GUC(dev_priv)); > > intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); > - ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC); > + ret = gen6_hw_domain_reset(dev_priv, guc_domain); > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > > return ret; >
Quoting Daniele Ceraolo Spurio (2018-03-16 20:28:25) > > > On 16/03/18 05:14, Mika Kuoppala wrote: > > From: Michel Thierry <michel.thierry@intel.com> > > > > The bits used to reset the different engines/domains have changed in > > GEN11, this patch maps the reset engine mask bits with the new bits > > in the reset control register. > > > > v2: Use shift-left instead of BIT macro to match the file style (Paulo). > > v3: Reuse gen8_reset_engines (Daniele). > > v4: Do not call intel_uncore_forcewake_reset after reset, we may be > > using the forcewake to read protected registers elsewhere and those > > results may be clobbered by the concurrent dropping of forcewake. > > > > bspec: 19212 > > Cc: Oscar Mateo <oscar.mateo@intel.com> > > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++ > > drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++-- > > 2 files changed, 62 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 9eaaa96287ec..f3cc77690124 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > #define GEN6_GRDOM_VECS (1 << 4) > > #define GEN9_GRDOM_GUC (1 << 5) > > #define GEN8_GRDOM_MEDIA2 (1 << 7) > > +/* GEN11 changed all bit defs except for FULL & RENDER */ > > +#define GEN11_GRDOM_FULL GEN6_GRDOM_FULL > > +#define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER > > +#define GEN11_GRDOM_BLT (1 << 2) > > +#define GEN11_GRDOM_GUC (1 << 3) > > +#define GEN11_GRDOM_MEDIA (1 << 5) > > +#define GEN11_GRDOM_MEDIA2 (1 << 6) > > +#define GEN11_GRDOM_MEDIA3 (1 << 7) > > +#define GEN11_GRDOM_MEDIA4 (1 << 8) > > +#define GEN11_GRDOM_VECS (1 << 13) > > +#define GEN11_GRDOM_VECS2 (1 << 14) > > > > #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base+0x228) > > #define RING_PP_DIR_BASE_READ(engine) _MMIO((engine)->mmio_base+0x518) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index 4c616d074a97..cabbf0e682e7 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, > > return gen6_hw_domain_reset(dev_priv, hw_mask); > > } > > > > +/** > > + * gen11_reset_engines - reset individual engines > > + * @dev_priv: i915 device > > + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset > > + * > > + * This function will reset the individual engines that are set in engine_mask. > > + * If you provide ALL_ENGINES as mask, full global domain reset will be issued. > > + * > > + * Note: It is responsibility of the caller to handle the difference between > > + * asking full domain reset versus reset for all available individual engines. > > + * > > + * Returns 0 on success, nonzero on error. > > + */ > > +static int gen11_reset_engines(struct drm_i915_private *dev_priv, > > + unsigned engine_mask) > > +{ > > + struct intel_engine_cs *engine; > > + const u32 hw_engine_mask[I915_NUM_ENGINES] = { > > + [RCS] = GEN11_GRDOM_RENDER, > > + [BCS] = GEN11_GRDOM_BLT, > > + [VCS] = GEN11_GRDOM_MEDIA, > > + [VCS2] = GEN11_GRDOM_MEDIA2, > > + [VCS3] = GEN11_GRDOM_MEDIA3, > > + [VCS4] = GEN11_GRDOM_MEDIA4, > > + [VECS] = GEN11_GRDOM_VECS, > > + [VECS2] = GEN11_GRDOM_VECS2, > > + }; > > Just a thought, but since this function is a copy of gen6_reset_engines > with the only difference being the array (GEN11_GRDOM_FULL is also the > same as GEN6_GRDOM_FULL), would it make sense to just add the array to > the gen6 function? e.g.: > > const u32 gen6_hw_engine_mask[] = { > .... > } > const u32 gen11_hw_engine_mask[] = { > .... > } > > const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ? > gen11_hw_engine_mask : gen6_hw_engine_mask; > Oh, and we are definitely in the territory where static const should result in a smaller binary (.text + .data). -Chris
On 3/16/2018 1:28 PM, Daniele Ceraolo Spurio wrote: > > > On 16/03/18 05:14, Mika Kuoppala wrote: >> From: Michel Thierry <michel.thierry@intel.com> >> >> The bits used to reset the different engines/domains have changed in >> GEN11, this patch maps the reset engine mask bits with the new bits >> in the reset control register. >> >> v2: Use shift-left instead of BIT macro to match the file style (Paulo). >> v3: Reuse gen8_reset_engines (Daniele). >> v4: Do not call intel_uncore_forcewake_reset after reset, we may be >> using the forcewake to read protected registers elsewhere and those >> results may be clobbered by the concurrent dropping of forcewake. >> >> bspec: 19212 >> Cc: Oscar Mateo <oscar.mateo@intel.com> >> Cc: Antonio Argenziano <antonio.argenziano@intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++ >> drivers/gpu/drm/i915/intel_uncore.c | 53 >> +++++++++++++++++++++++++++++++++++-- >> 2 files changed, 62 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 9eaaa96287ec..f3cc77690124 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t >> reg) >> #define GEN6_GRDOM_VECS (1 << 4) >> #define GEN9_GRDOM_GUC (1 << 5) >> #define GEN8_GRDOM_MEDIA2 (1 << 7) >> +/* GEN11 changed all bit defs except for FULL & RENDER */ >> +#define GEN11_GRDOM_FULL GEN6_GRDOM_FULL >> +#define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER >> +#define GEN11_GRDOM_BLT (1 << 2) >> +#define GEN11_GRDOM_GUC (1 << 3) >> +#define GEN11_GRDOM_MEDIA (1 << 5) >> +#define GEN11_GRDOM_MEDIA2 (1 << 6) >> +#define GEN11_GRDOM_MEDIA3 (1 << 7) >> +#define GEN11_GRDOM_MEDIA4 (1 << 8) >> +#define GEN11_GRDOM_VECS (1 << 13) >> +#define GEN11_GRDOM_VECS2 (1 << 14) >> #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base+0x228) >> #define RING_PP_DIR_BASE_READ(engine) >> _MMIO((engine)->mmio_base+0x518) >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c >> b/drivers/gpu/drm/i915/intel_uncore.c >> index 4c616d074a97..cabbf0e682e7 100644 >> --- a/drivers/gpu/drm/i915/intel_uncore.c >> +++ b/drivers/gpu/drm/i915/intel_uncore.c >> @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct >> drm_i915_private *dev_priv, >> return gen6_hw_domain_reset(dev_priv, hw_mask); >> } >> +/** >> + * gen11_reset_engines - reset individual engines >> + * @dev_priv: i915 device >> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for >> full reset >> + * >> + * This function will reset the individual engines that are set in >> engine_mask. >> + * If you provide ALL_ENGINES as mask, full global domain reset will >> be issued. >> + * >> + * Note: It is responsibility of the caller to handle the difference >> between >> + * asking full domain reset versus reset for all available individual >> engines. >> + * >> + * Returns 0 on success, nonzero on error. >> + */ >> +static int gen11_reset_engines(struct drm_i915_private *dev_priv, >> + unsigned engine_mask) >> +{ >> + struct intel_engine_cs *engine; >> + const u32 hw_engine_mask[I915_NUM_ENGINES] = { >> + [RCS] = GEN11_GRDOM_RENDER, >> + [BCS] = GEN11_GRDOM_BLT, >> + [VCS] = GEN11_GRDOM_MEDIA, >> + [VCS2] = GEN11_GRDOM_MEDIA2, >> + [VCS3] = GEN11_GRDOM_MEDIA3, >> + [VCS4] = GEN11_GRDOM_MEDIA4, >> + [VECS] = GEN11_GRDOM_VECS, >> + [VECS2] = GEN11_GRDOM_VECS2, >> + }; > > Just a thought, but since this function is a copy of gen6_reset_engines > with the only difference being the array (GEN11_GRDOM_FULL is also the > same as GEN6_GRDOM_FULL), would it make sense to just add the array to > the gen6 function? e.g.: There are more changes for gen11 coming (locking/unlocking the shared SFC units), so I don't think it's a good idea to combine them. > > const u32 gen6_hw_engine_mask[] = { > .... > } > const u32 gen11_hw_engine_mask[] = { > .... > } > > const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ? > gen11_hw_engine_mask : gen6_hw_engine_mask; > > > My Ack still stands regardless and I also agree with renaming the > defines to be closer to the specs. > > Daniele > >> + u32 hw_mask; >> + >> + BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES); >> + >> + if (engine_mask == ALL_ENGINES) { >> + hw_mask = GEN11_GRDOM_FULL; >> + } else { >> + unsigned int tmp; >> + >> + hw_mask = 0; >> + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) >> + hw_mask |= hw_engine_mask[engine->id]; >> + } >> + >> + return gen6_hw_domain_reset(dev_priv, hw_mask); >> +} >> + >> /** >> * __intel_wait_for_register_fw - wait until register matches >> expected state >> * @dev_priv: the i915 device >> @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct >> drm_i915_private *dev_priv, >> if (gen8_reset_engine_start(engine)) >> goto not_ready; >> - return gen6_reset_engines(dev_priv, engine_mask); >> + if (INTEL_GEN(dev_priv) >= 11) >> + return gen11_reset_engines(dev_priv, engine_mask); >> + else >> + return gen6_reset_engines(dev_priv, engine_mask); >> not_ready: >> for_each_engine_masked(engine, dev_priv, engine_mask, tmp) >> @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct >> drm_i915_private *dev_priv) >> int intel_reset_guc(struct drm_i915_private *dev_priv) >> { >> + u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC : >> + GEN9_GRDOM_GUC; >> int ret; >> GEM_BUG_ON(!HAS_GUC(dev_priv)); >> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); >> - ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC); >> + ret = gen6_hw_domain_reset(dev_priv, guc_domain); >> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); >> return ret; >>
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9eaaa96287ec..f3cc77690124 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN6_GRDOM_VECS (1 << 4) #define GEN9_GRDOM_GUC (1 << 5) #define GEN8_GRDOM_MEDIA2 (1 << 7) +/* GEN11 changed all bit defs except for FULL & RENDER */ +#define GEN11_GRDOM_FULL GEN6_GRDOM_FULL +#define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER +#define GEN11_GRDOM_BLT (1 << 2) +#define GEN11_GRDOM_GUC (1 << 3) +#define GEN11_GRDOM_MEDIA (1 << 5) +#define GEN11_GRDOM_MEDIA2 (1 << 6) +#define GEN11_GRDOM_MEDIA3 (1 << 7) +#define GEN11_GRDOM_MEDIA4 (1 << 8) +#define GEN11_GRDOM_VECS (1 << 13) +#define GEN11_GRDOM_VECS2 (1 << 14) #define RING_PP_DIR_BASE(engine) _MMIO((engine)->mmio_base+0x228) #define RING_PP_DIR_BASE_READ(engine) _MMIO((engine)->mmio_base+0x518) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 4c616d074a97..cabbf0e682e7 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, return gen6_hw_domain_reset(dev_priv, hw_mask); } +/** + * gen11_reset_engines - reset individual engines + * @dev_priv: i915 device + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset + * + * This function will reset the individual engines that are set in engine_mask. + * If you provide ALL_ENGINES as mask, full global domain reset will be issued. + * + * Note: It is responsibility of the caller to handle the difference between + * asking full domain reset versus reset for all available individual engines. + * + * Returns 0 on success, nonzero on error. + */ +static int gen11_reset_engines(struct drm_i915_private *dev_priv, + unsigned engine_mask) +{ + struct intel_engine_cs *engine; + const u32 hw_engine_mask[I915_NUM_ENGINES] = { + [RCS] = GEN11_GRDOM_RENDER, + [BCS] = GEN11_GRDOM_BLT, + [VCS] = GEN11_GRDOM_MEDIA, + [VCS2] = GEN11_GRDOM_MEDIA2, + [VCS3] = GEN11_GRDOM_MEDIA3, + [VCS4] = GEN11_GRDOM_MEDIA4, + [VECS] = GEN11_GRDOM_VECS, + [VECS2] = GEN11_GRDOM_VECS2, + }; + u32 hw_mask; + + BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES); + + if (engine_mask == ALL_ENGINES) { + hw_mask = GEN11_GRDOM_FULL; + } else { + unsigned int tmp; + + hw_mask = 0; + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) + hw_mask |= hw_engine_mask[engine->id]; + } + + return gen6_hw_domain_reset(dev_priv, hw_mask); +} + /** * __intel_wait_for_register_fw - wait until register matches expected state * @dev_priv: the i915 device @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv, if (gen8_reset_engine_start(engine)) goto not_ready; - return gen6_reset_engines(dev_priv, engine_mask); + if (INTEL_GEN(dev_priv) >= 11) + return gen11_reset_engines(dev_priv, engine_mask); + else + return gen6_reset_engines(dev_priv, engine_mask); not_ready: for_each_engine_masked(engine, dev_priv, engine_mask, tmp) @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv) int intel_reset_guc(struct drm_i915_private *dev_priv) { + u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC : + GEN9_GRDOM_GUC; int ret; GEM_BUG_ON(!HAS_GUC(dev_priv)); intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC); + ret = gen6_hw_domain_reset(dev_priv, guc_domain); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret;