On Thu, Jul 27, 2023 at 08:35:17AM +0300, Jouni Högander wrote:
> We want to wait dma fences in dirtyfb ioctl. As we don't want to make
> dirtyfb ioctl as blocking call we need to use
> dma_fence_add_callback. Callback used for dma_fence_add_callback is
> called from atomic context. Due to this we need to add a new
> frontbuffer tracking interface to queue flush.
>
> v2: Check if flush work is already pending
>
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> .../gpu/drm/i915/display/intel_frontbuffer.c | 33 +++++++++++++++++++
> .../gpu/drm/i915/display/intel_frontbuffer.h | 4 +++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 17a7aa8b28c2..d33b6021d9ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -202,6 +202,39 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
> frontbuffer_flush(i915, frontbuffer_bits, origin);
> }
>
> +static void intel_frontbuffer_flush_work(struct work_struct *work)
> +{
> + struct intel_frontbuffer *front =
> + container_of(work, struct intel_frontbuffer, flush_work);
> +
> + i915_gem_object_flush_if_display(front->obj);
> + intel_frontbuffer_flush(front, ORIGIN_DIRTYFB);
> + intel_frontbuffer_put(front);
> +}
> +
> +/**
> + * intel_frontbuffer_queue_flush - queue flushing frontbuffer object
> + * @front: GEM object to flush
> + *
> + * This function is targeted for our dirty callback for queueing flush when
> + * dma fence is signales
> + */
> +void intel_frontbuffer_queue_flush(struct intel_frontbuffer *front)
> +{
> + unsigned int frontbuffer_bits;
> +
> + if (!front)
> + return;
> +
> + frontbuffer_bits = atomic_read(&front->bits);
I'm thinking we could check that already before we add the fence cb.
> + if (!frontbuffer_bits || work_pending(&front->flush_work))
The work_pending() check is here to kep the kref in sync?
Maybe better to just use the return value of schedule_work() for
that?
> + return;
> +
> + kref_get(&front->ref);
> + INIT_WORK(&front->flush_work, intel_frontbuffer_flush_work);
Don't we have a better place for the INIT_WORK() (eg. when we initialize
the intel_frontbuffer)?
> + schedule_work(&front->flush_work);
> +}
> +
> static int frontbuffer_active(struct i915_active *ref)
> {
> struct intel_frontbuffer *front =
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> index 3c474ed937fb..11760b5ce9fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> @@ -47,6 +47,8 @@ struct intel_frontbuffer {
> struct i915_active write;
> struct drm_i915_gem_object *obj;
> struct rcu_head rcu;
> +
> + struct work_struct flush_work;
> };
>
> /*
> @@ -163,6 +165,8 @@ static inline void intel_frontbuffer_flush(struct intel_frontbuffer *front,
> __intel_fb_flush(front, origin, frontbuffer_bits);
> }
>
> +void intel_frontbuffer_queue_flush(struct intel_frontbuffer *front);
> +
> void intel_frontbuffer_track(struct intel_frontbuffer *old,
> struct intel_frontbuffer *new,
> unsigned int frontbuffer_bits);
> --
> 2.34.1
--
Ville Syrjälä
Intel
