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

[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?
>
> 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!