| Message ID | 20240619141413.7983-1-quic_jkona@quicinc.com |
|---|---|
| Headers | show |
| Series | Add control for switching back and forth to HW control | expand |
On 6/19/2024 7:44 PM, Jagadeesh Kona wrote: > For Venus V6 variant SoCs(sm8250, sc7280), the venus driver uses the newly > introduced dev_pm_genpd_set_hwmode() API to switch the vcodec GDSC to > HW/SW control modes at runtime. Hence use HW_CTRL_TRIGGER flag for vcodec > GDSC's on sm8250, sc7280 to register the set_hwmode_dev & get_hwmode_dev > callbacks for vcodec GDSC and allow the GDSC mode to be changed using > dev_pm_genpd_set_hwmode() API. > > Signed-off-by: Jagadeesh Kona<quic_jkona@quicinc.com> > Signed-off-by: Abel Vesa<abel.vesa@linaro.org> > --- > drivers/clk/qcom/videocc-sc7280.c | 2 +- > drivers/clk/qcom/videocc-sm8250.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Taniya Das <quic_tdas@quicinc.com>
On 6/20/2024 3:40 AM, Caleb Connolly wrote: > Hi Jagadeesh, > > Sorry, some grammar nitpicks. > > On 19/06/2024 16:14, Jagadeesh Kona wrote: >> Some GDSC client drivers require the GDSC mode to be switched dynamically >> to HW mode at runtime to gain the power benefits. Typically such client >> drivers require the GDSC to be brought up in SW mode initially to enable >> the required dependent clocks and configure the hardware to proper state. >> Once initial hardware set up is done, they switch the GDSC to HW mode to >> save power. At the end of usecase, they switch the GDSC back to SW mode >> and disable the GDSC. >> >> Introduce HW_CTRL_TRIGGER flag to register the set_hwmode_dev and >> get_hwmode_dev callbacks for GDSC's whose respective client drivers >> require the GDSC mode to be switched dynamically at runtime using >> dev_pm_genpd_set_hwmode() API. >> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> drivers/clk/qcom/gdsc.c | 42 +++++++++++++++++++++++++++++++++++++++++ >> drivers/clk/qcom/gdsc.h | 1 + >> 2 files changed, 43 insertions(+) >> >> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c >> index df9618ab7eea..6acc7af82255 100644 >> --- a/drivers/clk/qcom/gdsc.c >> +++ b/drivers/clk/qcom/gdsc.c >> @@ -363,6 +363,44 @@ static int gdsc_disable(struct generic_pm_domain >> *domain) >> return 0; >> } >> +static int gdsc_set_hwmode(struct generic_pm_domain *domain, struct >> device *dev, bool mode) >> +{ >> + struct gdsc *sc = domain_to_gdsc(domain); >> + int ret; >> + >> + ret = gdsc_hwctrl(sc, mode); >> + if (ret) >> + return ret; >> + >> + /* >> + * Wait for the GDSC to go through a power down and >> + * up cycle. In case SW/FW end up polling status >> + * bits for the gdsc before the power cycle is completed >> + * it might read the status wrongly. > > If we poll the status register before the power cycle is finished we > might read incorrect values. Thanks Caleb for your review. Sure, will take care of these comments in next series. Thanks, Jagadeesh >> + */ >> + udelay(1); >> + >> + /* >> + * When GDSC is switched to HW mode, HW can disable the GDSC. > The GDSC >> + * When GDSC is switched back to SW mode, the GDSC will be enabled > The GDSC >> + * again, hence need to poll for GDSC to complete the power >> uphence we need to poll > > Kind regards, >> + */ >> + if (!mode) >> + return gdsc_poll_status(sc, GDSC_ON); >> + >> + return 0; >> +} >> + >> +static bool gdsc_get_hwmode(struct generic_pm_domain *domain, struct >> device *dev) >> +{ >> + struct gdsc *sc = domain_to_gdsc(domain); >> + u32 val; >> + >> + regmap_read(sc->regmap, sc->gdscr, &val); >> + >> + return !!(val & HW_CONTROL_MASK); >> +} >> + >> static int gdsc_init(struct gdsc *sc) >> { >> u32 mask, val; >> @@ -451,6 +489,10 @@ static int gdsc_init(struct gdsc *sc) >> sc->pd.power_off = gdsc_disable; >> if (!sc->pd.power_on) >> sc->pd.power_on = gdsc_enable; >> + if (sc->flags & HW_CTRL_TRIGGER) { >> + sc->pd.set_hwmode_dev = gdsc_set_hwmode; >> + sc->pd.get_hwmode_dev = gdsc_get_hwmode; >> + } >> ret = pm_genpd_init(&sc->pd, NULL, !on); >> if (ret) >> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h >> index 803512688336..1e2779b823d1 100644 >> --- a/drivers/clk/qcom/gdsc.h >> +++ b/drivers/clk/qcom/gdsc.h >> @@ -67,6 +67,7 @@ struct gdsc { >> #define ALWAYS_ON BIT(6) >> #define RETAIN_FF_ENABLE BIT(7) >> #define NO_RET_PERIPH BIT(8) >> +#define HW_CTRL_TRIGGER BIT(9) >> struct reset_controller_dev *rcdev; >> unsigned int *resets; >> unsigned int reset_count; >
This series adds support for dev_pm_genpd_set_hwmode() and dev_pm_genpd_get_hwmode() APIs and support in gdsc provider drivers to register respective callbacks and venus consumer driver example using above API to switch the power domain(GDSC) to HW/SW modes dynamically at runtime. Changes in V6: - [PATCH 3/5]: Added details for 1usec delay in gdsc_set_hwmode() - [PATCH 4/5]: Updated commit text - Added R-By and T-By tags received on V5 RESEND - Link to V5 RESEND: https://lore.kernel.org/all/20240413152013.22307-1-quic_jkona@quicinc.com/ - Link to V5: https://lore.kernel.org/all/20240315111046.22136-1-quic_jkona@quicinc.com/ Changes in V5: - Updated 1st patch as per V4 review comments to synchronize the initial HW mode state by invoking ->get_hwmode_dev()callback in genpd_add_device() - With above change, SW cached hwmode will contain correct value initially, and it will be updated everytime mode is changed in set_hwmode, hence updated dev_pm_genpd_get_hwmode() to just return SW cached hwmode in 1st patch - Updated commit text for 1st, 3rd, 4th and 5th patches - Updated 3rd and 5th patches as per review comments received on V4 series - Added R-By tags received in older series to 1st and 2nd patches - Link to V4: https://lore.kernel.org/all/20240122-gdsc-hwctrl-v4-0-9061e8a7aa07@linaro.org/ Changes in V4: - Re-worded 1st patch commit message, as per Bjorn's suggestion, and added Dmitry's R-b tag - Added Bjorn's and Dmitry's R-b tags to the 2nd patch - Re-worded 3rd patch commit message, to better explain the HW_CTRL_TRIGGER flag. - Added mode transition delay when setting mode for GDSC - Added status polling if GDSSC is enabled when transitioning from HW to SW - Re-worded 4th patch commit message to better explain why the HW_CTRL_TRIGGER needs to be used instead - Drop changes to SC7180, SDM845 and SM8550 video CC drivers, as only SC7280 and SM8250 have been tested so far. More platforms (with v6 venus) will be added eventually. - Call genpd set_hwmode API only for v6 and dropped the vcodec_pmdomains_hwctrl. - Re-worded 5th patch commit message accordingly. - Link to V3: https://lore.kernel.org/lkml/20230823114528.3677667-1-abel.vesa@linaro.org/ Changes in V3: - 5th patch has been squashed in the 4th one - Link to V2: https://lore.kernel.org/lkml/20230816145741.1472721-1-abel.vesa@linaro.org/ Changes in V2: - patch for printing domain HW-managed mode in the summary - patch that adds one consumer (venus) - patch for gdsc with new (different) flag - patch for videocc GDSC provider to update flags - Link to V1: https://lore.kernel.org/all/20230628105652.1670316-1-abel.vesa@linaro.org/ Abel Vesa (1): PM: domains: Add the domain HW-managed mode to the summary Jagadeesh Kona (3): clk: qcom: gdsc: Add set and get hwmode callbacks to switch GDSC mode clk: qcom: videocc: Use HW_CTRL_TRIGGER for SM8250, SC7280 vcodec GDSC's venus: pm_helpers: Use dev_pm_genpd_set_hwmode to switch GDSC mode on V6 Ulf Hansson (1): PM: domains: Allow devices attached to genpd to be managed by HW drivers/clk/qcom/gdsc.c | 42 ++++++++++ drivers/clk/qcom/gdsc.h | 1 + drivers/clk/qcom/videocc-sc7280.c | 2 +- drivers/clk/qcom/videocc-sm8250.c | 4 +- .../media/platform/qcom/venus/pm_helpers.c | 39 ++++++---- drivers/pmdomain/core.c | 78 ++++++++++++++++++- include/linux/pm_domain.h | 17 ++++ 7 files changed, 162 insertions(+), 21 deletions(-)