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 at linux.intel.com>
> ---
> 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?
> }
>
> 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?
Ander
> - work->enable_stall_check = true;
>
> i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
> to_intel_plane(primary)->frontbuffer_bit);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e0fcfa1683cc..ce2a6d985bbe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -952,7 +952,6 @@ struct intel_unpin_work {
> struct drm_i915_gem_request *flip_queued_req;
> u32 flip_queued_vblank;
> u32 flip_ready_vblank;
> - bool enable_stall_check;
> };
>
> struct intel_load_detect_pipe {
