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@xxxxxxxxxxxxxxx > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx> > > > --- > > > 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
