* [RFC] drm/i915: lpsp with hdmi/dp outputs @ 2020-06-01 10:15 Anshuman Gupta 2020-06-01 12:49 ` [Intel-gfx] " Shankar, Uma 2020-06-01 14:11 ` Ville Syrjälä 0 siblings, 2 replies; 7+ messages in thread From: Anshuman Gupta @ 2020-06-01 10:15 UTC (permalink / raw) To: intel-gfx Cc: Anshuman Gupta, stable, Ville Syrjälä, Maarten Lankhorst 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. + */ + 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-01 10:15 [RFC] drm/i915: lpsp with hdmi/dp outputs Anshuman Gupta @ 2020-06-01 12:49 ` Shankar, Uma 2020-06-02 12:06 ` Anshuman Gupta 2020-06-01 14:11 ` Ville Syrjälä 1 sibling, 1 reply; 7+ messages in thread From: Shankar, Uma @ 2020-06-01 12:49 UTC (permalink / raw) To: Gupta, Anshuman, intel-gfx@lists.freedesktop.org; +Cc: stable@vger.kernel.org > -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > Anshuman Gupta > Sent: Monday, June 1, 2020 3:45 PM > To: intel-gfx@lists.freedesktop.org > Cc: stable@vger.kernel.org > Subject: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs > > 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. > + */ > + if (INTEL_GEN(dev_priv) < 12 && HAS_DDI(dev_priv) && > +crtc_state->has_audio) > mask |= BIT_ULL(POWER_DOMAIN_AUDIO); As part of ddi_get_config we determine has_audio using power well enabled: pipe_config->has_audio = intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder); If audio power domain is not enabled, we may end up with this as false. Later this may get checked in intel_enable_ddi_hdmi to call audio codec enable if (crtc_state->has_audio) intel_audio_codec_enable(encoder, crtc_state, conn_state); This may cause detection to fail. Please verify this usecase once and confirm. Regards, Uma Shankar > if (crtc_state->shared_dpll) > -- > 2.26.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-01 12:49 ` [Intel-gfx] " Shankar, Uma @ 2020-06-02 12:06 ` Anshuman Gupta 2020-06-02 13:02 ` Shankar, Uma 0 siblings, 1 reply; 7+ messages in thread From: Anshuman Gupta @ 2020-06-02 12:06 UTC (permalink / raw) To: Shankar, Uma; +Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org On 2020-06-01 at 18:19:44 +0530, Shankar, Uma wrote: > > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > > Anshuman Gupta > > Sent: Monday, June 1, 2020 3:45 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: stable@vger.kernel.org > > Subject: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs > > > > 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. > > + */ > > + if (INTEL_GEN(dev_priv) < 12 && HAS_DDI(dev_priv) && > > +crtc_state->has_audio) > > mask |= BIT_ULL(POWER_DOMAIN_AUDIO); > > As part of ddi_get_config we determine has_audio using power well enabled: > pipe_config->has_audio = > intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder); IMO AUDIO power will also be requested by i915_audio_component get request, we can always use HDMI display without audio playback, AUDIO power should be enabled when audio driver request for it. if we get AUDIO_POWER_DOMAIN while enabling crtc PG3 will always kept on till CRTC is disabled, that is the issue required to be addressed here. This is just RFC to initiate a discussion around it. Thanks, Anshuman Gupta. > > If audio power domain is not enabled, we may end up with this as false. > Later this may get checked in intel_enable_ddi_hdmi to call audio codec enable > > if (crtc_state->has_audio) > intel_audio_codec_enable(encoder, crtc_state, conn_state); > > This may cause detection to fail. Please verify this usecase once and confirm. > > Regards, > Uma Shankar > > > if (crtc_state->shared_dpll) > > -- > > 2.26.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-02 12:06 ` Anshuman Gupta @ 2020-06-02 13:02 ` Shankar, Uma 0 siblings, 0 replies; 7+ messages in thread From: Shankar, Uma @ 2020-06-02 13:02 UTC (permalink / raw) To: Gupta, Anshuman; +Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org > -----Original Message----- > From: Gupta, Anshuman <anshuman.gupta@intel.com> > Sent: Tuesday, June 2, 2020 5:37 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org > Subject: Re: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs > > On 2020-06-01 at 18:19:44 +0530, Shankar, Uma wrote: > > > > > > > -----Original Message----- > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf > > > Of Anshuman Gupta > > > Sent: Monday, June 1, 2020 3:45 PM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: stable@vger.kernel.org > > > Subject: [Intel-gfx] [RFC] drm/i915: lpsp with hdmi/dp outputs > > > > > > 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. > > > + */ > > > + if (INTEL_GEN(dev_priv) < 12 && HAS_DDI(dev_priv) && > > > +crtc_state->has_audio) > > > mask |= BIT_ULL(POWER_DOMAIN_AUDIO); > > > > As part of ddi_get_config we determine has_audio using power well enabled: > > pipe_config->has_audio = > > intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder); > IMO AUDIO power will also be requested by i915_audio_component get request, > we can always use HDMI display without audio playback, AUDIO power should > be enabled when audio driver request for it. > if we get AUDIO_POWER_DOMAIN while enabling crtc PG3 will always kept on till > CRTC is disabled, that is the issue required to be addressed here. Yes HDMI can be enabled without audio, but if we want audio we will need to notify audio driver through HSW_AUD_PIN_ELD_CP_VLD and also prepare and write ELD data in hardware register. If I understand correctly this will need power and by this time audio driver would not have requested for it. Hence this will fail audio detection. > This is just RFC to initiate a discussion around it. > Thanks, > Anshuman Gupta. > > > > If audio power domain is not enabled, we may end up with this as false. > > Later this may get checked in intel_enable_ddi_hdmi to call audio > > codec enable > > > > if (crtc_state->has_audio) > > intel_audio_codec_enable(encoder, crtc_state, > > conn_state); > > > > This may cause detection to fail. Please verify this usecase once and confirm. > > > > Regards, > > Uma Shankar > > > > > if (crtc_state->shared_dpll) > > > -- > > > 2.26.2 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-01 10:15 [RFC] drm/i915: lpsp with hdmi/dp outputs Anshuman Gupta 2020-06-01 12:49 ` [Intel-gfx] " Shankar, Uma @ 2020-06-01 14:11 ` Ville Syrjälä 2020-06-02 12:28 ` Anshuman Gupta 1 sibling, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2020-06-01 14:11 UTC (permalink / raw) To: Anshuman Gupta; +Cc: intel-gfx, stable, Maarten Lankhorst 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. That said, looks like audio is moving into the always on well, but not yet in tgl. . > + */ > + 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-01 14:11 ` Ville Syrjälä @ 2020-06-02 12:28 ` Anshuman Gupta 2020-06-03 10:05 ` Ville Syrjälä 0 siblings, 1 reply; 7+ messages in thread From: Anshuman Gupta @ 2020-06-02 12:28 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, stable, Maarten Lankhorst 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] drm/i915: lpsp with hdmi/dp outputs 2020-06-02 12:28 ` Anshuman Gupta @ 2020-06-03 10:05 ` Ville Syrjälä 0 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2020-06-03 10:05 UTC (permalink / raw) To: Anshuman Gupta; +Cc: intel-gfx, stable, Maarten Lankhorst 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-03 10:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-01 10:15 [RFC] drm/i915: lpsp with hdmi/dp outputs Anshuman Gupta 2020-06-01 12:49 ` [Intel-gfx] " Shankar, Uma 2020-06-02 12:06 ` Anshuman Gupta 2020-06-02 13:02 ` Shankar, Uma 2020-06-01 14:11 ` Ville Syrjälä 2020-06-02 12:28 ` Anshuman Gupta 2020-06-03 10:05 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).