Re: [PATCH v2 1/3] drm/i915/gt: Support fixed CCS mode

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/01/2024 15:13, Joonas Lahtinen wrote:
> Quoting Tvrtko Ursulin (2024-01-05 12:39:31)
>>
>> On 04/01/2024 21:23, Andi Shyti wrote:
> 
> <SNIP>
> 
>>>>> +void intel_gt_apply_ccs_mode(struct intel_gt *gt)
>>>>> +{
>>>>> +   mutex_lock(&gt->ccs.mutex);
>>>>> +   __intel_gt_apply_ccs_mode(gt);
>>>>> +   mutex_unlock(&gt->ccs.mutex);
>>>>> +}
>>>>> +
>>>>> +void intel_gt_init_ccs_mode(struct intel_gt *gt)
>>>>> +{
>>>>> +   mutex_init(&gt->ccs.mutex);
>>>>> +   gt->ccs.mode = 1;
>>>>
>>>> What is '1'? And this question carries over to the sysfs interface in the
>>>> following patch - who will use it and where it is documented how to use it?
>>>
>>> The value '1' is explained in the comment above[1] and in the
>>
>> Do you mean this is mode '1':
>>
>>    * With 1 engine (ccs0):
>>    *   slice 0, 1, 2, 3: ccs0
>>
>> ?
>>
>> But I don't see where it says what do different modes mean on different
>> SKU configurations.
>>
>> It also does not say what should the num_slices sysfs file be used for.
>>
>> Does "mode N" mean "assign each command streamer N compute slices"? Or
>> "assign each compute slice N command streamers"?
>>
>> I wonder if we should add something user friendly into
>> Documentation/ABI/*/sysfs-... Joonas your thoughts?
> 
> We definitely should always properly document all sysfs additions, just
> seems like we less frequently remember to do so. So yeah, this should be
> documented just like other uAPI.
> 
> I also like the idea of not exposing the the file at all if the value
> can't be modified.
> 
> The ccs_mode is just supposed to allow user to select how many CCS
> engines they want to expose, and always make an even split of slices
> between them, nothing more nothing less.

Hmm I can't see that the series changes anywhere what command streamers 
will get reported as available.

Regards,

Tvrtko