| Message ID | 1515695148-18237-1-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/01/18 18:25, Oscar Mateo wrote: > From: Kelvin Gardiner <kelvin.gardiner@intel.com> > > ICL 11 has a greater number of maximum subslices. This patch > reflects this. > > v2: GEN11 updates to MCR_SELECTOR (Oscar) > > Bspec: 21139 > BSpec: 21108 > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> (v1) > Signed-off-by: Kelvin Gardiner <kelvin.gardiner@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > drivers/gpu/drm/i915/intel_engine_cs.c | 22 ++++++++++++++++++---- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > 3 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c9b6250..c79ca5b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2444,6 +2444,10 @@ enum i915_power_well_id { > #define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3) > #define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24) > #define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3) > +#define GEN11_MCR_SLICE(slice) (((slice) & 0xf) << 27) > +#define GEN11_MCR_SLICE_MASK GEN8_MCR_SLICE(0xf) I think you got that line above wrong, should be GEN11_MCR_SLICE(0xf) > +#define GEN11_MCR_SUBSLICE(subslice) (((subslice) & 0x7) << 24) > +#define GEN11_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(0x7) Same issue on the line above : GEN11_MCR_SUBSLICE(0x7) Otherwise, looks good. > #define RING_IPEIR(base) _MMIO((base)+0x64) > #define RING_IPEHR(base) _MMIO((base)+0x68) > /* > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index a373bcb..8c0da94 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -775,10 +775,24 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) > read_subslice_reg(struct drm_i915_private *dev_priv, int slice, > int subslice, i915_reg_t reg) > { > + uint32_t mcr_slice_subslice_mask; > + uint32_t mcr_slice_subslice_select; > uint32_t mcr; > uint32_t ret; > enum forcewake_domains fw_domains; > > + if (INTEL_GEN(dev_priv) >= 11) { > + mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK | > + GEN11_MCR_SUBSLICE_MASK; > + mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) | > + GEN11_MCR_SUBSLICE(subslice); > + } else { > + mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK | > + GEN8_MCR_SUBSLICE_MASK; > + mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) | > + GEN8_MCR_SUBSLICE(subslice); > + } > + > fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, > FW_REG_READ); > fw_domains |= intel_uncore_forcewake_for_reg(dev_priv, > @@ -793,14 +807,14 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) > * The HW expects the slice and sublice selectors to be reset to 0 > * after reading out the registers. > */ > - WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK)); > - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); > - mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice); > + WARN_ON_ONCE(mcr & mcr_slice_subslice_mask); > + mcr &= ~mcr_slice_subslice_mask; > + mcr |= mcr_slice_subslice_select; > I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); > > ret = I915_READ_FW(reg); > > - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); > + mcr &= ~mcr_slice_subslice_mask; > I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); > > intel_uncore_forcewake_put__locked(dev_priv, fw_domains); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 2a88231..029093a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -82,7 +82,7 @@ enum intel_engine_hangcheck_action { > } > > #define I915_MAX_SLICES 3 > -#define I915_MAX_SUBSLICES 3 > +#define I915_MAX_SUBSLICES 8 > > #define instdone_slice_mask(dev_priv__) \ > (INTEL_GEN(dev_priv__) == 7 ? \
On 02/08/2018 08:35 AM, Lionel Landwerlin wrote: > On 11/01/18 18:25, Oscar Mateo wrote: >> From: Kelvin Gardiner <kelvin.gardiner@intel.com> >> >> ICL 11 has a greater number of maximum subslices. This patch >> reflects this. >> >> v2: GEN11 updates to MCR_SELECTOR (Oscar) >> >> Bspec: 21139 >> BSpec: 21108 >> >> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> (v1) >> Signed-off-by: Kelvin Gardiner <kelvin.gardiner@intel.com> >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 4 ++++ >> drivers/gpu/drm/i915/intel_engine_cs.c | 22 ++++++++++++++++++---- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- >> 3 files changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index c9b6250..c79ca5b 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -2444,6 +2444,10 @@ enum i915_power_well_id { >> #define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3) >> #define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24) >> #define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3) >> +#define GEN11_MCR_SLICE(slice) (((slice) & 0xf) << 27) >> +#define GEN11_MCR_SLICE_MASK GEN8_MCR_SLICE(0xf) > I think you got that line above wrong, should be GEN11_MCR_SLICE(0xf) >> +#define GEN11_MCR_SUBSLICE(subslice) (((subslice) & 0x7) << 24) >> +#define GEN11_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(0x7) > > Same issue on the line above : GEN11_MCR_SUBSLICE(0x7) > > Otherwise, looks good. Oops, thank you for spotting this. Do I have your r-b for the fixed version then? > >> #define RING_IPEIR(base) _MMIO((base)+0x64) >> #define RING_IPEHR(base) _MMIO((base)+0x68) >> /* >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >> b/drivers/gpu/drm/i915/intel_engine_cs.c >> index a373bcb..8c0da94 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -775,10 +775,24 @@ const char *i915_cache_level_str(struct >> drm_i915_private *i915, int type) >> read_subslice_reg(struct drm_i915_private *dev_priv, int slice, >> int subslice, i915_reg_t reg) >> { >> + uint32_t mcr_slice_subslice_mask; >> + uint32_t mcr_slice_subslice_select; >> uint32_t mcr; >> uint32_t ret; >> enum forcewake_domains fw_domains; >> + if (INTEL_GEN(dev_priv) >= 11) { >> + mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK | >> + GEN11_MCR_SUBSLICE_MASK; >> + mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) | >> + GEN11_MCR_SUBSLICE(subslice); >> + } else { >> + mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK | >> + GEN8_MCR_SUBSLICE_MASK; >> + mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) | >> + GEN8_MCR_SUBSLICE(subslice); >> + } >> + >> fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, >> FW_REG_READ); >> fw_domains |= intel_uncore_forcewake_for_reg(dev_priv, >> @@ -793,14 +807,14 @@ const char *i915_cache_level_str(struct >> drm_i915_private *i915, int type) >> * The HW expects the slice and sublice selectors to be reset to 0 >> * after reading out the registers. >> */ >> - WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK)); >> - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); >> - mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice); >> + WARN_ON_ONCE(mcr & mcr_slice_subslice_mask); >> + mcr &= ~mcr_slice_subslice_mask; >> + mcr |= mcr_slice_subslice_select; >> I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); >> ret = I915_READ_FW(reg); >> - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); >> + mcr &= ~mcr_slice_subslice_mask; >> I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); >> intel_uncore_forcewake_put__locked(dev_priv, fw_domains); >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >> b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 2a88231..029093a 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -82,7 +82,7 @@ enum intel_engine_hangcheck_action { >> } >> #define I915_MAX_SLICES 3 >> -#define I915_MAX_SUBSLICES 3 >> +#define I915_MAX_SUBSLICES 8 >> #define instdone_slice_mask(dev_priv__) \ >> (INTEL_GEN(dev_priv__) == 7 ? \ > >
On 09/02/18 17:44, Oscar Mateo wrote: > > > On 02/08/2018 08:35 AM, Lionel Landwerlin wrote: >> On 11/01/18 18:25, Oscar Mateo wrote: >>> From: Kelvin Gardiner <kelvin.gardiner@intel.com> >>> >>> ICL 11 has a greater number of maximum subslices. This patch >>> reflects this. >>> >>> v2: GEN11 updates to MCR_SELECTOR (Oscar) >>> >>> Bspec: 21139 >>> BSpec: 21108 >>> >>> Reviewed-by: Daniele Ceraolo Spurio >>> <daniele.ceraolospurio@intel.com> (v1) >>> Signed-off-by: Kelvin Gardiner <kelvin.gardiner@intel.com> >>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_reg.h | 4 ++++ >>> drivers/gpu/drm/i915/intel_engine_cs.c | 22 ++++++++++++++++++---- >>> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- >>> 3 files changed, 23 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index c9b6250..c79ca5b 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -2444,6 +2444,10 @@ enum i915_power_well_id { >>> #define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3) >>> #define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24) >>> #define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3) >>> +#define GEN11_MCR_SLICE(slice) (((slice) & 0xf) << 27) >>> +#define GEN11_MCR_SLICE_MASK GEN8_MCR_SLICE(0xf) >> I think you got that line above wrong, should be GEN11_MCR_SLICE(0xf) >>> +#define GEN11_MCR_SUBSLICE(subslice) (((subslice) & 0x7) << 24) >>> +#define GEN11_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(0x7) >> >> Same issue on the line above : GEN11_MCR_SUBSLICE(0x7) >> >> Otherwise, looks good. > > Oops, thank you for spotting this. Do I have your r-b for the fixed > version then? Sure, with the change, it's : Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > >> >>> #define RING_IPEIR(base) _MMIO((base)+0x64) >>> #define RING_IPEHR(base) _MMIO((base)+0x68) >>> /* >>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >>> b/drivers/gpu/drm/i915/intel_engine_cs.c >>> index a373bcb..8c0da94 100644 >>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >>> @@ -775,10 +775,24 @@ const char *i915_cache_level_str(struct >>> drm_i915_private *i915, int type) >>> read_subslice_reg(struct drm_i915_private *dev_priv, int slice, >>> int subslice, i915_reg_t reg) >>> { >>> + uint32_t mcr_slice_subslice_mask; >>> + uint32_t mcr_slice_subslice_select; >>> uint32_t mcr; >>> uint32_t ret; >>> enum forcewake_domains fw_domains; >>> + if (INTEL_GEN(dev_priv) >= 11) { >>> + mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK | >>> + GEN11_MCR_SUBSLICE_MASK; >>> + mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) | >>> + GEN11_MCR_SUBSLICE(subslice); >>> + } else { >>> + mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK | >>> + GEN8_MCR_SUBSLICE_MASK; >>> + mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) | >>> + GEN8_MCR_SUBSLICE(subslice); >>> + } >>> + >>> fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, >>> FW_REG_READ); >>> fw_domains |= intel_uncore_forcewake_for_reg(dev_priv, >>> @@ -793,14 +807,14 @@ const char *i915_cache_level_str(struct >>> drm_i915_private *i915, int type) >>> * The HW expects the slice and sublice selectors to be reset >>> to 0 >>> * after reading out the registers. >>> */ >>> - WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | >>> GEN8_MCR_SUBSLICE_MASK)); >>> - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); >>> - mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice); >>> + WARN_ON_ONCE(mcr & mcr_slice_subslice_mask); >>> + mcr &= ~mcr_slice_subslice_mask; >>> + mcr |= mcr_slice_subslice_select; >>> I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); >>> ret = I915_READ_FW(reg); >>> - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); >>> + mcr &= ~mcr_slice_subslice_mask; >>> I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); >>> intel_uncore_forcewake_put__locked(dev_priv, fw_domains); >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> index 2a88231..029093a 100644 >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> @@ -82,7 +82,7 @@ enum intel_engine_hangcheck_action { >>> } >>> #define I915_MAX_SLICES 3 >>> -#define I915_MAX_SUBSLICES 3 >>> +#define I915_MAX_SUBSLICES 8 >>> #define instdone_slice_mask(dev_priv__) \ >>> (INTEL_GEN(dev_priv__) == 7 ? \ >> >> > >
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c9b6250..c79ca5b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2444,6 +2444,10 @@ enum i915_power_well_id { #define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3) #define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24) #define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3) +#define GEN11_MCR_SLICE(slice) (((slice) & 0xf) << 27) +#define GEN11_MCR_SLICE_MASK GEN8_MCR_SLICE(0xf) +#define GEN11_MCR_SUBSLICE(subslice) (((subslice) & 0x7) << 24) +#define GEN11_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(0x7) #define RING_IPEIR(base) _MMIO((base)+0x64) #define RING_IPEHR(base) _MMIO((base)+0x68) /* diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a373bcb..8c0da94 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -775,10 +775,24 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) read_subslice_reg(struct drm_i915_private *dev_priv, int slice, int subslice, i915_reg_t reg) { + uint32_t mcr_slice_subslice_mask; + uint32_t mcr_slice_subslice_select; uint32_t mcr; uint32_t ret; enum forcewake_domains fw_domains; + if (INTEL_GEN(dev_priv) >= 11) { + mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK | + GEN11_MCR_SUBSLICE_MASK; + mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) | + GEN11_MCR_SUBSLICE(subslice); + } else { + mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK | + GEN8_MCR_SUBSLICE_MASK; + mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) | + GEN8_MCR_SUBSLICE(subslice); + } + fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ); fw_domains |= intel_uncore_forcewake_for_reg(dev_priv, @@ -793,14 +807,14 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) * The HW expects the slice and sublice selectors to be reset to 0 * after reading out the registers. */ - WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK)); - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); - mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice); + WARN_ON_ONCE(mcr & mcr_slice_subslice_mask); + mcr &= ~mcr_slice_subslice_mask; + mcr |= mcr_slice_subslice_select; I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); ret = I915_READ_FW(reg); - mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK); + mcr &= ~mcr_slice_subslice_mask; I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); intel_uncore_forcewake_put__locked(dev_priv, fw_domains); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 2a88231..029093a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -82,7 +82,7 @@ enum intel_engine_hangcheck_action { } #define I915_MAX_SLICES 3 -#define I915_MAX_SUBSLICES 3 +#define I915_MAX_SUBSLICES 8 #define instdone_slice_mask(dev_priv__) \ (INTEL_GEN(dev_priv__) == 7 ? \