* [RFC PATCH] dma-fence: Remove 64-bit flag @ 2025-10-17 9:31 Philipp Stanner 2025-10-17 21:28 ` Matthew Brost 0 siblings, 1 reply; 5+ messages in thread From: Philipp Stanner @ 2025-10-17 9:31 UTC (permalink / raw) To: Sumit Semwal, Gustavo Padovan, Christian König, tursulin Cc: linux-media, dri-devel, linaro-mm-sig, linux-kernel, Philipp Stanner It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore, since seqno is a u64 everywhere. Remove the unneeded flag. Signed-off-by: Philipp Stanner <phasta@kernel.org> --- Seems to me that this flag doesn't really do anything anymore? I *suspect* that it could be that some drivers pass a u32 to dma_fence_init()? I guess they could be ported, couldn't they. P. --- drivers/dma-buf/dma-fence.c | 3 +-- include/linux/dma-fence.h | 10 +--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 3f78c56b58dc..24794c027813 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1078,8 +1078,7 @@ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno) { - __dma_fence_init(fence, ops, lock, context, seqno, - BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); + __dma_fence_init(fence, ops, lock, context, seqno, 0); } EXPORT_SYMBOL(dma_fence_init64); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 64639e104110..4eca2db28625 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -98,7 +98,6 @@ struct dma_fence { }; enum dma_fence_flag_bits { - DMA_FENCE_FLAG_SEQNO64_BIT, DMA_FENCE_FLAG_SIGNALED_BIT, DMA_FENCE_FLAG_TIMESTAMP_BIT, DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence) */ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) { - /* This is for backward compatibility with drivers which can only handle - * 32bit sequence numbers. Use a 64bit compare when the driver says to - * do so. - */ - if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) - return f1 > f2; - - return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; + return f1 > f2; } /** -- 2.49.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] dma-fence: Remove 64-bit flag 2025-10-17 9:31 [RFC PATCH] dma-fence: Remove 64-bit flag Philipp Stanner @ 2025-10-17 21:28 ` Matthew Brost 2025-10-20 8:16 ` Philipp Stanner 0 siblings, 1 reply; 5+ messages in thread From: Matthew Brost @ 2025-10-17 21:28 UTC (permalink / raw) To: Philipp Stanner Cc: Sumit Semwal, Gustavo Padovan, Christian König, tursulin, linux-media, dri-devel, linaro-mm-sig, linux-kernel On Fri, Oct 17, 2025 at 11:31:47AM +0200, Philipp Stanner wrote: > It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore, > since seqno is a u64 everywhere. > > Remove the unneeded flag. > > Signed-off-by: Philipp Stanner <phasta@kernel.org> > --- > Seems to me that this flag doesn't really do anything anymore? > > I *suspect* that it could be that some drivers pass a u32 to > dma_fence_init()? I guess they could be ported, couldn't they. > Xe uses 32-bit hardware fence sequence numbers—see [1] and [2]. We could switch to 64-bit hardware fence sequence numbers, but that would require changes on the driver side. If you sent this to our CI, I’m fairly certain we’d see a bunch of failures. I suspect this would also break several other drivers. As I mentioned, all Xe-supported platforms could be updated since their rings support 64-bit store instructions. However, I suspect that very old i915 platforms don’t support such instructions in the ring. I agree this is a legacy issue, and we should probably use 64-bit sequence numbers in Xe. But again, platforms and drivers that are decades old might break as a result. Matt [1] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence.c#L264 [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence_types.h#L51 > P. > --- > drivers/dma-buf/dma-fence.c | 3 +-- > include/linux/dma-fence.h | 10 +--------- > 2 files changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 3f78c56b58dc..24794c027813 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -1078,8 +1078,7 @@ void > dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > spinlock_t *lock, u64 context, u64 seqno) > { > - __dma_fence_init(fence, ops, lock, context, seqno, > - BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); > + __dma_fence_init(fence, ops, lock, context, seqno, 0); > } > EXPORT_SYMBOL(dma_fence_init64); > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 64639e104110..4eca2db28625 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -98,7 +98,6 @@ struct dma_fence { > }; > > enum dma_fence_flag_bits { > - DMA_FENCE_FLAG_SEQNO64_BIT, > DMA_FENCE_FLAG_SIGNALED_BIT, > DMA_FENCE_FLAG_TIMESTAMP_BIT, > DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, > @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence) > */ > static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) > { > - /* This is for backward compatibility with drivers which can only handle > - * 32bit sequence numbers. Use a 64bit compare when the driver says to > - * do so. > - */ > - if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) > - return f1 > f2; > - > - return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; > + return f1 > f2; > } > > /** > -- > 2.49.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] dma-fence: Remove 64-bit flag 2025-10-17 21:28 ` Matthew Brost @ 2025-10-20 8:16 ` Philipp Stanner 2025-10-20 11:18 ` Matthew Brost 0 siblings, 1 reply; 5+ messages in thread From: Philipp Stanner @ 2025-10-20 8:16 UTC (permalink / raw) To: Matthew Brost, Philipp Stanner Cc: Sumit Semwal, Gustavo Padovan, Christian König, tursulin, linux-media, dri-devel, linaro-mm-sig, linux-kernel On Fri, 2025-10-17 at 14:28 -0700, Matthew Brost wrote: > On Fri, Oct 17, 2025 at 11:31:47AM +0200, Philipp Stanner wrote: > > It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore, > > since seqno is a u64 everywhere. > > > > Remove the unneeded flag. > > > > Signed-off-by: Philipp Stanner <phasta@kernel.org> > > --- > > Seems to me that this flag doesn't really do anything anymore? > > > > I *suspect* that it could be that some drivers pass a u32 to > > dma_fence_init()? I guess they could be ported, couldn't they. > > > > Xe uses 32-bit hardware fence sequence numbers—see [1] and [2]. We could > switch to 64-bit hardware fence sequence numbers, but that would require > changes on the driver side. If you sent this to our CI, I’m fairly > certain we’d see a bunch of failures. I suspect this would also break > several other drivers. What exactly breaks? Help me out here; if you pass a u32 for a u64, doesn't the C standard guarantee that the higher, unused 32 bits will be 0? Because the only thing the flag still does is do this lower_32 check in fence_is_later. P. > > As I mentioned, all Xe-supported platforms could be updated since their > rings support 64-bit store instructions. However, I suspect that very > old i915 platforms don’t support such instructions in the ring. I agree > this is a legacy issue, and we should probably use 64-bit sequence > numbers in Xe. But again, platforms and drivers that are decades old > might break as a result. > > Matt > > [1] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence.c#L264 > [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence_types.h#L51 > > > P. > > --- > > drivers/dma-buf/dma-fence.c | 3 +-- > > include/linux/dma-fence.h | 10 +--------- > > 2 files changed, 2 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > index 3f78c56b58dc..24794c027813 100644 > > --- a/drivers/dma-buf/dma-fence.c > > +++ b/drivers/dma-buf/dma-fence.c > > @@ -1078,8 +1078,7 @@ void > > dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > > spinlock_t *lock, u64 context, u64 seqno) > > { > > - __dma_fence_init(fence, ops, lock, context, seqno, > > - BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); > > + __dma_fence_init(fence, ops, lock, context, seqno, 0); > > } > > EXPORT_SYMBOL(dma_fence_init64); > > > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > > index 64639e104110..4eca2db28625 100644 > > --- a/include/linux/dma-fence.h > > +++ b/include/linux/dma-fence.h > > @@ -98,7 +98,6 @@ struct dma_fence { > > }; > > > > enum dma_fence_flag_bits { > > - DMA_FENCE_FLAG_SEQNO64_BIT, > > DMA_FENCE_FLAG_SIGNALED_BIT, > > DMA_FENCE_FLAG_TIMESTAMP_BIT, > > DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, > > @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence) > > */ > > static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) > > { > > - /* This is for backward compatibility with drivers which can only handle > > - * 32bit sequence numbers. Use a 64bit compare when the driver says to > > - * do so. > > - */ > > - if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) > > - return f1 > f2; > > - > > - return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; > > + return f1 > f2; > > } > > > > /** > > -- > > 2.49.0 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] dma-fence: Remove 64-bit flag 2025-10-20 8:16 ` Philipp Stanner @ 2025-10-20 11:18 ` Matthew Brost 2025-10-27 7:41 ` Christian König 0 siblings, 1 reply; 5+ messages in thread From: Matthew Brost @ 2025-10-20 11:18 UTC (permalink / raw) To: phasta Cc: Sumit Semwal, Gustavo Padovan, Christian König, tursulin, linux-media, dri-devel, linaro-mm-sig, linux-kernel On Mon, Oct 20, 2025 at 10:16:23AM +0200, Philipp Stanner wrote: > On Fri, 2025-10-17 at 14:28 -0700, Matthew Brost wrote: > > On Fri, Oct 17, 2025 at 11:31:47AM +0200, Philipp Stanner wrote: > > > It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore, > > > since seqno is a u64 everywhere. > > > > > > Remove the unneeded flag. > > > > > > Signed-off-by: Philipp Stanner <phasta@kernel.org> > > > --- > > > Seems to me that this flag doesn't really do anything anymore? > > > > > > I *suspect* that it could be that some drivers pass a u32 to > > > dma_fence_init()? I guess they could be ported, couldn't they. > > > > > > > Xe uses 32-bit hardware fence sequence numbers—see [1] and [2]. We could > > switch to 64-bit hardware fence sequence numbers, but that would require > > changes on the driver side. If you sent this to our CI, I’m fairly > > certain we’d see a bunch of failures. I suspect this would also break > > several other drivers. > > What exactly breaks? Help me out here; if you pass a u32 for a u64, Seqno wraps. > doesn't the C standard guarantee that the higher, unused 32 bits will > be 0? return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; Look at the above logic. f1 = 0x0; f2 = 0xffffffff; /* -1 */ The above statement will correctly return true. Compared to the below statement which returns false. return f1 > f2; We test seqno wraps in Xe by setting our initial seqno to -127, again if you send this patch to our CI any test which sends more than 127 job on queue will likely fail. Matt > > Because the only thing the flag still does is do this lower_32 check in > fence_is_later. > > P. > > > > > As I mentioned, all Xe-supported platforms could be updated since their > > rings support 64-bit store instructions. However, I suspect that very > > old i915 platforms don’t support such instructions in the ring. I agree > > this is a legacy issue, and we should probably use 64-bit sequence > > numbers in Xe. But again, platforms and drivers that are decades old > > might break as a result. > > > > Matt > > > > [1] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence.c#L264 > > [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence_types.h#L51 > > > > > P. > > > --- > > > drivers/dma-buf/dma-fence.c | 3 +-- > > > include/linux/dma-fence.h | 10 +--------- > > > 2 files changed, 2 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > > index 3f78c56b58dc..24794c027813 100644 > > > --- a/drivers/dma-buf/dma-fence.c > > > +++ b/drivers/dma-buf/dma-fence.c > > > @@ -1078,8 +1078,7 @@ void > > > dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > > > spinlock_t *lock, u64 context, u64 seqno) > > > { > > > - __dma_fence_init(fence, ops, lock, context, seqno, > > > - BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); > > > + __dma_fence_init(fence, ops, lock, context, seqno, 0); > > > } > > > EXPORT_SYMBOL(dma_fence_init64); > > > > > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > > > index 64639e104110..4eca2db28625 100644 > > > --- a/include/linux/dma-fence.h > > > +++ b/include/linux/dma-fence.h > > > @@ -98,7 +98,6 @@ struct dma_fence { > > > }; > > > > > > enum dma_fence_flag_bits { > > > - DMA_FENCE_FLAG_SEQNO64_BIT, > > > DMA_FENCE_FLAG_SIGNALED_BIT, > > > DMA_FENCE_FLAG_TIMESTAMP_BIT, > > > DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, > > > @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence) > > > */ > > > static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) > > > { > > > - /* This is for backward compatibility with drivers which can only handle > > > - * 32bit sequence numbers. Use a 64bit compare when the driver says to > > > - * do so. > > > - */ > > > - if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) > > > - return f1 > f2; > > > - > > > - return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; > > > + return f1 > f2; > > > } > > > > > > /** > > > -- > > > 2.49.0 > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] dma-fence: Remove 64-bit flag 2025-10-20 11:18 ` Matthew Brost @ 2025-10-27 7:41 ` Christian König 0 siblings, 0 replies; 5+ messages in thread From: Christian König @ 2025-10-27 7:41 UTC (permalink / raw) To: Matthew Brost, phasta Cc: Sumit Semwal, Gustavo Padovan, tursulin, linux-media, dri-devel, linaro-mm-sig, linux-kernel On 10/20/25 13:18, Matthew Brost wrote: > On Mon, Oct 20, 2025 at 10:16:23AM +0200, Philipp Stanner wrote: >> On Fri, 2025-10-17 at 14:28 -0700, Matthew Brost wrote: >>> On Fri, Oct 17, 2025 at 11:31:47AM +0200, Philipp Stanner wrote: >>>> It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore, >>>> since seqno is a u64 everywhere. >>>> >>>> Remove the unneeded flag. >>>> >>>> Signed-off-by: Philipp Stanner <phasta@kernel.org> >>>> --- >>>> Seems to me that this flag doesn't really do anything anymore? >>>> >>>> I *suspect* that it could be that some drivers pass a u32 to >>>> dma_fence_init()? I guess they could be ported, couldn't they. >>>> >>> >>> Xe uses 32-bit hardware fence sequence numbers—see [1] and [2]. We could >>> switch to 64-bit hardware fence sequence numbers, but that would require >>> changes on the driver side. If you sent this to our CI, I’m fairly >>> certain we’d see a bunch of failures. I suspect this would also break >>> several other drivers. >> >> What exactly breaks? Help me out here; if you pass a u32 for a u64, > > Seqno wraps. > >> doesn't the C standard guarantee that the higher, unused 32 bits will >> be 0? > > return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; > > Look at the above logic. > > f1 = 0x0; > f2 = 0xffffffff; /* -1 */ > > The above statement will correctly return true. > > Compared to the below statement which returns false. > > return f1 > f2; > > We test seqno wraps in Xe by setting our initial seqno to -127, again if > you send this patch to our CI any test which sends more than 127 job on > queue will likely fail. Yeah, exactly that's why this flag is needed for quite a lot of things. Question is what is missing in the documentation to make that clear? Regards, Christian. > > Matt > >> >> Because the only thing the flag still does is do this lower_32 check in >> fence_is_later. >> >> P. >> >>> >>> As I mentioned, all Xe-supported platforms could be updated since their >>> rings support 64-bit store instructions. However, I suspect that very >>> old i915 platforms don’t support such instructions in the ring. I agree >>> this is a legacy issue, and we should probably use 64-bit sequence >>> numbers in Xe. But again, platforms and drivers that are decades old >>> might break as a result. >>> >>> Matt >>> >>> [1] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence.c#L264 >>> [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence_types.h#L51 >>> >>>> P. >>>> --- >>>> drivers/dma-buf/dma-fence.c | 3 +-- >>>> include/linux/dma-fence.h | 10 +--------- >>>> 2 files changed, 2 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c >>>> index 3f78c56b58dc..24794c027813 100644 >>>> --- a/drivers/dma-buf/dma-fence.c >>>> +++ b/drivers/dma-buf/dma-fence.c >>>> @@ -1078,8 +1078,7 @@ void >>>> dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, >>>> spinlock_t *lock, u64 context, u64 seqno) >>>> { >>>> - __dma_fence_init(fence, ops, lock, context, seqno, >>>> - BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); >>>> + __dma_fence_init(fence, ops, lock, context, seqno, 0); >>>> } >>>> EXPORT_SYMBOL(dma_fence_init64); >>>> >>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h >>>> index 64639e104110..4eca2db28625 100644 >>>> --- a/include/linux/dma-fence.h >>>> +++ b/include/linux/dma-fence.h >>>> @@ -98,7 +98,6 @@ struct dma_fence { >>>> }; >>>> >>>> enum dma_fence_flag_bits { >>>> - DMA_FENCE_FLAG_SEQNO64_BIT, >>>> DMA_FENCE_FLAG_SIGNALED_BIT, >>>> DMA_FENCE_FLAG_TIMESTAMP_BIT, >>>> DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >>>> @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence) >>>> */ >>>> static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) >>>> { >>>> - /* This is for backward compatibility with drivers which can only handle >>>> - * 32bit sequence numbers. Use a 64bit compare when the driver says to >>>> - * do so. >>>> - */ >>>> - if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) >>>> - return f1 > f2; >>>> - >>>> - return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; >>>> + return f1 > f2; >>>> } >>>> >>>> /** >>>> -- >>>> 2.49.0 >>>> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-27 7:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-17 9:31 [RFC PATCH] dma-fence: Remove 64-bit flag Philipp Stanner 2025-10-17 21:28 ` Matthew Brost 2025-10-20 8:16 ` Philipp Stanner 2025-10-20 11:18 ` Matthew Brost 2025-10-27 7:41 ` Christian König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox