On Mon, Oct 06, 2014 at 03:15:18PM +0100, john.c.harri...@intel.com wrote:
> From: John Harrison <john.c.harri...@intel.com>

Again on the topic of thin commit message: Some additional words about why
we can noodle around in request structures from hardirq context (those
fields are invariant after add_request) would be good. Since with the
scheduler a lot of that stuff suddenly is a lot less invariant (if we
delay the seqno allocation until we emit requests, which I'm pretty sure
we have to for a bunch of reasons).

> 
> For: VIZ-4377
> Signed-off-by: john.c.harri...@intel.com
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |    4 ++--
>  drivers/gpu/drm/i915/intel_display.c |    9 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |    2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 726a8f0..52ddd19 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -547,11 +547,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)));

Yeah it's no your patch, but this stuff has defintely fallen over the
edge. A prep patch to extract some helper functions to bring sanity to
i915_gem_pageflip_info's indentation levels is needed here imo.
-Daniel

>                       } 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 f13bc30..26fdd96 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9879,7 +9879,7 @@ static bool __intel_pageflip_stall_check(struct 
> drm_device *dev,
>       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))
> +                                    
> i915_gem_request_get_seqno(work->flip_queued_req)))
>                       return false;
>  
>               work->flip_ready_vblank = drm_vblank_count(dev, 
> intel_crtc->pipe);
> @@ -10047,7 +10047,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>               if (ret)
>                       goto cleanup_unpin;
>  
> -             work->flip_queued_seqno = 
> i915_gem_request_get_seqno(obj->last_write_req);
> +             work->flip_queued_req = obj->last_write_req;
>               work->flip_queued_ring = obj->ring;
>       } else {
>               ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring,
> @@ -10055,10 +10055,13 @@ static int intel_crtc_page_flip(struct drm_crtc 
> *crtc,
>               if (ret)
>                       goto cleanup_unpin;
>  
> -             work->flip_queued_seqno = 
> i915_gem_request_get_seqno(intel_ring_get_request(ring));
> +             work->flip_queued_req = intel_ring_get_request(ring);
>               work->flip_queued_ring = ring;
>       }
>  
> +     if (work->flip_queued_req)
> +             i915_gem_request_reference(work->flip_queued_req);
> +
>       work->flip_queued_vblank = drm_vblank_count(dev, intel_crtc->pipe);
>       work->enable_stall_check = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 418ac13..cb5af63 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -687,7 +687,7 @@ struct intel_unpin_work {
>       u32 flip_count;
>       u32 gtt_offset;
>       struct intel_engine_cs *flip_queued_ring;
> -     u32 flip_queued_seqno;
> +     struct drm_i915_gem_request *flip_queued_req;
>       int flip_queued_vblank;
>       int flip_ready_vblank;
>       bool enable_stall_check;
> -- 
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to