[PATCH 1/9] dma-buf: make fence sequence numbers 64 bit v2

Christian König ckoenig.leichtzumerken at gmail.com
Wed Dec 5 13:09:58 UTC 2018


Hi David,

with a bit of luck that should do it.

Can you please run your test as well as the igt tests on this once more 
and see if everything works as expected?

Thanks,
Christian.

Am 05.12.18 um 14:08 schrieb Christian König:
> For a lot of use cases we need 64bit sequence numbers. Currently drivers
> overload the dma_fence structure to store the additional bits.
>
> Stop doing that and make the sequence number in the dma_fence always
> 64bit.
>
> For compatibility with hardware which can do only 32bit sequences the
> comparisons in __dma_fence_is_later only takes the lower 32bits as significant
> when the upper 32bits are all zero.
>
> v2: change the logic in __dma_fence_is_later
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/dma-buf/dma-fence.c            |  2 +-
>   drivers/dma-buf/sw_sync.c              |  2 +-
>   drivers/dma-buf/sync_file.c            |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c |  2 +-
>   drivers/gpu/drm/i915/i915_sw_fence.c   |  2 +-
>   drivers/gpu/drm/i915/intel_engine_cs.c |  2 +-
>   drivers/gpu/drm/vgem/vgem_fence.c      |  4 ++--
>   include/linux/dma-fence.h              | 22 +++++++++++++++-------
>   8 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 136ec04d683f..3aa8733f832a 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -649,7 +649,7 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
>    */
>   void
>   dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> -	       spinlock_t *lock, u64 context, unsigned seqno)
> +	       spinlock_t *lock, u64 context, u64 seqno)
>   {
>   	BUG_ON(!lock);
>   	BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 53c1d6d36a64..32dcf7b4c935 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -172,7 +172,7 @@ static bool timeline_fence_enable_signaling(struct dma_fence *fence)
>   static void timeline_fence_value_str(struct dma_fence *fence,
>   				    char *str, int size)
>   {
> -	snprintf(str, size, "%d", fence->seqno);
> +	snprintf(str, size, "%lld", fence->seqno);
>   }
>   
>   static void timeline_fence_timeline_value_str(struct dma_fence *fence,
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 35dd06479867..4f6305ca52c8 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -144,7 +144,7 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
>   	} else {
>   		struct dma_fence *fence = sync_file->fence;
>   
> -		snprintf(buf, len, "%s-%s%llu-%d",
> +		snprintf(buf, len, "%s-%s%llu-%lld",
>   			 fence->ops->get_driver_name(fence),
>   			 fence->ops->get_timeline_name(fence),
>   			 fence->context,
> @@ -258,7 +258,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>   
>   			i_b++;
>   		} else {
> -			if (pt_a->seqno - pt_b->seqno <= INT_MAX)
> +			if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno))
>   				add_fence(fences, &i, pt_a);
>   			else
>   				add_fence(fences, &i, pt_b);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> index 12f2bf97611f..bfaf5c6323be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> @@ -388,7 +388,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
>   			   soffset, eoffset, eoffset - soffset);
>   
>   		if (i->fence)
> -			seq_printf(m, " protected by 0x%08x on context %llu",
> +			seq_printf(m, " protected by 0x%016llx on context %llu",
>   				   i->fence->seqno, i->fence->context);
>   
>   		seq_printf(m, "\n");
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index 6dbeed079ae5..11bcdabd5177 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -393,7 +393,7 @@ static void timer_i915_sw_fence_wake(struct timer_list *t)
>   	if (!fence)
>   		return;
>   
> -	pr_notice("Asynchronous wait on fence %s:%s:%x timed out (hint:%pS)\n",
> +	pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%pS)\n",
>   		  cb->dma->ops->get_driver_name(cb->dma),
>   		  cb->dma->ops->get_timeline_name(cb->dma),
>   		  cb->dma->seqno,
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 217ed3ee1cab..f28a66c67d34 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1236,7 +1236,7 @@ static void print_request(struct drm_printer *m,
>   
>   	x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf));
>   
> -	drm_printf(m, "%s%x%s [%llx:%x]%s @ %dms: %s\n",
> +	drm_printf(m, "%s%x%s [%llx:%llx]%s @ %dms: %s\n",
>   		   prefix,
>   		   rq->global_seqno,
>   		   i915_request_completed(rq) ? "!" : "",
> diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
> index c1c420afe2dd..eb17c0cd3727 100644
> --- a/drivers/gpu/drm/vgem/vgem_fence.c
> +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> @@ -53,13 +53,13 @@ static void vgem_fence_release(struct dma_fence *base)
>   
>   static void vgem_fence_value_str(struct dma_fence *fence, char *str, int size)
>   {
> -	snprintf(str, size, "%u", fence->seqno);
> +	snprintf(str, size, "%llu", fence->seqno);
>   }
>   
>   static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
>   					  int size)
>   {
> -	snprintf(str, size, "%u",
> +	snprintf(str, size, "%llu",
>   		 dma_fence_is_signaled(fence) ? fence->seqno : 0);
>   }
>   
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 999e4b104410..6b788467b2e3 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -77,7 +77,7 @@ struct dma_fence {
>   	struct list_head cb_list;
>   	spinlock_t *lock;
>   	u64 context;
> -	unsigned seqno;
> +	u64 seqno;
>   	unsigned long flags;
>   	ktime_t timestamp;
>   	int error;
> @@ -244,7 +244,7 @@ struct dma_fence_ops {
>   };
>   
>   void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> -		    spinlock_t *lock, u64 context, unsigned seqno);
> +		    spinlock_t *lock, u64 context, u64 seqno);
>   
>   void dma_fence_release(struct kref *kref);
>   void dma_fence_free(struct dma_fence *fence);
> @@ -414,9 +414,17 @@ dma_fence_is_signaled(struct dma_fence *fence)
>    * Returns true if f1 is chronologically later than f2. Both fences must be
>    * from the same context, since a seqno is not common across contexts.
>    */
> -static inline bool __dma_fence_is_later(u32 f1, u32 f2)
> +static inline bool __dma_fence_is_later(u64 f1, u64 f2)
>   {
> -	return (int)(f1 - f2) > 0;
> +	/* This is for backward compatibility with drivers which can only handle
> +	 * 32bit sequence numbers. Use a 64bit compare when any of the higher
> +	 * bits are none zero, otherwise use a 32bit compare with wrap around
> +	 * handling.
> +	 */
> +	if (upper_32_bits(f1) || upper_32_bits(f2))
> +		return f1 > f2;
> +
> +	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>   }
>   
>   /**
> @@ -548,21 +556,21 @@ u64 dma_fence_context_alloc(unsigned num);
>   	do {								\
>   		struct dma_fence *__ff = (f);				\
>   		if (IS_ENABLED(CONFIG_DMA_FENCE_TRACE))			\
> -			pr_info("f %llu#%u: " fmt,			\
> +			pr_info("f %llu#%llu: " fmt,			\
>   				__ff->context, __ff->seqno, ##args);	\
>   	} while (0)
>   
>   #define DMA_FENCE_WARN(f, fmt, args...) \
>   	do {								\
>   		struct dma_fence *__ff = (f);				\
> -		pr_warn("f %llu#%u: " fmt, __ff->context, __ff->seqno,	\
> +		pr_warn("f %llu#%llu: " fmt, __ff->context, __ff->seqno,\
>   			 ##args);					\
>   	} while (0)
>   
>   #define DMA_FENCE_ERR(f, fmt, args...) \
>   	do {								\
>   		struct dma_fence *__ff = (f);				\
> -		pr_err("f %llu#%u: " fmt, __ff->context, __ff->seqno,	\
> +		pr_err("f %llu#%llu: " fmt, __ff->context, __ff->seqno,	\
>   			##args);					\
>   	} while (0)
>   



More information about the amd-gfx mailing list