Re: [PATCH v3 15/28] drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'

Daniel Vetter
2014-11-26T13:06:53Z

On Mon, Nov 24, 2014 at 06:49:37PM +0000, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Converted the flip_queued_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.
>
> 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/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++--------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 3 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2720eb5..bc9b1c8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -546,11 +546,11 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
> if (work->flip_queued_ring) {
> seq_printf(m, "Flip queued on %s at seqno %u, next seqno %u [current breadcrumb %u], completed? %d\n",
> work->flip_queued_ring->name,
> - work->flip_queued_seqno,
> + i915_gem_request_get_seqno(work->flip_queued_req),
> dev_priv->next_seqno,
> work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
> i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
> - work->flip_queued_seqno));
> + i915_gem_request_get_seqno(work->flip_queued_req)));
> } else
> seq_printf(m, "Flip not associated with any ring\n");
> seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 98a5ed0..4a801e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9669,10 +9669,18 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
> return false;
>
> if (work->flip_ready_vblank == 0) {
> - if (work->flip_queued_ring &&
> - !i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
> - work->flip_queued_seqno))
> - return false;
> + if (work->flip_queued_ring) {
> + uint32_t s1 = work->flip_queued_ring->get_seqno(
> + work->flip_queued_ring, true);
> + uint32_t s2 = i915_gem_request_get_seqno(
> + work->flip_queued_req);
> + if (!i915_seqno_passed(s1, s2))
> + return false;
> +
> + i915_gem_request_unreference_irq(work->flip_queued_req);
> + work->flip_queued_req = NULL;
> + work->flip_queued_ring = NULL;

Ok, here's the next one. I've fixed this up with

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1df3387af59b..c8eb109acc3b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9122,6 +9122,11 @@ static void intel_unpin_work_fn(struct work_struct *__work)
drm_gem_object_unreference(&work->old_fb_obj->base);

intel_update_fbc(dev);
+
+ if (work->flip_queued_req)
+ i915_gem_request_unreference_irq(work->flip_queued_req);
+ work->flip_queued_req = NULL;
+ work->flip_queued_ring = NULL;
mutex_unlock(&dev->struct_mutex);

intel_frontbuffer_flip_complete(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
@@ -9733,10 +9738,6 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
work->flip_queued_req);
if (!i915_seqno_passed(s1, s2))
return false;
-
- i915_gem_request_unreference_irq(work->flip_queued_req);
- work->flip_queued_req = NULL;
- work->flip_queued_ring = NULL;
}

work->flip_ready_vblank = drm_vblank_count(dev, intel_crtc->pipe);
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch