Skip to content

Commit

Permalink
drm/i915: Fix up fifo underrun tracking, take N
Browse files Browse the repository at this point in the history
So apparently this is tricky.

We need to consider:
- We start out with all the hw enabling bits disabled, both the
  individual fifo underrun interrupts and the shared display error
  interrupts masked. Otherwise if the bios config is broken we'll blow
  up with a NULL deref in our interrupt handler since the crtc
  structures aren't set up yet at driver load time.
- On gmch we need to mask fifo underruns on the sw side, so always
  need to set that in sanitize_crtc for those platforms.
- On other platforms we try to set the sw tracking so that it reflects
  the real state. But since a few platforms have shared bits we must
  _not_ disable fifo underrun reporting. Otherwise we'll never enable
  the shared error interrupt.

This is the state before out patch, but unfortunately this is not good
enough. But after a suspend resume operation this is broken:
1. We don't enable the hw interrupts since the same code runs on
resume as on driver load.
2. The fifo underrun state adjustments we do in sanitize_crtc doesn't
fire on resume since (except for hilarious firmware) all pipes are off
at that point. But they also don't hurt since the subsequent crtc
enabling due to force_restore will enable fifo underruns.

Which means when we enable fifo underrun reporting we notice that the
per-crtc state is already correct and short-circuit everthing out. And
the interrupt doesn't get enabled.

A similar problem would happen if the bios doesn't light up anything
when the driver loads. Which is exactly what happens when we reload
the driver since our unload functions disables all outputs.

Now we can't just rip out the short-circuit logic and unconditionally
update the fifo underrun reporting interrupt masking: We have some
checks for shared error interrupts to catch issues that happened when
the shared error interrupt was disabled.

The right fix is to push down this logic so that we can always update
the hardware state, but only check for missed fifo underruns on a real
enabled->disabled transition and ignore them when we're already
disabled.

On platforms with shared error interrupt the pipe CRC interrupts are
grouped together with the fifo underrun reporting this fixes pipe CRC
support after suspend and driver reloads.

Testcase: igt/kms_pipe_crc_basic/suspend-*
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  • Loading branch information
Daniel Vetter committed Jun 5, 2014
1 parent cace841 commit 2ae2a50
Showing 1 changed file with 19 additions and 25 deletions.
44 changes: 19 additions & 25 deletions drivers/gpu/drm/i915/i915_irq.c
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ void i9xx_check_fifo_underruns(struct drm_device *dev)
}

static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
enum pipe pipe, bool enable)
enum pipe pipe,
bool enable, bool old)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u32 reg = PIPESTAT(pipe);
Expand All @@ -347,7 +348,7 @@ static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
POSTING_READ(reg);
} else {
if (pipestat & PIPE_FIFO_UNDERRUN_STATUS)
if (old && pipestat & PIPE_FIFO_UNDERRUN_STATUS)
DRM_ERROR("pipe %c underrun\n", pipe_name(pipe));
}
}
Expand All @@ -366,7 +367,8 @@ static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev,
}

static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev,
enum pipe pipe, bool enable)
enum pipe pipe,
bool enable, bool old)
{
struct drm_i915_private *dev_priv = dev->dev_private;
if (enable) {
Expand All @@ -379,7 +381,8 @@ static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev,
} else {
ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);

if (I915_READ(GEN7_ERR_INT) & ERR_INT_FIFO_UNDERRUN(pipe)) {
if (old &&
I915_READ(GEN7_ERR_INT) & ERR_INT_FIFO_UNDERRUN(pipe)) {
DRM_ERROR("uncleared fifo underrun on pipe %c\n",
pipe_name(pipe));
}
Expand Down Expand Up @@ -444,7 +447,7 @@ static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,

static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
enum transcoder pch_transcoder,
bool enable)
bool enable, bool old)
{
struct drm_i915_private *dev_priv = dev->dev_private;

Expand All @@ -459,7 +462,8 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
} else {
ibx_disable_display_interrupt(dev_priv, SDE_ERROR_CPT);

if (I915_READ(SERR_INT) & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
if (old && I915_READ(SERR_INT) &
SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
DRM_ERROR("uncleared pch fifo underrun on pch transcoder %c\n",
transcoder_name(pch_transcoder));
}
Expand All @@ -486,28 +490,23 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
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);
bool ret;
bool old;

assert_spin_locked(&dev_priv->irq_lock);

ret = !intel_crtc->cpu_fifo_underrun_disabled;

if (enable == ret)
goto done;

old = !intel_crtc->cpu_fifo_underrun_disabled;
intel_crtc->cpu_fifo_underrun_disabled = !enable;

if (INTEL_INFO(dev)->gen < 5 || IS_VALLEYVIEW(dev))
i9xx_set_fifo_underrun_reporting(dev, pipe, enable);
i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
else if (IS_GEN5(dev) || IS_GEN6(dev))
ironlake_set_fifo_underrun_reporting(dev, pipe, enable);
else if (IS_GEN7(dev))
ivybridge_set_fifo_underrun_reporting(dev, pipe, enable);
ivybridge_set_fifo_underrun_reporting(dev, pipe, enable, old);
else if (IS_GEN8(dev))
broadwell_set_fifo_underrun_reporting(dev, pipe, enable);

done:
return ret;
return old;
}

bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
Expand Down Expand Up @@ -556,7 +555,7 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long flags;
bool ret;
bool old;

/*
* NOTE: Pre-LPT has a fixed cpu pipe -> pch transcoder mapping, but LPT
Expand All @@ -569,21 +568,16 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,

spin_lock_irqsave(&dev_priv->irq_lock, flags);

ret = !intel_crtc->pch_fifo_underrun_disabled;

if (enable == ret)
goto done;

old = !intel_crtc->pch_fifo_underrun_disabled;
intel_crtc->pch_fifo_underrun_disabled = !enable;

if (HAS_PCH_IBX(dev))
ibx_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
else
cpt_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
cpt_set_fifo_underrun_reporting(dev, pch_transcoder, enable, old);

done:
spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
return ret;
return old;
}


Expand Down

0 comments on commit 2ae2a50

Please sign in to comment.