Re: [PATCH v3 12/28] drm/i915: Convert mmio_flip::seqno to struct request

Daniel Vetter
2014-11-26T09:23:33Z

On Mon, Nov 24, 2014 at 06:49:34PM +0000, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Converted the mmio_flip 'seqno' value to be a request structure as part of the
> on going seqno to request changes. This includes reference counting the request
> being saved away to ensure it can not be retired and freed while the flip code
> is still waiting on it.
>
> v2: Used the IRQ friendly request dereference call in the notify handler as that
> code is called asynchronously without holding any useful mutex locks.
>
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
> drivers/gpu/drm/i915/intel_drv.h | 3 +--
> 2 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 097e8a1..cbf3cb7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9550,18 +9550,19 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
> {
> struct intel_crtc *intel_crtc =
> container_of(work, struct intel_crtc, mmio_flip.work);
> - struct intel_engine_cs *ring;
> - uint32_t seqno;
> -
> - seqno = intel_crtc->mmio_flip.seqno;
> - ring = intel_crtc->mmio_flip.ring;
> + struct intel_mmio_flip *mmio_flip;
>
> - if (seqno)
> - WARN_ON(__i915_wait_seqno(ring, seqno,
> + mmio_flip = &intel_crtc->mmio_flip;
> + if (mmio_flip->req)
> + WARN_ON(__i915_wait_seqno(i915_gem_request_get_ring(mmio_flip->req),
> + i915_gem_request_get_seqno(mmio_flip->req),
> intel_crtc->reset_counter,
> false, NULL, NULL) != 0);
>
> intel_do_mmio_flip(intel_crtc);
> + if (mmio_flip->req)
> + i915_gem_request_unreference_irq(mmio_flip->req);

Can't we just grab dev->struct_mutex around here and reuse the normal
request_unref? That would allow us to ditch all the unref_irq complexity.
-Daniel

> + mmio_flip->req = NULL;
> }
>
> static int intel_queue_mmio_flip(struct drm_device *dev,
> @@ -9573,9 +9574,8 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> - intel_crtc->mmio_flip.seqno =
> - i915_gem_request_get_seqno(obj->last_write_req);
> - intel_crtc->mmio_flip.ring = obj->ring;
> + i915_gem_request_assign(&intel_crtc->mmio_flip.req,
> + obj->last_write_req);
>
> schedule_work(&intel_crtc->mmio_flip.work);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 44b153c5..2755532 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -406,8 +406,7 @@ struct intel_pipe_wm {
> };
>
> struct intel_mmio_flip {
> - u32 seqno;
> - struct intel_engine_cs *ring;
> + struct drm_i915_gem_request *req;
> struct work_struct work;
> };
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch