| Message ID | 20230111090222.2016499-15-Vijendar.Mukunda@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add soundwire support for Pink Sardine platform | expand |
On 11/01/23 21:17, Pierre-Louis Bossart wrote: > > On 1/11/23 03:02, Vijendar Mukunda wrote: >> Add support for runtime pm ops for AMD master driver. >> >> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> >> Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com> >> --- >> drivers/soundwire/amd_master.c | 205 ++++++++++++++++++++++++++++++ >> drivers/soundwire/amd_master.h | 3 + >> include/linux/soundwire/sdw_amd.h | 1 + >> 3 files changed, 209 insertions(+) >> >> diff --git a/drivers/soundwire/amd_master.c b/drivers/soundwire/amd_master.c >> index c7063b8bdd7b..d2d7f07de202 100644 >> --- a/drivers/soundwire/amd_master.c >> +++ b/drivers/soundwire/amd_master.c >> @@ -15,6 +15,7 @@ >> #include <linux/soundwire/sdw.h> >> #include <linux/soundwire/sdw_registers.h> >> #include <linux/soundwire/sdw_amd.h> >> +#include <linux/pm_runtime.h> >> #include <linux/wait.h> >> #include <sound/pcm_params.h> >> #include <sound/soc.h> >> @@ -290,6 +291,17 @@ static int amd_disable_sdw_interrupts(struct amd_sdwc_ctrl *ctrl) >> return 0; >> } >> >> +static int amd_deinit_sdw_controller(struct amd_sdwc_ctrl *ctrl) >> +{ >> + int ret; >> + >> + ret = amd_disable_sdw_interrupts(ctrl); >> + if (ret) >> + return ret; >> + ret = amd_disable_sdw_controller(ctrl); >> + return ret; >> +} >> + >> static int amd_sdwc_set_frameshape(struct amd_sdwc_ctrl *ctrl, u32 rows, u32 cols) >> { >> u32 sdw_rows, sdw_cols, frame_size; >> @@ -1387,6 +1399,12 @@ static int amd_sdwc_probe(struct platform_device *pdev) >> INIT_WORK(&ctrl->amd_sdw_work, amd_sdwc_update_slave_status_work); >> INIT_WORK(&ctrl->probe_work, amd_sdwc_probe_work); >> schedule_work(&ctrl->probe_work); >> + /* Enable runtime PM */ >> + pm_runtime_set_autosuspend_delay(dev, AMD_SDW_MASTER_SUSPEND_DELAY_MS); >> + pm_runtime_use_autosuspend(dev); >> + pm_runtime_mark_last_busy(dev); >> + pm_runtime_set_active(dev); >> + pm_runtime_enable(dev); >> return 0; >> } >> >> @@ -1398,14 +1416,201 @@ static int amd_sdwc_remove(struct platform_device *pdev) >> amd_disable_sdw_interrupts(ctrl); >> sdw_bus_master_delete(&ctrl->bus); >> ret = amd_disable_sdw_controller(ctrl); >> + pm_runtime_disable(&pdev->dev); >> return ret; >> } >> >> +static int amd_sdwc_clock_stop(struct amd_sdwc_ctrl *ctrl) >> +{ >> + u32 clk_resume_ctrl_reg; >> + u32 wake_en_reg; >> + u32 val; >> + u32 retry_count = 0; >> + int ret; >> + >> + ret = sdw_bus_prep_clk_stop(&ctrl->bus); >> + if (ret < 0 && ret != -ENODATA) { >> + dev_err(ctrl->dev, "prepare clock stop failed %d", ret); >> + return ret; >> + } >> + ret = sdw_bus_clk_stop(&ctrl->bus); >> + if (ret < 0 && ret != -ENODATA) { >> + dev_err(ctrl->dev, "bus clock stop failed %d", ret); >> + return ret; > You need to be very careful here, because returning an error may prevent > the device from suspending. > > If it's safe and possible to recover during the resume step, you > probably want to log the error but let the suspend continue. will fix it. >> + } >> + switch (ctrl->instance) { >> + case ACP_SDW0: >> + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; >> + wake_en_reg = ACP_SW_WAKE_EN; >> + break; >> + case ACP_SDW1: >> + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; >> + wake_en_reg = ACP_SW1_WAKE_EN; >> + break; >> + default: >> + return -EINVAL; >> + } > why not store these offsets during the probe and use them directly here? > You know at probe time which master you're using. will fix it. >> + >> + do { >> + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); >> + if (val & AMD_SDW_CLK_STOP_DONE) { >> + ctrl->clk_stopped = true; >> + break; >> + } >> + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); >> + >> + if (!ctrl->clk_stopped) { >> + dev_err(ctrl->dev, "SDW%x clock stop failed\n", ctrl->instance); >> + return -ETIMEDOUT; >> + } >> + >> + if (ctrl->wake_en_mask) >> + acp_reg_writel(0x01, ctrl->mmio + wake_en_reg); >> + >> + dev_dbg(ctrl->dev, "SDW%x clock stop successful\n", ctrl->instance); >> + return 0; >> +} >> + >> +static int amd_sdwc_clock_stop_exit(struct amd_sdwc_ctrl *ctrl) >> +{ >> + int ret; >> + u32 clk_resume_ctrl_reg; >> + u32 val = 0; >> + u32 retry_count = 0; >> + >> + switch (ctrl->instance) { >> + case ACP_SDW0: >> + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; >> + break; >> + case ACP_SDW1: >> + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; >> + break; >> + default: >> + return -EINVAL; >> + } >> + if (ctrl->clk_stopped) { >> + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); >> + val |= AMD_SDW_CLK_RESUME_REQ; >> + acp_reg_writel(val, ctrl->mmio + clk_resume_ctrl_reg); >> + do { >> + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); >> + if (val & AMD_SDW_CLK_RESUME_DONE) >> + break; >> + usleep_range(10, 100); >> + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); >> + if (val & AMD_SDW_CLK_RESUME_DONE) { >> + acp_reg_writel(0, ctrl->mmio + clk_resume_ctrl_reg); >> + ret = sdw_bus_exit_clk_stop(&ctrl->bus); >> + if (ret < 0) >> + dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret); >> + ctrl->clk_stopped = false; >> + } >> + } >> + if (ctrl->clk_stopped) { >> + dev_err(ctrl->dev, "SDW%x clock stop exit failed\n", ctrl->instance); >> + return -ETIMEDOUT; >> + } >> + >> + dev_dbg(ctrl->dev, "SDW%x clock stop exit successful\n", ctrl->instance); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused amd_suspend_runtime(struct device *dev) >> +{ >> + struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(dev); >> + struct sdw_bus *bus = &ctrl->bus; >> + int ret; >> + >> + if (bus->prop.hw_disabled || !ctrl->startup_done) { > do you have a case where the startup is not done? This was an > Intel-specific thing. We have included startup_done flag in probe_work to check whether Manager has started. In case if manager init sequence fails, then there is no need to apply any PM ops. >> + dev_dbg(bus->dev, "SoundWire master %d is disabled or not-started, ignoring\n", >> + bus->link_id); >> + return 0; >> + } >> + if (ctrl->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { >> + ret = amd_sdwc_clock_stop(ctrl); >> + if (ret) >> + return ret; >> + } else if (ctrl->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { >> + ret = amd_sdwc_clock_stop(ctrl); >> + if (ret) >> + return ret; >> + ret = amd_deinit_sdw_controller(ctrl); >> + if (ret) >> + return ret; >> + } >> + return 0; >> +} >> + >> +static int __maybe_unused amd_resume_runtime(struct device *dev) >> +{ >> + struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(dev); >> + struct sdw_bus *bus = &ctrl->bus; >> + int ret; >> + u32 clk_resume_ctrl_reg; >> + u32 val = 0; >> + u32 retry_count = 0; >> + >> + if (bus->prop.hw_disabled || !ctrl->startup_done) { > same here > >> + dev_dbg(bus->dev, "SoundWire master %d is disabled or not-started, ignoring\n", >> + bus->link_id); >> + return 0; >> + } >> + >> + switch (ctrl->instance) { >> + case ACP_SDW0: >> + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; >> + break; >> + case ACP_SDW1: >> + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; >> + break; >> + default: >> + return -EINVAL; >> + } > select registers in the probe. will fix it. >> + >> + if (ctrl->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { >> + ret = amd_sdwc_clock_stop_exit(ctrl); >> + if (ret) >> + return ret; >> + } else if (ctrl->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { >> + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); >> + if (val) { >> + val |= AMD_SDW_CLK_RESUME_REQ; >> + acp_reg_writel(val, ctrl->mmio + clk_resume_ctrl_reg); >> + do { >> + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); >> + if (val & AMD_SDW_CLK_RESUME_DONE) >> + break; >> + usleep_range(10, 100); >> + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); >> + if (val & AMD_SDW_CLK_RESUME_DONE) { >> + acp_reg_writel(0, ctrl->mmio + clk_resume_ctrl_reg); >> + ctrl->clk_stopped = false; >> + } >> + } >> + sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET); >> + amd_init_sdw_controller(ctrl); >> + amd_enable_sdw_interrupts(ctrl); >> + ret = amd_enable_sdw_controller(ctrl); >> + if (ret) >> + return ret; >> + ret = amd_sdwc_set_frameshape(ctrl, 50, 10); > this should be defined at probe time, using magic numbers like this will > not work in all cases and totally depends on the frame rate and > bandwidth needs. Will fix it. >> + if (ret) >> + return ret; >> + } >> + return 0; >> +}
>>> +static int __maybe_unused amd_suspend_runtime(struct device *dev) >>> +{ >>> + struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(dev); >>> + struct sdw_bus *bus = &ctrl->bus; >>> + int ret; >>> + >>> + if (bus->prop.hw_disabled || !ctrl->startup_done) { >> do you have a case where the startup is not done? This was an >> Intel-specific thing. > We have included startup_done flag in probe_work to check whether Manager > has started. In case if manager init sequence fails, then there is no need > to apply any PM ops. Not following, sorry. We introduced the .startup callback for intel because of a power dependency where we could not access and initialize the registers at the .probe time for the master driver. Do you have a similar dependency, and if not why not remove this flag?
diff --git a/drivers/soundwire/amd_master.c b/drivers/soundwire/amd_master.c index c7063b8bdd7b..d2d7f07de202 100644 --- a/drivers/soundwire/amd_master.c +++ b/drivers/soundwire/amd_master.c @@ -15,6 +15,7 @@ #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_registers.h> #include <linux/soundwire/sdw_amd.h> +#include <linux/pm_runtime.h> #include <linux/wait.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -290,6 +291,17 @@ static int amd_disable_sdw_interrupts(struct amd_sdwc_ctrl *ctrl) return 0; } +static int amd_deinit_sdw_controller(struct amd_sdwc_ctrl *ctrl) +{ + int ret; + + ret = amd_disable_sdw_interrupts(ctrl); + if (ret) + return ret; + ret = amd_disable_sdw_controller(ctrl); + return ret; +} + static int amd_sdwc_set_frameshape(struct amd_sdwc_ctrl *ctrl, u32 rows, u32 cols) { u32 sdw_rows, sdw_cols, frame_size; @@ -1387,6 +1399,12 @@ static int amd_sdwc_probe(struct platform_device *pdev) INIT_WORK(&ctrl->amd_sdw_work, amd_sdwc_update_slave_status_work); INIT_WORK(&ctrl->probe_work, amd_sdwc_probe_work); schedule_work(&ctrl->probe_work); + /* Enable runtime PM */ + pm_runtime_set_autosuspend_delay(dev, AMD_SDW_MASTER_SUSPEND_DELAY_MS); + pm_runtime_use_autosuspend(dev); + pm_runtime_mark_last_busy(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); return 0; } @@ -1398,14 +1416,201 @@ static int amd_sdwc_remove(struct platform_device *pdev) amd_disable_sdw_interrupts(ctrl); sdw_bus_master_delete(&ctrl->bus); ret = amd_disable_sdw_controller(ctrl); + pm_runtime_disable(&pdev->dev); return ret; } +static int amd_sdwc_clock_stop(struct amd_sdwc_ctrl *ctrl) +{ + u32 clk_resume_ctrl_reg; + u32 wake_en_reg; + u32 val; + u32 retry_count = 0; + int ret; + + ret = sdw_bus_prep_clk_stop(&ctrl->bus); + if (ret < 0 && ret != -ENODATA) { + dev_err(ctrl->dev, "prepare clock stop failed %d", ret); + return ret; + } + ret = sdw_bus_clk_stop(&ctrl->bus); + if (ret < 0 && ret != -ENODATA) { + dev_err(ctrl->dev, "bus clock stop failed %d", ret); + return ret; + } + switch (ctrl->instance) { + case ACP_SDW0: + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; + wake_en_reg = ACP_SW_WAKE_EN; + break; + case ACP_SDW1: + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; + wake_en_reg = ACP_SW1_WAKE_EN; + break; + default: + return -EINVAL; + } + + do { + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); + if (val & AMD_SDW_CLK_STOP_DONE) { + ctrl->clk_stopped = true; + break; + } + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); + + if (!ctrl->clk_stopped) { + dev_err(ctrl->dev, "SDW%x clock stop failed\n", ctrl->instance); + return -ETIMEDOUT; + } + + if (ctrl->wake_en_mask) + acp_reg_writel(0x01, ctrl->mmio + wake_en_reg); + + dev_dbg(ctrl->dev, "SDW%x clock stop successful\n", ctrl->instance); + return 0; +} + +static int amd_sdwc_clock_stop_exit(struct amd_sdwc_ctrl *ctrl) +{ + int ret; + u32 clk_resume_ctrl_reg; + u32 val = 0; + u32 retry_count = 0; + + switch (ctrl->instance) { + case ACP_SDW0: + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; + break; + case ACP_SDW1: + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; + break; + default: + return -EINVAL; + } + if (ctrl->clk_stopped) { + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); + val |= AMD_SDW_CLK_RESUME_REQ; + acp_reg_writel(val, ctrl->mmio + clk_resume_ctrl_reg); + do { + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); + if (val & AMD_SDW_CLK_RESUME_DONE) + break; + usleep_range(10, 100); + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); + if (val & AMD_SDW_CLK_RESUME_DONE) { + acp_reg_writel(0, ctrl->mmio + clk_resume_ctrl_reg); + ret = sdw_bus_exit_clk_stop(&ctrl->bus); + if (ret < 0) + dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret); + ctrl->clk_stopped = false; + } + } + if (ctrl->clk_stopped) { + dev_err(ctrl->dev, "SDW%x clock stop exit failed\n", ctrl->instance); + return -ETIMEDOUT; + } + + dev_dbg(ctrl->dev, "SDW%x clock stop exit successful\n", ctrl->instance); + + return 0; +} + +static int __maybe_unused amd_suspend_runtime(struct device *dev) +{ + struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(dev); + struct sdw_bus *bus = &ctrl->bus; + int ret; + + if (bus->prop.hw_disabled || !ctrl->startup_done) { + dev_dbg(bus->dev, "SoundWire master %d is disabled or not-started, ignoring\n", + bus->link_id); + return 0; + } + if (ctrl->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { + ret = amd_sdwc_clock_stop(ctrl); + if (ret) + return ret; + } else if (ctrl->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { + ret = amd_sdwc_clock_stop(ctrl); + if (ret) + return ret; + ret = amd_deinit_sdw_controller(ctrl); + if (ret) + return ret; + } + return 0; +} + +static int __maybe_unused amd_resume_runtime(struct device *dev) +{ + struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(dev); + struct sdw_bus *bus = &ctrl->bus; + int ret; + u32 clk_resume_ctrl_reg; + u32 val = 0; + u32 retry_count = 0; + + if (bus->prop.hw_disabled || !ctrl->startup_done) { + dev_dbg(bus->dev, "SoundWire master %d is disabled or not-started, ignoring\n", + bus->link_id); + return 0; + } + + switch (ctrl->instance) { + case ACP_SDW0: + clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL; + break; + case ACP_SDW1: + clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL; + break; + default: + return -EINVAL; + } + + if (ctrl->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { + ret = amd_sdwc_clock_stop_exit(ctrl); + if (ret) + return ret; + } else if (ctrl->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); + if (val) { + val |= AMD_SDW_CLK_RESUME_REQ; + acp_reg_writel(val, ctrl->mmio + clk_resume_ctrl_reg); + do { + val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg); + if (val & AMD_SDW_CLK_RESUME_DONE) + break; + usleep_range(10, 100); + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); + if (val & AMD_SDW_CLK_RESUME_DONE) { + acp_reg_writel(0, ctrl->mmio + clk_resume_ctrl_reg); + ctrl->clk_stopped = false; + } + } + sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET); + amd_init_sdw_controller(ctrl); + amd_enable_sdw_interrupts(ctrl); + ret = amd_enable_sdw_controller(ctrl); + if (ret) + return ret; + ret = amd_sdwc_set_frameshape(ctrl, 50, 10); + if (ret) + return ret; + } + return 0; +} + +static const struct dev_pm_ops amd_pm = { + SET_RUNTIME_PM_OPS(amd_suspend_runtime, amd_resume_runtime, NULL) +}; + static struct platform_driver amd_sdwc_driver = { .probe = &amd_sdwc_probe, .remove = &amd_sdwc_remove, .driver = { .name = "amd_sdw_controller", + .pm = &amd_pm, } }; module_platform_driver(amd_sdwc_driver); diff --git a/drivers/soundwire/amd_master.h b/drivers/soundwire/amd_master.h index b43a5d6496cb..cc254255512b 100644 --- a/drivers/soundwire/amd_master.h +++ b/drivers/soundwire/amd_master.h @@ -237,6 +237,9 @@ #define AMD_SDW0_PAD_KEEPER_DISABLE_MASK 0x1E #define AMD_SDW1_PAD_KEEPER_DISABLE_MASK 0xF #define AMD_SDW_PREQ_INTR_STAT BIT(19) +#define AMD_SDW_CLK_STOP_DONE 1 +#define AMD_SDW_CLK_RESUME_REQ 2 +#define AMD_SDW_CLK_RESUME_DONE 3 enum amd_sdw_channel { /* SDW0 */ diff --git a/include/linux/soundwire/sdw_amd.h b/include/linux/soundwire/sdw_amd.h index 2db03b2f0c3b..f362f195b887 100644 --- a/include/linux/soundwire/sdw_amd.h +++ b/include/linux/soundwire/sdw_amd.h @@ -38,6 +38,7 @@ struct amd_sdwc_ctrl { u32 quirks; u32 wake_en_mask; int num_ports; + bool clk_stopped; bool startup_done; u32 power_mode_mask; };