Skip to content

[SQUASH ME] ASoC: SOF: Intel: hda: release display power for D0ix #1767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kv2019i wants to merge 3 commits into from
Closed

[SQUASH ME] ASoC: SOF: Intel: hda: release display power for D0ix #1767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6b95b0
[SQUASH ME] ASoC: SOF: Intel: hda: release display power for D0ix
kv2019i Feb 6, 2020
58a3a52
ALSA: hda: do not override probe codec_mask at in link_get()
kv2019i Feb 6, 2020
d31cd92
[SQUASH ME] ASoC: SOF: Intel: hda: align with upstream
kv2019i Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions sound/hda/ext/hdac_ext_controller.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all);
int snd_hdac_ext_bus_link_get(struct hdac_bus *bus,
struct hdac_ext_link *link)
{
unsigned long codec_mask;
int ret = 0;

mutex_lock(&bus->lock);
Expand All @@ -280,9 +281,11 @@ int snd_hdac_ext_bus_link_get(struct hdac_bus *bus,
* HDA spec section 4.3 - Codec Discovery
*/
udelay(521);
bus->codec_mask = snd_hdac_chip_readw(bus, STATESTS);
dev_dbg(bus->dev, "codec_mask = 0x%lx\n", bus->codec_mask);
snd_hdac_chip_writew(bus, STATESTS, bus->codec_mask);
codec_mask = snd_hdac_chip_readw(bus, STATESTS);
dev_dbg(bus->dev, "codec_mask = 0x%lx\n", codec_mask);
snd_hdac_chip_writew(bus, STATESTS, codec_mask);
if (!bus->codec_mask)
bus->codec_mask = codec_mask;

@plbossart plbossart Feb 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify in the commit message what the impact of overriding the bus->codec_mask is? the patch looks ok, but it's not clear why it's needed.

@kv2019i kv2019i Feb 6, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart Ack. So basicly my patch to power up/down display, at runtime suspend/resume, depends on codec_mask being correct. If hdac library changes it on its own, our PM runtime flow stops working correctly. We could copy the data to a SOF local variable outside HDAC, but it seems that's not really reasonable -- there doesn't seem to be any reason for hdac_ext_controller.c to modify codec_mask like this.

When sending upstream, this should patch should be first in the series (as later patches depend on it). I can still do this :)

@plbossart plbossart Feb 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll approve then and let you send all the i915-related upstream on top of Mark's tree. Please add my reviewed-by tag.

when it's merged, we'll deal with conflicts with Ranjani's changes and send a v2.

@lyakh lyakh Feb 14, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there's really no reason for hdac_ext_controller to modify .codec_mask maybe that should be fixed instead?

}

mutex_unlock(&bus->lock);
Expand Down
12 changes: 8 additions & 4 deletions sound/soc/sof/intel/hda-codec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,10 @@ void hda_codec_i915_display_power(struct snd_sof_dev *sdev, bool enable)
{
struct hdac_bus *bus = sof_to_bus(sdev);

dev_dbg(bus->dev, "Turning i915 HDAC power %d\n", enable);
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, enable);
if (HDA_IDISP_CODEC(bus->codec_mask)) {
dev_dbg(bus->dev, "Turning i915 HDAC power %d\n", enable);
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, enable);
}
}
EXPORT_SYMBOL_NS(hda_codec_i915_display_power, SND_SOC_SOF_HDA_AUDIO_CODEC_I915);

Expand All @@ -189,7 +191,8 @@ int hda_codec_i915_init(struct snd_sof_dev *sdev)
if (ret < 0)
return ret;

hda_codec_i915_display_power(sdev, true);
/* codec_mask not yet known, power up for probe */
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);

return 0;
}
Expand All @@ -200,7 +203,8 @@ int hda_codec_i915_exit(struct snd_sof_dev *sdev)
struct hdac_bus *bus = sof_to_bus(sdev);
int ret;

hda_codec_i915_display_power(sdev, false);
/* power down unconditionally */
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
Comment thread
plbossart marked this conversation as resolved.
Resolved

ret = snd_hdac_i915_exit(bus);

Expand Down
15 changes: 7 additions & 8 deletions sound/soc/sof/intel/hda-dsp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,8 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
return ret;
}

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* display codec can powered off after link reset */
if (HDA_IDISP_CODEC(bus->codec_mask))
hda_codec_i915_display_power(sdev, false);
#endif
hda_codec_i915_display_power(sdev, false);
Comment thread
plbossart marked this conversation as resolved.
Resolved

return 0;
}
Expand All @@ -612,11 +609,8 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
#endif
int ret;

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* display codec must be powered before link reset */
if (HDA_IDISP_CODEC(bus->codec_mask))
hda_codec_i915_display_power(sdev, true);
#endif
hda_codec_i915_display_power(sdev, true);

/*
* clear TCSEL to clear playback on some HD Audio
Expand Down Expand Up @@ -675,6 +669,8 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
return ret;
}

hda_codec_i915_display_power(sdev, true);

/* restore L1SEN bit */
if (hda->l1_support_changed)
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
Expand Down Expand Up @@ -763,6 +759,9 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
return ret;
}

/* we can't keep a wakeref to display driver at suspend */
hda_codec_i915_display_power(sdev, false);

/* enable L1SEN to make sure the system can enter S0Ix */
hda->l1_support_changed =
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
Expand Down
5 changes: 2 additions & 3 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,14 +519,12 @@ static int hda_init(struct snd_sof_dev *sdev)
/* HDA base */
sdev->bar[HDA_DSP_HDA_BAR] = bus->remap_addr;

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* init i915 and HDMI codecs */
ret = hda_codec_i915_init(sdev);
if (ret < 0) {
dev_err(sdev->dev, "error: init i915 and HDMI codec failed\n");
return ret;
}
#endif

/* get controller capabilities */
ret = hda_dsp_ctrl_get_caps(sdev);
Expand Down Expand Up @@ -645,7 +643,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
hda_codec_probe_bus(sdev, hda_codec_use_common_hdmi);

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

/*
* we are done probing so decrement link counts
Expand Down Expand Up @@ -884,6 +882,7 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
iounmap(sdev->bar[HDA_DSP_BAR]);
hdac_bus_unmap:
iounmap(bus->remap_addr);
hda_codec_i915_exit(sdev);
err:
return ret;
}
Expand Down