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

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2211 bytes --]

Nayan Deshmukh <nayan26deshmukh@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.

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!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel