Re: [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts

Chris Wilson
2016-10-10T11:46:39Z

On Mon, Oct 10, 2016 at 02:42:01PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 10, 2016 at 12:34:54PM +0100, Chris Wilson wrote:
> > To enable the vblank itself, we need to have an RPM wakeref for the mmio
> > access, and whilst generating the vblank interrupts we continue to
> > require the rpm wakeref. The assumption is that the RPM wakeref is held
> > by the display powerwell held by the active pipe. As this chain was not
> > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN
> > during *_vblank_enable().
> >
> > v2: Check the display power well rather than digging inside the atomic
> > CRTC state.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 1e43fe30da11..f0f17055dbb9 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2715,6 +2715,14 @@ void i915_handle_error(struct drm_i915_private *dev_priv,
> > i915_reset_and_wakeup(dev_priv);
> > }
> >
> > +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv,
> > + enum pipe pipe)
> > +{
> > + WARN_ON(IS_ENABLED(CONFIG_DRM_I915_DEBUG) &&
> > + !intel_display_power_is_enabled(dev_priv,
> > + POWER_DOMAIN_PIPE(pipe)));
>
> Uses a mutex. And having a power well enabled doesn't mean much. It
> doesn't guarantee that vblanks work.

Impasse. :|

There should be no point in an explicit assert_rpm_wakeref here as the
register access should catch an error there. Is there no safe way we can
assert the current state of the CRTC is correct for enabling vblanks?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre