On Tue, Apr 19, 2016 at 09:52:28AM +0200, Maarten Lankhorst wrote: > This will be required to allow more than 1 update in the future. > > Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 90 +++++++++++++++++------------- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 105 > ++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 4 files changed, 114 insertions(+), 87 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index aabd7a13cec7..513c7da24c3d 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -589,6 +589,53 @@ static int i915_gem_gtt_info(struct seq_file *m, void > *data) > return 0; > } > > +static void i915_dump_pageflip(struct seq_file *m, > + struct drm_i915_private *dev_priv, > + struct intel_crtc *crtc, > + struct intel_flip_work *work) > +{ > + const char pipe = pipe_name(crtc->pipe); > + const char plane = plane_name(crtc->plane); > + u32 pending; > + u32 addr; > + > + pending = atomic_read(&work->pending); > + if (pending == INTEL_FLIP_INACTIVE) { > + seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n", > + pipe, plane); > + } else { > + seq_printf(m, "Flip pending (waiting for vsync) on pipe %c > (plane %c)\n", > + pipe, plane); > + } > + if (work->flip_queued_req) { > + struct intel_engine_cs *engine = > i915_gem_request_get_engine(work->flip_queued_req); > + > + seq_printf(m, "Flip queued on %s at seqno %x, next seqno %x > [current breadcrumb %x], completed? %d\n", > + engine->name, > + i915_gem_request_get_seqno(work->flip_queued_req), > + dev_priv->next_seqno, > + engine->get_seqno(engine), > + i915_gem_request_completed(work->flip_queued_req, > true)); > + } 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", > + work->flip_queued_vblank, > + work->flip_ready_vblank, > + intel_crtc_get_vblank_counter(crtc)); > + seq_printf(m, "%d prepares\n", atomic_read(&work->pending)); > + > + if (INTEL_INFO(dev_priv)->gen >= 4) > + addr = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); > + else > + addr = I915_READ(DSPADDR(crtc->plane)); > + seq_printf(m, "Current scanout address 0x%08x\n", addr); > + > + if (work->pending_flip_obj) { > + seq_printf(m, "New framebuffer address 0x%08lx\n", > (long)work->gtt_offset); > + seq_printf(m, "MMIO update completed? %d\n", addr == > work->gtt_offset); > + } > +} > + > static int i915_gem_pageflip_info(struct seq_file *m, void *data) > { > struct drm_info_node *node = m->private; > @@ -607,48 +654,13 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > struct intel_flip_work *work; > > spin_lock_irq(&dev->event_lock); > - work = crtc->flip_work; > - if (work == NULL) { > + if (list_empty(&crtc->flip_work)) { > seq_printf(m, "No flip due on pipe %c (plane %c)\n", > pipe, plane); > } else { > - u32 pending; > - u32 addr; > - > - pending = atomic_read(&work->pending); > - if (pending == INTEL_FLIP_INACTIVE) { > - seq_printf(m, "Flip ioctl preparing on pipe %c > (plane %c)\n", > - pipe, plane); > - } else { > - seq_printf(m, "Flip pending (waiting for vsync) > on pipe %c (plane %c)\n", > - pipe, plane); > - } > - if (work->flip_queued_req) { > - struct intel_engine_cs *engine = > i915_gem_request_get_engine(work->flip_queued_req); > - > - seq_printf(m, "Flip queued on %s at seqno %x, > next seqno %x [current breadcrumb %x], completed? %d\n", > - engine->name, > - > i915_gem_request_get_seqno(work->flip_queued_req), > - dev_priv->next_seqno, > - engine->get_seqno(engine), > - > i915_gem_request_completed(work->flip_queued_req, true)); > - } 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", > - work->flip_queued_vblank, > - work->flip_ready_vblank, > - intel_crtc_get_vblank_counter(crtc)); > - seq_printf(m, "%d prepares\n", > atomic_read(&work->pending)); > - > - if (INTEL_INFO(dev)->gen >= 4) > - addr = > I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); > - else > - addr = I915_READ(DSPADDR(crtc->plane)); > - seq_printf(m, "Current scanout address 0x%08x\n", addr); > - > - if (work->pending_flip_obj) { > - seq_printf(m, "New framebuffer address > 0x%08lx\n", (long)work->gtt_offset); > - seq_printf(m, "MMIO update completed? %d\n", > addr == work->gtt_offset); > + list_for_each_entry(work, &crtc->flip_work, head); {
Accidental semi-colon after list_for_each_entry() > + i915_dump_pageflip(m, dev_priv, crtc, work); > + seq_puts(m, "\n"); > } > } > spin_unlock_irq(&dev->event_lock); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 85102ad75962..fb19fee24584 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -611,7 +611,7 @@ struct drm_i915_display_funcs { > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags); > + uint64_t gtt_offset); > void (*hpd_irq_setup)(struct drm_device *dev); > /* clock updates for mode set */ > /* cursor updates */ > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index d1181089512a..a2b4be06eb84 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3300,17 +3300,12 @@ static bool intel_crtc_has_pending_flip(struct > drm_crtc *crtc) > struct drm_device *dev = crtc->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > unsigned reset_counter; > - bool pending; > > reset_counter = i915_reset_counter(&to_i915(dev)->gpu_error); > if (intel_crtc->reset_counter != reset_counter) > return false; > > - spin_lock_irq(&dev->event_lock); > - pending = to_intel_crtc(crtc)->flip_work != NULL; > - spin_unlock_irq(&dev->event_lock); > - > - return pending; > + return !list_empty_careful(&to_intel_crtc(crtc)->flip_work); > } > > static void intel_update_pipe_config(struct intel_crtc *crtc, > @@ -3886,7 +3881,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev) > if (atomic_read(&crtc->unpin_work_count) == 0) > continue; > > - if (crtc->flip_work) > + if (!list_empty_careful(&crtc->flip_work)) > intel_wait_for_vblank(dev, crtc->pipe); > > return true; > @@ -3895,12 +3890,11 @@ bool intel_has_pending_fb_unpin(struct drm_device > *dev) > return false; > } > > -static void page_flip_completed(struct intel_crtc *intel_crtc) > +static void page_flip_completed(struct intel_crtc *intel_crtc, struct > intel_flip_work *work) > { > struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > - struct intel_flip_work *work = intel_crtc->flip_work; > > - intel_crtc->flip_work = NULL; > + list_del_init(&work->head); > > if (work->event) > drm_crtc_send_vblank_event(&intel_crtc->base, work->event); > @@ -3935,10 +3929,11 @@ static int intel_crtc_wait_for_pending_flips(struct > drm_crtc *crtc) > struct intel_flip_work *work; > > spin_lock_irq(&dev->event_lock); > - work = intel_crtc->flip_work; > + work = list_first_entry_or_null(&intel_crtc->flip_work, > + struct intel_flip_work, head); I'm not quite following this. Why is it enough for us to only look at the first entry. Is it hardcoded atm because we only have a list with a single entry. If so we should write a comment about this. > if (work && !is_mmio_work(work)) { > WARN_ONCE(1, "Removing stuck page flip\n"); > - page_flip_completed(intel_crtc); > + page_flip_completed(intel_crtc, work); > } > spin_unlock_irq(&dev->event_lock); > } > @@ -6335,7 +6330,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc > *crtc) > return; > > if (to_intel_plane_state(crtc->primary->state)->visible) { > - WARN_ON(intel_crtc->flip_work); > + WARN_ON(list_empty(&intel_crtc->flip_work)); > > intel_pre_disable_primary_noatomic(crtc); > > @@ -10924,17 +10919,24 @@ static void intel_crtc_destroy(struct drm_crtc > *crtc) > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct drm_device *dev = crtc->dev; > struct intel_flip_work *work; > + struct list_head head; > + > + INIT_LIST_HEAD(&head); What's this? > > spin_lock_irq(&dev->event_lock); > - work = intel_crtc->flip_work; > - intel_crtc->flip_work = NULL; > - spin_unlock_irq(&dev->event_lock); > + while (!list_empty(&intel_crtc->flip_work)) { > + work = list_first_entry(&intel_crtc->flip_work, > + struct intel_flip_work, head); Hardcoded assumption here again? > + list_del_init(&work->head); > + spin_unlock_irq(&dev->event_lock); > > - if (work) { > cancel_work_sync(&work->mmio_work); > cancel_work_sync(&work->unpin_work); > kfree(work); > + > + spin_lock_irq(&dev->event_lock); > } > + spin_unlock_irq(&dev->event_lock); > > drm_crtc_cleanup(crtc); > > @@ -10976,7 +10978,8 @@ static bool g4x_flip_count_after_eq(u32 a, u32 b) > return !((a - b) & 0x80000000); > } > > -static bool page_flip_finished(struct intel_crtc *crtc) > +static bool page_flip_finished(struct intel_crtc *crtc, > + struct intel_flip_work *work) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -11021,9 +11024,9 @@ static bool page_flip_finished(struct intel_crtc > *crtc) > * anyway, we don't really care. > */ > return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) == > - crtc->flip_work->gtt_offset && > + work->gtt_offset && > > g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)), > - crtc->flip_work->flip_count); > + work->flip_count); > } > > static void do_intel_finish_page_flip(struct drm_device *dev, > @@ -11042,12 +11045,14 @@ static void do_intel_finish_page_flip(struct > drm_device *dev, > * lost pageflips) so needs the full irqsave spinlocks. > */ > spin_lock_irqsave(&dev->event_lock, flags); > - work = intel_crtc->flip_work; > + work = list_first_entry_or_null(&intel_crtc->flip_work, > + struct intel_flip_work, > + head); Hardcoded...? > > if (work != NULL && > atomic_read(&work->pending) == INTEL_FLIP_PENDING && > - page_flip_finished(intel_crtc)) > - page_flip_completed(intel_crtc); > + page_flip_finished(intel_crtc, work)) > + page_flip_completed(intel_crtc, work); > > spin_unlock_irqrestore(&dev->event_lock, flags); > } > @@ -11083,7 +11088,7 @@ static int intel_gen2_queue_flip(struct drm_device > *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > struct intel_engine_cs *engine = req->engine; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -11106,7 +11111,7 @@ static int intel_gen2_queue_flip(struct drm_device > *dev, > intel_ring_emit(engine, MI_DISPLAY_FLIP | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > intel_ring_emit(engine, fb->pitches[0]); > - intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset); > + intel_ring_emit(engine, gtt_offset); > intel_ring_emit(engine, 0); /* aux display base address, unused */ > > return 0; > @@ -11117,7 +11122,7 @@ static int intel_gen3_queue_flip(struct drm_device > *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > struct intel_engine_cs *engine = req->engine; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -11137,7 +11142,7 @@ static int intel_gen3_queue_flip(struct drm_device > *dev, > intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > intel_ring_emit(engine, fb->pitches[0]); > - intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset); > + intel_ring_emit(engine, gtt_offset); > intel_ring_emit(engine, MI_NOOP); > > return 0; > @@ -11148,7 +11153,7 @@ static int intel_gen4_queue_flip(struct drm_device > *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > struct intel_engine_cs *engine = req->engine; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -11167,8 +11172,7 @@ static int intel_gen4_queue_flip(struct drm_device > *dev, > intel_ring_emit(engine, MI_DISPLAY_FLIP | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > intel_ring_emit(engine, fb->pitches[0]); > - intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset | > - obj->tiling_mode); > + intel_ring_emit(engine, gtt_offset | obj->tiling_mode); > > /* XXX Enabling the panel-fitter across page-flip is so far > * untested on non-native modes, so ignore it for now. > @@ -11186,7 +11190,7 @@ static int intel_gen6_queue_flip(struct drm_device > *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > struct intel_engine_cs *engine = req->engine; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -11201,7 +11205,7 @@ static int intel_gen6_queue_flip(struct drm_device > *dev, > intel_ring_emit(engine, MI_DISPLAY_FLIP | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode); > - intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset); > + intel_ring_emit(engine, gtt_offset); > > /* Contrary to the suggestions in the documentation, > * "Enable Panel Fitter" does not seem to be required when page > @@ -11221,7 +11225,7 @@ static int intel_gen7_queue_flip(struct drm_device > *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > struct intel_engine_cs *engine = req->engine; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -11304,7 +11308,7 @@ static int intel_gen7_queue_flip(struct drm_device > *dev, > > intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit); > intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode)); > - intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset); > + intel_ring_emit(engine, gtt_offset); > intel_ring_emit(engine, (MI_NOOP)); > > return 0; > @@ -11371,17 +11375,17 @@ static int intel_default_queue_flip(struct > drm_device *dev, > struct drm_framebuffer *fb, > struct drm_i915_gem_object *obj, > struct drm_i915_gem_request *req, > - uint32_t flags) > + uint64_t gtt_offset) > { > return -ENODEV; > } > > static bool __intel_pageflip_stall_check(struct drm_device *dev, > - struct drm_crtc *crtc) > + struct drm_crtc *crtc, > + struct intel_flip_work *work) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_flip_work *work = intel_crtc->flip_work; > u32 addr, vblank; > u32 pending; > > @@ -11437,12 +11441,14 @@ void intel_check_page_flip(struct drm_device *dev, > int pipe) > return; > > spin_lock(&dev->event_lock); > - work = intel_crtc->flip_work; > - if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) { > + work = list_first_entry_or_null(&intel_crtc->flip_work, > + struct intel_flip_work, head); ... > + > + if (work != NULL && __intel_pageflip_stall_check(dev, crtc, work)) { > WARN_ONCE(!is_mmio_work(work), > "Kicking stuck page flip: queued at %d, now %d\n", > work->flip_queued_vblank, > intel_crtc_get_vblank_counter(intel_crtc)); > - page_flip_completed(intel_crtc); > + page_flip_completed(intel_crtc, work); > work = NULL; > } > if (work != NULL && !is_mmio_work(work) && > @@ -11508,13 +11514,18 @@ static int intel_crtc_page_flip(struct drm_crtc > *crtc, > > /* We borrow the event spin lock for protecting flip_work */ > spin_lock_irq(&dev->event_lock); > - if (intel_crtc->flip_work) { > + if (!list_empty(&intel_crtc->flip_work)) { > + struct intel_flip_work *old_work; > + > + old_work = list_first_entry(&intel_crtc->flip_work, > + struct intel_flip_work, head); > + ... > /* Before declaring the flip queue wedged, check if > * the hardware completed the operation behind our backs. > */ > - if (__intel_pageflip_stall_check(dev, crtc)) { > + if (__intel_pageflip_stall_check(dev, crtc, old_work)) { > DRM_DEBUG_DRIVER("flip queue: previous flip completed, > continuing\n"); > - page_flip_completed(intel_crtc); > + page_flip_completed(intel_crtc, old_work); > } else { > DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); > spin_unlock_irq(&dev->event_lock); > @@ -11524,7 +11535,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > return -EBUSY; > } > } > - intel_crtc->flip_work = work; > + list_add_tail(&work->head, &intel_crtc->flip_work); > spin_unlock_irq(&dev->event_lock); > > if (atomic_read(&intel_crtc->unpin_work_count) >= 2) > @@ -11606,7 +11617,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > } else { > i915_gem_request_assign(&work->flip_queued_req, request); > ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, > - page_flip_flags); > + work->gtt_offset); > if (ret) > goto cleanup_unpin; > > @@ -11641,7 +11652,7 @@ cleanup: > drm_framebuffer_unreference(work->old_fb); > > spin_lock_irq(&dev->event_lock); > - intel_crtc->flip_work = NULL; > + list_del(&work->head); > spin_unlock_irq(&dev->event_lock); > > drm_crtc_vblank_put(crtc); > @@ -14253,6 +14264,8 @@ static void intel_crtc_init(struct drm_device *dev, > int pipe) > intel_crtc->base.state = &crtc_state->base; > crtc_state->base.crtc = &intel_crtc->base; > > + INIT_LIST_HEAD(&intel_crtc->flip_work); > + > /* initialize shared scalers */ > if (INTEL_INFO(dev)->gen >= 9) { > if (pipe == PIPE_C) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index bc310513adb8..0a2472d69af8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -611,7 +611,7 @@ struct intel_crtc { > unsigned long enabled_power_domains; > bool lowfreq_avail; > struct intel_overlay *overlay; > - struct intel_flip_work *flip_work; > + struct list_head flip_work; > > atomic_t unpin_work_count; > > @@ -935,6 +935,8 @@ intel_get_crtc_for_plane(struct drm_device *dev, int > plane) > } > > struct intel_flip_work { > + struct list_head head; > + > struct work_struct unpin_work; > struct work_struct mmio_work; > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx