[PATCH 01/11] dma-buf: make fence sequence numbers 64 bit

Christian König ckoenig.leichtzumerken at gmail.com
Thu Nov 29 08:43:21 UTC 2018


Am 29.11.18 um 09:08 schrieb zhoucm1:
>
>
> On 2018年11月28日 22:50, Christian König wrote:
>> 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.
> Can't we compare 64bit variable directly?  Can we do it as below?

As I wrote we don't want to do this because we have tons of hardware 
(including AMDs UVD/VCE engines) which can only do 32bit fences.

So only the lower 32bits are used as significant here.

Christian.

>
> -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 (f1 > f2) ? true : false;
>
>  }
>
> -David
>
>>
>> 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              | 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 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 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)
>



More information about the amd-gfx mailing list