| Message ID | 20230213094031.2231058-7-Vijendar.Mukunda@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2,1/8] soundwire: export sdw_compute_slave_ports() function | expand |
On 2/13/23 03:40, Vijendar Mukunda wrote: > Add support for runtime pm ops for AMD soundwire manager driver. > > Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> > Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com> > --- > drivers/soundwire/amd_manager.c | 163 ++++++++++++++++++++++++++++++ > drivers/soundwire/amd_manager.h | 3 + > include/linux/soundwire/sdw_amd.h | 16 +++ > 3 files changed, 182 insertions(+) > > diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c > index 87f9a987d93a..eced189ba6e0 100644 > --- a/drivers/soundwire/amd_manager.c > +++ b/drivers/soundwire/amd_manager.c > @@ -14,6 +14,7 @@ > #include <linux/slab.h> > #include <linux/soundwire/sdw.h> > #include <linux/soundwire/sdw_registers.h> > +#include <linux/pm_runtime.h> > #include <linux/wait.h> > #include <sound/pcm_params.h> > #include <sound/soc.h> > @@ -185,6 +186,15 @@ static void amd_disable_sdw_interrupts(struct amd_sdw_manager *amd_manager) > acp_reg_writel(0x00, amd_manager->mmio + ACP_SW_ERROR_INTR_MASK); > } > > +static int amd_deinit_sdw_manager(struct amd_sdw_manager *amd_manager) > +{ > + int ret; > + > + amd_disable_sdw_interrupts(amd_manager); > + ret = amd_disable_sdw_manager(amd_manager); > + return ret; > +} > + > static void amd_sdw_set_frameshape(struct amd_sdw_manager *amd_manager) > { > u32 frame_size; > @@ -1043,6 +1053,12 @@ static int amd_sdw_manager_probe(struct platform_device *pdev) > INIT_WORK(&amd_manager->amd_sdw_work, amd_sdw_update_slave_status_work); > INIT_WORK(&amd_manager->probe_work, amd_sdw_probe_work); > schedule_work(&amd_manager->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); that doesn't sound good to me, why do this here and not in the work function? That creates a racy case where the device might suspend before being initialized. > return 0; > } > > @@ -1057,14 +1073,161 @@ static int amd_sdw_manager_remove(struct platform_device *pdev) > amd_disable_sdw_interrupts(amd_manager); > sdw_bus_master_delete(&amd_manager->bus); > ret = amd_disable_sdw_manager(amd_manager); > + pm_runtime_disable(&pdev->dev); shouldn't you do the pm_runtime_disable first? > return ret; > } > +/* AMD pm_runtime quirk definitions */ > + > +/* > + * Force the clock to stop(ClockStopMode0) when suspend callback > + * is invoked. > + */ > +#define AMD_SDW_CLK_STOP_MODE 1 > + > +/* > + * Stop the bus when runtime suspend/system level suspend callback > + * is invoked. If set, a complete bus reset and re-enumeration will > + * be performed when the bus restarts. > + */ > +#define AMD_SDW_POWER_OFF_MODE 2 You need to clarify this mode, can you deal with device in-band wakes if the power is off? > #define ACP_SDW0 0 > #define ACP_SDW1 1 > > @@ -57,6 +71,7 @@ struct sdw_amd_dai_runtime { > * @instance: soundwire manager instance > * @quirks: soundwire manager quirks > * @wake_en_mask: wake enable mask per soundwire manager > + * @clk_stopped: flag set to true when clock is stopped > * @power_mode_mask: flag interprets amd soundwire manager power mode > * @dai_runtime_array: dai runtime array > */ > @@ -86,6 +101,7 @@ struct amd_sdw_manager { > u32 quirks; > u32 wake_en_mask; > u32 power_mode_mask; > + bool clk_stopped; > > struct sdw_amd_dai_runtime **dai_runtime_array; > };
On 13/02/23 23:50, Pierre-Louis Bossart wrote: > > On 2/13/23 03:40, Vijendar Mukunda wrote: >> Add support for runtime pm ops for AMD soundwire manager driver. >> >> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> >> Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com> >> --- >> drivers/soundwire/amd_manager.c | 163 ++++++++++++++++++++++++++++++ >> drivers/soundwire/amd_manager.h | 3 + >> include/linux/soundwire/sdw_amd.h | 16 +++ >> 3 files changed, 182 insertions(+) >> >> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c >> index 87f9a987d93a..eced189ba6e0 100644 >> --- a/drivers/soundwire/amd_manager.c >> +++ b/drivers/soundwire/amd_manager.c >> @@ -14,6 +14,7 @@ >> #include <linux/slab.h> >> #include <linux/soundwire/sdw.h> >> #include <linux/soundwire/sdw_registers.h> >> +#include <linux/pm_runtime.h> >> #include <linux/wait.h> >> #include <sound/pcm_params.h> >> #include <sound/soc.h> >> @@ -185,6 +186,15 @@ static void amd_disable_sdw_interrupts(struct amd_sdw_manager *amd_manager) >> acp_reg_writel(0x00, amd_manager->mmio + ACP_SW_ERROR_INTR_MASK); >> } >> >> +static int amd_deinit_sdw_manager(struct amd_sdw_manager *amd_manager) >> +{ >> + int ret; >> + >> + amd_disable_sdw_interrupts(amd_manager); >> + ret = amd_disable_sdw_manager(amd_manager); >> + return ret; >> +} >> + >> static void amd_sdw_set_frameshape(struct amd_sdw_manager *amd_manager) >> { >> u32 frame_size; >> @@ -1043,6 +1053,12 @@ static int amd_sdw_manager_probe(struct platform_device *pdev) >> INIT_WORK(&amd_manager->amd_sdw_work, amd_sdw_update_slave_status_work); >> INIT_WORK(&amd_manager->probe_work, amd_sdw_probe_work); >> schedule_work(&amd_manager->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); > that doesn't sound good to me, why do this here and not in the work > function? That creates a racy case where the device might suspend before > being initialized. This can be moved to work function. > >> return 0; >> } >> >> @@ -1057,14 +1073,161 @@ static int amd_sdw_manager_remove(struct platform_device *pdev) >> amd_disable_sdw_interrupts(amd_manager); >> sdw_bus_master_delete(&amd_manager->bus); >> ret = amd_disable_sdw_manager(amd_manager); >> + pm_runtime_disable(&pdev->dev); > shouldn't you do the pm_runtime_disable first? I agree. Will fix it. > >> return ret; >> } >> +/* AMD pm_runtime quirk definitions */ >> + >> +/* >> + * Force the clock to stop(ClockStopMode0) when suspend callback >> + * is invoked. >> + */ >> +#define AMD_SDW_CLK_STOP_MODE 1 >> + >> +/* >> + * Stop the bus when runtime suspend/system level suspend callback >> + * is invoked. If set, a complete bus reset and re-enumeration will >> + * be performed when the bus restarts. >> + */ >> +#define AMD_SDW_POWER_OFF_MODE 2 > You need to clarify this mode, can you deal with device in-band wakes if > the power is off? On the Current platform, in-band wakes are not supported if the power is off. > >> #define ACP_SDW0 0 >> #define ACP_SDW1 1 >> >> @@ -57,6 +71,7 @@ struct sdw_amd_dai_runtime { >> * @instance: soundwire manager instance >> * @quirks: soundwire manager quirks >> * @wake_en_mask: wake enable mask per soundwire manager >> + * @clk_stopped: flag set to true when clock is stopped >> * @power_mode_mask: flag interprets amd soundwire manager power mode >> * @dai_runtime_array: dai runtime array >> */ >> @@ -86,6 +101,7 @@ struct amd_sdw_manager { >> u32 quirks; >> u32 wake_en_mask; >> u32 power_mode_mask; >> + bool clk_stopped; >> >> struct sdw_amd_dai_runtime **dai_runtime_array; >> };
>>> +/* AMD pm_runtime quirk definitions */ >>> + >>> +/* >>> + * Force the clock to stop(ClockStopMode0) when suspend callback >>> + * is invoked. >>> + */ >>> +#define AMD_SDW_CLK_STOP_MODE 1 >>> + >>> +/* >>> + * Stop the bus when runtime suspend/system level suspend callback >>> + * is invoked. If set, a complete bus reset and re-enumeration will >>> + * be performed when the bus restarts. >>> + */ >>> +#define AMD_SDW_POWER_OFF_MODE 2 >> You need to clarify this mode, can you deal with device in-band wakes if >> the power is off? > On the Current platform, in-band wakes are not supported if the power is off. Humm, that's an important clarification. Does this mean your link0 will never use the POWER_OFF_MODE since it includes support for headset codecs and you want the ability to detect jacks status changes? Or is this more like CLK_STOP_MODE is used in runtime pm and POWER_OFF in system suspend?
On 14/02/23 19:03, Pierre-Louis Bossart wrote: > >>>> +/* AMD pm_runtime quirk definitions */ >>>> + >>>> +/* >>>> + * Force the clock to stop(ClockStopMode0) when suspend callback >>>> + * is invoked. >>>> + */ >>>> +#define AMD_SDW_CLK_STOP_MODE 1 >>>> + >>>> +/* >>>> + * Stop the bus when runtime suspend/system level suspend callback >>>> + * is invoked. If set, a complete bus reset and re-enumeration will >>>> + * be performed when the bus restarts. >>>> + */ >>>> +#define AMD_SDW_POWER_OFF_MODE 2 >>> You need to clarify this mode, can you deal with device in-band wakes if >>> the power is off? >> On the Current platform, in-band wakes are not supported if the power is off. > Humm, that's an important clarification. > > Does this mean your link0 will never use the POWER_OFF_MODE since it > includes support for headset codecs and you want the ability to detect > jacks status changes? Yes. we will use CLK_STOP_MODE in runtime pm and system suspend for Link0 when jack codec is connected over Link 0. > > Or is this more like CLK_STOP_MODE is used in runtime pm and POWER_OFF > in system suspend?
diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c index 87f9a987d93a..eced189ba6e0 100644 --- a/drivers/soundwire/amd_manager.c +++ b/drivers/soundwire/amd_manager.c @@ -14,6 +14,7 @@ #include <linux/slab.h> #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_registers.h> +#include <linux/pm_runtime.h> #include <linux/wait.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -185,6 +186,15 @@ static void amd_disable_sdw_interrupts(struct amd_sdw_manager *amd_manager) acp_reg_writel(0x00, amd_manager->mmio + ACP_SW_ERROR_INTR_MASK); } +static int amd_deinit_sdw_manager(struct amd_sdw_manager *amd_manager) +{ + int ret; + + amd_disable_sdw_interrupts(amd_manager); + ret = amd_disable_sdw_manager(amd_manager); + return ret; +} + static void amd_sdw_set_frameshape(struct amd_sdw_manager *amd_manager) { u32 frame_size; @@ -1043,6 +1053,12 @@ static int amd_sdw_manager_probe(struct platform_device *pdev) INIT_WORK(&amd_manager->amd_sdw_work, amd_sdw_update_slave_status_work); INIT_WORK(&amd_manager->probe_work, amd_sdw_probe_work); schedule_work(&amd_manager->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; } @@ -1057,14 +1073,161 @@ static int amd_sdw_manager_remove(struct platform_device *pdev) amd_disable_sdw_interrupts(amd_manager); sdw_bus_master_delete(&amd_manager->bus); ret = amd_disable_sdw_manager(amd_manager); + pm_runtime_disable(&pdev->dev); return ret; } +static int amd_sdw_clock_stop(struct amd_sdw_manager *amd_manager) +{ + u32 val; + u32 retry_count = 0; + int ret; + + ret = sdw_bus_prep_clk_stop(&amd_manager->bus); + if (ret < 0 && ret != -ENODATA) { + dev_err(amd_manager->dev, "prepare clock stop failed %d", ret); + return 0; + } + ret = sdw_bus_clk_stop(&amd_manager->bus); + if (ret < 0 && ret != -ENODATA) { + dev_err(amd_manager->dev, "bus clock stop failed %d", ret); + return 0; + } + + do { + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + if (val & AMD_SDW_CLK_STOP_DONE) { + amd_manager->clk_stopped = true; + break; + } + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT); + + if (!amd_manager->clk_stopped) { + dev_err(amd_manager->dev, "SDW%x clock stop failed\n", amd_manager->instance); + return 0; + } + + if (amd_manager->wake_en_mask) + acp_reg_writel(0x01, amd_manager->acp_mmio + ACP_SW_WAKE_EN(amd_manager->instance)); + + dev_dbg(amd_manager->dev, "SDW%x clock stop successful\n", amd_manager->instance); + return 0; +} + +static int amd_sdw_clock_stop_exit(struct amd_sdw_manager *amd_manager) +{ + int ret; + u32 val = 0; + u32 retry_count = 0; + + if (amd_manager->clk_stopped) { + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + val |= AMD_SDW_CLK_RESUME_REQ; + acp_reg_writel(val, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + do { + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + 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, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + ret = sdw_bus_exit_clk_stop(&amd_manager->bus); + if (ret < 0) + dev_err(amd_manager->dev, "bus failed to exit clock stop %d\n", + ret); + amd_manager->clk_stopped = false; + } + } + if (amd_manager->clk_stopped) { + dev_err(amd_manager->dev, "SDW%x clock stop exit failed\n", amd_manager->instance); + return 0; + } + dev_dbg(amd_manager->dev, "SDW%x clock stop exit successful\n", amd_manager->instance); + return 0; +} + +static int __maybe_unused amd_suspend_runtime(struct device *dev) +{ + struct amd_sdw_manager *amd_manager = dev_get_drvdata(dev); + struct sdw_bus *bus = &amd_manager->bus; + int ret; + + if (bus->prop.hw_disabled) { + dev_dbg(bus->dev, "Soundwire manager %d is disabled,\n", + bus->link_id); + return 0; + } + if (amd_manager->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { + ret = amd_sdw_clock_stop(amd_manager); + if (ret) + return ret; + } else if (amd_manager->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { + ret = amd_sdw_clock_stop(amd_manager); + if (ret) + return ret; + ret = amd_deinit_sdw_manager(amd_manager); + if (ret) + return ret; + } + return 0; +} + +static int __maybe_unused amd_resume_runtime(struct device *dev) +{ + struct amd_sdw_manager *amd_manager = dev_get_drvdata(dev); + struct sdw_bus *bus = &amd_manager->bus; + int ret; + u32 val = 0; + u32 retry_count = 0; + + if (bus->prop.hw_disabled) { + dev_dbg(bus->dev, "Soundwire manager %d is disabled, ignoring\n", + bus->link_id); + return 0; + } + + if (amd_manager->power_mode_mask & AMD_SDW_CLK_STOP_MODE) { + ret = amd_sdw_clock_stop_exit(amd_manager); + if (ret) + return ret; + } else if (amd_manager->power_mode_mask & AMD_SDW_POWER_OFF_MODE) { + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + if (val) { + val |= AMD_SDW_CLK_RESUME_REQ; + acp_reg_writel(val, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + do { + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + 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, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL); + amd_manager->clk_stopped = false; + } + } + sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET); + amd_init_sdw_manager(amd_manager); + amd_enable_sdw_interrupts(amd_manager); + ret = amd_enable_sdw_manager(amd_manager); + if (ret) + return ret; + amd_sdw_set_frameshape(amd_manager); + } + 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_sdw_driver = { .probe = &amd_sdw_manager_probe, .remove = &amd_sdw_manager_remove, .driver = { .name = "amd_sdw_manager", + .pm = &amd_pm, } }; module_platform_driver(amd_sdw_driver); diff --git a/drivers/soundwire/amd_manager.h b/drivers/soundwire/amd_manager.h index 5bcaf7a763bb..6ec37612ae4e 100644 --- a/drivers/soundwire/amd_manager.h +++ b/drivers/soundwire/amd_manager.h @@ -187,6 +187,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_cmd_type { AMD_SDW_CMD_PING = 0, diff --git a/include/linux/soundwire/sdw_amd.h b/include/linux/soundwire/sdw_amd.h index 003b8a197fc5..9074c7736ff7 100644 --- a/include/linux/soundwire/sdw_amd.h +++ b/include/linux/soundwire/sdw_amd.h @@ -8,6 +8,20 @@ #include <linux/soundwire/sdw.h> +/* AMD pm_runtime quirk definitions */ + +/* + * Force the clock to stop(ClockStopMode0) when suspend callback + * is invoked. + */ +#define AMD_SDW_CLK_STOP_MODE 1 + +/* + * Stop the bus when runtime suspend/system level suspend callback + * is invoked. If set, a complete bus reset and re-enumeration will + * be performed when the bus restarts. + */ +#define AMD_SDW_POWER_OFF_MODE 2 #define ACP_SDW0 0 #define ACP_SDW1 1 @@ -57,6 +71,7 @@ struct sdw_amd_dai_runtime { * @instance: soundwire manager instance * @quirks: soundwire manager quirks * @wake_en_mask: wake enable mask per soundwire manager + * @clk_stopped: flag set to true when clock is stopped * @power_mode_mask: flag interprets amd soundwire manager power mode * @dai_runtime_array: dai runtime array */ @@ -86,6 +101,7 @@ struct amd_sdw_manager { u32 quirks; u32 wake_en_mask; u32 power_mode_mask; + bool clk_stopped; struct sdw_amd_dai_runtime **dai_runtime_array; };