[Intel-gfx] [PATCH] drm/i915: Get audio power domain during initial hw readout

[Bob Paauwe]

On Wed, 13 Apr 2016 11:59:43 +0300
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> On Tue, Apr 12, 2016 at 03:52:48PM -0700, Bob Paauwe wrote:
> > if the crtc has audio is enabled. Otherwise, when the first atomic
> > modeset happens it will warn when trying to drop the audio power
> > domain.
> > 
> > Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 5155efb6..caeb3e1 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10561,6 +10561,7 @@ found:
> >  	}
> >  
> >  	ret = intel_modeset_setup_plane_state(state, crtc, mode, fb, 0, 0);
> > +
> >  	if (ret)
> >  		goto fail;
> >  
> > @@ -15998,6 +15999,10 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  
> >  		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> >  		if (crtc->base.state->active) {
> > +			if (crtc->config->has_audio)
> > +				intel_display_power_get(dev_priv,
> > +							POWER_DOMAIN_AUDIO);
> > +  
> 
> Hmm. Ideally we would do this alongside the other power doamin stuff in
> intel_modeset_setup_hw_state(), but that's too late for
> intel_sanitize_{crtc,encoder}(). So I guess we'll have to do it before
> those, or we have to pull the audio power domain stuff out from the
> encoder hooks. Or perhaps even throw it out entirely, since I'm not sure
> it's doing anything for us.
> 
> If we do leave the power domain handling in the encoder hooks, then
> this is not quite the right place to grab the reference. We could have
> an encoder enabled w/o an active pipe, in which case
> intel_sanitize_encoder() would shut off the encoder, so we'd still end
> up with a bad refcount.
>

Thanks Ville for the details.  I'm not at all familiar with how this is
supposed to work so this helps some.  But I'm still not sure I follow.

If I move this to get_crtc_power_domains() by sticking 

   if (crtc_state->has_audio)
	mask |= BIT(POWER_DOMAIN_AUDIO);

at the end, which is where I think you're suggesting initially, it
seems to work.  So I'm not sure why that's too late.  Also, I'm not
seeing where shutting off the encoder will effect the power domain
refcounts.

Bob

> >  			intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> >  			intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> >  			WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx  
> 



-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193