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!
