[Intel-gfx] [PATCH 08/19] drm/i915: Convert flip_work to a list.
Patrik Jakobsson
patrik.jakobsson at linux.intel.com
Mon May 2 09:22:01 UTC 2016
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.lankhorst at 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 at 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
More information about the Intel-gfx
mailing list