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 <[email protected]> > > --- > > 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 > >
