Re: [RFC] drm/i915: lpsp with hdmi/dp outputs

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 02, 2020 at 05:58:07PM +0530, Anshuman Gupta wrote:
> On 2020-06-01 at 17:11:32 +0300, Ville Syrjälä wrote:
> > On Mon, Jun 01, 2020 at 03:45:16PM +0530, Anshuman Gupta wrote:
> > > Gen12 hw are failing to enable lpsp configuration due to PG3 was left on
> > > due to valid usgae count of POWER_DOMAIN_AUDIO.
> > > It is not required to get POWER_DOMAIN_AUDIO ref-count when enabling
> > > a crtc, it should be always i915_audio_component request to get/put
> > > AUDIO_POWER_DOMAIN.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 6c3b11de2daf..f31a579d7a52 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7356,7 +7356,11 @@ static u64 get_crtc_power_domains(struct intel_crtc_state *crtc_state)
> > >  		mask |= BIT_ULL(intel_encoder->power_domain);
> > >  	}
> > >  
> > > -	if (HAS_DDI(dev_priv) && crtc_state->has_audio)
> > > +	/*
> > > +	 * Gen12 can drive lpsp on hdmi/dp outpus, it doesn't require to
> > > +	 * enable AUDIO power in order to enable a crtc
> > 
> > Nothing requires audio power to enable a crtc. What this is saying is
> > that if we want audio then we must enable the audio power. If you
> > didn't want audio then you wouldn't have .has_audio set.
> IMO i915_audio_component_get_power also enables audio power, and
> i915_audio_component_put_power releases the usage count based upon audio
> runtime idleness but here get_crtc_power_domains() gets the POWER_DOMAIN_AUDIO usages
> count, which will be released only when this crtc get disbaled.
> It may enable AUDIO power despite of fact that audio driver has released the
> usage count.
> Please correct me if i am wrong here.

The audio component stuff doesn't actually do the audio enable/disable
sequence.

> 
> > 
> > That said, looks like audio is moving into the always on well, but not
> > yet in tgl.
> Still some of audio functional stuff lies in PG3, not completely removed
> from PG3.
> Thanks,
> Anshuman Gupta.
> > 
> > .
> > > +	 */
> > > +	if (INTEL_GEN(dev_priv) < 12 && HAS_DDI(dev_priv) && crtc_state->has_audio)
> > >  		mask |= BIT_ULL(POWER_DOMAIN_AUDIO);
> > >  
> > >  	if (crtc_state->shared_dpll)
> > > -- 
> > > 2.26.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel