diff mbox series

[6/6] drm/i915/icl: Support co-existence between per-context SSEU and OA

Message ID 20180917113058.28994-7-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Per context dynamic (sub)slice power-gating | expand

Commit Message

Tvrtko Ursulin Sept. 17, 2018, 11:30 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

When OA is active we want to lock the powergating configuration, but on
Icelake, users like the media stack will have issues if we lock to the
full device configuration.

Instead lock to a subset of (sub)slices which are currently a known
working configuration for all users.

v2:
 * Fix commit message spelling.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

Comments

Tvrtko Ursulin Oct. 1, 2018, 8:49 a.m. UTC | #1
Hi,

One final unreviewed patch on this series. Hopefully uncontroversial 
enough for a quick review?

Thanks,

Tvrtko

On 17/09/2018 12:30, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> When OA is active we want to lock the powergating configuration, but on
> Icelake, users like the media stack will have issues if we lock to the
> full device configuration.
> 
> Instead lock to a subset of (sub)slices which are currently a known
> working configuration for all users.
> 
> v2:
>   * Fix commit message spelling.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index b5603e977a3f..cded1f1d9ec2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
>   
>   	/*
>   	 * If i915/perf is active, we want a stable powergating configuration
> -	 * on the system. The most natural configuration to take in that case
> -	 * is the default (i.e maximum the hardware can do).
> +	 * on the system.
> +	 *
> +	 * We could choose full enablement, but on ICL we know there are use
> +	 * cases which disable slices for functional, apart for performance
> +	 * reasons. So in this case we select a known stable subset.
>   	 */
> -	if (unlikely(i915->perf.oa.exclusive_stream))
> -		ctx_sseu = intel_device_default_sseu(i915);
> -	else
> +	if (!i915->perf.oa.exclusive_stream) {
>   		ctx_sseu = *req_sseu;
> +	} else {
> +		ctx_sseu = intel_device_default_sseu(i915);
> +
> +		if (IS_GEN11(i915)) {
> +			/*
> +			 * We only need subslice count so it doesn't matter
> +			 * which ones we select - just turn of low bits in the
> +			 * amount of half of all available subslices per slice.
> +			 */
> +			ctx_sseu.subslice_mask =
> +				~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
> +			ctx_sseu.slice_mask = 0x1;
> +		}
> +	}
>   
>   	slices = hweight8(ctx_sseu.slice_mask);
>   	subslices = hweight8(ctx_sseu.subslice_mask);
>
Lionel Landwerlin Oct. 1, 2018, 9:50 a.m. UTC | #2
On 17/09/2018 13:30, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> When OA is active we want to lock the powergating configuration, but on
> Icelake, users like the media stack will have issues if we lock to the
> full device configuration.
>
> Instead lock to a subset of (sub)slices which are currently a known
> working configuration for all users.
>
> v2:
>   * Fix commit message spelling.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index b5603e977a3f..cded1f1d9ec2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
>   
>   	/*
>   	 * If i915/perf is active, we want a stable powergating configuration
> -	 * on the system. The most natural configuration to take in that case
> -	 * is the default (i.e maximum the hardware can do).
> +	 * on the system.
> +	 *
> +	 * We could choose full enablement, but on ICL we know there are use
> +	 * cases which disable slices for functional, apart for performance
> +	 * reasons. So in this case we select a known stable subset.
>   	 */
> -	if (unlikely(i915->perf.oa.exclusive_stream))
> -		ctx_sseu = intel_device_default_sseu(i915);
> -	else
> +	if (!i915->perf.oa.exclusive_stream) {
>   		ctx_sseu = *req_sseu;
> +	} else {
> +		ctx_sseu = intel_device_default_sseu(i915);
> +
> +		if (IS_GEN11(i915)) {
> +			/*
> +			 * We only need subslice count so it doesn't matter
> +			 * which ones we select - just turn of low bits in the

s/turn of/turn off/


> +			 * amount of half of all available subslices per slice.
> +			 */
> +			ctx_sseu.subslice_mask =
> +				~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));


I would go with :


ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;


Documentation says that the first 4 subslices are the "big" ones 
(gathered from the fusing register fields which go from 
slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this should 
be equally media/3d capable.



> +			ctx_sseu.slice_mask = 0x1;
> +		}
> +	}
>   
>   	slices = hweight8(ctx_sseu.slice_mask);
>   	subslices = hweight8(ctx_sseu.subslice_mask);
Tvrtko Ursulin Oct. 1, 2018, 10:20 a.m. UTC | #3
On 01/10/2018 10:50, Lionel Landwerlin wrote:
> On 17/09/2018 13:30, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> When OA is active we want to lock the powergating configuration, but on
>> Icelake, users like the media stack will have issues if we lock to the
>> full device configuration.
>>
>> Instead lock to a subset of (sub)slices which are currently a known
>> working configuration for all users.
>>
>> v2:
>>   * Fix commit message spelling.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index b5603e977a3f..cded1f1d9ec2 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private 
>> *i915, struct intel_sseu *req_sseu)
>>       /*
>>        * If i915/perf is active, we want a stable powergating 
>> configuration
>> -     * on the system. The most natural configuration to take in that 
>> case
>> -     * is the default (i.e maximum the hardware can do).
>> +     * on the system.
>> +     *
>> +     * We could choose full enablement, but on ICL we know there are use
>> +     * cases which disable slices for functional, apart for performance
>> +     * reasons. So in this case we select a known stable subset.
>>        */
>> -    if (unlikely(i915->perf.oa.exclusive_stream))
>> -        ctx_sseu = intel_device_default_sseu(i915);
>> -    else
>> +    if (!i915->perf.oa.exclusive_stream) {
>>           ctx_sseu = *req_sseu;
>> +    } else {
>> +        ctx_sseu = intel_device_default_sseu(i915);
>> +
>> +        if (IS_GEN11(i915)) {
>> +            /*
>> +             * We only need subslice count so it doesn't matter
>> +             * which ones we select - just turn of low bits in the
> 
> s/turn of/turn off/

Yep, thanks.

> 
>> +             * amount of half of all available subslices per slice.
>> +             */
>> +            ctx_sseu.subslice_mask =
>> +                ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
> 
> 
> I would go with :
> 
> 
> ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;
> 
> 
> Documentation says that the first 4 subslices are the "big" ones 
> (gathered from the fusing register fields which go from 
> slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this should 
> be equally media/3d capable.

Doesn't work I think - one 1x6x8 part I've seen has a subslice mask of 
0b11111100 and there we want to have three subslices enabled.

Regards,

Tvrtko

> 
> 
>> +            ctx_sseu.slice_mask = 0x1;
>> +        }
>> +    }
>>       slices = hweight8(ctx_sseu.slice_mask);
>>       subslices = hweight8(ctx_sseu.subslice_mask);
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lionel Landwerlin Oct. 1, 2018, 11:06 a.m. UTC | #4
On 01/10/2018 12:20, Tvrtko Ursulin wrote:
>
> On 01/10/2018 10:50, Lionel Landwerlin wrote:
>> On 17/09/2018 13:30, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> When OA is active we want to lock the powergating configuration, but on
>>> Icelake, users like the media stack will have issues if we lock to the
>>> full device configuration.
>>>
>>> Instead lock to a subset of (sub)slices which are currently a known
>>> working configuration for all users.
>>>
>>> v2:
>>>   * Fix commit message spelling.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index b5603e977a3f..cded1f1d9ec2 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private 
>>> *i915, struct intel_sseu *req_sseu)
>>>       /*
>>>        * If i915/perf is active, we want a stable powergating 
>>> configuration
>>> -     * on the system. The most natural configuration to take in 
>>> that case
>>> -     * is the default (i.e maximum the hardware can do).
>>> +     * on the system.
>>> +     *
>>> +     * We could choose full enablement, but on ICL we know there 
>>> are use
>>> +     * cases which disable slices for functional, apart for 
>>> performance
>>> +     * reasons. So in this case we select a known stable subset.
>>>        */
>>> -    if (unlikely(i915->perf.oa.exclusive_stream))
>>> -        ctx_sseu = intel_device_default_sseu(i915);
>>> -    else
>>> +    if (!i915->perf.oa.exclusive_stream) {
>>>           ctx_sseu = *req_sseu;
>>> +    } else {
>>> +        ctx_sseu = intel_device_default_sseu(i915);
>>> +
>>> +        if (IS_GEN11(i915)) {
>>> +            /*
>>> +             * We only need subslice count so it doesn't matter
>>> +             * which ones we select - just turn of low bits in the
>>
>> s/turn of/turn off/
>
> Yep, thanks.
>
>>
>>> +             * amount of half of all available subslices per slice.
>>> +             */
>>> +            ctx_sseu.subslice_mask =
>>> +                ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
>>
>>
>> I would go with :
>>
>>
>> ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;
>>
>>
>> Documentation says that the first 4 subslices are the "big" ones 
>> (gathered from the fusing register fields which go from 
>> slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this 
>> should be equally media/3d capable.
>
> Doesn't work I think - one 1x6x8 part I've seen has a subslice mask of 
> 0b11111100 and there we want to have three subslices enabled.
>
> Regards,
>
> Tvrtko


Thanks, then the fusing fields don't match for a big/small sets of 
subslices.

I guess the fusing might be in this pattern (Small/Big) : SBSBSBSB

I was hoping we could know so that we program the powergating at the 
most capable configuration.


-

Lionel


>
>>
>>
>>> +            ctx_sseu.slice_mask = 0x1;
>>> +        }
>>> +    }
>>>       slices = hweight8(ctx_sseu.slice_mask);
>>>       subslices = hweight8(ctx_sseu.subslice_mask);
>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Tvrtko Ursulin Oct. 1, 2018, 11:42 a.m. UTC | #5
On 01/10/2018 12:06, Lionel Landwerlin wrote:
> On 01/10/2018 12:20, Tvrtko Ursulin wrote:
>>
>> On 01/10/2018 10:50, Lionel Landwerlin wrote:
>>> On 17/09/2018 13:30, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> When OA is active we want to lock the powergating configuration, but on
>>>> Icelake, users like the media stack will have issues if we lock to the
>>>> full device configuration.
>>>>
>>>> Instead lock to a subset of (sub)slices which are currently a known
>>>> working configuration for all users.
>>>>
>>>> v2:
>>>>   * Fix commit message spelling.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>>> index b5603e977a3f..cded1f1d9ec2 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private 
>>>> *i915, struct intel_sseu *req_sseu)
>>>>       /*
>>>>        * If i915/perf is active, we want a stable powergating 
>>>> configuration
>>>> -     * on the system. The most natural configuration to take in 
>>>> that case
>>>> -     * is the default (i.e maximum the hardware can do).
>>>> +     * on the system.
>>>> +     *
>>>> +     * We could choose full enablement, but on ICL we know there 
>>>> are use
>>>> +     * cases which disable slices for functional, apart for 
>>>> performance
>>>> +     * reasons. So in this case we select a known stable subset.
>>>>        */
>>>> -    if (unlikely(i915->perf.oa.exclusive_stream))
>>>> -        ctx_sseu = intel_device_default_sseu(i915);
>>>> -    else
>>>> +    if (!i915->perf.oa.exclusive_stream) {
>>>>           ctx_sseu = *req_sseu;
>>>> +    } else {
>>>> +        ctx_sseu = intel_device_default_sseu(i915);
>>>> +
>>>> +        if (IS_GEN11(i915)) {
>>>> +            /*
>>>> +             * We only need subslice count so it doesn't matter
>>>> +             * which ones we select - just turn of low bits in the
>>>
>>> s/turn of/turn off/
>>
>> Yep, thanks.
>>
>>>
>>>> +             * amount of half of all available subslices per slice.
>>>> +             */
>>>> +            ctx_sseu.subslice_mask =
>>>> +                ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
>>>
>>>
>>> I would go with :
>>>
>>>
>>> ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;
>>>
>>>
>>> Documentation says that the first 4 subslices are the "big" ones 
>>> (gathered from the fusing register fields which go from 
>>> slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this 
>>> should be equally media/3d capable.
>>
>> Doesn't work I think - one 1x6x8 part I've seen has a subslice mask of 
>> 0b11111100 and there we want to have three subslices enabled.
>>
>> Regards,
>>
>> Tvrtko
> 
> 
> Thanks, then the fusing fields don't match for a big/small sets of 
> subslices.
> 
> I guess the fusing might be in this pattern (Small/Big) : SBSBSBSB
> 
> I was hoping we could know so that we program the powergating at the 
> most capable configuration.

It will be the most capable configuration. Did you forget the RPCS 
register in ICL works with counts and not masks? According the that 
Bspec table we cannot select anything but the most capable configuration.

Regards,

Tvrtko
Lionel Landwerlin Oct. 1, 2018, 2:18 p.m. UTC | #6
On 01/10/2018 13:42, Tvrtko Ursulin wrote:
>
> On 01/10/2018 12:06, Lionel Landwerlin wrote:
>> On 01/10/2018 12:20, Tvrtko Ursulin wrote:
>>>
>>> On 01/10/2018 10:50, Lionel Landwerlin wrote:
>>>> On 17/09/2018 13:30, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> When OA is active we want to lock the powergating configuration, 
>>>>> but on
>>>>> Icelake, users like the media stack will have issues if we lock to 
>>>>> the
>>>>> full device configuration.
>>>>>
>>>>> Instead lock to a subset of (sub)slices which are currently a known
>>>>> working configuration for all users.
>>>>>
>>>>> v2:
>>>>>   * Fix commit message spelling.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>>>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>>>> index b5603e977a3f..cded1f1d9ec2 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>>> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private 
>>>>> *i915, struct intel_sseu *req_sseu)
>>>>>       /*
>>>>>        * If i915/perf is active, we want a stable powergating 
>>>>> configuration
>>>>> -     * on the system. The most natural configuration to take in 
>>>>> that case
>>>>> -     * is the default (i.e maximum the hardware can do).
>>>>> +     * on the system.
>>>>> +     *
>>>>> +     * We could choose full enablement, but on ICL we know there 
>>>>> are use
>>>>> +     * cases which disable slices for functional, apart for 
>>>>> performance
>>>>> +     * reasons. So in this case we select a known stable subset.
>>>>>        */
>>>>> -    if (unlikely(i915->perf.oa.exclusive_stream))
>>>>> -        ctx_sseu = intel_device_default_sseu(i915);
>>>>> -    else
>>>>> +    if (!i915->perf.oa.exclusive_stream) {
>>>>>           ctx_sseu = *req_sseu;
>>>>> +    } else {
>>>>> +        ctx_sseu = intel_device_default_sseu(i915);
>>>>> +
>>>>> +        if (IS_GEN11(i915)) {
>>>>> +            /*
>>>>> +             * We only need subslice count so it doesn't matter
>>>>> +             * which ones we select - just turn of low bits in the
>>>>
>>>> s/turn of/turn off/
>>>
>>> Yep, thanks.
>>>
>>>>
>>>>> +             * amount of half of all available subslices per slice.
>>>>> +             */
>>>>> +            ctx_sseu.subslice_mask =
>>>>> +                ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
>>>>
>>>>
>>>> I would go with :
>>>>
>>>>
>>>> ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;
>>>>
>>>>
>>>> Documentation says that the first 4 subslices are the "big" ones 
>>>> (gathered from the fusing register fields which go from 
>>>> slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this 
>>>> should be equally media/3d capable.
>>>
>>> Doesn't work I think - one 1x6x8 part I've seen has a subslice mask 
>>> of 0b11111100 and there we want to have three subslices enabled.
>>>
>>> Regards,
>>>
>>> Tvrtko
>>
>>
>> Thanks, then the fusing fields don't match for a big/small sets of 
>> subslices.
>>
>> I guess the fusing might be in this pattern (Small/Big) : SBSBSBSB
>>
>> I was hoping we could know so that we program the powergating at the 
>> most capable configuration.
>
> It will be the most capable configuration. Did you forget the RPCS 
> register in ICL works with counts and not masks? According the that 
> Bspec table we cannot select anything but the most capable configuration.
>
> Regards,
>
> Tvrtko
>
Sorry, I must have forgotten. I was trying to read the docs again 
looking for something that said the fusing always happened in pairs.


I'm confident we need to limit the subslices when OA is on and if you 
know where the details are written down (maybe add the bspec number?), 
this is :


Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Tvrtko Ursulin Oct. 1, 2018, 3:05 p.m. UTC | #7
On 01/10/2018 15:18, Lionel Landwerlin wrote:
> On 01/10/2018 13:42, Tvrtko Ursulin wrote:
>>
>> On 01/10/2018 12:06, Lionel Landwerlin wrote:
>>> On 01/10/2018 12:20, Tvrtko Ursulin wrote:
>>>>
>>>> On 01/10/2018 10:50, Lionel Landwerlin wrote:
>>>>> On 17/09/2018 13:30, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> When OA is active we want to lock the powergating configuration, 
>>>>>> but on
>>>>>> Icelake, users like the media stack will have issues if we lock to 
>>>>>> the
>>>>>> full device configuration.
>>>>>>
>>>>>> Instead lock to a subset of (sub)slices which are currently a known
>>>>>> working configuration for all users.
>>>>>>
>>>>>> v2:
>>>>>>   * Fix commit message spelling.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++-----
>>>>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>>>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>>>>> index b5603e977a3f..cded1f1d9ec2 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>>>> @@ -2521,13 +2521,28 @@ u32 gen8_make_rpcs(struct drm_i915_private 
>>>>>> *i915, struct intel_sseu *req_sseu)
>>>>>>       /*
>>>>>>        * If i915/perf is active, we want a stable powergating 
>>>>>> configuration
>>>>>> -     * on the system. The most natural configuration to take in 
>>>>>> that case
>>>>>> -     * is the default (i.e maximum the hardware can do).
>>>>>> +     * on the system.
>>>>>> +     *
>>>>>> +     * We could choose full enablement, but on ICL we know there 
>>>>>> are use
>>>>>> +     * cases which disable slices for functional, apart for 
>>>>>> performance
>>>>>> +     * reasons. So in this case we select a known stable subset.
>>>>>>        */
>>>>>> -    if (unlikely(i915->perf.oa.exclusive_stream))
>>>>>> -        ctx_sseu = intel_device_default_sseu(i915);
>>>>>> -    else
>>>>>> +    if (!i915->perf.oa.exclusive_stream) {
>>>>>>           ctx_sseu = *req_sseu;
>>>>>> +    } else {
>>>>>> +        ctx_sseu = intel_device_default_sseu(i915);
>>>>>> +
>>>>>> +        if (IS_GEN11(i915)) {
>>>>>> +            /*
>>>>>> +             * We only need subslice count so it doesn't matter
>>>>>> +             * which ones we select - just turn of low bits in the
>>>>>
>>>>> s/turn of/turn off/
>>>>
>>>> Yep, thanks.
>>>>
>>>>>
>>>>>> +             * amount of half of all available subslices per slice.
>>>>>> +             */
>>>>>> +            ctx_sseu.subslice_mask =
>>>>>> +                ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
>>>>>
>>>>>
>>>>> I would go with :
>>>>>
>>>>>
>>>>> ctx_sseu.subslice_mask = ctx_sseu.subslice_mask & 0xf;
>>>>>
>>>>>
>>>>> Documentation says that the first 4 subslices are the "big" ones 
>>>>> (gathered from the fusing register fields which go from 
>>>>> slice0-subslice[0-3] then slice1-subslice[0-3], etc...), so this 
>>>>> should be equally media/3d capable.
>>>>
>>>> Doesn't work I think - one 1x6x8 part I've seen has a subslice mask 
>>>> of 0b11111100 and there we want to have three subslices enabled.
>>>>
>>>> Regards,
>>>>
>>>> Tvrtko
>>>
>>>
>>> Thanks, then the fusing fields don't match for a big/small sets of 
>>> subslices.
>>>
>>> I guess the fusing might be in this pattern (Small/Big) : SBSBSBSB
>>>
>>> I was hoping we could know so that we program the powergating at the 
>>> most capable configuration.
>>
>> It will be the most capable configuration. Did you forget the RPCS 
>> register in ICL works with counts and not masks? According the that 
>> Bspec table we cannot select anything but the most capable configuration.
>>
>> Regards,
>>
>> Tvrtko
>>
> Sorry, I must have forgotten. I was trying to read the docs again 
> looking for something that said the fusing always happened in pairs.
> 
> 
> I'm confident we need to limit the subslices when OA is on and if you 
> know where the details are written down (maybe add the bspec number?), 
> this is :

Hm I was certain I have put the link in some time ago, but you are 
right, I have not so will do!

> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Thanks!

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b5603e977a3f..cded1f1d9ec2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2521,13 +2521,28 @@  u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
 
 	/*
 	 * If i915/perf is active, we want a stable powergating configuration
-	 * on the system. The most natural configuration to take in that case
-	 * is the default (i.e maximum the hardware can do).
+	 * on the system.
+	 *
+	 * We could choose full enablement, but on ICL we know there are use
+	 * cases which disable slices for functional, apart for performance
+	 * reasons. So in this case we select a known stable subset.
 	 */
-	if (unlikely(i915->perf.oa.exclusive_stream))
-		ctx_sseu = intel_device_default_sseu(i915);
-	else
+	if (!i915->perf.oa.exclusive_stream) {
 		ctx_sseu = *req_sseu;
+	} else {
+		ctx_sseu = intel_device_default_sseu(i915);
+
+		if (IS_GEN11(i915)) {
+			/*
+			 * We only need subslice count so it doesn't matter
+			 * which ones we select - just turn of low bits in the
+			 * amount of half of all available subslices per slice.
+			 */
+			ctx_sseu.subslice_mask =
+				~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
+			ctx_sseu.slice_mask = 0x1;
+		}
+	}
 
 	slices = hweight8(ctx_sseu.slice_mask);
 	subslices = hweight8(ctx_sseu.subslice_mask);