Commit 5e2d7afc authored by Daniel Vetter's avatar Daniel Vetter

drm/i915: Clarify event_lock locking, process context

It's good practice to use the more specific versions for irq save
spinlocks both as executable documentation and to enforce saner
design. The _irqsave version really should only be used if the calling
context is unknown and there's a good reason to call a function from
all kinds of places.

This is the first step whice replaces all occurances of _irqsave in
process context with the simpler irq disable/enable variants. We don't
have any funky spinlock nesting going on, especially since the
event_lock is the outermost of the irq/vblank related spinlocks.
Reviewed-by: default avatarJesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 4b3a9526
...@@ -516,7 +516,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) ...@@ -516,7 +516,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private; struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev; struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
struct intel_crtc *crtc; struct intel_crtc *crtc;
int ret; int ret;
...@@ -529,7 +528,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) ...@@ -529,7 +528,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
const char plane = plane_name(crtc->plane); const char plane = plane_name(crtc->plane);
struct intel_unpin_work *work; struct intel_unpin_work *work;
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
work = crtc->unpin_work; work = crtc->unpin_work;
if (work == NULL) { if (work == NULL) {
seq_printf(m, "No flip due on pipe %c (plane %c)\n", seq_printf(m, "No flip due on pipe %c (plane %c)\n",
...@@ -575,7 +574,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) ...@@ -575,7 +574,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
seq_printf(m, "MMIO update completed? %d\n", addr == work->gtt_offset); seq_printf(m, "MMIO update completed? %d\n", addr == work->gtt_offset);
} }
} }
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
} }
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
......
...@@ -2711,16 +2711,15 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) ...@@ -2711,16 +2711,15 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev; struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long flags;
bool pending; bool pending;
if (i915_reset_in_progress(&dev_priv->gpu_error) || if (i915_reset_in_progress(&dev_priv->gpu_error) ||
intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)) intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
return false; return false;
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
pending = to_intel_crtc(crtc)->unpin_work != NULL; pending = to_intel_crtc(crtc)->unpin_work != NULL;
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
return pending; return pending;
} }
...@@ -3431,14 +3430,13 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) ...@@ -3431,14 +3430,13 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
!intel_crtc_has_pending_flip(crtc), !intel_crtc_has_pending_flip(crtc),
60*HZ) == 0)) { 60*HZ) == 0)) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
if (intel_crtc->unpin_work) { if (intel_crtc->unpin_work) {
WARN_ONCE(1, "Removing stuck page flip\n"); WARN_ONCE(1, "Removing stuck page flip\n");
page_flip_completed(intel_crtc); page_flip_completed(intel_crtc);
} }
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
} }
if (crtc->primary->fb) { if (crtc->primary->fb) {
...@@ -9280,12 +9278,11 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) ...@@ -9280,12 +9278,11 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc->dev; struct drm_device *dev = crtc->dev;
struct intel_unpin_work *work; struct intel_unpin_work *work;
unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
work = intel_crtc->unpin_work; work = intel_crtc->unpin_work;
intel_crtc->unpin_work = NULL; intel_crtc->unpin_work = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
if (work) { if (work) {
cancel_work_sync(&work->work); cancel_work_sync(&work->work);
...@@ -9896,7 +9893,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -9896,7 +9893,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe; enum pipe pipe = intel_crtc->pipe;
struct intel_unpin_work *work; struct intel_unpin_work *work;
struct intel_engine_cs *ring; struct intel_engine_cs *ring;
unsigned long flags;
int ret; int ret;
//trigger software GT busyness calculation //trigger software GT busyness calculation
...@@ -9940,7 +9936,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -9940,7 +9936,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
goto free_work; goto free_work;
/* We borrow the event spin lock for protecting unpin_work */ /* We borrow the event spin lock for protecting unpin_work */
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
if (intel_crtc->unpin_work) { if (intel_crtc->unpin_work) {
/* Before declaring the flip queue wedged, check if /* Before declaring the flip queue wedged, check if
* the hardware completed the operation behind our backs. * the hardware completed the operation behind our backs.
...@@ -9950,7 +9946,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -9950,7 +9946,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
page_flip_completed(intel_crtc); page_flip_completed(intel_crtc);
} else { } else {
DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
drm_crtc_vblank_put(crtc); drm_crtc_vblank_put(crtc);
kfree(work); kfree(work);
...@@ -9958,7 +9954,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -9958,7 +9954,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
} }
} }
intel_crtc->unpin_work = work; intel_crtc->unpin_work = work;
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
if (atomic_read(&intel_crtc->unpin_work_count) >= 2) if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
flush_workqueue(dev_priv->wq); flush_workqueue(dev_priv->wq);
...@@ -10045,9 +10041,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -10045,9 +10041,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
cleanup: cleanup:
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
intel_crtc->unpin_work = NULL; intel_crtc->unpin_work = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
drm_crtc_vblank_put(crtc); drm_crtc_vblank_put(crtc);
free_work: free_work:
...@@ -10058,9 +10054,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ...@@ -10058,9 +10054,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
intel_crtc_wait_for_pending_flips(crtc); intel_crtc_wait_for_pending_flips(crtc);
ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
if (ret == 0 && event) { if (ret == 0 && event) {
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irq(&dev->event_lock);
drm_send_vblank_event(dev, pipe, event); drm_send_vblank_event(dev, pipe, event);
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irq(&dev->event_lock);
} }
} }
return ret; return ret;
...@@ -13769,9 +13765,8 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) ...@@ -13769,9 +13765,8 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file)
for_each_intel_crtc(dev, crtc) { for_each_intel_crtc(dev, crtc) {
struct intel_unpin_work *work; struct intel_unpin_work *work;
unsigned long irqflags;
spin_lock_irqsave(&dev->event_lock, irqflags); spin_lock_irq(&dev->event_lock);
work = crtc->unpin_work; work = crtc->unpin_work;
...@@ -13781,6 +13776,6 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) ...@@ -13781,6 +13776,6 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file)
work->event = NULL; work->event = NULL;
} }
spin_unlock_irqrestore(&dev->event_lock, irqflags); spin_unlock_irq(&dev->event_lock);
} }
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment