lkml.org 
[lkml]   [2023]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 14/19] soundwire: amd: add runtime pm ops for AMD master driver
    From
    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;
    >> +}

    \
     
     \ /
      Last update: 2023-03-26 23:37    [W:6.476 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site