Re: [PATCH 01/10] dma-buf: make fence sequence numbers 64 bit v2

[Chris Wilson]

Quoting Chris Wilson (2018-12-04 12:52:15)
> Quoting Christian König (2018-12-04 11:59:39)
> > -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;
> 
> Or just "(int64_t)(f1 - f2) > 0", works if drivers are only using the low
> 32b and if they use the full 64b.

No, it doesn't.
-Chris
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] dma-buf: make fence sequence numbers 64 bit v2

[Chris Wilson]

Quoting Christian König (2018-12-04 11:59:39)
> -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;

Or just "(int64_t)(f1 - f2) > 0", works if drivers are only using the low
32b and if they use the full 64b.
-Chris
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] dma-buf: make fence sequence numbers 64 bit v2

[Chunming Zhou]

在 2018/12/4 19:59, 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 
Reviewed-by: Chunming Zhou 

> ---
>   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, , pt_a);
>   else
>   add_fence(fences, , 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,
>