RE: [PATCH 1/7] dma-buf: make fence sequence numbers 64 bit

["Zhou, David(ChunMing)" <David1.Zhou@amd.com>]

Acked-by: Chunming  Zhou <david1.zhou@amd.com>, it is better that other people from outside can take a look.

> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Thursday, November 15, 2018 7:13 PM
> To: dri-devel@lists.freedesktop.org
> Cc: chris@chris-wilson.co.uk; daniel.vetter@ffwll.ch; eric@anholt.net; Zhou,
> David(ChunMing) <David1.Zhou@amd.com>
> Subject: [PATCH 1/7] dma-buf: make fence sequence numbers 64 bit
> 
> 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 still only takes the lower 32bits as
> significant.
> 
> Signed-off-by: Christian König <christian.koenig@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              | 14 +++++++-------
>  8 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 1551ca7df394..37e24b69e94b 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -615,7 +615,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 e6ee71323a66..1fdb09fe71d3 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
> 02dba8cd033d..1393529c0c1f 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,9 @@ 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;
> +	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>  }
> 
>  /**
> @@ -547,21 +547,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)
> 
> --
> 2.14.1

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