* [PATCH 0/6] drm/i915: Rework page flip to be more atomic like.
@ 2016-03-23 13:24 Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 1/6] drm/i915: Remove stallcheck special handling Maarten Lankhorst
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
This patch series renames intel_unpin_work to intel_flip_work to better
describe what it does. It will also contain pointers to the old and
new crtc/plane states, which means that intel_flip_work becomes
self-contained and usable for atomic updates.
This patch also enables mmio updates for all platforms, which is needed
for implementing async atomic commit properly.
Maarten Lankhorst (6):
drm/i915: Remove stallcheck special handling.
drm/i915: Remove intel_prepare_page_flip.
drm/i915: Add the exclusive fence to plane_state.
drm/i915: Allow mmio updates on all platforms.
drm/i915: Convert flip_work to a list.
drm/i915: Rework intel_crtc_page_flip to be almost atomic.
drivers/gpu/drm/i915/i915_debugfs.c | 108 ++--
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 18 +-
drivers/gpu/drm/i915/intel_atomic_plane.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 940 +++++++++++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 34 +-
6 files changed, 566 insertions(+), 537 deletions(-)
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] drm/i915: Remove stallcheck special handling.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip Maarten Lankhorst
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
Re-use unpin_work->pending, but also set vblank count before
intel_mark_page_flip_active to be sure.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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 e0ba3e38000f..310043d73bf7 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -581,9 +581,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 {
@@ -605,10 +610,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 51f913fb199d..6303438df9f2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11315,8 +11315,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)
@@ -11326,6 +11324,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);
}
static void intel_mmio_flip_work_func(struct work_struct *work)
@@ -11392,15 +11392,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 &&
@@ -11576,6 +11572,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;
}
@@ -11587,6 +11588,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);
@@ -11596,14 +11598,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)
@@ -11616,7 +11610,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);
- 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 c87b4503435d..7f0779783851 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -950,7 +950,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 {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 1/6] drm/i915: Remove stallcheck special handling Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
2016-03-23 15:06 ` Ville Syrjälä
2016-03-23 13:24 ` [PATCH 3/6] drm/i915: Add the exclusive fence to plane_state Maarten Lankhorst
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
is no longer useful.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 --
drivers/gpu/drm/i915/i915_irq.c | 18 ++-----
drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 2 -
4 files changed, 40 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 310043d73bf7..4a0078c17cd1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -588,9 +588,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
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 {
seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
pipe, plane);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a55a7cc317f8..9d4089f6c91b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
intel_pipe_handle_vblank(dev, pipe))
intel_check_page_flip(dev, pipe);
- if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
- intel_prepare_page_flip(dev, pipe);
+ if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
intel_finish_page_flip(dev, pipe);
- }
if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
i9xx_pipe_crc_irq_handler(dev, pipe);
/* plane/pipes map 1:1 on ilk+ */
- if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
- intel_prepare_page_flip(dev, pipe);
+ if (de_iir & DE_PLANE_FLIP_DONE(pipe))
intel_finish_page_flip_plane(dev, pipe);
- }
}
/* check event from PCH */
@@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
intel_check_page_flip(dev, pipe);
/* plane/pipes map 1:1 on ilk+ */
- if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
- intel_prepare_page_flip(dev, pipe);
+ if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
intel_finish_page_flip_plane(dev, pipe);
- }
}
/* check event from PCH */
@@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
else
flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
- if (flip_done) {
- intel_prepare_page_flip(dev, pipe);
+ if (flip_done)
intel_finish_page_flip_plane(dev, pipe);
- }
if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
hsw_pipe_crc_irq_handler(dev, pipe);
@@ -3984,7 +3976,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
if (I915_READ16(ISR) & flip_pending)
goto check_page_flip;
- intel_prepare_page_flip(dev, plane);
intel_finish_page_flip(dev, pipe);
return true;
@@ -4175,7 +4166,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
if (I915_READ(ISR) & flip_pending)
goto check_page_flip;
- intel_prepare_page_flip(dev, plane);
intel_finish_page_flip(dev, pipe);
return true;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6303438df9f2..c15c08dc251c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3138,7 +3138,6 @@ static void intel_complete_page_flips(struct drm_device *dev)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum plane plane = intel_crtc->plane;
- intel_prepare_page_flip(dev, plane);
intel_finish_page_flip_plane(dev, plane);
}
}
@@ -10825,53 +10824,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
kfree(work);
}
-static void do_intel_finish_page_flip(struct drm_device *dev,
- struct drm_crtc *crtc)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_unpin_work *work;
- unsigned long flags;
-
- /* Ignore early vblank irqs */
- if (intel_crtc == NULL)
- return;
-
- /*
- * This is called both by irq handlers and the reset code (to complete
- * lost pageflips) so needs the full irqsave spinlocks.
- */
- spin_lock_irqsave(&dev->event_lock, flags);
- work = intel_crtc->unpin_work;
-
- /* Ensure we don't miss a work->pending update ... */
- smp_rmb();
-
- if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
- spin_unlock_irqrestore(&dev->event_lock, flags);
- return;
- }
-
- page_flip_completed(intel_crtc);
-
- spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-void intel_finish_page_flip(struct drm_device *dev, int pipe)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
-
- do_intel_finish_page_flip(dev, crtc);
-}
-
-void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
-
- do_intel_finish_page_flip(dev, crtc);
-}
-
/* Is 'a' after or equal to 'b'? */
static bool g4x_flip_count_after_eq(u32 a, u32 b)
{
@@ -10924,28 +10876,52 @@ static bool page_flip_finished(struct intel_crtc *crtc)
crtc->unpin_work->flip_count);
}
-void intel_prepare_page_flip(struct drm_device *dev, int plane)
+static void do_intel_finish_page_flip(struct drm_device *dev,
+ struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv->plane_to_crtc_mapping[plane]);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_unpin_work *work;
unsigned long flags;
+ /* Ignore early vblank irqs */
+ if (intel_crtc == NULL)
+ return;
/*
* This is called both by irq handlers and the reset code (to complete
* lost pageflips) so needs the full irqsave spinlocks.
- *
- * NB: An MMIO update of the plane base pointer will also
- * generate a page-flip completion irq, i.e. every modeset
- * is also accompanied by a spurious intel_prepare_page_flip().
*/
spin_lock_irqsave(&dev->event_lock, flags);
- if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
- atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
+ work = intel_crtc->unpin_work;
+
+ if (work == NULL ||
+ atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
+ !page_flip_finished(intel_crtc)) {
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+ return;
+ }
+
+ page_flip_completed(intel_crtc);
+
spin_unlock_irqrestore(&dev->event_lock, flags);
}
+void intel_finish_page_flip(struct drm_device *dev, int pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+
+ do_intel_finish_page_flip(dev, crtc);
+}
+
+void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
+
+ do_intel_finish_page_flip(dev, crtc);
+}
+
static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
{
/* Ensure that the work item is consistent when activating it ... */
@@ -11395,8 +11371,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
u32 pending;
pending = atomic_read(&work->pending);
- if (pending != INTEL_FLIP_PENDING)
- return pending == INTEL_FLIP_COMPLETE;
+ if (pending == INTEL_FLIP_INACTIVE)
+ return false;
if (work->flip_ready_vblank == 0) {
if (work->flip_queued_req &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7f0779783851..e26293ead94f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -944,7 +944,6 @@ struct intel_unpin_work {
atomic_t pending;
#define INTEL_FLIP_INACTIVE 0
#define INTEL_FLIP_PENDING 1
-#define INTEL_FLIP_COMPLETE 2
u32 flip_count;
u32 gtt_offset;
struct drm_i915_gem_request *flip_queued_req;
@@ -1155,7 +1154,6 @@ struct drm_framebuffer *
__intel_framebuffer_create(struct drm_device *dev,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj);
-void intel_prepare_page_flip(struct drm_device *dev, int plane);
void intel_finish_page_flip(struct drm_device *dev, int pipe);
void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
void intel_check_page_flip(struct drm_device *dev, int pipe);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] drm/i915: Add the exclusive fence to plane_state.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 1/6] drm/i915: Remove stallcheck special handling Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 4/6] drm/i915: Allow mmio updates on all platforms Maarten Lankhorst
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
XXX: Write me!
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 54 +++++++++++++++++++++++--------
2 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7de7721f65bc..2ab45f16fa65 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -102,6 +102,7 @@ intel_plane_destroy_state(struct drm_plane *plane,
struct drm_plane_state *state)
{
WARN_ON(state && to_intel_plane_state(state)->wait_req);
+ WARN_ON(state && state->fence);
drm_atomic_helper_plane_destroy_state(plane, state);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c15c08dc251c..c52eb1bb4f24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13349,6 +13349,15 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
struct intel_plane_state *intel_plane_state =
to_intel_plane_state(plane_state);
+ if (plane_state->fence) {
+ long lret = fence_wait(plane_state->fence, true);
+
+ if (lret < 0) {
+ ret = lret;
+ break;
+ }
+ }
+
if (!intel_plane_state->wait_req)
continue;
@@ -13681,6 +13690,33 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.atomic_destroy_state = intel_crtc_destroy_state,
};
+static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
+{
+ struct reservation_object *resv;
+
+
+ if (!obj->base.dma_buf)
+ return NULL;
+
+ resv = obj->base.dma_buf->resv;
+
+ /* For framebuffer backed by dmabuf, wait for fence */
+ while (1) {
+ struct fence *fence_excl, *ret = NULL;
+
+ rcu_read_lock();
+
+ fence_excl = rcu_dereference(resv->fence_excl);
+ if (fence_excl)
+ ret = fence_get_rcu(fence_excl);
+
+ rcu_read_unlock();
+
+ if (ret == fence_excl)
+ return ret;
+ }
+}
+
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -13732,19 +13768,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
return ret;
}
- /* For framebuffer backed by dmabuf, wait for fence */
- if (obj && obj->base.dma_buf) {
- long lret;
-
- lret = reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
- false, true,
- MAX_SCHEDULE_TIMEOUT);
- if (lret == -ERESTARTSYS)
- return lret;
-
- WARN(lret < 0, "waiting returns %li\n", lret);
- }
-
if (!obj) {
ret = 0;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR &&
@@ -13764,6 +13787,8 @@ intel_prepare_plane_fb(struct drm_plane *plane,
i915_gem_request_assign(&plane_state->wait_req,
obj->last_write_req);
+
+ plane_state->base.fence = intel_get_excl_fence(obj);
}
i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
@@ -13806,6 +13831,9 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
i915_gem_request_assign(&old_intel_state->wait_req, NULL);
+
+ fence_put(old_intel_state->base.fence);
+ old_intel_state->base.fence = NULL;
}
int
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
` (2 preceding siblings ...)
2016-03-23 13:24 ` [PATCH 3/6] drm/i915: Add the exclusive fence to plane_state Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
2016-03-23 15:07 ` Ville Syrjälä
2016-03-23 13:24 ` [PATCH 5/6] drm/i915: Convert flip_work to a list Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 6/6] drm/i915: Rework intel_crtc_page_flip to be almost atomic Maarten Lankhorst
5 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
Rename intel_unpin_work to intel_flip_work and use it for mmio flips
and unpinning. Use flip_queued_req to hold the wait request in the
mmio case and allow the vblank interrupt to complete mmio work to
have mmio flips run correctly on g4 and earlier.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
drivers/gpu/drm/i915/intel_display.c | 271 +++++++++++------------------------
drivers/gpu/drm/i915/intel_drv.h | 16 +--
3 files changed, 87 insertions(+), 204 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4a0078c17cd1..bc2ae70abcea 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -573,10 +573,10 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
for_each_intel_crtc(dev, crtc) {
const char pipe = pipe_name(crtc->pipe);
const char plane = plane_name(crtc->plane);
- struct intel_unpin_work *work;
+ struct intel_flip_work *work;
spin_lock_irq(&dev->event_lock);
- work = crtc->unpin_work;
+ work = crtc->flip_work;
if (work == NULL) {
seq_printf(m, "No flip due on pipe %c (plane %c)\n",
pipe, plane);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c52eb1bb4f24..3b082020aab8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,11 @@
#include <linux/reservation.h>
#include <linux/dma-buf.h>
+static bool is_mmio_work(struct intel_flip_work *work)
+{
+ return work->mmio_work.func;
+}
+
/* Primary plane formats for gen <= 3 */
static const uint32_t i8xx_primary_formats[] = {
DRM_FORMAT_C8,
@@ -3243,7 +3248,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
return false;
spin_lock_irq(&dev->event_lock);
- pending = to_intel_crtc(crtc)->unpin_work != NULL;
+ pending = to_intel_crtc(crtc)->flip_work != NULL;
spin_unlock_irq(&dev->event_lock);
return pending;
@@ -3825,7 +3830,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
if (atomic_read(&crtc->unpin_work_count) == 0)
continue;
- if (crtc->unpin_work)
+ if (crtc->flip_work)
intel_wait_for_vblank(dev, crtc->pipe);
return true;
@@ -3837,11 +3842,11 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
static void page_flip_completed(struct intel_crtc *intel_crtc)
{
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
- struct intel_unpin_work *work = intel_crtc->unpin_work;
+ struct intel_flip_work *work = intel_crtc->flip_work;
- /* ensure that the unpin work is consistent wrt ->pending. */
+ /* ensure that the flip work is consistent wrt ->pending. */
smp_rmb();
- intel_crtc->unpin_work = NULL;
+ intel_crtc->flip_work = NULL;
if (work->event)
drm_send_vblank_event(intel_crtc->base.dev,
@@ -3851,7 +3856,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc)
drm_crtc_vblank_put(&intel_crtc->base);
wake_up_all(&dev_priv->pending_flip_queue);
- queue_work(dev_priv->wq, &work->work);
+ queue_work(dev_priv->wq, &work->unpin_work);
trace_i915_flip_complete(intel_crtc->plane,
work->pending_flip_obj);
@@ -3875,9 +3880,11 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
if (ret == 0) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_flip_work *work;
spin_lock_irq(&dev->event_lock);
- if (intel_crtc->unpin_work) {
+ work = intel_crtc->flip_work;
+ if (work && !is_mmio_work(work)) {
WARN_ONCE(1, "Removing stuck page flip\n");
page_flip_completed(intel_crtc);
}
@@ -6259,7 +6266,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->unpin_work);
+ WARN_ON(intel_crtc->flip_work);
intel_pre_disable_primary_noatomic(crtc);
@@ -10781,15 +10788,16 @@ 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_unpin_work *work;
+ struct intel_flip_work *work;
spin_lock_irq(&dev->event_lock);
- work = intel_crtc->unpin_work;
- intel_crtc->unpin_work = NULL;
+ work = intel_crtc->flip_work;
+ intel_crtc->flip_work = NULL;
spin_unlock_irq(&dev->event_lock);
if (work) {
- cancel_work_sync(&work->work);
+ cancel_work_sync(&work->mmio_work);
+ cancel_work_sync(&work->unpin_work);
kfree(work);
}
@@ -10800,12 +10808,15 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
static void intel_unpin_work_fn(struct work_struct *__work)
{
- struct intel_unpin_work *work =
- container_of(__work, struct intel_unpin_work, work);
+ struct intel_flip_work *work =
+ container_of(__work, struct intel_flip_work, unpin_work);
struct intel_crtc *crtc = to_intel_crtc(work->crtc);
struct drm_device *dev = crtc->base.dev;
struct drm_plane *primary = crtc->base.primary;
+ if (is_mmio_work(work))
+ flush_work(&work->mmio_work);
+
mutex_lock(&dev->struct_mutex);
intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
drm_gem_object_unreference(&work->pending_flip_obj->base);
@@ -10871,16 +10882,16 @@ static bool page_flip_finished(struct intel_crtc *crtc)
* anyway, we don't really care.
*/
return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
- crtc->unpin_work->gtt_offset &&
+ crtc->flip_work->gtt_offset &&
g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
- crtc->unpin_work->flip_count);
+ crtc->flip_work->flip_count);
}
static void do_intel_finish_page_flip(struct drm_device *dev,
struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_unpin_work *work;
+ struct intel_flip_work *work;
unsigned long flags;
/* Ignore early vblank irqs */
@@ -10892,11 +10903,11 @@ 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->unpin_work;
+ work = intel_crtc->flip_work;
if (work == NULL ||
atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
- !page_flip_finished(intel_crtc)) {
+ !page_flip_finished(intel_crtc)) {
spin_unlock_irqrestore(&dev->event_lock, flags);
return;
}
@@ -10922,7 +10933,7 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
do_intel_finish_page_flip(dev, crtc);
}
-static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
+static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
{
/* Ensure that the work item is consistent when activating it ... */
smp_wmb();
@@ -10959,10 +10970,9 @@ 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->unpin_work->gtt_offset);
+ intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
intel_ring_emit(engine, 0); /* aux display base address, unused */
- intel_mark_page_flip_active(intel_crtc->unpin_work);
return 0;
}
@@ -10991,10 +11001,9 @@ 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->unpin_work->gtt_offset);
+ intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
intel_ring_emit(engine, MI_NOOP);
- intel_mark_page_flip_active(intel_crtc->unpin_work);
return 0;
}
@@ -11022,7 +11031,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->unpin_work->gtt_offset |
+ intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
obj->tiling_mode);
/* XXX Enabling the panel-fitter across page-flip is so far
@@ -11033,7 +11042,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
intel_ring_emit(engine, pf | pipesrc);
- intel_mark_page_flip_active(intel_crtc->unpin_work);
return 0;
}
@@ -11057,7 +11065,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->unpin_work->gtt_offset);
+ intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
/* Contrary to the suggestions in the documentation,
* "Enable Panel Fitter" does not seem to be required when page
@@ -11069,7 +11077,6 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
intel_ring_emit(engine, pf | pipesrc);
- intel_mark_page_flip_active(intel_crtc->unpin_work);
return 0;
}
@@ -11161,10 +11168,9 @@ 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->unpin_work->gtt_offset);
+ intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
intel_ring_emit(engine, (MI_NOOP));
- intel_mark_page_flip_active(intel_crtc->unpin_work);
return 0;
}
@@ -11182,9 +11188,6 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
if (engine == NULL)
return true;
- if (INTEL_INFO(engine->dev)->gen < 5)
- return false;
-
if (i915.use_mmio_flip < 0)
return false;
else if (i915.use_mmio_flip > 0)
@@ -11199,126 +11202,21 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
return engine != i915_gem_request_get_engine(obj->last_write_req);
}
-static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
- unsigned int rotation,
- struct intel_unpin_work *work)
-{
- struct drm_device *dev = intel_crtc->base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
- const enum pipe pipe = intel_crtc->pipe;
- u32 ctl, stride, tile_height;
-
- ctl = I915_READ(PLANE_CTL(pipe, 0));
- ctl &= ~PLANE_CTL_TILED_MASK;
- switch (fb->modifier[0]) {
- case DRM_FORMAT_MOD_NONE:
- break;
- case I915_FORMAT_MOD_X_TILED:
- ctl |= PLANE_CTL_TILED_X;
- break;
- case I915_FORMAT_MOD_Y_TILED:
- ctl |= PLANE_CTL_TILED_Y;
- break;
- case I915_FORMAT_MOD_Yf_TILED:
- ctl |= PLANE_CTL_TILED_YF;
- break;
- default:
- MISSING_CASE(fb->modifier[0]);
- }
-
- /*
- * The stride is either expressed as a multiple of 64 bytes chunks for
- * linear buffers or in number of tiles for tiled buffers.
- */
- if (intel_rotation_90_or_270(rotation)) {
- /* stride = Surface height in tiles */
- tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
- stride = DIV_ROUND_UP(fb->height, tile_height);
- } else {
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
- }
-
- /*
- * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
- * PLANE_SURF updates, the update is then guaranteed to be atomic.
- */
- I915_WRITE(PLANE_CTL(pipe, 0), ctl);
- I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
-
- I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
- POSTING_READ(PLANE_SURF(pipe, 0));
-}
-
-static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
- struct intel_unpin_work *work)
+static void intel_mmio_flip_work_func(struct work_struct *w)
{
- struct drm_device *dev = intel_crtc->base.dev;
+ struct intel_flip_work *work =
+ container_of(w, struct intel_flip_work, mmio_work);
+ struct intel_crtc *crtc = to_intel_crtc(work->crtc);
+ struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_framebuffer *intel_fb =
- to_intel_framebuffer(intel_crtc->base.primary->fb);
- struct drm_i915_gem_object *obj = intel_fb->obj;
- i915_reg_t reg = DSPCNTR(intel_crtc->plane);
- u32 dspcntr;
-
- dspcntr = I915_READ(reg);
-
- if (obj->tiling_mode != I915_TILING_NONE)
- dspcntr |= DISPPLANE_TILED;
- else
- dspcntr &= ~DISPPLANE_TILED;
-
- I915_WRITE(reg, dspcntr);
+ struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+ struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
- I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
- POSTING_READ(DSPSURF(intel_crtc->plane));
-}
-
-/*
- * XXX: This is the temporary way to update the plane registers until we get
- * around to using the usual plane update functions for MMIO flips
- */
-static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
-{
- struct intel_crtc *crtc = mmio_flip->crtc;
- struct intel_unpin_work *work;
-
- spin_lock_irq(&crtc->base.dev->event_lock);
- work = crtc->unpin_work;
- spin_unlock_irq(&crtc->base.dev->event_lock);
- if (work == NULL)
- return;
-
- intel_pipe_update_start(crtc);
-
- if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
- skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
- else
- /* use_mmio_flip() retricts MMIO flips to ilk+ */
- ilk_do_mmio_flip(crtc, work);
-
- intel_pipe_update_end(crtc);
-
- intel_mark_page_flip_active(work);
-}
-
-static void intel_mmio_flip_work_func(struct work_struct *work)
-{
- struct intel_mmio_flip *mmio_flip =
- container_of(work, struct intel_mmio_flip, work);
- struct intel_framebuffer *intel_fb =
- to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
- struct drm_i915_gem_object *obj = intel_fb->obj;
-
- if (mmio_flip->req) {
- WARN_ON(__i915_wait_request(mmio_flip->req,
- mmio_flip->crtc->reset_counter,
+ if (work->flip_queued_req)
+ WARN_ON(__i915_wait_request(work->flip_queued_req,
+ crtc->reset_counter,
false, NULL,
- &mmio_flip->i915->rps.mmioflips));
- i915_gem_request_unreference__unlocked(mmio_flip->req);
- }
+ &dev_priv->rps.mmioflips));
/* For framebuffer backed by dmabuf, wait for fence */
if (obj->base.dma_buf)
@@ -11326,29 +11224,12 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
false, false,
MAX_SCHEDULE_TIMEOUT) < 0);
- intel_do_mmio_flip(mmio_flip);
- kfree(mmio_flip);
-}
-
-static int intel_queue_mmio_flip(struct drm_device *dev,
- struct drm_crtc *crtc,
- struct drm_i915_gem_object *obj)
-{
- struct intel_mmio_flip *mmio_flip;
-
- mmio_flip = kmalloc(sizeof(*mmio_flip), GFP_KERNEL);
- if (mmio_flip == NULL)
- return -ENOMEM;
-
- mmio_flip->i915 = to_i915(dev);
- mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
- mmio_flip->crtc = to_intel_crtc(crtc);
- mmio_flip->rotation = crtc->primary->state->rotation;
-
- INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
- schedule_work(&mmio_flip->work);
-
- return 0;
+ intel_pipe_update_start(crtc);
+ primary->update_plane(&primary->base,
+ crtc->config,
+ to_intel_plane_state(primary->base.state));
+ atomic_set(&work->pending, INTEL_FLIP_PENDING);
+ intel_pipe_update_end(crtc);
}
static int intel_default_queue_flip(struct drm_device *dev,
@@ -11366,7 +11247,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_unpin_work *work = intel_crtc->unpin_work;
+ struct intel_flip_work *work = intel_crtc->flip_work;
u32 addr;
u32 pending;
@@ -11374,6 +11255,13 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
if (pending == INTEL_FLIP_INACTIVE)
return false;
+ /*
+ * If this is a mmio flip return true because state
+ * is changed to INTEL_FLIP_PENDING after updating.
+ */
+ if (is_mmio_work(work))
+ return true;
+
if (work->flip_ready_vblank == 0) {
if (work->flip_queued_req &&
!i915_gem_request_completed(work->flip_queued_req, true))
@@ -11404,7 +11292,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_unpin_work *work;
+ struct intel_flip_work *work;
WARN_ON(!in_interrupt());
@@ -11412,14 +11300,15 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
return;
spin_lock(&dev->event_lock);
- work = intel_crtc->unpin_work;
+ work = intel_crtc->flip_work;
if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
- WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
+ WARN_ONCE(!is_mmio_work(work),
+ "Kicking stuck page flip: queued at %d, now %d\n",
work->flip_queued_vblank, drm_vblank_count(dev, pipe));
page_flip_completed(intel_crtc);
work = NULL;
}
- if (work != NULL &&
+ if (work != NULL && !is_mmio_work(work) &&
drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
intel_queue_rps_boost_for_request(dev, work->flip_queued_req);
spin_unlock(&dev->event_lock);
@@ -11437,7 +11326,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *primary = crtc->primary;
enum pipe pipe = intel_crtc->pipe;
- struct intel_unpin_work *work;
+ struct intel_flip_work *work;
struct intel_engine_cs *engine;
bool mmio_flip;
struct drm_i915_gem_request *request = NULL;
@@ -11474,19 +11363,20 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->event = event;
work->crtc = crtc;
work->old_fb = old_fb;
- INIT_WORK(&work->work, intel_unpin_work_fn);
+ INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
ret = drm_crtc_vblank_get(crtc);
if (ret)
goto free_work;
- /* We borrow the event spin lock for protecting unpin_work */
+ /* We borrow the event spin lock for protecting flip_work */
spin_lock_irq(&dev->event_lock);
- if (intel_crtc->unpin_work) {
+ if (intel_crtc->flip_work) {
/* Before declaring the flip queue wedged, check if
* the hardware completed the operation behind our backs.
*/
- if (__intel_pageflip_stall_check(dev, crtc)) {
+ if (!is_mmio_work(intel_crtc->flip_work) &&
+ __intel_pageflip_stall_check(dev, crtc)) {
DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
page_flip_completed(intel_crtc);
} else {
@@ -11498,7 +11388,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
return -EBUSY;
}
}
- intel_crtc->unpin_work = work;
+ intel_crtc->flip_work = work;
spin_unlock_irq(&dev->event_lock);
if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
@@ -11567,23 +11457,22 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
if (mmio_flip) {
- ret = intel_queue_mmio_flip(dev, crtc, obj);
- if (ret)
- goto cleanup_unpin;
+ INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
i915_gem_request_assign(&work->flip_queued_req,
obj->last_write_req);
+
+ schedule_work(&work->mmio_work);
} else {
+ i915_gem_request_assign(&work->flip_queued_req, request);
ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
page_flip_flags);
if (ret)
goto cleanup_unpin;
- i915_gem_request_assign(&work->flip_queued_req, request);
- }
-
- if (request)
+ intel_mark_page_flip_active(work);
i915_add_request_no_flush(request);
+ }
work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
@@ -11613,7 +11502,7 @@ cleanup:
drm_framebuffer_unreference(work->old_fb);
spin_lock_irq(&dev->event_lock);
- intel_crtc->unpin_work = NULL;
+ intel_crtc->flip_work = NULL;
spin_unlock_irq(&dev->event_lock);
drm_crtc_vblank_put(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e26293ead94f..cac368138764 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -592,14 +592,6 @@ struct vlv_wm_state {
bool cxsr;
};
-struct intel_mmio_flip {
- struct work_struct work;
- struct drm_i915_private *i915;
- struct drm_i915_gem_request *req;
- struct intel_crtc *crtc;
- unsigned int rotation;
-};
-
struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
@@ -614,7 +606,7 @@ struct intel_crtc {
unsigned long enabled_power_domains;
bool lowfreq_avail;
struct intel_overlay *overlay;
- struct intel_unpin_work *unpin_work;
+ struct intel_flip_work *flip_work;
atomic_t unpin_work_count;
@@ -935,8 +927,10 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
return dev_priv->plane_to_crtc_mapping[plane];
}
-struct intel_unpin_work {
- struct work_struct work;
+struct intel_flip_work {
+ struct work_struct unpin_work;
+ struct work_struct mmio_work;
+
struct drm_crtc *crtc;
struct drm_framebuffer *old_fb;
struct drm_i915_gem_object *pending_flip_obj;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] drm/i915: Convert flip_work to a list.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
` (3 preceding siblings ...)
2016-03-23 13:24 ` [PATCH 4/6] drm/i915: Allow mmio updates on all platforms Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 6/6] drm/i915: Rework intel_crtc_page_flip to be almost atomic Maarten Lankhorst
5 siblings, 0 replies; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
This will be required to allow more than 1 update in the future.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 91 ++++++++++++++++-------------
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_drv.h | 4 +-
4 files changed, 116 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bc2ae70abcea..1f9cbe83285e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -558,6 +558,54 @@ 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, true),
+ i915_gem_request_completed(work->flip_queued_req, true));
+ } else
+ seq_printf(m, "Flip not associated with any engine\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,
+ drm_crtc_vblank_count(&crtc->base));
+ 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;
@@ -576,48 +624,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, true),
- 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,
- drm_crtc_vblank_count(&crtc->base));
- 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); {
+ 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 9d29ab06c99a..06e4773ae7f6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -622,7 +622,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 3b082020aab8..8af1ddbd3549 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3241,17 +3241,12 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- bool pending;
if (i915_reset_in_progress(&dev_priv->gpu_error) ||
intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.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,
@@ -3830,7 +3825,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;
@@ -3839,14 +3834,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;
- /* ensure that the flip work is consistent wrt ->pending. */
- smp_rmb();
- intel_crtc->flip_work = NULL;
+ list_del_init(&work->head);
if (work->event)
drm_send_vblank_event(intel_crtc->base.dev,
@@ -3883,10 +3875,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);
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);
}
@@ -6266,7 +6259,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);
@@ -10789,17 +10782,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);
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);
+ 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);
@@ -10841,7 +10841,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;
@@ -10882,9 +10883,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,
@@ -10903,16 +10904,18 @@ 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);
if (work == NULL ||
atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
- !page_flip_finished(intel_crtc)) {
+ !page_flip_finished(intel_crtc, work)) {
spin_unlock_irqrestore(&dev->event_lock, flags);
return;
}
- page_flip_completed(intel_crtc);
+ page_flip_completed(intel_crtc, work);
spin_unlock_irqrestore(&dev->event_lock, flags);
}
@@ -10947,7 +10950,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);
@@ -10970,7 +10973,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;
@@ -10981,7 +10984,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);
@@ -11001,7 +11004,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;
@@ -11012,7 +11015,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;
@@ -11031,8 +11034,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.
@@ -11050,7 +11052,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;
@@ -11065,7 +11067,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
@@ -11085,7 +11087,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);
@@ -11168,7 +11170,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;
@@ -11237,17 +11239,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;
u32 pending;
@@ -11300,12 +11302,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, drm_vblank_count(dev, pipe));
- page_flip_completed(intel_crtc);
+ page_flip_completed(intel_crtc, work);
work = NULL;
}
if (work != NULL && !is_mmio_work(work) &&
@@ -11371,14 +11375,19 @@ 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 (!is_mmio_work(intel_crtc->flip_work) &&
- __intel_pageflip_stall_check(dev, crtc)) {
+ if (!is_mmio_work(old_work) &&
+ __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);
@@ -11388,7 +11397,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)
@@ -11466,7 +11475,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;
@@ -11502,7 +11511,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);
@@ -14107,6 +14116,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 cac368138764..697cf685858d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -606,7 +606,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;
@@ -928,6 +928,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] drm/i915: Rework intel_crtc_page_flip to be almost atomic.
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
` (4 preceding siblings ...)
2016-03-23 13:24 ` [PATCH 5/6] drm/i915: Convert flip_work to a list Maarten Lankhorst
@ 2016-03-23 13:24 ` Maarten Lankhorst
5 siblings, 0 replies; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-23 13:24 UTC (permalink / raw)
To: intel-gfx
Create a work structure that will be used for all changes. This will
be used later on in the atomic commit function.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 37 +-
drivers/gpu/drm/i915/intel_display.c | 661 +++++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_drv.h | 13 +-
3 files changed, 424 insertions(+), 287 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f9cbe83285e..9b740d8a3f5e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -564,30 +564,43 @@ static void i915_dump_pageflip(struct seq_file *m,
struct intel_flip_work *work)
{
const char pipe = pipe_name(crtc->pipe);
- const char plane = plane_name(crtc->plane);
u32 pending;
u32 addr;
+ int i;
pending = atomic_read(&work->pending);
if (pending == INTEL_FLIP_INACTIVE) {
seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
- pipe, plane);
+ pipe, plane_name(crtc->plane));
} else {
seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
- pipe, plane);
+ pipe, plane_name(crtc->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",
+
+ for (i = 0; i < work->num_planes; i++) {
+ struct intel_plane_state *old_plane_state = work->old_plane_state[i];
+ struct drm_plane *plane = old_plane_state->base.plane;
+ struct drm_i915_gem_request *req = old_plane_state->wait_req;
+ struct intel_engine_cs *engine;
+
+ seq_printf(m, "[PLANE:%i] part of flip.\n", plane->base.id);
+
+ if (!req) {
+ seq_printf(m, "Plane not associated with any engine\n");
+ continue;
+ }
+
+ engine = i915_gem_request_get_engine(req);
+
+ seq_printf(m, "Plane blocked 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),
+ i915_gem_request_get_seqno(req),
dev_priv->next_seqno,
engine->get_seqno(engine, true),
- i915_gem_request_completed(work->flip_queued_req, true));
- } else
- seq_printf(m, "Flip not associated with any engine\n");
+ i915_gem_request_completed(req, true));
+ }
+
seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
work->flip_queued_vblank,
work->flip_ready_vblank,
@@ -600,7 +613,7 @@ static void i915_dump_pageflip(struct seq_file *m,
addr = I915_READ(DSPADDR(crtc->plane));
seq_printf(m, "Current scanout address 0x%08x\n", addr);
- if (work->pending_flip_obj) {
+ if (work->flip_queued_req) {
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);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8af1ddbd3549..5b09b45c54a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -49,7 +49,7 @@
static bool is_mmio_work(struct intel_flip_work *work)
{
- return work->mmio_work.func;
+ return !work->flip_queued_req;
}
/* Primary plane formats for gen <= 3 */
@@ -2549,20 +2549,6 @@ out_unref_obj:
return false;
}
-/* Update plane->state->fb to match plane->fb after driver-internal updates */
-static void
-update_state_fb(struct drm_plane *plane)
-{
- if (plane->fb == plane->state->fb)
- return;
-
- if (plane->state->fb)
- drm_framebuffer_unreference(plane->state->fb);
- plane->state->fb = plane->fb;
- if (plane->state->fb)
- drm_framebuffer_reference(plane->state->fb);
-}
-
static void
intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
struct intel_initial_plane_config *plane_config)
@@ -3837,8 +3823,8 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
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);
-
- list_del_init(&work->head);
+ struct drm_plane_state *new_plane_state;
+ struct drm_plane *primary = intel_crtc->base.primary;
if (work->event)
drm_send_vblank_event(intel_crtc->base.dev,
@@ -3847,11 +3833,19 @@ static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip
drm_crtc_vblank_put(&intel_crtc->base);
- wake_up_all(&dev_priv->pending_flip_queue);
- queue_work(dev_priv->wq, &work->unpin_work);
+ new_plane_state = &work->old_plane_state[0]->base;
+ if (work->num_planes >= 1 &&
+ new_plane_state->plane == primary &&
+ new_plane_state->fb)
+ trace_i915_flip_complete(intel_crtc->plane,
+ intel_fb_obj(new_plane_state->fb));
- trace_i915_flip_complete(intel_crtc->plane,
- work->pending_flip_obj);
+ if (work->can_async_unpin) {
+ list_del_init(&work->head);
+ wake_up_all(&dev_priv->pending_flip_queue);
+ }
+
+ queue_work(dev_priv->wq, &work->unpin_work);
}
static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
@@ -3877,7 +3871,9 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
spin_lock_irq(&dev->event_lock);
work = list_first_entry_or_null(&intel_crtc->flip_work,
struct intel_flip_work, head);
- if (work && !is_mmio_work(work)) {
+
+ if (work && !is_mmio_work(work) &&
+ !work_busy(&work->unpin_work)) {
WARN_ONCE(1, "Removing stuck page flip\n");
page_flip_completed(intel_crtc, work);
}
@@ -10806,31 +10802,105 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
kfree(intel_crtc);
}
+static void intel_crtc_post_flip_update(struct intel_flip_work *work,
+ struct drm_crtc *crtc)
+{
+ struct intel_crtc_state *crtc_state = work->new_crtc_state;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ if (crtc_state->disable_cxsr)
+ intel_crtc->wm.cxsr_allowed = true;
+
+ if (crtc_state->update_wm_post && crtc_state->base.active)
+ intel_update_watermarks(crtc);
+
+ if (work->num_planes > 0 &&
+ work->old_plane_state[0]->base.plane == crtc->primary) {
+ struct intel_plane_state *plane_state =
+ work->new_plane_state[0];
+
+ if (plane_state->visible &&
+ (needs_modeset(&crtc_state->base) ||
+ !work->old_plane_state[0]->visible))
+ intel_post_enable_primary(crtc);
+ }
+}
+
static void intel_unpin_work_fn(struct work_struct *__work)
{
struct intel_flip_work *work =
container_of(__work, struct intel_flip_work, unpin_work);
- struct intel_crtc *crtc = to_intel_crtc(work->crtc);
- struct drm_device *dev = crtc->base.dev;
- struct drm_plane *primary = crtc->base.primary;
+ struct drm_crtc *crtc = work->old_crtc_state->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int i;
- if (is_mmio_work(work))
- flush_work(&work->mmio_work);
+ if (work->fb_bits)
+ intel_frontbuffer_flip_complete(dev, work->fb_bits);
- mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
- drm_gem_object_unreference(&work->pending_flip_obj->base);
+ /*
+ * Unless work->can_async_unpin is false, there's no way to ensure
+ * that work->new_crtc_state contains valid memory during unpin
+ * because intel_atomic_commit may free it before this runs.
+ */
+ if (!work->can_async_unpin)
+ intel_crtc_post_flip_update(work, crtc);
+
+ if (work->fb_bits & to_intel_plane(crtc->primary)->frontbuffer_bit)
+ intel_fbc_post_update(intel_crtc);
+
+ if (work->put_power_domains)
+ modeset_put_power_domains(dev_priv, work->put_power_domains);
+
+ /* Make sure mmio work is completely finished before freeing all state here. */
+ flush_work(&work->mmio_work);
+
+ if (!work->can_async_unpin || !list_empty(&work->head)) {
+ spin_lock_irq(&dev->event_lock);
+ WARN(list_empty(&work->head) != work->can_async_unpin,
+ "[CRTC:%i] Pin work %p async %i with %i planes, active %i -> %i ms %i\n",
+ crtc->base.id, work, work->can_async_unpin, work->num_planes,
+ work->old_crtc_state->base.active, work->new_crtc_state->base.active,
+ needs_modeset(&work->new_crtc_state->base));
+
+ if (!list_empty(&work->head))
+ list_del(&work->head);
+
+ wake_up_all(&dev_priv->pending_flip_queue);
+ spin_unlock_irq(&dev->event_lock);
+ }
if (work->flip_queued_req)
- i915_gem_request_assign(&work->flip_queued_req, NULL);
- mutex_unlock(&dev->struct_mutex);
+ i915_gem_request_unreference__unlocked(work->flip_queued_req);
+
+ for (i = 0; i < work->num_planes; i++) {
+ struct intel_plane_state *old_plane_state =
+ work->old_plane_state[i];
+ struct drm_framebuffer *old_fb = old_plane_state->base.fb;
+ struct drm_plane *plane = old_plane_state->base.plane;
+ struct drm_i915_gem_request *req;
- intel_frontbuffer_flip_complete(dev, to_intel_plane(primary)->frontbuffer_bit);
- intel_fbc_post_update(crtc);
- drm_framebuffer_unreference(work->old_fb);
+ req = old_plane_state->wait_req;
+ old_plane_state->wait_req = NULL;
+ i915_gem_request_unreference__unlocked(req);
- BUG_ON(atomic_read(&crtc->unpin_work_count) == 0);
- atomic_dec(&crtc->unpin_work_count);
+ fence_put(old_plane_state->base.fence);
+ old_plane_state->base.fence = NULL;
+
+ if (old_fb &&
+ (plane->type != DRM_PLANE_TYPE_CURSOR ||
+ !INTEL_INFO(dev_priv)->cursor_needs_physical)) {
+ mutex_lock(&dev->struct_mutex);
+ intel_unpin_fb_obj(old_fb, old_plane_state->base.rotation);
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ intel_plane_destroy_state(plane, &old_plane_state->base);
+ }
+
+ if (!WARN_ON(atomic_read(&intel_crtc->unpin_work_count) == 0))
+ atomic_dec(&intel_crtc->unpin_work_count);
kfree(work);
}
@@ -10910,7 +10980,7 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
if (work == NULL ||
atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
- !page_flip_finished(intel_crtc, work)) {
+ !page_flip_finished(intel_crtc, work) || work_busy(&work->unpin_work)) {
spin_unlock_irqrestore(&dev->event_lock, flags);
return;
}
@@ -10936,15 +11006,6 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
do_intel_finish_page_flip(dev, crtc);
}
-static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
-{
- /* Ensure that the work item is consistent when activating it ... */
- smp_wmb();
- atomic_set(&work->pending, INTEL_FLIP_PENDING);
- /* and that it is marked active as soon as the irq could fire. */
- smp_wmb();
-}
-
static int intel_gen2_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
@@ -11176,72 +11237,201 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
return 0;
}
-static bool use_mmio_flip(struct intel_engine_cs *engine,
- struct drm_i915_gem_object *obj)
+static struct intel_engine_cs *
+intel_get_flip_engine(struct drm_device *dev,
+ struct drm_i915_private *dev_priv,
+ struct drm_i915_gem_object *obj)
{
- /*
- * This is not being used for older platforms, because
- * non-availability of flip done interrupt forces us to use
- * CS flips. Older platforms derive flip done using some clever
- * tricks involving the flip_pending status bits and vblank irqs.
- * So using MMIO flips there would disrupt this mechanism.
- */
+ if (IS_VALLEYVIEW(dev) || IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
+ return &dev_priv->engine[BCS];
- if (engine == NULL)
- return true;
+ if (dev_priv->info.gen >= 7) {
+ struct intel_engine_cs *engine;
- if (i915.use_mmio_flip < 0)
+ engine = i915_gem_request_get_engine(obj->last_write_req);
+ if (engine && engine->id == RCS)
+ return engine;
+
+ return &dev_priv->engine[BCS];
+ } else
+ return &dev_priv->engine[RCS];
+}
+
+static bool
+flip_fb_compatible(struct drm_device *dev,
+ struct drm_framebuffer *fb,
+ struct drm_framebuffer *old_fb)
+{
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
+
+ if (old_fb->pixel_format != fb->pixel_format)
return false;
- else if (i915.use_mmio_flip > 0)
- return true;
- else if (i915.enable_execlists)
- return true;
- else if (obj->base.dma_buf &&
- !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv,
- false))
- return true;
- else
- return engine != i915_gem_request_get_engine(obj->last_write_req);
+
+ if (INTEL_INFO(dev)->gen > 3 &&
+ (fb->offsets[0] != old_fb->offsets[0] ||
+ fb->pitches[0] != old_fb->pitches[0]))
+ return false;
+
+ /* vlv: DISPLAY_FLIP fails to change tiling */
+ if (IS_VALLEYVIEW(dev) && obj->tiling_mode != old_obj->tiling_mode)
+ return false;
+
+ return true;
+}
+
+static void
+intel_display_flip_prepare(struct drm_device *dev, 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);
+
+ if (work->flip_prepared)
+ return;
+
+ work->flip_prepared = true;
+
+ if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+ work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(intel_crtc->pipe)) + 1;
+ work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+
+ intel_frontbuffer_flip_prepare(dev, work->new_crtc_state->fb_bits);
+}
+
+static void intel_flip_schedule_request(struct intel_flip_work *work, struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane_state *new_state = work->new_plane_state[0];
+ struct intel_plane_state *old_state = work->old_plane_state[0];
+ struct drm_framebuffer *fb, *old_fb;
+ struct drm_i915_gem_request *request = NULL;
+ struct intel_engine_cs *engine;
+ struct drm_i915_gem_object *obj;
+ struct fence *fence;
+ int ret;
+
+ if (i915_terminally_wedged(&dev_priv->gpu_error) ||
+ i915_reset_in_progress(&dev_priv->gpu_error) ||
+ i915.enable_execlists || i915.use_mmio_flip > 0 ||
+ !dev_priv->display.queue_flip)
+ goto mmio;
+
+ /* Not right after modesetting, surface parameters need to be updated */
+ if (needs_modeset(crtc->state) ||
+ to_intel_crtc_state(crtc->state)->update_pipe)
+ goto mmio;
+
+ /* Only allow a mmio flip for a primary plane without a dma-buf fence */
+ if (work->num_planes != 1 ||
+ new_state->base.plane != crtc->primary ||
+ new_state->base.fence)
+ goto mmio;
+
+ fence = work->old_plane_state[0]->base.fence;
+ if (fence && !fence_is_signaled(fence))
+ goto mmio;
+
+ old_fb = old_state->base.fb;
+ fb = new_state->base.fb;
+ obj = intel_fb_obj(fb);
+
+ trace_i915_flip_request(to_intel_crtc(crtc)->plane, obj);
+
+ /* Only when updating a already visible fb. */
+ if (!new_state->visible || !old_state->visible)
+ goto mmio;
+
+ if (!flip_fb_compatible(dev, fb, old_fb))
+ goto mmio;
+
+ engine = intel_get_flip_engine(dev, dev_priv, obj);
+ if (i915.use_mmio_flip == 0 && obj->last_write_req &&
+ i915_gem_request_get_engine(obj->last_write_req) != engine)
+ goto mmio;
+
+ work->gtt_offset = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj, 0);
+ work->gtt_offset += to_intel_crtc(crtc)->dspaddr_offset;
+
+ 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)
+ request = NULL;
+ }
+
+ intel_display_flip_prepare(dev, crtc, work);
+
+ if (!ret)
+ ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, 0);
+
+ if (!ret) {
+ i915_gem_request_assign(&work->flip_queued_req, request);
+ smp_mb__before_atomic();
+ atomic_set(&work->pending, INTEL_FLIP_PENDING);
+ i915_add_request_no_flush(request);
+ return;
+ }
+ if (request)
+ i915_gem_request_cancel(request);
+
+mmio:
+ to_intel_crtc(crtc)->reset_counter =
+ atomic_read(&dev_priv->gpu_error.reset_counter);
+ schedule_work(&work->mmio_work);
}
static void intel_mmio_flip_work_func(struct work_struct *w)
{
struct intel_flip_work *work =
container_of(w, struct intel_flip_work, mmio_work);
- struct intel_crtc *crtc = to_intel_crtc(work->crtc);
- struct drm_device *dev = crtc->base.dev;
+ struct drm_crtc *crtc = work->old_crtc_state->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *crtc_state = work->new_crtc_state;
+ struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_plane *primary = to_intel_plane(crtc->base.primary);
- struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
+ struct drm_i915_gem_request *req;
+ int i;
- if (work->flip_queued_req)
- WARN_ON(__i915_wait_request(work->flip_queued_req,
- crtc->reset_counter,
+ for (i = 0; i < work->num_planes; i++) {
+ struct intel_plane_state *old_plane_state = work->old_plane_state[i];
+
+ /* For framebuffer backed by dmabuf, wait for fence */
+ if (old_plane_state->base.fence)
+ WARN_ON(fence_wait(old_plane_state->base.fence, false) < 0);
+
+ req = old_plane_state->wait_req;
+ if (!req)
+ continue;
+
+ WARN_ON(__i915_wait_request(req, intel_crtc->reset_counter,
false, NULL,
&dev_priv->rps.mmioflips));
+ }
+
+ intel_display_flip_prepare(dev, crtc, work);
+
+ intel_pipe_update_start(intel_crtc);
+ if (!needs_modeset(&crtc_state->base)) {
+ if (crtc_state->update_pipe)
+ intel_update_pipe_config(intel_crtc, work->old_crtc_state);
+ else if (INTEL_INFO(dev)->gen >= 9)
+ skl_detach_scalers(intel_crtc);
+ }
+
+ for (i = 0; i < work->num_planes; i++) {
+ struct intel_plane_state *new_plane_state = work->new_plane_state[i];
+ struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
+
+ plane->update_plane(&plane->base, crtc_state, new_plane_state);
+ }
- /* For framebuffer backed by dmabuf, wait for fence */
- if (obj->base.dma_buf)
- WARN_ON(reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
- false, false,
- MAX_SCHEDULE_TIMEOUT) < 0);
-
- intel_pipe_update_start(crtc);
- primary->update_plane(&primary->base,
- crtc->config,
- to_intel_plane_state(primary->base.state));
atomic_set(&work->pending, INTEL_FLIP_PENDING);
- intel_pipe_update_end(crtc);
-}
-static int intel_default_queue_flip(struct drm_device *dev,
- struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj,
- struct drm_i915_gem_request *req,
- uint64_t gtt_offset)
-{
- return -ENODEV;
+ intel_pipe_update_end(intel_crtc);
}
static bool __intel_pageflip_stall_check(struct drm_device *dev,
@@ -11254,7 +11444,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
u32 pending;
pending = atomic_read(&work->pending);
- if (pending == INTEL_FLIP_INACTIVE)
+ if (pending == INTEL_FLIP_INACTIVE ||
+ work_busy(&work->unpin_work))
return false;
/*
@@ -11318,6 +11509,33 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
spin_unlock(&dev->event_lock);
}
+static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
+{
+ struct reservation_object *resv;
+
+
+ if (!obj->base.dma_buf)
+ return NULL;
+
+ resv = obj->base.dma_buf->resv;
+
+ /* For framebuffer backed by dmabuf, wait for fence */
+ while (1) {
+ struct fence *fence_excl, *ret = NULL;
+
+ rcu_read_lock();
+
+ fence_excl = rcu_dereference(resv->fence_excl);
+ if (fence_excl)
+ ret = fence_get_rcu(fence_excl);
+
+ rcu_read_unlock();
+
+ if (ret == fence_excl)
+ return ret;
+ }
+}
+
static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_pending_vblank_event *event,
@@ -11325,17 +11543,20 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *old_fb = crtc->primary->fb;
+ struct drm_plane_state *old_state, *new_state = NULL;
+ struct drm_crtc_state *new_crtc_state = NULL;
+ struct drm_framebuffer *old_fb = crtc->primary->state->fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *primary = crtc->primary;
- enum pipe pipe = intel_crtc->pipe;
struct intel_flip_work *work;
- struct intel_engine_cs *engine;
- bool mmio_flip;
- struct drm_i915_gem_request *request = NULL;
int ret;
+ old_state = crtc->primary->state;
+
+ if (!crtc->state->active)
+ return -EINVAL;
+
/*
* drm_mode_page_flip_ioctl() should already catch this, but double
* check to be safe. In the future we may enable pageflipping from
@@ -11345,7 +11566,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
return -EBUSY;
/* Can't change pixel format via MI display flips. */
- if (fb->pixel_format != crtc->primary->fb->pixel_format)
+ if (fb->pixel_format != old_fb->pixel_format)
return -EINVAL;
/*
@@ -11353,25 +11574,44 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
* Note that pitch changes could also affect these register.
*/
if (INTEL_INFO(dev)->gen > 3 &&
- (fb->offsets[0] != crtc->primary->fb->offsets[0] ||
- fb->pitches[0] != crtc->primary->fb->pitches[0]))
+ (fb->offsets[0] != old_fb->offsets[0] ||
+ fb->pitches[0] != old_fb->pitches[0]))
return -EINVAL;
- if (i915_terminally_wedged(&dev_priv->gpu_error))
- goto out_hang;
-
work = kzalloc(sizeof(*work), GFP_KERNEL);
- if (work == NULL)
- return -ENOMEM;
+ new_crtc_state = intel_crtc_duplicate_state(crtc);
+ new_state = intel_plane_duplicate_state(primary);
+
+ if (!work || !new_crtc_state || !new_state) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
+ drm_framebuffer_unreference(new_state->fb);
+ drm_framebuffer_reference(fb);
+ new_state->fb = fb;
work->event = event;
- work->crtc = crtc;
- work->old_fb = old_fb;
INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
+ INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
+
+ work->new_crtc_state = to_intel_crtc_state(new_crtc_state);
+ work->old_crtc_state = intel_crtc->config;
+
+ work->fb_bits = to_intel_plane(primary)->frontbuffer_bit;
+ work->new_crtc_state->fb_bits = work->fb_bits;
+ work->can_async_unpin = true;
+ work->num_planes = 1;
+ work->old_plane_state[0] = to_intel_plane_state(old_state);
+ work->new_plane_state[0] = to_intel_plane_state(new_state);
+
+ /* Step 1: vblank waiting and workqueue throttling,
+ * similar to intel_atomic_prepare_commit
+ */
ret = drm_crtc_vblank_get(crtc);
if (ret)
- goto free_work;
+ goto cleanup;
/* We borrow the event spin lock for protecting flip_work */
spin_lock_irq(&dev->event_lock);
@@ -11392,9 +11632,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
spin_unlock_irq(&dev->event_lock);
- drm_crtc_vblank_put(crtc);
- kfree(work);
- return -EBUSY;
+ ret = -EBUSY;
+ goto cleanup_vblank;
}
}
list_add_tail(&work->head, &intel_crtc->flip_work);
@@ -11403,157 +11642,62 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
flush_workqueue(dev_priv->wq);
- /* Reference the objects for the scheduled work. */
- drm_framebuffer_reference(work->old_fb);
- drm_gem_object_reference(&obj->base);
-
- crtc->primary->fb = fb;
- update_state_fb(crtc->primary);
- intel_fbc_pre_update(intel_crtc);
-
- work->pending_flip_obj = obj;
-
- ret = i915_mutex_lock_interruptible(dev);
+ /* step 2, similar to intel_prepare_plane_fb */
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
- goto cleanup;
-
- atomic_inc(&intel_crtc->unpin_work_count);
- intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
-
- if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
- work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(pipe)) + 1;
-
- if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
- engine = &dev_priv->engine[BCS];
- if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode)
- /* vlv: DISPLAY_FLIP fails to change tiling */
- engine = NULL;
- } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
- engine = &dev_priv->engine[BCS];
- } else if (INTEL_INFO(dev)->gen >= 7) {
- engine = i915_gem_request_get_engine(obj->last_write_req);
- if (engine == NULL || engine->id != RCS)
- engine = &dev_priv->engine[BCS];
- } else {
- engine = &dev_priv->engine[RCS];
- }
-
- mmio_flip = use_mmio_flip(engine, obj);
-
- /* When using CS flips, we want to emit semaphores between rings.
- * However, when using mmio flips we will create a task to do the
- * synchronisation, so all we want here is to pin the framebuffer
- * into the display plane and skip any waits.
- */
- 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);
- }
+ goto cleanup_work;
- if (ret)
- goto cleanup_pending;
- }
-
- ret = intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+ ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
if (ret)
- goto cleanup_pending;
+ goto cleanup_unlock;
- 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);
+ i915_gem_track_fb(intel_fb_obj(old_fb), obj,
+ to_intel_plane(primary)->frontbuffer_bit);
+
+ /* point of no return, swap state */
+ primary->state = new_state;
+ crtc->state = new_crtc_state;
+ intel_crtc->config = to_intel_crtc_state(new_crtc_state);
+ primary->fb = fb;
- if (mmio_flip) {
- INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
+ /* scheduling flip work */
+ atomic_inc(&intel_crtc->unpin_work_count);
- i915_gem_request_assign(&work->flip_queued_req,
+ if (obj->last_write_req &&
+ !i915_gem_request_completed(obj->last_write_req, true))
+ i915_gem_request_assign(&work->old_plane_state[0]->wait_req,
obj->last_write_req);
- schedule_work(&work->mmio_work);
- } else {
- i915_gem_request_assign(&work->flip_queued_req, request);
- ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
- work->gtt_offset);
- if (ret)
- goto cleanup_unpin;
+ if (obj->base.dma_buf)
+ work->old_plane_state[0]->base.fence = intel_get_excl_fence(obj);
- intel_mark_page_flip_active(work);
- i915_add_request_no_flush(request);
- }
+ intel_fbc_pre_update(intel_crtc);
- work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+ intel_flip_schedule_request(work, crtc);
- i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
- to_intel_plane(primary)->frontbuffer_bit);
mutex_unlock(&dev->struct_mutex);
- intel_frontbuffer_flip_prepare(dev,
- to_intel_plane(primary)->frontbuffer_bit);
-
trace_i915_flip_request(intel_crtc->plane, obj);
return 0;
-cleanup_unpin:
- intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
-cleanup_pending:
- if (!IS_ERR_OR_NULL(request))
- i915_gem_request_cancel(request);
- atomic_dec(&intel_crtc->unpin_work_count);
+cleanup_unlock:
mutex_unlock(&dev->struct_mutex);
-cleanup:
- crtc->primary->fb = old_fb;
- update_state_fb(crtc->primary);
-
- drm_gem_object_unreference_unlocked(&obj->base);
- drm_framebuffer_unreference(work->old_fb);
-
+cleanup_work:
spin_lock_irq(&dev->event_lock);
list_del(&work->head);
spin_unlock_irq(&dev->event_lock);
+cleanup_vblank:
drm_crtc_vblank_put(crtc);
-free_work:
- kfree(work);
-
- if (ret == -EIO) {
- struct drm_atomic_state *state;
- struct drm_plane_state *plane_state;
-
-out_hang:
- state = drm_atomic_state_alloc(dev);
- if (!state)
- return -ENOMEM;
- state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
-
-retry:
- plane_state = drm_atomic_get_plane_state(state, primary);
- ret = PTR_ERR_OR_ZERO(plane_state);
- if (!ret) {
- drm_atomic_set_fb_for_plane(plane_state, fb);
-
- ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
- if (!ret)
- ret = drm_atomic_commit(state);
- }
-
- if (ret == -EDEADLK) {
- drm_modeset_backoff(state->acquire_ctx);
- drm_atomic_state_clear(state);
- goto retry;
- }
+cleanup:
+ if (new_state)
+ intel_plane_destroy_state(primary, new_state);
- if (ret)
- drm_atomic_state_free(state);
+ if (new_crtc_state)
+ intel_crtc_destroy_state(crtc, new_crtc_state);
- if (ret == 0 && event) {
- spin_lock_irq(&dev->event_lock);
- drm_send_vblank_event(dev, pipe, event);
- spin_unlock_irq(&dev->event_lock);
- }
- }
+ kfree(work);
return ret;
}
@@ -13588,33 +13732,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.atomic_destroy_state = intel_crtc_destroy_state,
};
-static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
-{
- struct reservation_object *resv;
-
-
- if (!obj->base.dma_buf)
- return NULL;
-
- resv = obj->base.dma_buf->resv;
-
- /* For framebuffer backed by dmabuf, wait for fence */
- while (1) {
- struct fence *fence_excl, *ret = NULL;
-
- rcu_read_lock();
-
- fence_excl = rcu_dereference(resv->fence_excl);
- if (fence_excl)
- ret = fence_get_rcu(fence_excl);
-
- rcu_read_unlock();
-
- if (ret == fence_excl)
- return ret;
- }
-}
-
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -14876,7 +14993,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
/* Drop through - unsupported since execlist only. */
default:
/* Default just returns -ENODEV to indicate unsupported */
- dev_priv->display.queue_flip = intel_default_queue_flip;
+ break;
}
}
@@ -15832,9 +15949,9 @@ void intel_modeset_gem_init(struct drm_device *dev)
DRM_ERROR("failed to pin boot fb on pipe %d\n",
to_intel_crtc(c)->pipe);
drm_framebuffer_unreference(c->primary->fb);
- c->primary->fb = NULL;
+ drm_framebuffer_unreference(c->primary->state->fb);
+ c->primary->fb = c->primary->state->fb = NULL;
c->primary->crtc = c->primary->state->crtc = NULL;
- update_state_fb(c->primary);
c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
}
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 697cf685858d..b494824874bd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -933,9 +933,6 @@ struct intel_flip_work {
struct work_struct unpin_work;
struct work_struct mmio_work;
- struct drm_crtc *crtc;
- struct drm_framebuffer *old_fb;
- struct drm_i915_gem_object *pending_flip_obj;
struct drm_pending_vblank_event *event;
atomic_t pending;
#define INTEL_FLIP_INACTIVE 0
@@ -945,6 +942,16 @@ struct intel_flip_work {
struct drm_i915_gem_request *flip_queued_req;
u32 flip_queued_vblank;
u32 flip_ready_vblank;
+
+ unsigned put_power_domains;
+ unsigned num_planes;
+
+ bool can_async_unpin, flip_prepared;
+ unsigned fb_bits;
+
+ struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+ struct intel_plane_state *old_plane_state[I915_MAX_PLANES + 1];
+ struct intel_plane_state *new_plane_state[I915_MAX_PLANES + 1];
};
struct intel_load_detect_pipe {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip.
2016-03-23 13:24 ` [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip Maarten Lankhorst
@ 2016-03-23 15:06 ` Ville Syrjälä
0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-23 15:06 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, Mar 23, 2016 at 02:24:28PM +0100, Maarten Lankhorst wrote:
> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
> is no longer useful.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 3 --
> drivers/gpu/drm/i915/i915_irq.c | 18 ++-----
> drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 -
> 4 files changed, 40 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 310043d73bf7..4a0078c17cd1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -588,9 +588,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
> 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 {
> seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
> pipe, plane);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a55a7cc317f8..9d4089f6c91b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
> intel_pipe_handle_vblank(dev, pipe))
> intel_check_page_flip(dev, pipe);
>
> - if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
> - intel_prepare_page_flip(dev, pipe);
> + if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
> intel_finish_page_flip(dev, pipe);
> - }
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> i9xx_pipe_crc_irq_handler(dev, pipe);
>
> /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE(pipe))
> intel_finish_page_flip_plane(dev, pipe);
> - }
> }
>
> /* check event from PCH */
> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> intel_check_page_flip(dev, pipe);
>
> /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
> intel_finish_page_flip_plane(dev, pipe);
> - }
> }
>
> /* check event from PCH */
> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> else
> flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>
> - if (flip_done) {
> - intel_prepare_page_flip(dev, pipe);
> + if (flip_done)
> intel_finish_page_flip_plane(dev, pipe);
> - }
>
> if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
> hsw_pipe_crc_irq_handler(dev, pipe);
> @@ -3984,7 +3976,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
> if (I915_READ16(ISR) & flip_pending)
> goto check_page_flip;
>
> - intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip(dev, pipe);
> return true;
>
> @@ -4175,7 +4166,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
> if (I915_READ(ISR) & flip_pending)
> goto check_page_flip;
>
> - intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip(dev, pipe);
> return true;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6303438df9f2..c15c08dc251c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3138,7 +3138,6 @@ static void intel_complete_page_flips(struct drm_device *dev)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum plane plane = intel_crtc->plane;
>
> - intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip_plane(dev, plane);
> }
> }
> @@ -10825,53 +10824,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
> kfree(work);
> }
>
> -static void do_intel_finish_page_flip(struct drm_device *dev,
> - struct drm_crtc *crtc)
> -{
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_unpin_work *work;
> - unsigned long flags;
> -
> - /* Ignore early vblank irqs */
> - if (intel_crtc == NULL)
> - return;
> -
> - /*
> - * This is called both by irq handlers and the reset code (to complete
> - * lost pageflips) so needs the full irqsave spinlocks.
> - */
> - spin_lock_irqsave(&dev->event_lock, flags);
> - work = intel_crtc->unpin_work;
> -
> - /* Ensure we don't miss a work->pending update ... */
> - smp_rmb();
> -
> - if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - return;
> - }
> -
> - page_flip_completed(intel_crtc);
> -
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
> -void intel_finish_page_flip(struct drm_device *dev, int pipe)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> -
> - do_intel_finish_page_flip(dev, crtc);
> -}
> -
> -void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> -
> - do_intel_finish_page_flip(dev, crtc);
> -}
> -
> /* Is 'a' after or equal to 'b'? */
> static bool g4x_flip_count_after_eq(u32 a, u32 b)
> {
> @@ -10924,28 +10876,52 @@ static bool page_flip_finished(struct intel_crtc *crtc)
> crtc->unpin_work->flip_count);
> }
>
> -void intel_prepare_page_flip(struct drm_device *dev, int plane)
> +static void do_intel_finish_page_flip(struct drm_device *dev,
> + struct drm_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc =
> - to_intel_crtc(dev_priv->plane_to_crtc_mapping[plane]);
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_unpin_work *work;
> unsigned long flags;
>
> + /* Ignore early vblank irqs */
> + if (intel_crtc == NULL)
> + return;
>
> /*
> * This is called both by irq handlers and the reset code (to complete
> * lost pageflips) so needs the full irqsave spinlocks.
> - *
> - * NB: An MMIO update of the plane base pointer will also
> - * generate a page-flip completion irq, i.e. every modeset
> - * is also accompanied by a spurious intel_prepare_page_flip().
> */
> spin_lock_irqsave(&dev->event_lock, flags);
> - if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
> - atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
> + work = intel_crtc->unpin_work;
> +
> + if (work == NULL ||
> + atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> + !page_flip_finished(intel_crtc)) {
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + return;
> + }
> +
> + page_flip_completed(intel_crtc);
> +
> spin_unlock_irqrestore(&dev->event_lock, flags);
> }
>
> +void intel_finish_page_flip(struct drm_device *dev, int pipe)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +
> + do_intel_finish_page_flip(dev, crtc);
> +}
> +
> +void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> +
> + do_intel_finish_page_flip(dev, crtc);
> +}
I don't think you need the _plane() variant anywhere actually, so might
as well kill it.
And since this patch is non-controversial I'd move it to the head of the
series.
> +
> static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
> {
> /* Ensure that the work item is consistent when activating it ... */
> @@ -11395,8 +11371,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
> u32 pending;
>
> pending = atomic_read(&work->pending);
> - if (pending != INTEL_FLIP_PENDING)
> - return pending == INTEL_FLIP_COMPLETE;
> + if (pending == INTEL_FLIP_INACTIVE)
> + return false;
>
> if (work->flip_ready_vblank == 0) {
> if (work->flip_queued_req &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7f0779783851..e26293ead94f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -944,7 +944,6 @@ struct intel_unpin_work {
> atomic_t pending;
> #define INTEL_FLIP_INACTIVE 0
> #define INTEL_FLIP_PENDING 1
> -#define INTEL_FLIP_COMPLETE 2
> u32 flip_count;
> u32 gtt_offset;
> struct drm_i915_gem_request *flip_queued_req;
> @@ -1155,7 +1154,6 @@ struct drm_framebuffer *
> __intel_framebuffer_create(struct drm_device *dev,
> struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_i915_gem_object *obj);
> -void intel_prepare_page_flip(struct drm_device *dev, int plane);
> void intel_finish_page_flip(struct drm_device *dev, int pipe);
> void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> void intel_check_page_flip(struct drm_device *dev, int pipe);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-23 13:24 ` [PATCH 4/6] drm/i915: Allow mmio updates on all platforms Maarten Lankhorst
@ 2016-03-23 15:07 ` Ville Syrjälä
2016-03-24 8:35 ` Maarten Lankhorst
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-23 15:07 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> and unpinning. Use flip_queued_req to hold the wait request in the
> mmio case and allow the vblank interrupt to complete mmio work to
> have mmio flips run correctly on g4 and earlier.
Before you actually go and trust drm_vblank_count() you should make it
race free.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 271 +++++++++++------------------------
> drivers/gpu/drm/i915/intel_drv.h | 16 +--
> 3 files changed, 87 insertions(+), 204 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4a0078c17cd1..bc2ae70abcea 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -573,10 +573,10 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
> for_each_intel_crtc(dev, crtc) {
> const char pipe = pipe_name(crtc->pipe);
> const char plane = plane_name(crtc->plane);
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
>
> spin_lock_irq(&dev->event_lock);
> - work = crtc->unpin_work;
> + work = crtc->flip_work;
> if (work == NULL) {
> seq_printf(m, "No flip due on pipe %c (plane %c)\n",
> pipe, plane);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c52eb1bb4f24..3b082020aab8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -47,6 +47,11 @@
> #include <linux/reservation.h>
> #include <linux/dma-buf.h>
>
> +static bool is_mmio_work(struct intel_flip_work *work)
> +{
> + return work->mmio_work.func;
> +}
> +
> /* Primary plane formats for gen <= 3 */
> static const uint32_t i8xx_primary_formats[] = {
> DRM_FORMAT_C8,
> @@ -3243,7 +3248,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> return false;
>
> spin_lock_irq(&dev->event_lock);
> - pending = to_intel_crtc(crtc)->unpin_work != NULL;
> + pending = to_intel_crtc(crtc)->flip_work != NULL;
> spin_unlock_irq(&dev->event_lock);
>
> return pending;
> @@ -3825,7 +3830,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
> if (atomic_read(&crtc->unpin_work_count) == 0)
> continue;
>
> - if (crtc->unpin_work)
> + if (crtc->flip_work)
> intel_wait_for_vblank(dev, crtc->pipe);
>
> return true;
> @@ -3837,11 +3842,11 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
> static void page_flip_completed(struct intel_crtc *intel_crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> - struct intel_unpin_work *work = intel_crtc->unpin_work;
> + struct intel_flip_work *work = intel_crtc->flip_work;
>
> - /* ensure that the unpin work is consistent wrt ->pending. */
> + /* ensure that the flip work is consistent wrt ->pending. */
> smp_rmb();
> - intel_crtc->unpin_work = NULL;
> + intel_crtc->flip_work = NULL;
>
> if (work->event)
> drm_send_vblank_event(intel_crtc->base.dev,
> @@ -3851,7 +3856,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc)
> drm_crtc_vblank_put(&intel_crtc->base);
>
> wake_up_all(&dev_priv->pending_flip_queue);
> - queue_work(dev_priv->wq, &work->work);
> + queue_work(dev_priv->wq, &work->unpin_work);
>
> trace_i915_flip_complete(intel_crtc->plane,
> work->pending_flip_obj);
> @@ -3875,9 +3880,11 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>
> if (ret == 0) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_flip_work *work;
>
> spin_lock_irq(&dev->event_lock);
> - if (intel_crtc->unpin_work) {
> + work = intel_crtc->flip_work;
> + if (work && !is_mmio_work(work)) {
> WARN_ONCE(1, "Removing stuck page flip\n");
> page_flip_completed(intel_crtc);
> }
> @@ -6259,7 +6266,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->unpin_work);
> + WARN_ON(intel_crtc->flip_work);
>
> intel_pre_disable_primary_noatomic(crtc);
>
> @@ -10781,15 +10788,16 @@ 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_unpin_work *work;
> + struct intel_flip_work *work;
>
> spin_lock_irq(&dev->event_lock);
> - work = intel_crtc->unpin_work;
> - intel_crtc->unpin_work = NULL;
> + work = intel_crtc->flip_work;
> + intel_crtc->flip_work = NULL;
> spin_unlock_irq(&dev->event_lock);
>
> if (work) {
> - cancel_work_sync(&work->work);
> + cancel_work_sync(&work->mmio_work);
> + cancel_work_sync(&work->unpin_work);
> kfree(work);
> }
>
> @@ -10800,12 +10808,15 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
>
> static void intel_unpin_work_fn(struct work_struct *__work)
> {
> - struct intel_unpin_work *work =
> - container_of(__work, struct intel_unpin_work, work);
> + struct intel_flip_work *work =
> + container_of(__work, struct intel_flip_work, unpin_work);
> struct intel_crtc *crtc = to_intel_crtc(work->crtc);
> struct drm_device *dev = crtc->base.dev;
> struct drm_plane *primary = crtc->base.primary;
>
> + if (is_mmio_work(work))
> + flush_work(&work->mmio_work);
> +
> mutex_lock(&dev->struct_mutex);
> intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
> drm_gem_object_unreference(&work->pending_flip_obj->base);
> @@ -10871,16 +10882,16 @@ static bool page_flip_finished(struct intel_crtc *crtc)
> * anyway, we don't really care.
> */
> return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
> - crtc->unpin_work->gtt_offset &&
> + crtc->flip_work->gtt_offset &&
> g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
> - crtc->unpin_work->flip_count);
> + crtc->flip_work->flip_count);
> }
>
> static void do_intel_finish_page_flip(struct drm_device *dev,
> struct drm_crtc *crtc)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
> unsigned long flags;
>
> /* Ignore early vblank irqs */
> @@ -10892,11 +10903,11 @@ 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->unpin_work;
> + work = intel_crtc->flip_work;
>
> if (work == NULL ||
> atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> - !page_flip_finished(intel_crtc)) {
> + !page_flip_finished(intel_crtc)) {
> spin_unlock_irqrestore(&dev->event_lock, flags);
> return;
> }
> @@ -10922,7 +10933,7 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> do_intel_finish_page_flip(dev, crtc);
> }
>
> -static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
> +static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
> {
> /* Ensure that the work item is consistent when activating it ... */
> smp_wmb();
> @@ -10959,10 +10970,9 @@ 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->unpin_work->gtt_offset);
> + intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
> intel_ring_emit(engine, 0); /* aux display base address, unused */
>
> - intel_mark_page_flip_active(intel_crtc->unpin_work);
> return 0;
> }
>
> @@ -10991,10 +11001,9 @@ 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->unpin_work->gtt_offset);
> + intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
> intel_ring_emit(engine, MI_NOOP);
>
> - intel_mark_page_flip_active(intel_crtc->unpin_work);
> return 0;
> }
>
> @@ -11022,7 +11031,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->unpin_work->gtt_offset |
> + intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
> obj->tiling_mode);
>
> /* XXX Enabling the panel-fitter across page-flip is so far
> @@ -11033,7 +11042,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
> pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
> intel_ring_emit(engine, pf | pipesrc);
>
> - intel_mark_page_flip_active(intel_crtc->unpin_work);
> return 0;
> }
>
> @@ -11057,7 +11065,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->unpin_work->gtt_offset);
> + intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>
> /* Contrary to the suggestions in the documentation,
> * "Enable Panel Fitter" does not seem to be required when page
> @@ -11069,7 +11077,6 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
> pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
> intel_ring_emit(engine, pf | pipesrc);
>
> - intel_mark_page_flip_active(intel_crtc->unpin_work);
> return 0;
> }
>
> @@ -11161,10 +11168,9 @@ 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->unpin_work->gtt_offset);
> + intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
> intel_ring_emit(engine, (MI_NOOP));
>
> - intel_mark_page_flip_active(intel_crtc->unpin_work);
> return 0;
> }
>
> @@ -11182,9 +11188,6 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
> if (engine == NULL)
> return true;
>
> - if (INTEL_INFO(engine->dev)->gen < 5)
> - return false;
> -
> if (i915.use_mmio_flip < 0)
> return false;
> else if (i915.use_mmio_flip > 0)
> @@ -11199,126 +11202,21 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
> return engine != i915_gem_request_get_engine(obj->last_write_req);
> }
>
> -static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> - unsigned int rotation,
> - struct intel_unpin_work *work)
> -{
> - struct drm_device *dev = intel_crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> - const enum pipe pipe = intel_crtc->pipe;
> - u32 ctl, stride, tile_height;
> -
> - ctl = I915_READ(PLANE_CTL(pipe, 0));
> - ctl &= ~PLANE_CTL_TILED_MASK;
> - switch (fb->modifier[0]) {
> - case DRM_FORMAT_MOD_NONE:
> - break;
> - case I915_FORMAT_MOD_X_TILED:
> - ctl |= PLANE_CTL_TILED_X;
> - break;
> - case I915_FORMAT_MOD_Y_TILED:
> - ctl |= PLANE_CTL_TILED_Y;
> - break;
> - case I915_FORMAT_MOD_Yf_TILED:
> - ctl |= PLANE_CTL_TILED_YF;
> - break;
> - default:
> - MISSING_CASE(fb->modifier[0]);
> - }
> -
> - /*
> - * The stride is either expressed as a multiple of 64 bytes chunks for
> - * linear buffers or in number of tiles for tiled buffers.
> - */
> - if (intel_rotation_90_or_270(rotation)) {
> - /* stride = Surface height in tiles */
> - tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
> - stride = DIV_ROUND_UP(fb->height, tile_height);
> - } else {
> - stride = fb->pitches[0] /
> - intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> - fb->pixel_format);
> - }
> -
> - /*
> - * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> - * PLANE_SURF updates, the update is then guaranteed to be atomic.
> - */
> - I915_WRITE(PLANE_CTL(pipe, 0), ctl);
> - I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> -
> - I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
> - POSTING_READ(PLANE_SURF(pipe, 0));
> -}
> -
> -static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
> - struct intel_unpin_work *work)
> +static void intel_mmio_flip_work_func(struct work_struct *w)
> {
> - struct drm_device *dev = intel_crtc->base.dev;
> + struct intel_flip_work *work =
> + container_of(w, struct intel_flip_work, mmio_work);
> + struct intel_crtc *crtc = to_intel_crtc(work->crtc);
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_framebuffer *intel_fb =
> - to_intel_framebuffer(intel_crtc->base.primary->fb);
> - struct drm_i915_gem_object *obj = intel_fb->obj;
> - i915_reg_t reg = DSPCNTR(intel_crtc->plane);
> - u32 dspcntr;
> -
> - dspcntr = I915_READ(reg);
> -
> - if (obj->tiling_mode != I915_TILING_NONE)
> - dspcntr |= DISPPLANE_TILED;
> - else
> - dspcntr &= ~DISPPLANE_TILED;
> -
> - I915_WRITE(reg, dspcntr);
> + struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> + struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
>
> - I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
> - POSTING_READ(DSPSURF(intel_crtc->plane));
> -}
> -
> -/*
> - * XXX: This is the temporary way to update the plane registers until we get
> - * around to using the usual plane update functions for MMIO flips
> - */
> -static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> -{
> - struct intel_crtc *crtc = mmio_flip->crtc;
> - struct intel_unpin_work *work;
> -
> - spin_lock_irq(&crtc->base.dev->event_lock);
> - work = crtc->unpin_work;
> - spin_unlock_irq(&crtc->base.dev->event_lock);
> - if (work == NULL)
> - return;
> -
> - intel_pipe_update_start(crtc);
> -
> - if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> - skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> - else
> - /* use_mmio_flip() retricts MMIO flips to ilk+ */
> - ilk_do_mmio_flip(crtc, work);
> -
> - intel_pipe_update_end(crtc);
> -
> - intel_mark_page_flip_active(work);
> -}
> -
> -static void intel_mmio_flip_work_func(struct work_struct *work)
> -{
> - struct intel_mmio_flip *mmio_flip =
> - container_of(work, struct intel_mmio_flip, work);
> - struct intel_framebuffer *intel_fb =
> - to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
> - struct drm_i915_gem_object *obj = intel_fb->obj;
> -
> - if (mmio_flip->req) {
> - WARN_ON(__i915_wait_request(mmio_flip->req,
> - mmio_flip->crtc->reset_counter,
> + if (work->flip_queued_req)
> + WARN_ON(__i915_wait_request(work->flip_queued_req,
> + crtc->reset_counter,
> false, NULL,
> - &mmio_flip->i915->rps.mmioflips));
> - i915_gem_request_unreference__unlocked(mmio_flip->req);
> - }
> + &dev_priv->rps.mmioflips));
>
> /* For framebuffer backed by dmabuf, wait for fence */
> if (obj->base.dma_buf)
> @@ -11326,29 +11224,12 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
> false, false,
> MAX_SCHEDULE_TIMEOUT) < 0);
>
> - intel_do_mmio_flip(mmio_flip);
> - kfree(mmio_flip);
> -}
> -
> -static int intel_queue_mmio_flip(struct drm_device *dev,
> - struct drm_crtc *crtc,
> - struct drm_i915_gem_object *obj)
> -{
> - struct intel_mmio_flip *mmio_flip;
> -
> - mmio_flip = kmalloc(sizeof(*mmio_flip), GFP_KERNEL);
> - if (mmio_flip == NULL)
> - return -ENOMEM;
> -
> - mmio_flip->i915 = to_i915(dev);
> - mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> - mmio_flip->crtc = to_intel_crtc(crtc);
> - mmio_flip->rotation = crtc->primary->state->rotation;
> -
> - INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
> - schedule_work(&mmio_flip->work);
> -
> - return 0;
> + intel_pipe_update_start(crtc);
> + primary->update_plane(&primary->base,
> + crtc->config,
> + to_intel_plane_state(primary->base.state));
> + atomic_set(&work->pending, INTEL_FLIP_PENDING);
> + intel_pipe_update_end(crtc);
> }
>
> static int intel_default_queue_flip(struct drm_device *dev,
> @@ -11366,7 +11247,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_unpin_work *work = intel_crtc->unpin_work;
> + struct intel_flip_work *work = intel_crtc->flip_work;
> u32 addr;
> u32 pending;
>
> @@ -11374,6 +11255,13 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
> if (pending == INTEL_FLIP_INACTIVE)
> return false;
>
> + /*
> + * If this is a mmio flip return true because state
> + * is changed to INTEL_FLIP_PENDING after updating.
> + */
> + if (is_mmio_work(work))
> + return true;
> +
> if (work->flip_ready_vblank == 0) {
> if (work->flip_queued_req &&
> !i915_gem_request_completed(work->flip_queued_req, true))
> @@ -11404,7 +11292,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
>
> WARN_ON(!in_interrupt());
>
> @@ -11412,14 +11300,15 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
> return;
>
> spin_lock(&dev->event_lock);
> - work = intel_crtc->unpin_work;
> + work = intel_crtc->flip_work;
> if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
> - WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
> + WARN_ONCE(!is_mmio_work(work),
> + "Kicking stuck page flip: queued at %d, now %d\n",
> work->flip_queued_vblank, drm_vblank_count(dev, pipe));
> page_flip_completed(intel_crtc);
> work = NULL;
> }
> - if (work != NULL &&
> + if (work != NULL && !is_mmio_work(work) &&
> drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
> intel_queue_rps_boost_for_request(dev, work->flip_queued_req);
> spin_unlock(&dev->event_lock);
> @@ -11437,7 +11326,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_plane *primary = crtc->primary;
> enum pipe pipe = intel_crtc->pipe;
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
> struct intel_engine_cs *engine;
> bool mmio_flip;
> struct drm_i915_gem_request *request = NULL;
> @@ -11474,19 +11363,20 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->event = event;
> work->crtc = crtc;
> work->old_fb = old_fb;
> - INIT_WORK(&work->work, intel_unpin_work_fn);
> + INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
>
> ret = drm_crtc_vblank_get(crtc);
> if (ret)
> goto free_work;
>
> - /* We borrow the event spin lock for protecting unpin_work */
> + /* We borrow the event spin lock for protecting flip_work */
> spin_lock_irq(&dev->event_lock);
> - if (intel_crtc->unpin_work) {
> + if (intel_crtc->flip_work) {
> /* Before declaring the flip queue wedged, check if
> * the hardware completed the operation behind our backs.
> */
> - if (__intel_pageflip_stall_check(dev, crtc)) {
> + if (!is_mmio_work(intel_crtc->flip_work) &&
> + __intel_pageflip_stall_check(dev, crtc)) {
> DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
> page_flip_completed(intel_crtc);
> } else {
> @@ -11498,7 +11388,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> return -EBUSY;
> }
> }
> - intel_crtc->unpin_work = work;
> + intel_crtc->flip_work = work;
> spin_unlock_irq(&dev->event_lock);
>
> if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
> @@ -11567,23 +11457,22 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>
> if (mmio_flip) {
> - ret = intel_queue_mmio_flip(dev, crtc, obj);
> - if (ret)
> - goto cleanup_unpin;
> + INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
>
> i915_gem_request_assign(&work->flip_queued_req,
> obj->last_write_req);
> +
> + schedule_work(&work->mmio_work);
> } else {
> + i915_gem_request_assign(&work->flip_queued_req, request);
> ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
> page_flip_flags);
> if (ret)
> goto cleanup_unpin;
>
> - i915_gem_request_assign(&work->flip_queued_req, request);
> - }
> -
> - if (request)
> + intel_mark_page_flip_active(work);
> i915_add_request_no_flush(request);
> + }
>
> work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>
> @@ -11613,7 +11502,7 @@ cleanup:
> drm_framebuffer_unreference(work->old_fb);
>
> spin_lock_irq(&dev->event_lock);
> - intel_crtc->unpin_work = NULL;
> + intel_crtc->flip_work = NULL;
> spin_unlock_irq(&dev->event_lock);
>
> drm_crtc_vblank_put(crtc);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e26293ead94f..cac368138764 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -592,14 +592,6 @@ struct vlv_wm_state {
> bool cxsr;
> };
>
> -struct intel_mmio_flip {
> - struct work_struct work;
> - struct drm_i915_private *i915;
> - struct drm_i915_gem_request *req;
> - struct intel_crtc *crtc;
> - unsigned int rotation;
> -};
> -
> struct intel_crtc {
> struct drm_crtc base;
> enum pipe pipe;
> @@ -614,7 +606,7 @@ struct intel_crtc {
> unsigned long enabled_power_domains;
> bool lowfreq_avail;
> struct intel_overlay *overlay;
> - struct intel_unpin_work *unpin_work;
> + struct intel_flip_work *flip_work;
>
> atomic_t unpin_work_count;
>
> @@ -935,8 +927,10 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
> return dev_priv->plane_to_crtc_mapping[plane];
> }
>
> -struct intel_unpin_work {
> - struct work_struct work;
> +struct intel_flip_work {
> + struct work_struct unpin_work;
> + struct work_struct mmio_work;
> +
> struct drm_crtc *crtc;
> struct drm_framebuffer *old_fb;
> struct drm_i915_gem_object *pending_flip_obj;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-23 15:07 ` Ville Syrjälä
@ 2016-03-24 8:35 ` Maarten Lankhorst
2016-03-24 14:26 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-24 8:35 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 23-03-16 om 16:07 schreef Ville Syrjälä:
> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>> and unpinning. Use flip_queued_req to hold the wait request in the
>> mmio case and allow the vblank interrupt to complete mmio work to
>> have mmio flips run correctly on g4 and earlier.
> Before you actually go and trust drm_vblank_count() you should make it
> race free.
How about adding the below to the patch?
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index befac649aa96..05ec832e02de 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11224,12 +11224,11 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
false, false,
MAX_SCHEDULE_TIMEOUT) < 0);
- intel_pipe_update_start(crtc);
+ intel_pipe_update_start(crtc, work);
primary->update_plane(&primary->base,
crtc->config,
to_intel_plane_state(primary->base.state));
- atomic_set(&work->pending, INTEL_FLIP_PENDING);
- intel_pipe_update_end(crtc);
+ intel_pipe_update_end(crtc, work);
}
static int intel_default_queue_flip(struct drm_device *dev,
@@ -13800,7 +13799,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
bool modeset = needs_modeset(crtc->state);
/* Perform vblank evasion around commit operation */
- intel_pipe_update_start(intel_crtc);
+ intel_pipe_update_start(intel_crtc, NULL);
if (modeset)
return;
@@ -13816,7 +13815,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- intel_pipe_update_end(intel_crtc);
+ intel_pipe_update_end(intel_crtc, NULL);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cac368138764..86d486cfd778 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1604,8 +1604,8 @@ bool intel_sdvo_init(struct drm_device *dev,
int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-void intel_pipe_update_start(struct intel_crtc *crtc);
-void intel_pipe_update_end(struct intel_crtc *crtc);
+void intel_pipe_update_start(struct intel_crtc *crtc, struct intel_flip_work *work);
+void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work);
/* intel_tv.c */
void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 8821533561b1..8da59a1eca56 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -78,7 +78,8 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
* avoid random delays. The value written to @start_vbl_count should be
* supplied to intel_pipe_update_end() for error checking.
*/
-void intel_pipe_update_start(struct intel_crtc *crtc)
+void intel_pipe_update_start(struct intel_crtc *crtc,
+ struct intel_flip_work *work)
{
struct drm_device *dev = crtc->base.dev;
const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
@@ -142,6 +143,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
crtc->debug.start_vbl_count =
dev->driver->get_vblank_counter(dev, pipe);
+ if (work)
+ work->flip_queued_vblank = crtc->debug.start_vbl_count;
+
trace_i915_pipe_update_vblank_evaded(crtc);
}
@@ -154,7 +158,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
* re-enables interrupts and verifies the update was actually completed
* before a vblank using the value of @start_vbl_count.
*/
-void intel_pipe_update_end(struct intel_crtc *crtc)
+void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work)
{
struct drm_device *dev = crtc->base.dev;
enum pipe pipe = crtc->pipe;
@@ -162,6 +166,12 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
ktime_t end_vbl_time = ktime_get();
+ if (work) {
+ work->flip_queued_vblank = end_vbl_count;
+ smp_mb__before_atomic();
+ atomic_set(&work->pending, INTEL_FLIP_PENDING);
+ }
+
trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
local_irq_enable();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 8:35 ` Maarten Lankhorst
@ 2016-03-24 14:26 ` Ville Syrjälä
2016-03-24 14:42 ` Maarten Lankhorst
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-24 14:26 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
> > On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
> >> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> >> and unpinning. Use flip_queued_req to hold the wait request in the
> >> mmio case and allow the vblank interrupt to complete mmio work to
> >> have mmio flips run correctly on g4 and earlier.
> > Before you actually go and trust drm_vblank_count() you should make it
> > race free.
>
> How about adding the below to the patch?
You can't just mix the hw and sw counter. Using the hw counter would be
neat because it doesn't require so much work to be race-free, but that
leaves out gen2 which I don't like. My atomic code used the hw counter
though, but I had plans to fall back to the sw counter on gen2
eventually.
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index befac649aa96..05ec832e02de 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11224,12 +11224,11 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
> false, false,
> MAX_SCHEDULE_TIMEOUT) < 0);
>
> - intel_pipe_update_start(crtc);
> + intel_pipe_update_start(crtc, work);
> primary->update_plane(&primary->base,
> crtc->config,
> to_intel_plane_state(primary->base.state));
> - atomic_set(&work->pending, INTEL_FLIP_PENDING);
> - intel_pipe_update_end(crtc);
> + intel_pipe_update_end(crtc, work);
> }
>
> static int intel_default_queue_flip(struct drm_device *dev,
> @@ -13800,7 +13799,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
> bool modeset = needs_modeset(crtc->state);
>
> /* Perform vblank evasion around commit operation */
> - intel_pipe_update_start(intel_crtc);
> + intel_pipe_update_start(intel_crtc, NULL);
>
> if (modeset)
> return;
> @@ -13816,7 +13815,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> - intel_pipe_update_end(intel_crtc);
> + intel_pipe_update_end(intel_crtc, NULL);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cac368138764..86d486cfd778 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1604,8 +1604,8 @@ bool intel_sdvo_init(struct drm_device *dev,
> int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
> int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> -void intel_pipe_update_start(struct intel_crtc *crtc);
> -void intel_pipe_update_end(struct intel_crtc *crtc);
> +void intel_pipe_update_start(struct intel_crtc *crtc, struct intel_flip_work *work);
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work);
>
> /* intel_tv.c */
> void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 8821533561b1..8da59a1eca56 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -78,7 +78,8 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> * avoid random delays. The value written to @start_vbl_count should be
> * supplied to intel_pipe_update_end() for error checking.
> */
> -void intel_pipe_update_start(struct intel_crtc *crtc)
> +void intel_pipe_update_start(struct intel_crtc *crtc,
> + struct intel_flip_work *work)
> {
> struct drm_device *dev = crtc->base.dev;
> const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> @@ -142,6 +143,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
> crtc->debug.start_vbl_count =
> dev->driver->get_vblank_counter(dev, pipe);
>
> + if (work)
> + work->flip_queued_vblank = crtc->debug.start_vbl_count;
> +
> trace_i915_pipe_update_vblank_evaded(crtc);
> }
>
> @@ -154,7 +158,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
> * re-enables interrupts and verifies the update was actually completed
> * before a vblank using the value of @start_vbl_count.
> */
> -void intel_pipe_update_end(struct intel_crtc *crtc)
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work)
> {
> struct drm_device *dev = crtc->base.dev;
> enum pipe pipe = crtc->pipe;
> @@ -162,6 +166,12 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
> u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
> ktime_t end_vbl_time = ktime_get();
>
> + if (work) {
> + work->flip_queued_vblank = end_vbl_count;
> + smp_mb__before_atomic();
> + atomic_set(&work->pending, INTEL_FLIP_PENDING);
> + }
> +
> trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
>
> local_irq_enable();
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 14:26 ` Ville Syrjälä
@ 2016-03-24 14:42 ` Maarten Lankhorst
2016-03-24 14:48 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-24 14:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 24-03-16 om 15:26 schreef Ville Syrjälä:
> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>>>> and unpinning. Use flip_queued_req to hold the wait request in the
>>>> mmio case and allow the vblank interrupt to complete mmio work to
>>>> have mmio flips run correctly on g4 and earlier.
>>> Before you actually go and trust drm_vblank_count() you should make it
>>> race free.
>> How about adding the below to the patch?
> You can't just mix the hw and sw counter. Using the hw counter would be
> neat because it doesn't require so much work to be race-free, but that
> leaves out gen2 which I don't like. My atomic code used the hw counter
> though, but I had plans to fall back to the sw counter on gen2
> eventually.
>
So I dug in a little bit more..
MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
The only thing it's used for is for cs flips, if more than 1 vblank passed since submission, the request gets a rps boost.
If more than 3 vblanks have passed after the request the cs flip waits on completed, the cs page flip is considered stuck
and will be forcefully completed.
With stale values the worst case rps boost might get applied right away, or the cs flip is forcefully completed after only 2 vblanks.
Neither affects mmio flips, so ignore this extra hunk and look at the original patch. :)
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 14:42 ` Maarten Lankhorst
@ 2016-03-24 14:48 ` Ville Syrjälä
2016-03-24 15:19 ` Maarten Lankhorst
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-24 14:48 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
> > On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
> >> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
> >>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
> >>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> >>>> and unpinning. Use flip_queued_req to hold the wait request in the
> >>>> mmio case and allow the vblank interrupt to complete mmio work to
> >>>> have mmio flips run correctly on g4 and earlier.
> >>> Before you actually go and trust drm_vblank_count() you should make it
> >>> race free.
> >> How about adding the below to the patch?
> > You can't just mix the hw and sw counter. Using the hw counter would be
> > neat because it doesn't require so much work to be race-free, but that
> > leaves out gen2 which I don't like. My atomic code used the hw counter
> > though, but I had plans to fall back to the sw counter on gen2
> > eventually.
> >
> So I dug in a little bit more..
>
> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
Yes they do if you want to use the vblank irq for completing them.
> The only thing it's used for is for cs flips, if more than 1 vblank passed since submission, the request gets a rps boost.
>
> If more than 3 vblanks have passed after the request the cs flip waits on completed, the cs page flip is considered stuck
> and will be forcefully completed.
>
> With stale values the worst case rps boost might get applied right away, or the cs flip is forcefully completed after only 2 vblanks.
> Neither affects mmio flips, so ignore this extra hunk and look at the original patch. :)
>
> ~Maarten
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 14:48 ` Ville Syrjälä
@ 2016-03-24 15:19 ` Maarten Lankhorst
2016-03-24 15:32 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-24 15:19 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 24-03-16 om 15:48 schreef Ville Syrjälä:
> On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
>> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
>>> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
>>>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
>>>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
>>>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>>>>>> and unpinning. Use flip_queued_req to hold the wait request in the
>>>>>> mmio case and allow the vblank interrupt to complete mmio work to
>>>>>> have mmio flips run correctly on g4 and earlier.
>>>>> Before you actually go and trust drm_vblank_count() you should make it
>>>>> race free.
>>>> How about adding the below to the patch?
>>> You can't just mix the hw and sw counter. Using the hw counter would be
>>> neat because it doesn't require so much work to be race-free, but that
>>> leaves out gen2 which I don't like. My atomic code used the hw counter
>>> though, but I had plans to fall back to the sw counter on gen2
>>> eventually.
>>>
>> So I dug in a little bit more..
>>
>> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
> Yes they do if you want to use the vblank irq for completing them.
>
Why? We only need a vblank irq, after the mmio updates are done we set an atomic that indicates mmio is done, which is checked in the vblank irq.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 15:19 ` Maarten Lankhorst
@ 2016-03-24 15:32 ` Ville Syrjälä
2016-03-29 8:03 ` Maarten Lankhorst
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-24 15:32 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Mar 24, 2016 at 04:19:13PM +0100, Maarten Lankhorst wrote:
> Op 24-03-16 om 15:48 schreef Ville Syrjälä:
> > On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
> >> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
> >>> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
> >>>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
> >>>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
> >>>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> >>>>>> and unpinning. Use flip_queued_req to hold the wait request in the
> >>>>>> mmio case and allow the vblank interrupt to complete mmio work to
> >>>>>> have mmio flips run correctly on g4 and earlier.
> >>>>> Before you actually go and trust drm_vblank_count() you should make it
> >>>>> race free.
> >>>> How about adding the below to the patch?
> >>> You can't just mix the hw and sw counter. Using the hw counter would be
> >>> neat because it doesn't require so much work to be race-free, but that
> >>> leaves out gen2 which I don't like. My atomic code used the hw counter
> >>> though, but I had plans to fall back to the sw counter on gen2
> >>> eventually.
> >>>
> >> So I dug in a little bit more..
> >>
> >> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
> > Yes they do if you want to use the vblank irq for completing them.
> >
> Why? We only need a vblank irq, after the mmio updates are done we set an atomic that indicates mmio is done, which is checked in the vblank irq.
You can race with the irq handler, especially on gen2/3 where the vblank
irq is actually the frame start irq that is delayed by at least ~1
scanline. So you absolutely need to know on which frame actually
submitted the flip so as not to complete it prematurely.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-24 15:32 ` Ville Syrjälä
@ 2016-03-29 8:03 ` Maarten Lankhorst
2016-03-30 13:04 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Maarten Lankhorst @ 2016-03-29 8:03 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 24-03-16 om 16:32 schreef Ville Syrjälä:
> On Thu, Mar 24, 2016 at 04:19:13PM +0100, Maarten Lankhorst wrote:
>> Op 24-03-16 om 15:48 schreef Ville Syrjälä:
>>> On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
>>>> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
>>>>> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
>>>>>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
>>>>>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
>>>>>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>>>>>>>> and unpinning. Use flip_queued_req to hold the wait request in the
>>>>>>>> mmio case and allow the vblank interrupt to complete mmio work to
>>>>>>>> have mmio flips run correctly on g4 and earlier.
>>>>>>> Before you actually go and trust drm_vblank_count() you should make it
>>>>>>> race free.
>>>>>> How about adding the below to the patch?
>>>>> You can't just mix the hw and sw counter. Using the hw counter would be
>>>>> neat because it doesn't require so much work to be race-free, but that
>>>>> leaves out gen2 which I don't like. My atomic code used the hw counter
>>>>> though, but I had plans to fall back to the sw counter on gen2
>>>>> eventually.
>>>>>
>>>> So I dug in a little bit more..
>>>>
>>>> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
>>> Yes they do if you want to use the vblank irq for completing them.
>>>
>> Why? We only need a vblank irq, after the mmio updates are done we set an atomic that indicates mmio is done, which is checked in the vblank irq.
> You can race with the irq handler, especially on gen2/3 where the vblank
> irq is actually the frame start irq that is delayed by at least ~1
> scanline. So you absolutely need to know on which frame actually
> submitted the flip so as not to complete it prematurely.
In that case won't you get a DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n"); anyway?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.
2016-03-29 8:03 ` Maarten Lankhorst
@ 2016-03-30 13:04 ` Ville Syrjälä
0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2016-03-30 13:04 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Tue, Mar 29, 2016 at 10:03:56AM +0200, Maarten Lankhorst wrote:
> Op 24-03-16 om 16:32 schreef Ville Syrjälä:
> > On Thu, Mar 24, 2016 at 04:19:13PM +0100, Maarten Lankhorst wrote:
> >> Op 24-03-16 om 15:48 schreef Ville Syrjälä:
> >>> On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
> >>>> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
> >>>>> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
> >>>>>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
> >>>>>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
> >>>>>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> >>>>>>>> and unpinning. Use flip_queued_req to hold the wait request in the
> >>>>>>>> mmio case and allow the vblank interrupt to complete mmio work to
> >>>>>>>> have mmio flips run correctly on g4 and earlier.
> >>>>>>> Before you actually go and trust drm_vblank_count() you should make it
> >>>>>>> race free.
> >>>>>> How about adding the below to the patch?
> >>>>> You can't just mix the hw and sw counter. Using the hw counter would be
> >>>>> neat because it doesn't require so much work to be race-free, but that
> >>>>> leaves out gen2 which I don't like. My atomic code used the hw counter
> >>>>> though, but I had plans to fall back to the sw counter on gen2
> >>>>> eventually.
> >>>>>
> >>>> So I dug in a little bit more..
> >>>>
> >>>> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
> >>> Yes they do if you want to use the vblank irq for completing them.
> >>>
> >> Why? We only need a vblank irq, after the mmio updates are done we set an atomic that indicates mmio is done, which is checked in the vblank irq.
> > You can race with the irq handler, especially on gen2/3 where the vblank
> > irq is actually the frame start irq that is delayed by at least ~1
> > scanline. So you absolutely need to know on which frame actually
> > submitted the flip so as not to complete it prematurely.
> In that case won't you get a DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n"); anyway?
Nope. The update start+end will happen within the same frame (unless we
messed up the vblank evade) so the hw counter will show the same value
on both sides. The irq handler isn't involved with that code at all.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-03-30 13:04 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-23 13:24 [PATCH 0/6] drm/i915: Rework page flip to be more atomic like Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 1/6] drm/i915: Remove stallcheck special handling Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 2/6] drm/i915: Remove intel_prepare_page_flip Maarten Lankhorst
2016-03-23 15:06 ` Ville Syrjälä
2016-03-23 13:24 ` [PATCH 3/6] drm/i915: Add the exclusive fence to plane_state Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 4/6] drm/i915: Allow mmio updates on all platforms Maarten Lankhorst
2016-03-23 15:07 ` Ville Syrjälä
2016-03-24 8:35 ` Maarten Lankhorst
2016-03-24 14:26 ` Ville Syrjälä
2016-03-24 14:42 ` Maarten Lankhorst
2016-03-24 14:48 ` Ville Syrjälä
2016-03-24 15:19 ` Maarten Lankhorst
2016-03-24 15:32 ` Ville Syrjälä
2016-03-29 8:03 ` Maarten Lankhorst
2016-03-30 13:04 ` Ville Syrjälä
2016-03-23 13:24 ` [PATCH 5/6] drm/i915: Convert flip_work to a list Maarten Lankhorst
2016-03-23 13:24 ` [PATCH 6/6] drm/i915: Rework intel_crtc_page_flip to be almost atomic Maarten Lankhorst
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.