From: Rob Clark <[email protected]>
Add a way to initialize a fence without touching the refcount. This is
useful, for example, if the fence is embedded in a drm_sched_job. In
this case the refcount will be initialized before the job is queued.
But the seqno of the hw_fence is not known until job_run().
Signed-off-by: Rob Clark <[email protected]>
---
drivers/dma-buf/dma-fence.c | 43 ++++++++++++++++++++++++++++---------
include/linux/dma-fence.h | 2 ++
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 74e36f6d05b0..97c05a465cb4 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -989,28 +989,27 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
EXPORT_SYMBOL(dma_fence_describe);
/**
- * dma_fence_init - Initialize a custom fence.
+ * dma_fence_init_noref - Initialize a custom fence without initializing refcount.
* @fence: the fence to initialize
* @ops: the dma_fence_ops for operations on this fence
* @lock: the irqsafe spinlock to use for locking this fence
* @context: the execution context this fence is run on
* @seqno: a linear increasing sequence number for this context
*
- * Initializes an allocated fence, the caller doesn't have to keep its
- * refcount after committing with this fence, but it will need to hold a
- * refcount again if &dma_fence_ops.enable_signaling gets called.
- *
- * context and seqno are used for easy comparison between fences, allowing
- * to check which fence is later by simply using dma_fence_later().
+ * Like &dma_fence_init but does not initialize the refcount. Suitable
+ * for cases where the fence is embedded in another struct which has it's
+ * refcount initialized before the fence is initialized. Such as embedding
+ * in a &drm_sched_job, where the job is created before knowing the seqno
+ * of the hw_fence.
*/
void
-dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
- spinlock_t *lock, u64 context, u64 seqno)
+dma_fence_init_noref(struct dma_fence *fence, const struct dma_fence_ops *ops,
+ spinlock_t *lock, u64 context, u64 seqno)
{
BUG_ON(!lock);
BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
+ BUG_ON(!kref_read(&fence->refcount));
- kref_init(&fence->refcount);
fence->ops = ops;
INIT_LIST_HEAD(&fence->cb_list);
fence->lock = lock;
@@ -1021,4 +1020,28 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
trace_dma_fence_init(fence);
}
+EXPORT_SYMBOL(dma_fence_init_noref);
+
+/**
+ * dma_fence_init - Initialize a custom fence.
+ * @fence: the fence to initialize
+ * @ops: the dma_fence_ops for operations on this fence
+ * @lock: the irqsafe spinlock to use for locking this fence
+ * @context: the execution context this fence is run on
+ * @seqno: a linear increasing sequence number for this context
+ *
+ * Initializes an allocated fence, the caller doesn't have to keep its
+ * refcount after committing with this fence, but it will need to hold a
+ * refcount again if &dma_fence_ops.enable_signaling gets called.
+ *
+ * context and seqno are used for easy comparison between fences, allowing
+ * to check which fence is later by simply using dma_fence_later().
+ */
+void
+dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
+ spinlock_t *lock, u64 context, u64 seqno)
+{
+ kref_init(&fence->refcount);
+ dma_fence_init_noref(fence, ops, lock, context, seqno);
+}
EXPORT_SYMBOL(dma_fence_init);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index d54b595a0fe0..f617c78a2e0a 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -279,6 +279,8 @@ struct dma_fence_ops {
void (*set_deadline)(struct dma_fence *fence, ktime_t deadline);
};
+void dma_fence_init_noref(struct dma_fence *fence, const struct dma_fence_ops *ops,
+ spinlock_t *lock, u64 context, u64 seqno);
void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
spinlock_t *lock, u64 context, u64 seqno);
--
2.39.2
Am 11.03.23 um 18:35 schrieb Rob Clark:
> From: Rob Clark <[email protected]>
>
> Add a way to initialize a fence without touching the refcount. This is
> useful, for example, if the fence is embedded in a drm_sched_job. In
> this case the refcount will be initialized before the job is queued.
> But the seqno of the hw_fence is not known until job_run().
>
> Signed-off-by: Rob Clark <[email protected]>
Well that approach won't work. The fence can only be initialized in the
job_run() callback because only then the sequence number can be determined.
Regards,
Christian.
> ---
> drivers/dma-buf/dma-fence.c | 43 ++++++++++++++++++++++++++++---------
> include/linux/dma-fence.h | 2 ++
> 2 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 74e36f6d05b0..97c05a465cb4 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -989,28 +989,27 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
> EXPORT_SYMBOL(dma_fence_describe);
>
> /**
> - * dma_fence_init - Initialize a custom fence.
> + * dma_fence_init_noref - Initialize a custom fence without initializing refcount.
> * @fence: the fence to initialize
> * @ops: the dma_fence_ops for operations on this fence
> * @lock: the irqsafe spinlock to use for locking this fence
> * @context: the execution context this fence is run on
> * @seqno: a linear increasing sequence number for this context
> *
> - * Initializes an allocated fence, the caller doesn't have to keep its
> - * refcount after committing with this fence, but it will need to hold a
> - * refcount again if &dma_fence_ops.enable_signaling gets called.
> - *
> - * context and seqno are used for easy comparison between fences, allowing
> - * to check which fence is later by simply using dma_fence_later().
> + * Like &dma_fence_init but does not initialize the refcount. Suitable
> + * for cases where the fence is embedded in another struct which has it's
> + * refcount initialized before the fence is initialized. Such as embedding
> + * in a &drm_sched_job, where the job is created before knowing the seqno
> + * of the hw_fence.
> */
> void
> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> - spinlock_t *lock, u64 context, u64 seqno)
> +dma_fence_init_noref(struct dma_fence *fence, const struct dma_fence_ops *ops,
> + spinlock_t *lock, u64 context, u64 seqno)
> {
> BUG_ON(!lock);
> BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
> + BUG_ON(!kref_read(&fence->refcount));
>
> - kref_init(&fence->refcount);
> fence->ops = ops;
> INIT_LIST_HEAD(&fence->cb_list);
> fence->lock = lock;
> @@ -1021,4 +1020,28 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>
> trace_dma_fence_init(fence);
> }
> +EXPORT_SYMBOL(dma_fence_init_noref);
> +
> +/**
> + * dma_fence_init - Initialize a custom fence.
> + * @fence: the fence to initialize
> + * @ops: the dma_fence_ops for operations on this fence
> + * @lock: the irqsafe spinlock to use for locking this fence
> + * @context: the execution context this fence is run on
> + * @seqno: a linear increasing sequence number for this context
> + *
> + * Initializes an allocated fence, the caller doesn't have to keep its
> + * refcount after committing with this fence, but it will need to hold a
> + * refcount again if &dma_fence_ops.enable_signaling gets called.
> + *
> + * context and seqno are used for easy comparison between fences, allowing
> + * to check which fence is later by simply using dma_fence_later().
> + */
> +void
> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> + spinlock_t *lock, u64 context, u64 seqno)
> +{
> + kref_init(&fence->refcount);
> + dma_fence_init_noref(fence, ops, lock, context, seqno);
> +}
> EXPORT_SYMBOL(dma_fence_init);
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index d54b595a0fe0..f617c78a2e0a 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -279,6 +279,8 @@ struct dma_fence_ops {
> void (*set_deadline)(struct dma_fence *fence, ktime_t deadline);
> };
>
> +void dma_fence_init_noref(struct dma_fence *fence, const struct dma_fence_ops *ops,
> + spinlock_t *lock, u64 context, u64 seqno);
> void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> spinlock_t *lock, u64 context, u64 seqno);
>
Am 13.03.23 um 08:13 schrieb Christian König:
> Am 11.03.23 um 18:35 schrieb Rob Clark:
>> From: Rob Clark <[email protected]>
>>
>> Add a way to initialize a fence without touching the refcount. This is
>> useful, for example, if the fence is embedded in a drm_sched_job. In
>> this case the refcount will be initialized before the job is queued.
>> But the seqno of the hw_fence is not known until job_run().
>>
>> Signed-off-by: Rob Clark <[email protected]>
>
> Well that approach won't work. The fence can only be initialized in
> the job_run() callback because only then the sequence number can be
> determined.
Ah, wait a second! After reading the msm code I realized you are going
to use this exactly the other way around that I think you use it.
In this case it would work, but that really needs better documentation.
And I'm pretty sure it's not a good idea for msm, but let's discuss that
on the other patch.
Regards,
Christian.
>
> Regards,
> Christian.
>
>> ---
>> drivers/dma-buf/dma-fence.c | 43 ++++++++++++++++++++++++++++---------
>> include/linux/dma-fence.h | 2 ++
>> 2 files changed, 35 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 74e36f6d05b0..97c05a465cb4 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -989,28 +989,27 @@ void dma_fence_describe(struct dma_fence
>> *fence, struct seq_file *seq)
>> EXPORT_SYMBOL(dma_fence_describe);
>> /**
>> - * dma_fence_init - Initialize a custom fence.
>> + * dma_fence_init_noref - Initialize a custom fence without
>> initializing refcount.
>> * @fence: the fence to initialize
>> * @ops: the dma_fence_ops for operations on this fence
>> * @lock: the irqsafe spinlock to use for locking this fence
>> * @context: the execution context this fence is run on
>> * @seqno: a linear increasing sequence number for this context
>> *
>> - * Initializes an allocated fence, the caller doesn't have to keep its
>> - * refcount after committing with this fence, but it will need to
>> hold a
>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>> - *
>> - * context and seqno are used for easy comparison between fences,
>> allowing
>> - * to check which fence is later by simply using dma_fence_later().
>> + * Like &dma_fence_init but does not initialize the refcount. Suitable
>> + * for cases where the fence is embedded in another struct which has
>> it's
>> + * refcount initialized before the fence is initialized. Such as
>> embedding
>> + * in a &drm_sched_job, where the job is created before knowing the
>> seqno
>> + * of the hw_fence.
>> */
>> void
>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops
>> *ops,
>> - spinlock_t *lock, u64 context, u64 seqno)
>> +dma_fence_init_noref(struct dma_fence *fence, const struct
>> dma_fence_ops *ops,
>> + spinlock_t *lock, u64 context, u64 seqno)
>> {
>> BUG_ON(!lock);
>> BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
>> + BUG_ON(!kref_read(&fence->refcount));
>> - kref_init(&fence->refcount);
>> fence->ops = ops;
>> INIT_LIST_HEAD(&fence->cb_list);
>> fence->lock = lock;
>> @@ -1021,4 +1020,28 @@ dma_fence_init(struct dma_fence *fence, const
>> struct dma_fence_ops *ops,
>> trace_dma_fence_init(fence);
>> }
>> +EXPORT_SYMBOL(dma_fence_init_noref);
>> +
>> +/**
>> + * dma_fence_init - Initialize a custom fence.
>> + * @fence: the fence to initialize
>> + * @ops: the dma_fence_ops for operations on this fence
>> + * @lock: the irqsafe spinlock to use for locking this fence
>> + * @context: the execution context this fence is run on
>> + * @seqno: a linear increasing sequence number for this context
>> + *
>> + * Initializes an allocated fence, the caller doesn't have to keep its
>> + * refcount after committing with this fence, but it will need to
>> hold a
>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>> + *
>> + * context and seqno are used for easy comparison between fences,
>> allowing
>> + * to check which fence is later by simply using dma_fence_later().
>> + */
>> +void
>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops
>> *ops,
>> + spinlock_t *lock, u64 context, u64 seqno)
>> +{
>> + kref_init(&fence->refcount);
>> + dma_fence_init_noref(fence, ops, lock, context, seqno);
>> +}
>> EXPORT_SYMBOL(dma_fence_init);
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index d54b595a0fe0..f617c78a2e0a 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -279,6 +279,8 @@ struct dma_fence_ops {
>> void (*set_deadline)(struct dma_fence *fence, ktime_t deadline);
>> };
>> +void dma_fence_init_noref(struct dma_fence *fence, const struct
>> dma_fence_ops *ops,
>> + spinlock_t *lock, u64 context, u64 seqno);
>> void dma_fence_init(struct dma_fence *fence, const struct
>> dma_fence_ops *ops,
>> spinlock_t *lock, u64 context, u64 seqno);
>