Skip to content

[DO NOT REVIEW YET]PM Support for BYT/CHT + module load/unload fix for BSW #1523

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
ranj063 wants to merge 5 commits into from
Closed

[DO NOT REVIEW YET]PM Support for BYT/CHT + module load/unload fix for BSW #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
add60d7
ASoC: SOF: Intel: BYT: add remove op
ranj063 Nov 20, 2019
8d4dd37
ASoC: SOF: ACPI: fix the runtime enable/disable sequence
ranj063 Nov 20, 2019
2ba5e92
ASoC: SOF: Intel: byt: Add PM callbacks
ranj063 Nov 20, 2019
c43a5b5
ASoC: SOF: ipc: ignore DSP replies received when they are not expected
ranj063 Nov 20, 2019
911388b
ASoC: SOF: pm: Fix checks for platform-specific callbacks
ranj063 Nov 20, 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
47 changes: 47 additions & 0 deletions sound/soc/sof/intel/byt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,39 @@ static void byt_set_mach_params(const struct snd_soc_acpi_mach *mach,
mach_params->platform = dev_name(dev);
}

static int byt_reset_dsp_disable_int(struct snd_sof_dev *sdev)
{
/* disable Interrupt from both sides */
snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x3);
snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x3);

/* put DSP into reset, set reset vector */
snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_CSR,
SHIM_BYT_CSR_RST | SHIM_BYT_CSR_VECTOR_SEL,
SHIM_BYT_CSR_RST | SHIM_BYT_CSR_VECTOR_SEL);

return 0;
}

static int byt_remove(struct snd_sof_dev *sdev)
{
return byt_reset_dsp_disable_int(sdev);
}

static int byt_suspend(struct snd_sof_dev *sdev)
{
return byt_reset_dsp_disable_int(sdev);
}

static int byt_resume(struct snd_sof_dev *sdev)
{
/* enable Interrupt from both sides */
snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x0);
snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x0);

return 0;
}

/* Baytrail DAIs */
static struct snd_soc_dai_driver byt_dai[] = {
{
Expand Down Expand Up @@ -732,6 +765,7 @@ static int byt_acpi_probe(struct snd_sof_dev *sdev)
const struct snd_sof_dsp_ops sof_byt_ops = {
/* device init */
.probe = byt_acpi_probe,
.remove = byt_remove,

/* DSP core boot / reset */
.run = byt_run,
Expand Down Expand Up @@ -781,6 +815,12 @@ const struct snd_sof_dsp_ops sof_byt_ops = {
/*Firmware loading */
.load_firmware = snd_sof_load_firmware_memcpy,

/* PM */
.suspend = byt_suspend,
.resume = byt_resume,
.runtime_suspend = byt_suspend,
.runtime_resume = byt_resume,

/* DAI drivers */
.drv = byt_dai,
.num_drv = 3, /* we have only 3 SSPs on byt*/
Expand All @@ -804,6 +844,7 @@ EXPORT_SYMBOL(byt_chip_info);
const struct snd_sof_dsp_ops sof_cht_ops = {
/* device init */
.probe = byt_acpi_probe,
.remove = byt_remove,

/* DSP core boot / reset */
.run = byt_run,
Expand Down Expand Up @@ -853,6 +894,12 @@ const struct snd_sof_dsp_ops sof_cht_ops = {
/*Firmware loading */
.load_firmware = snd_sof_load_firmware_memcpy,

/* PM */
.suspend = byt_suspend,
.resume = byt_resume,
.runtime_suspend = byt_suspend,
.runtime_resume = byt_resume,

/* DAI drivers */
.drv = byt_dai,
/* all 6 SSPs may be available for cherrytrail */
Expand Down
9 changes: 4 additions & 5 deletions sound/soc/sof/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,20 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
EXPORT_SYMBOL(sof_ipc_tx_message);

/* handle reply message from DSP */
int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
{
struct snd_sof_ipc_msg *msg = &sdev->ipc->msg;

if (msg->ipc_complete) {
dev_err(sdev->dev, "error: no reply expected, received 0x%x",
dev_dbg(sdev->dev,
"no reply expected, received 0x%x, will be ignored",
msg_id);
return -EINVAL;
return;
}

/* wake up and return the error if we have waiters on this message ? */
msg->ipc_complete = true;
wake_up(&msg->waitq);

return 0;
}
EXPORT_SYMBOL(snd_sof_ipc_reply);

Expand Down
18 changes: 13 additions & 5 deletions sound/soc/sof/pm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ static int sof_resume(struct device *dev, bool runtime_resume)
int ret;

/* do nothing if dsp resume callbacks are not set */
if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume)
return 0;
if (runtime_resume)
Copy link
Member

@plbossart plbossart Nov 21, 2019

Choose a reason for hiding this comment

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

This one is a bit controversial

I can see that you could support suspend/resume but not their pm_runtime versions.
would the opposite make any sense, support pm_runtime but not system suspend?

I would reword the commit message. This is not a fix, it's allowing for more granularity in what can be supported. this allows pm_runtime to be unsupported while normal resume is supported.
But I don't know if the other way around makes sense.

Copy link
Collaborator Author

@ranj063 ranj063 Nov 21, 2019

Choose a reason for hiding this comment

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

@plbossart this change makes no assumption about whether runtime PM or system PM is supported. sof_resume() and sof_suspend() are called for both the runtime PM and system PM cases. So all I am doing is if we're runtime suspending, check for the platform-specific runtime-suspend callback and so on for the other cases.
I called it a fix because previously, we didnt check for the appropriate op depending on whether it was runtime callback or system PM callback

Copy link
Member

@plbossart plbossart Nov 26, 2019

Choose a reason for hiding this comment

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

@ranj063 when maintainers see a commit message starting with 'fix' they think it's a real problem that needs to be handled on previous -stable versions. What you are doing is 'split checks for platform-specific callbacks', not a real fix.

if (!sof_ops(sdev)->runtime_resume)
return 0;
else
if (!sof_ops(sdev)->resume)
return 0;

/*
* if the runtime_resume flag is set, call the runtime_resume routine
Expand Down Expand Up @@ -124,9 +128,13 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
int ret;

/* do nothing if dsp suspend callback is not set */
if (!sof_ops(sdev)->suspend)
return 0;
/* do nothing if dsp suspend callbacks are not set */
if (runtime_suspend)
if (!sof_ops(sdev)->runtime_suspend)
return 0;
else
if (!sof_ops(sdev)->suspend)
return 0;
Copy link

@RanderWang RanderWang Nov 27, 2019

Choose a reason for hiding this comment

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

@ranj063 Do we need these checks here ? they are already checked in snd_sof_dsp_suspend and snd_sof_dsp_runtime_suspend


/* release trace */
snd_sof_release_trace(sdev);
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/sof/sof-acpi-dev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ static void sof_acpi_probe_complete(struct device *dev)
/* allow runtime_pm */
pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
pm_runtime_mark_last_busy(dev);
pm_runtime_idle(dev);
}

static int sof_acpi_probe(struct platform_device *pdev)
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/sof-priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset);
*/
struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev);
void snd_sof_ipc_free(struct snd_sof_dev *sdev);
int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id);
void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id);
void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev);
int snd_sof_ipc_stream_pcm_params(struct snd_sof_dev *sdev,
struct sof_ipc_pcm_params *params);
Expand Down