[Intel-gfx] [PATCH v2] drm/i915/icl: Update subslice define for ICL 11

[Lionel Landwerlin]

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 at 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 at intel.com> (v1)
>>> Signed-off-by: Kelvin Gardiner <kelvin.gardiner at intel.com>
>>> Signed-off-by: Oscar Mateo <oscar.mateo at 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 at 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 ? \
>>
>>
>
>