Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira: > On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote: >> Re-use unpin_work->pending, but also set vblank count before >> intel_mark_page_flip_active to be sure. > Be sure of what? > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++----- >> drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++------------------- >> drivers/gpu/drm/i915/intel_drv.h | 1 - >> 3 files changed, 18 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c >> b/drivers/gpu/drm/i915/i915_debugfs.c >> index 9640738aabf2..df8073a2ffbe 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m, >> void *data) >> seq_printf(m, "No flip due on pipe %c (plane %c)\n", >> pipe, plane); >> } else { >> + u32 pending; >> u32 addr; >> >> - if (atomic_read(&work->pending) < >> INTEL_FLIP_COMPLETE) { >> + 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 if (pending >= INTEL_FLIP_COMPLETE) { >> seq_printf(m, "Flip queued on pipe %c (plane >> %c)\n", >> pipe, plane); >> } else { >> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, >> void *data) >> work->flip_queued_vblank, >> work->flip_ready_vblank, >> drm_crtc_vblank_count(&crtc->base)); >> - if (work->enable_stall_check) >> - seq_puts(m, "Stall check enabled, "); >> - else >> - seq_puts(m, "Stall check waiting for page >> flip ioctl, "); >> seq_printf(m, "%d prepares\n", atomic_read(&work >> ->pending)); >> >> if (INTEL_INFO(dev)->gen >= 4) >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index f2be54a48727..618e034a7a5e 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip >> *mmio_flip) >> if (work == NULL) >> return; >> >> - intel_mark_page_flip_active(work); >> - >> intel_pipe_update_start(crtc); >> >> if (INTEL_INFO(mmio_flip->i915)->gen >= 9) >> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip >> *mmio_flip) >> ilk_do_mmio_flip(crtc, work); >> >> intel_pipe_update_end(crtc); >> + >> + intel_mark_page_flip_active(work); > Is this to avoid triggering the stall check during the wait from a vblank > evasion? It's to ensure that if a vblank happens before pipe_update_end, we don't mark the flip as completed until we actually updated the mmio registers. > >> } >> >> static void intel_mmio_flip_work_func(struct work_struct *work) >> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct >> drm_device *dev, >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> struct intel_unpin_work *work = intel_crtc->unpin_work; >> u32 addr; >> + u32 pending; >> >> - if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) >> - return true; >> - >> - if (atomic_read(&work->pending) < INTEL_FLIP_PENDING) >> - return false; >> - >> - if (!work->enable_stall_check) >> - return false; >> + pending = atomic_read(&work->pending); >> + if (pending != INTEL_FLIP_PENDING) >> + return pending == INTEL_FLIP_COMPLETE; >> >> if (work->flip_ready_vblank == 0) { >> if (work->flip_queued_req && >> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc >> *crtc, >> */ >> if (!mmio_flip) { >> ret = i915_gem_object_sync(obj, engine, &request); >> + if (!ret && !request) { >> + request = i915_gem_request_alloc(engine, NULL); >> + ret = PTR_ERR_OR_ZERO(request); >> + } >> + >> if (ret) >> goto cleanup_pending; >> } >> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, >> work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary), >> obj, 0); >> work->gtt_offset += intel_crtc->dspaddr_offset; >> + work->flip_queued_vblank = drm_crtc_vblank_count(crtc); >> >> if (mmio_flip) { >> ret = intel_queue_mmio_flip(dev, crtc, obj); >> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc >> *crtc, >> i915_gem_request_assign(&work->flip_queued_req, >> obj->last_write_req); >> } else { >> - if (!request) { >> - request = i915_gem_request_alloc(engine, NULL); >> - if (IS_ERR(request)) { >> - ret = PTR_ERR(request); >> - goto cleanup_unpin; >> - } >> - } >> - >> ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, >> request, >> page_flip_flags); >> if (ret) >> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, >> i915_add_request_no_flush(request); >> >> work->flip_queued_vblank = drm_crtc_vblank_count(crtc); > Do we still need the assigment above? > It's used for rps boosting, so likely... _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel