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

[Anshuman Gupta <anshuman.gupta@intel.com>]

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.

> 
> 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