[Intel-gfx] [PATCH 2/6] drm/i915/debugfs: reuse max slice/subslices already stored in sseu

[Lionel Landwerlin]

On 11/01/18 11:21, Tvrtko Ursulin wrote:
>
> On 18/12/2017 15:35, Lionel Landwerlin wrote:
>> Now that we have that information in topology fields, let's just 
>> reused it.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++----------------
>>   1 file changed, 10 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 0c7890b695c5..6ec7543e698f 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -4308,11 +4308,11 @@ static void gen10_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>                        struct sseu_dev_info *sseu)
>>   {
>>       const struct intel_device_info *info = INTEL_INFO(dev_priv);
>> -    int s_max = 6, ss_max = 4;
>>       int s, ss;
>> -    u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
>> +    u32 s_reg[info->sseu.max_slices],
>> +        eu_reg[2 * info->sseu.max_subslices], eu_mask[2];
>
> This is a bit unusual style, perhaps split into separate declarations.

Done.

>
>>   -    for (s = 0; s < s_max; s++) {
>> +    for (s = 0; s < info->sseu.max_slices; s++) {
>>           /*
>>            * FIXME: Valid SS Mask respects the spec and read
>>            * only valid bits for those registers, excluding reserverd
>> @@ -4334,7 +4334,7 @@ static void gen10_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>                GEN9_PGCTL_SSB_EU210_ACK |
>>                GEN9_PGCTL_SSB_EU311_ACK;
>>   -    for (s = 0; s < s_max; s++) {
>> +    for (s = 0; s < info->sseu.max_slices; s++) {
>>           if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>>               /* skip disabled slice */
>>               continue;
>> @@ -4342,7 +4342,7 @@ static void gen10_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>           sseu->slice_mask |= BIT(s);
>>           sseu->subslices_mask[s] = info->sseu.subslices_mask[s];
>>   -        for (ss = 0; ss < ss_max; ss++) {
>> +        for (ss = 0; ss < info->sseu.max_subslices; ss++) {
>>               unsigned int eu_cnt;
>>                 if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
>> @@ -4362,17 +4362,11 @@ static void gen10_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>   static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>>                       struct sseu_dev_info *sseu)
>>   {
>> -    int s_max = 3, ss_max = 4;
>> +    const struct intel_device_info *info = INTEL_INFO(dev_priv);
>>       int s, ss;
>> -    u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
>> -
>> -    /* BXT has a single slice and at most 3 subslices. */
>> -    if (IS_GEN9_LP(dev_priv)) {
>> -        s_max = 1;
>> -        ss_max = 3;
>> -    }
>> +    u32 s_reg[info->sseu.max_slices], 
>> eu_reg[2*info->sseu.max_subslices], eu_mask[2];
>
> Spaces around operators are preferred.

Done.

>
>>   -    for (s = 0; s < s_max; s++) {
>> +    for (s = 0; s < info->sseu.max_slices; s++) {
>>           s_reg[s] = I915_READ(GEN9_SLICE_PGCTL_ACK(s));
>>           eu_reg[2*s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
>>           eu_reg[2*s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
>> @@ -4387,7 +4381,7 @@ static void gen9_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>                GEN9_PGCTL_SSB_EU210_ACK |
>>                GEN9_PGCTL_SSB_EU311_ACK;
>>   -    for (s = 0; s < s_max; s++) {
>> +    for (s = 0; s < info->sseu.max_slices; s++) {
>>           if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>>               /* skip disabled slice */
>>               continue;
>> @@ -4398,7 +4392,7 @@ static void gen9_sseu_device_status(struct 
>> drm_i915_private *dev_priv,
>>               sseu->subslices_mask[s] =
>> INTEL_INFO(dev_priv)->sseu.subslices_mask[s];
>>   -        for (ss = 0; ss < ss_max; ss++) {
>> +        for (ss = 0; ss < info->sseu.max_subslices; ss++) {
>>               unsigned int eu_cnt;
>>                 if (IS_GEN9_LP(dev_priv)) {
>>
>
> With the formatting tweaks,
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Regards,
>
> Tvrtko
>