Re: [PATCH v2] drm/i915/icl: Update subslice define for ICL 11

Oscar Mateo
2018-02-09T17:44:57Z

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 ? \