Re: [PATCH] drm/scheduler: Fix drm_sched_entity_set_priority()

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 24/07/2024 12:16, Christian König wrote:
> Am 24.07.24 um 10:16 schrieb Tvrtko Ursulin:
>> [SNIP]
>>> Absolutely.
>>
>> Absolutely good and absolutely me, or absolutely you? :)
> 
> You, I don't even have time to finish all the stuff I already started :/

Okay, I think I can squeeze it in.

>> These are the TODO points and their opens:
>>
>> - Adjust amdgpu_ctx_set_entity_priority() to call 
>> drm_sched_entity_modify_sched() regardless of the hw_type - to fix 
>> priority changes on a single sched other than gfx or compute.
> 
> Either that or to stop using the scheduler priority to implement 
> userspace priorities and always use different HW queues for that.
> 
>>
>> - Document sched_list array lifetime must align with the entity and 
>> adjust the callers.
>>
>> Open:
>>
>> Do you still oppose keeping sched_list for num_scheds == 1?
> 
> Not if you can fix up all callers.
> 
>> If so do you propose drm_sched_entity_modify_sched() keeps disagreeing 
>> with drm_sched_entity_init() on this detail? And keep the "one shot 
>> single sched_list" quirk in? Why is that nicer than simply keeping the 
>> list and remove that quirk? Once lifetime rules are clear it IMO is 
>> okay to always keep the list.
> 
> Yeah if every caller of drm_sched_entity_init() can be fixed I'm fine 
> with that as well.

Okay so I will tackle the above few first.

>>
>> - Remove drm_sched_entity_set_priority().
>>
>> Open:
>>
>> Should we at this point also modify amdgpu_device_init_schedulers() to 
>> stop initialising schedulers with DRM_SCHED_PRIORITY_COUNT run queues?
> 
> One step at a time.

And leave this for later.

Regards,

Tvrtko