Skip to content

[RFC] ASoC: SOF: Intel: hda: refine init/stop chip in hda_suspend/hda_resume #965

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
zhuyingjiang wants to merge 4 commits into from
Closed

[RFC] ASoC: SOF: Intel: hda: refine init/stop chip in hda_suspend/hda_resume #965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e27abf2
ASoC: SOF: Intel: hda: use the SOF defined ppcap functions
zhuyingjiang May 21, 2019
51c7a50
ASoC: SOF: Intel: hda: add function for hda stop chip
zhuyingjiang May 21, 2019
dd69928
ASoC: SOF: Intel: hda: use the defined stop chip in suspend
zhuyingjiang May 21, 2019
eee6e43
ASoC: SOF: Intel: hda: use SOF defined init chip in resume
zhuyingjiang May 21, 2019
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
69 changes: 69 additions & 0 deletions sound/soc/sof/intel/hda-ctrl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset)
snd_hdac_chip_writel(bus, DPUBASE,
upper_32_bits(bus->posbuf.addr));
}

/* Reset stream-to-link mapping */
list_for_each_entry(hlink, &bus->hlink_list, list)
bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
#endif

bus->chip_init = true;
Expand All @@ -263,3 +267,68 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset)

return ret;
}

void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev)
{
struct hdac_bus *bus = sof_to_bus(sdev);
struct hdac_stream *stream;
int sd_offset;

if (!bus->chip_init)
return;

/* disable interrupts in stream descriptor */
list_for_each_entry(stream, &bus->stream_list, list) {
sd_offset = SOF_STREAM_SD_OFFSET(stream);
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
sd_offset +
SOF_HDA_ADSP_REG_CL_SD_CTL,
SOF_HDA_CL_DMA_SD_INT_MASK,
0);
}

/* disable SIE for all streams */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL,
SOF_HDA_INT_ALL_STREAM, 0);

/* disable hda bus irq */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL,
SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN,
0);

/* clear stream status */
list_for_each_entry(stream, &bus->stream_list, list) {
sd_offset = SOF_STREAM_SD_OFFSET(stream);
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
sd_offset +
SOF_HDA_ADSP_REG_CL_SD_STS,
SOF_HDA_CL_DMA_SD_INT_MASK,
SOF_HDA_CL_DMA_SD_INT_MASK);
}

/* clear WAKESTS */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS,
SOF_HDA_WAKESTS_INT_MASK,
SOF_HDA_WAKESTS_INT_MASK);

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* clear rirb status */
snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK);
#endif

/* clear interrupt status register */
snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTSTS,
SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_ALL_STREAM);

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* disable CORB/RIRB */
snd_hdac_bus_stop_cmd_io(bus);

/* disable position buffer */
if (bus->posbuf.addr) {
snd_hdac_chip_writel(bus, DPLBASE, 0);
snd_hdac_chip_writel(bus, DPUBASE, 0);
}
#endif
bus->chip_init = false;
}
55 changes: 2 additions & 53 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 @@ -307,23 +307,12 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state)
return ret;
}

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* disable ppcap interrupt */
snd_hdac_ext_bus_ppcap_int_enable(bus, false);
snd_hdac_ext_bus_ppcap_enable(bus, false);

/* disable hda bus irq and i/o */
snd_hdac_bus_stop_chip(bus);
#else
/* disable ppcap interrupt */
hda_dsp_ctrl_ppcap_enable(sdev, false);
hda_dsp_ctrl_ppcap_int_enable(sdev, false);

/* disable hda bus irq */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL,
SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN,
0);
#endif
/* disable hda bus irq and streams */
hda_dsp_ctrl_stop_chip(sdev);
Copy link
Member

@plbossart plbossart May 21, 2019

Choose a reason for hiding this comment

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

that's not an iso-feature change for the case where CONFIG_SND_SOC_SOF_HDA is not defined. You are doing a lot more and you'd need to explain a bit more the rationale for the changes.

Copy link
Author

@zhuyingjiang zhuyingjiang May 22, 2019

Choose a reason for hiding this comment

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

@plbossart The added steps are :
1 disable interrupts in stream descriptor
2 disable SIE for all streams
3 clear stream status
4 clear WAKESTS
5 clear interrupt status register
So all these are disable interrupts and clear registers of the streams, these are needed when shutdown/suspend to D3. Even with no HDA codecs.
So the previous code less them, although it seems still no problem with the DSP power off and on.
The PR has been tested with APL/WHL with HDA/nocodec mode.

Copy link
Member

@plbossart plbossart May 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

That one I could live with, but I would need a separate PR that adds those steps with the existing code. Once it's proven there is no impact we can do the code optimization. Doing all the changes at once is too risky and difficult to bisect.

Copy link
Author

@zhuyingjiang zhuyingjiang May 23, 2019

Choose a reason for hiding this comment

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

OK, #975 is the separate PR for this. I will modify the last commit and send a separate PR for the misc clock gating with init chip.


/* disable LP retention mode */
snd_sof_pci_update_bits(sdev, PCI_PGCTL,
Expand Down Expand Up @@ -354,7 +343,6 @@ static int hda_resume(struct snd_sof_dev *sdev)
*/
snd_sof_pci_update_bits(sdev, PCI_TCSEL, 0x07, 0);

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* reset and start hda controller */
ret = hda_dsp_ctrl_init_chip(sdev, true);
if (ret < 0) {
Expand All @@ -363,48 +351,9 @@ static int hda_resume(struct snd_sof_dev *sdev)
return ret;
}

hda_dsp_ctrl_misc_clock_gating(sdev, false);

/* Reset stream-to-link mapping */
list_for_each_entry(hlink, &bus->hlink_list, list)
bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);

hda_dsp_ctrl_misc_clock_gating(sdev, true);

/* enable ppcap interrupt */
snd_hdac_ext_bus_ppcap_enable(bus, true);
snd_hdac_ext_bus_ppcap_int_enable(bus, true);
#else

hda_dsp_ctrl_misc_clock_gating(sdev, false);

/* reset controller */
ret = hda_dsp_ctrl_link_reset(sdev, true);
if (ret < 0) {
dev_err(sdev->dev,
"error: failed to reset controller during resume\n");
return ret;
}

/* take controller out of reset */
ret = hda_dsp_ctrl_link_reset(sdev, false);
if (ret < 0) {
dev_err(sdev->dev,
"error: failed to ready controller during resume\n");
return ret;
}

/* enable hda bus irq */
snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL,
SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN,
SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN);

hda_dsp_ctrl_misc_clock_gating(sdev, true);

/* enable ppcap interrupt */
hda_dsp_ctrl_ppcap_enable(sdev, true);
hda_dsp_ctrl_ppcap_int_enable(sdev, true);
#endif

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* turn off the links that were off before suspend */
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ int hda_dsp_ctrl_link_reset(struct snd_sof_dev *sdev, bool reset);
void hda_dsp_ctrl_misc_clock_gating(struct snd_sof_dev *sdev, bool enable);
int hda_dsp_ctrl_clock_power_gating(struct snd_sof_dev *sdev, bool enable);
int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset);

void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev);
/*
* HDA bus operations.
*/
Expand Down