diff mbox series

drm/i915/audio: Hook up component bindings even if displays are disabled

Message ID 20180817100241.4628-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/audio: Hook up component bindings even if displays are disabled | expand

Commit Message

Chris Wilson Aug. 17, 2018, 10:02 a.m. UTC
If the display has been disabled by modparam, we still want to connect
together the HW bits and bobs with the associated drivers so that we can
continue to manage their runtime power gating.

Fixes: 108109444ff6 ("drm/i915: Check num_pipes before initializing audio component")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Elaine Wang <elaine.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Imre Deak Aug. 20, 2018, 1:23 p.m. UTC | #1
On Fri, Aug 17, 2018 at 11:02:41AM +0100, Chris Wilson wrote:
> If the display has been disabled by modparam, we still want to connect
> together the HW bits and bobs with the associated drivers so that we can
> continue to manage their runtime power gating.
> 
> Fixes: 108109444ff6 ("drm/i915: Check num_pipes before initializing audio component")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Elaine Wang <elaine.wang@intel.com>

Going through the hooks in i915_audio_component_ops, I haven't noticed
anything that would actually go wrong with num_pipes=0. Besides some
audio HW register programming from i915_audio_component_codec_wake_override(),
we'd skip doing everything else due to not reporting any active encoders
to the audio component (as we do during normal display modeset). So

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_audio.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index b725835b47ef..769f3f586661 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -962,9 +962,6 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
>  
> -	if (INTEL_INFO(dev_priv)->num_pipes == 0)
> -		return;
> -
>  	ret = component_add(dev_priv->drm.dev, &i915_audio_component_bind_ops);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to add audio component (%d)\n", ret);
> -- 
> 2.18.0
>
Chris Wilson Aug. 20, 2018, 2:05 p.m. UTC | #2
Quoting Imre Deak (2018-08-20 14:23:13)
> On Fri, Aug 17, 2018 at 11:02:41AM +0100, Chris Wilson wrote:
> > If the display has been disabled by modparam, we still want to connect
> > together the HW bits and bobs with the associated drivers so that we can
> > continue to manage their runtime power gating.
> > 
> > Fixes: 108109444ff6 ("drm/i915: Check num_pipes before initializing audio component")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Elaine Wang <elaine.wang@intel.com>
> 
> Going through the hooks in i915_audio_component_ops, I haven't noticed
> anything that would actually go wrong with num_pipes=0. Besides some
> audio HW register programming from i915_audio_component_codec_wake_override(),
> we'd skip doing everything else due to not reporting any active encoders
> to the audio component (as we do during normal display modeset). So
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

I'm confident in that we at least exercise the no-pipes case, less
confident that we test for fused-off hw. However, the audio component
simply should not find any output channels even in that case, so it
indeed should be safe.

Cross your fingers, thanks for the review,
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index b725835b47ef..769f3f586661 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -962,9 +962,6 @@  void i915_audio_component_init(struct drm_i915_private *dev_priv)
 {
 	int ret;
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return;
-
 	ret = component_add(dev_priv->drm.dev, &i915_audio_component_bind_ops);
 	if (ret < 0) {
 		DRM_ERROR("failed to add audio component (%d)\n", ret);