linux icon indicating copy to clipboard operation
linux copied to clipboard

HDA: i915 power-down no-op

Open plbossart opened this issue 1 year ago • 4 comments

Forking new issue from https://github.com/thesofproject/linux/pull/3933#discussion_r1014175537 discussion:

The i915 power management is a bit misleading in the power is turned on/off in multiple places. It's unconditionally turned on/off in hda_codev_i915_init/exit and conditionally everyone else. that's fine, but @ujfalusi has a point that this is a no-op in hda.c

if (!HDA_IDISP_CODEC(bus->codec_mask))
		hda_codec_i915_display_power(sdev, false);

this code was added by @kv2019i in 0c75419, but our very own @kv2019i added an additional filter in 816938b which now makes the initial code a no-op.

One of the two changes is correct, which one @kv2019i ?

plbossart avatar Nov 04 '22 17:11 plbossart

Good catch @ujfalusi @plbossart . Both of those patches are good, but I got trapped by my own cleverness in 71cc8abb6ec705ce4efbb54e401004687d40a641 which incorrectly added the no-op. That bit is harmless and can be removed as the power reference is not taken in hda_codec_i915_init() if no i915 codec is found.

kv2019i avatar Nov 04 '22 18:11 kv2019i

Not quite following @kv2019i, is this that you are suggesting?

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 348fbfb6a2c2..34c145ae439a 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -921,7 +921,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
        hda_codec_probe_bus(sdev);
 
        if (!HDA_IDISP_CODEC(bus->codec_mask))
-               hda_codec_i915_display_power(sdev, false);
+               hda_codec_i915_exit(sdev);
 
        hda_bus_ml_put_all(bus);
 

plbossart avatar Nov 04 '22 19:11 plbossart

I was actually thinking of removing both lines, but actually you are right, this is actually broken in the case that a) i915 is present, b) display codec for some reason doesn't show up on HDA bus. With current code, we keep display power on always.

But yeah, fixing this requires more thought and covering all the various ways display codec can go missing...

kv2019i avatar Nov 04 '22 19:11 kv2019i

ok, let's sleep on it @kv2019i. it's not urgent

plbossart avatar Nov 04 '22 19:11 plbossart