Re: [PATCH V2 02/23] drm/etnaviv: make it possible to allocate multiple events

[Lucas Stach <l.stach@pengutronix.de>]

Am Samstag, den 22.07.2017, 11:53 +0200 schrieb Christian Gmeiner:
> This makes it possible to allocate multiple events under the event
> spinlock. This change is needed to support 'sync'-points.
> 
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index fa9c7bd98e9c..ab108b0ed573 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1137,10 +1137,12 @@ int etnaviv_gpu_fence_sync_obj(struct etnaviv_gem_object *etnaviv_obj,
>   * event management:
>   */
>  
> -static unsigned int event_alloc(struct etnaviv_gpu *gpu)
> +static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> +	unsigned int *events)
>  {
>  	unsigned long ret, flags;
> -	unsigned int event;
> +	unsigned used, i;
> +	int err = 0;
>  
>  	ret = wait_for_completion_timeout(&gpu->event_free,
>  					  msecs_to_jiffies(10 * 10000));

This isn't obvious from the current code, but there are exactly as much
completions in the queue, as there are events. See initialization of the
completions in etnaviv_gpu_init(). This means the event allocation under
spinlock always succeeds if the wait hasn't timed out. To keep this
working you need to wait for the completion nr_event times, probably
changing this to an absolute timeout, so we don't lengthen the timeout
with the number of events.

> @@ -1149,16 +1151,24 @@ static unsigned int event_alloc(struct etnaviv_gpu *gpu)
>  
>  	spin_lock_irqsave(&gpu->event_spinlock, flags);
>  
> -	/* find first free event */
> -	event = find_first_zero_bit(gpu->event_bitmap, ETNA_NR_EVENTS);
> -	if (event < ETNA_NR_EVENTS)
> +	/* are there enough free events? */
> +	used = bitmap_weight(gpu->event_bitmap, ETNA_NR_EVENTS);
> +	if (used + nr_events > ETNA_NR_EVENTS) {
> +		err = -EBUSY;
> +		goto out;
> +	}

This isn't necessary if you waited successfully for the completion to
signal nr_event times. The allocation is guaranteed to succeed in that
case. We just want an early return before even locking the spinlock,
giving back the already collected completions if one of the waits timed
out.

> +
> +	for (i = 0; i < nr_events; i++) {
> +		int event = find_first_zero_bit(gpu->event_bitmap, ETNA_NR_EVENTS);
> +
> +		events[i] = event;
>  		set_bit(event, gpu->event_bitmap);
> -	else
> -		event = ~0U;
> +	}
>  
> +out:
>  	spin_unlock_irqrestore(&gpu->event_spinlock, flags);
>  
> -	return event;
> +	return err;
>  }
>  
>  static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> @@ -1327,10 +1337,9 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
>  	 *
>  	 */
>  
> -	event = event_alloc(gpu);
> -	if (unlikely(event == ~0U)) {
> +	ret = event_alloc(gpu, 1, &event);
> +	if (!ret) {
>  		DRM_ERROR("no free event\n");
> -		ret = -EBUSY;
>  		goto out_pm_put;
>  	}
>  


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