Commit f0355c4a authored by Daniel Vetter's avatar Daniel Vetter

drm/i915: Add locking to psr code

It's not really optional to have locking ...

The ugly part is how much locking the psr work needs since it has to
recheck everything. Which is way too much. But we need to ditch the
psr work in it's current form anyway and implement proper frontbuffer
tracking.

The other nasty bit that had to go was the delayed work cancle in
psr_exit. Which means a bunch of races just became a bit more likely,
but mea culpa.

v2: Fixup HAS_PSR checks, resulting in uninitialized mutex issues.
Reviewed-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 109fc2ad
...@@ -660,6 +660,7 @@ struct i915_drrs { ...@@ -660,6 +660,7 @@ struct i915_drrs {
struct intel_dp; struct intel_dp;
struct i915_psr { struct i915_psr {
struct mutex lock;
bool sink_support; bool sink_support;
bool source_ok; bool source_ok;
struct intel_dp *enabled; struct intel_dp *enabled;
......
...@@ -1767,6 +1767,11 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) ...@@ -1767,6 +1767,11 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
struct drm_i915_gem_object *obj = intel_fb_obj(crtc->primary->fb); struct drm_i915_gem_object *obj = intel_fb_obj(crtc->primary->fb);
struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base; struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
lockdep_assert_held(&dev_priv->psr.lock);
lockdep_assert_held(&dev->struct_mutex);
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
dev_priv->psr.source_ok = false; dev_priv->psr.source_ok = false;
if (!HAS_PSR(dev)) { if (!HAS_PSR(dev)) {
...@@ -1836,6 +1841,7 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp) ...@@ -1836,6 +1841,7 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE); WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
WARN_ON(dev_priv->psr.active); WARN_ON(dev_priv->psr.active);
lockdep_assert_held(&dev_priv->psr.lock);
/* Enable PSR on the panel */ /* Enable PSR on the panel */
intel_edp_psr_enable_sink(intel_dp); intel_edp_psr_enable_sink(intel_dp);
...@@ -1862,8 +1868,10 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) ...@@ -1862,8 +1868,10 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
return; return;
} }
mutex_lock(&dev_priv->psr.lock);
if (dev_priv->psr.enabled) { if (dev_priv->psr.enabled) {
DRM_DEBUG_KMS("PSR already in use\n"); DRM_DEBUG_KMS("PSR already in use\n");
mutex_unlock(&dev_priv->psr.lock);
return; return;
} }
...@@ -1872,6 +1880,7 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) ...@@ -1872,6 +1880,7 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
if (intel_edp_psr_match_conditions(intel_dp)) if (intel_edp_psr_match_conditions(intel_dp))
intel_edp_psr_do_enable(intel_dp); intel_edp_psr_do_enable(intel_dp);
mutex_unlock(&dev_priv->psr.lock);
} }
void intel_edp_psr_disable(struct intel_dp *intel_dp) void intel_edp_psr_disable(struct intel_dp *intel_dp)
...@@ -1879,9 +1888,15 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) ...@@ -1879,9 +1888,15 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
if (!dev_priv->psr.enabled) if (!HAS_PSR(dev))
return; return;
mutex_lock(&dev_priv->psr.lock);
if (!dev_priv->psr.enabled) {
mutex_unlock(&dev_priv->psr.lock);
return;
}
if (dev_priv->psr.active) { if (dev_priv->psr.active) {
I915_WRITE(EDP_PSR_CTL(dev), I915_WRITE(EDP_PSR_CTL(dev),
I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE); I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE);
...@@ -1897,19 +1912,30 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) ...@@ -1897,19 +1912,30 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
} }
dev_priv->psr.enabled = NULL; dev_priv->psr.enabled = NULL;
mutex_unlock(&dev_priv->psr.lock);
} }
static void intel_edp_psr_work(struct work_struct *work) static void intel_edp_psr_work(struct work_struct *work)
{ {
struct drm_i915_private *dev_priv = struct drm_i915_private *dev_priv =
container_of(work, typeof(*dev_priv), psr.work.work); container_of(work, typeof(*dev_priv), psr.work.work);
struct drm_device *dev = dev_priv->dev;
struct intel_dp *intel_dp = dev_priv->psr.enabled; struct intel_dp *intel_dp = dev_priv->psr.enabled;
drm_modeset_lock_all(dev);
mutex_lock(&dev->struct_mutex);
mutex_lock(&dev_priv->psr.lock);
intel_dp = dev_priv->psr.enabled;
if (!intel_dp) if (!intel_dp)
return; goto unlock;
if (intel_edp_psr_match_conditions(intel_dp)) if (intel_edp_psr_match_conditions(intel_dp))
intel_edp_psr_do_enable(intel_dp); intel_edp_psr_do_enable(intel_dp);
unlock:
mutex_unlock(&dev_priv->psr.lock);
mutex_unlock(&dev->struct_mutex);
drm_modeset_unlock_all(dev);
} }
void intel_edp_psr_exit(struct drm_device *dev) void intel_edp_psr_exit(struct drm_device *dev)
...@@ -1922,8 +1948,7 @@ void intel_edp_psr_exit(struct drm_device *dev) ...@@ -1922,8 +1948,7 @@ void intel_edp_psr_exit(struct drm_device *dev)
if (!dev_priv->psr.enabled) if (!dev_priv->psr.enabled)
return; return;
cancel_delayed_work_sync(&dev_priv->psr.work); mutex_lock(&dev_priv->psr.lock);
if (dev_priv->psr.active) { if (dev_priv->psr.active) {
u32 val = I915_READ(EDP_PSR_CTL(dev)); u32 val = I915_READ(EDP_PSR_CTL(dev));
...@@ -1936,16 +1961,15 @@ void intel_edp_psr_exit(struct drm_device *dev) ...@@ -1936,16 +1961,15 @@ void intel_edp_psr_exit(struct drm_device *dev)
schedule_delayed_work(&dev_priv->psr.work, schedule_delayed_work(&dev_priv->psr.work,
msecs_to_jiffies(100)); msecs_to_jiffies(100));
mutex_unlock(&dev_priv->psr.lock);
} }
void intel_edp_psr_init(struct drm_device *dev) void intel_edp_psr_init(struct drm_device *dev)
{ {
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
if (!HAS_PSR(dev))
return;
INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work);
mutex_init(&dev_priv->psr.lock);
} }
static void intel_disable_dp(struct intel_encoder *encoder) static void intel_disable_dp(struct intel_encoder *encoder)
......
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