[PATCH 3/3] drm/scheduler: modify args of drm_sched_entity_init

[Christian König]

Am 13.07.2018 um 09:20 schrieb Nayan Deshmukh:
> On Thu, Jul 12, 2018 at 11:21 PM Eric Anholt <eric at anholt.net> wrote:
>> Nayan Deshmukh <nayan26deshmukh at gmail.com> writes:
>>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> index 3dc1a4f07e3f..b2dbd1c1ba69 100644
>>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>> @@ -162,26 +162,32 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
>>>    * drm_sched_entity_init - Init a context entity used by scheduler when
>>>    * submit to HW ring.
>>>    *
>>> - * @sched: scheduler instance
>>>    * @entity: scheduler entity to init
>>> - * @rq: the run queue this entity belongs
>>> + * @rq_list: the list of run queue on which jobs from this
>>> + *           entity can be submitted
>>> + * @num_rq_list: number of run queue in rq_list
>>>    * @guilty: atomic_t set to 1 when a job on this queue
>>>    *          is found to be guilty causing a timeout
>>>    *
>>> + * Note: the rq_list should have atleast one element to schedule
>>> + *       the entity
>>> + *
>>>    * Returns 0 on success or a negative error code on failure.
>>>   */
>>> -int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
>>> -                       struct drm_sched_entity *entity,
>>> -                       struct drm_sched_rq *rq,
>>> +int drm_sched_entity_init(struct drm_sched_entity *entity,
>>> +                       struct drm_sched_rq **rq_list,
>>> +                       unsigned int num_rq_list,
>>>                          atomic_t *guilty)
>>>   {
>>> -     if (!(sched && entity && rq))
>>> +     if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
>>>                return -EINVAL;
>>>
>>>        memset(entity, 0, sizeof(struct drm_sched_entity));
>>>        INIT_LIST_HEAD(&entity->list);
>>> -     entity->rq = rq;
>>> -     entity->sched = sched;
>>> +     entity->rq_list = NULL;
>>> +     entity->rq = rq_list[0];
>>> +     entity->sched = rq_list[0]->sched;
>>> +     entity->num_rq_list = num_rq_list;
>> The API change makes sense as prep work, but I don't really like adding
>> the field to the struct (and changing the struct's docs for the existing
>> rq field) if it's going to always be NULL until a future change.
>>
>> Similarly, I'd rather see patch 2 as part of a series that uses the
>> value.
> I agree with you. I am fine with dropping the patch 2 for now and
> modifying patch 3. I am fine either way.
>
> What are your thoughts on this Christian?

Ok with me as well. In this case just send out the two patches with the 
API change.

Christian.

>> That said, while I don't currently have a usecase for load-balancing
>> between entities, I may in the future, so thanks for working on this!