thesofproject / linux

Linux kernel source tree
Other
91 stars 133 forks source link

HDA: i915 power-down no-op #3993

Open plbossart opened 2 years ago

plbossart commented 2 years ago

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 ?

kv2019i commented 2 years ago

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.

plbossart commented 2 years ago

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);
kv2019i commented 2 years ago

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

plbossart commented 2 years ago

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

plbossart commented 3 months ago

@ujfalusi @kv2019i I'll let you decide if we need to keep this issue open.