| Message ID | 20250409093025.2917087-3-m.wilczynski@samsung.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v1,1/2] dt-bindings: firmware: thead,th1520: Add clocks and resets | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | success | Success |
| bjorn/build-rv32-defconfig | success | build-rv32-defconfig |
| bjorn/build-rv64-clang-allmodconfig | success | build-rv64-clang-allmodconfig |
| bjorn/build-rv64-gcc-allmodconfig | success | build-rv64-gcc-allmodconfig |
| bjorn/build-rv64-nommu-k210-defconfig | success | build-rv64-nommu-k210-defconfig |
| bjorn/build-rv64-nommu-k210-virt | success | build-rv64-nommu-k210-virt |
| bjorn/checkpatch | success | checkpatch |
| bjorn/dtb-warn-rv64 | success | dtb-warn-rv64 |
| bjorn/header-inline | success | header-inline |
| bjorn/kdoc | success | kdoc |
| bjorn/module-param | success | module-param |
| bjorn/verify-fixes | success | verify-fixes |
| bjorn/verify-signedoff | success | verify-signedoff |
On Wed, 9 Apr 2025 at 11:30, Michal Wilczynski <m.wilczynski@samsung.com> wrote: > > Extend the TH1520 power domain driver to manage GPU related clocks and > resets via generic PM domain start/stop callbacks. > > The TH1520 GPU requires a special sequence to correctly initialize: > - Enable the GPU clocks > - Deassert the GPU clkgen reset > - Delay for a few cycles to satisfy hardware requirements > - Deassert the GPU core reset > > This sequence is SoC-specific and needs to be abstracted away from the > Imagination GPU driver, which expects a standard single reset line. > Following discussions with kernel maintainers, this logic is placed > inside a PM domain instead of polluting the clock or reset frameworks, > or the GPU driver itself [1]. > > Managing this inside a generic power domain allows better coordination > of clocks, resets, and power state, and aligns with the direction of > treating PM domains as SoC-specific "power management drivers". > > [1] - https://lore.kernel.org/all/CAPDyKFqsJaTrF0tBSY-TjpqdVt5=6aPQHYfnDebtphfRZSU=-Q@mail.gmail.com/ > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- > drivers/pmdomain/thead/th1520-pm-domains.c | 119 +++++++++++++++++++++ > 1 file changed, 119 insertions(+) > > diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c > index f702e20306f4..aa85c3954c39 100644 > --- a/drivers/pmdomain/thead/th1520-pm-domains.c > +++ b/drivers/pmdomain/thead/th1520-pm-domains.c > @@ -5,17 +5,29 @@ > * Author: Michal Wilczynski <m.wilczynski@samsung.com> > */ > > +#include <linux/clk.h> > +#include <linux/delay.h> > #include <linux/firmware/thead/thead,th1520-aon.h> > #include <linux/slab.h> > #include <linux/platform_device.h> > #include <linux/pm_domain.h> > +#include <linux/reset.h> > > #include <dt-bindings/power/thead,th1520-power.h> > > +#define TH1520_GPU_RESET_IDX 0 > +#define TH1520_GPU_CLKGEN_RESET_IDX 1 > + > struct th1520_power_domain { > struct th1520_aon_chan *aon_chan; > struct generic_pm_domain genpd; > u32 rsrc; > + > + struct clk_bulk_data *clks; > + int num_clks; > + struct reset_control_bulk_data *resets; > + int num_resets; > + > }; > > struct th1520_power_info { > @@ -61,6 +73,99 @@ static int th1520_pd_power_off(struct generic_pm_domain *domain) > return th1520_aon_power_update(pd->aon_chan, pd->rsrc, false); > } > > +static int th1520_gpu_init_clocks(struct device *dev, > + struct th1520_power_domain *pd) > +{ > + static const char *const clk_names[] = { "gpu-core", "gpu-sys" }; > + int i, ret; > + > + pd->num_clks = ARRAY_SIZE(clk_names); > + pd->clks = devm_kcalloc(dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL); > + if (!pd->clks) > + return -ENOMEM; > + > + for (i = 0; i < pd->num_clks; i++) > + pd->clks[i].id = clk_names[i]; > + > + ret = devm_clk_bulk_get(dev, pd->num_clks, pd->clks); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to get GPU clocks\n"); > + > + return 0; > +} > + > +static int th1520_gpu_init_resets(struct device *dev, > + struct th1520_power_domain *pd) > +{ > + static const char *const reset_names[] = { "gpu", "gpu-clkgen" }; > + int i, ret; > + > + pd->num_resets = ARRAY_SIZE(reset_names); > + pd->resets = devm_kcalloc(dev, pd->num_resets, sizeof(*pd->resets), > + GFP_KERNEL); > + if (!pd->resets) > + return -ENOMEM; > + > + for (i = 0; i < pd->num_resets; i++) > + pd->resets[i].id = reset_names[i]; > + > + ret = devm_reset_control_bulk_get_exclusive(dev, pd->num_resets, > + pd->resets); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to get GPU resets\n"); > + > + return 0; > +} > + > +static int th1520_gpu_domain_start(struct device *dev) > +{ > + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain); > + struct th1520_power_domain *pd = to_th1520_power_domain(genpd); > + int ret; > + > + ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks); > + if (ret) > + return ret; > + > + ret = reset_control_deassert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); > + if (ret) > + goto err_disable_clks; > + > + /* > + * According to the hardware manual, a delay of at least 32 clock > + * cycles is required between de-asserting the clkgen reset and > + * de-asserting the GPU reset. Assuming a worst-case scenario with > + * a very high GPU clock frequency, a delay of 1 microsecond is > + * sufficient to ensure this requirement is met across all > + * feasible GPU clock speeds. > + */ > + udelay(1); > + > + ret = reset_control_deassert(pd->resets[TH1520_GPU_RESET_IDX].rstc); > + if (ret) > + goto err_assert_clkgen; > + > + return 0; > + > +err_assert_clkgen: > + reset_control_assert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); > +err_disable_clks: > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > + return ret; > +} > + > +static int th1520_gpu_domain_stop(struct device *dev) > +{ > + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain); > + struct th1520_power_domain *pd = to_th1520_power_domain(genpd); > + > + reset_control_assert(pd->resets[TH1520_GPU_RESET_IDX].rstc); > + reset_control_assert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > + > + return 0; > +} > + > static struct generic_pm_domain *th1520_pd_xlate(const struct of_phandle_args *spec, > void *data) > { > @@ -99,6 +204,20 @@ th1520_add_pm_domain(struct device *dev, const struct th1520_power_info *pi) > pd->genpd.power_off = th1520_pd_power_off; > pd->genpd.name = pi->name; > > + /* there are special callbacks for the GPU */ > + if (pi == &th1520_pd_ranges[TH1520_GPU_PD]) { > + ret = th1520_gpu_init_clocks(dev, pd); Assuming both the clocks and the reset are really resources that belong to the GPU, I would suggest to use two callbacks for the genpd in question to implement the get/put of the clocks and reset from there. pd->genpd->attach_dev pd->genpd->detach_dev > + if (ret) > + return ERR_PTR(ret); > + > + ret = th1520_gpu_init_resets(dev, pd); > + if (ret) > + return ERR_PTR(ret); > + > + pd->genpd.dev_ops.start = th1520_gpu_domain_start; > + pd->genpd.dev_ops.stop = th1520_gpu_domain_stop; > + } > + > ret = pm_genpd_init(&pd->genpd, NULL, true); > if (ret) > return ERR_PTR(ret); > -- > 2.34.1 > Kind regards Uffe
diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c index f702e20306f4..aa85c3954c39 100644 --- a/drivers/pmdomain/thead/th1520-pm-domains.c +++ b/drivers/pmdomain/thead/th1520-pm-domains.c @@ -5,17 +5,29 @@ * Author: Michal Wilczynski <m.wilczynski@samsung.com> */ +#include <linux/clk.h> +#include <linux/delay.h> #include <linux/firmware/thead/thead,th1520-aon.h> #include <linux/slab.h> #include <linux/platform_device.h> #include <linux/pm_domain.h> +#include <linux/reset.h> #include <dt-bindings/power/thead,th1520-power.h> +#define TH1520_GPU_RESET_IDX 0 +#define TH1520_GPU_CLKGEN_RESET_IDX 1 + struct th1520_power_domain { struct th1520_aon_chan *aon_chan; struct generic_pm_domain genpd; u32 rsrc; + + struct clk_bulk_data *clks; + int num_clks; + struct reset_control_bulk_data *resets; + int num_resets; + }; struct th1520_power_info { @@ -61,6 +73,99 @@ static int th1520_pd_power_off(struct generic_pm_domain *domain) return th1520_aon_power_update(pd->aon_chan, pd->rsrc, false); } +static int th1520_gpu_init_clocks(struct device *dev, + struct th1520_power_domain *pd) +{ + static const char *const clk_names[] = { "gpu-core", "gpu-sys" }; + int i, ret; + + pd->num_clks = ARRAY_SIZE(clk_names); + pd->clks = devm_kcalloc(dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL); + if (!pd->clks) + return -ENOMEM; + + for (i = 0; i < pd->num_clks; i++) + pd->clks[i].id = clk_names[i]; + + ret = devm_clk_bulk_get(dev, pd->num_clks, pd->clks); + if (ret) + return dev_err_probe(dev, ret, "Failed to get GPU clocks\n"); + + return 0; +} + +static int th1520_gpu_init_resets(struct device *dev, + struct th1520_power_domain *pd) +{ + static const char *const reset_names[] = { "gpu", "gpu-clkgen" }; + int i, ret; + + pd->num_resets = ARRAY_SIZE(reset_names); + pd->resets = devm_kcalloc(dev, pd->num_resets, sizeof(*pd->resets), + GFP_KERNEL); + if (!pd->resets) + return -ENOMEM; + + for (i = 0; i < pd->num_resets; i++) + pd->resets[i].id = reset_names[i]; + + ret = devm_reset_control_bulk_get_exclusive(dev, pd->num_resets, + pd->resets); + if (ret) + return dev_err_probe(dev, ret, "Failed to get GPU resets\n"); + + return 0; +} + +static int th1520_gpu_domain_start(struct device *dev) +{ + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain); + struct th1520_power_domain *pd = to_th1520_power_domain(genpd); + int ret; + + ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks); + if (ret) + return ret; + + ret = reset_control_deassert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); + if (ret) + goto err_disable_clks; + + /* + * According to the hardware manual, a delay of at least 32 clock + * cycles is required between de-asserting the clkgen reset and + * de-asserting the GPU reset. Assuming a worst-case scenario with + * a very high GPU clock frequency, a delay of 1 microsecond is + * sufficient to ensure this requirement is met across all + * feasible GPU clock speeds. + */ + udelay(1); + + ret = reset_control_deassert(pd->resets[TH1520_GPU_RESET_IDX].rstc); + if (ret) + goto err_assert_clkgen; + + return 0; + +err_assert_clkgen: + reset_control_assert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); +err_disable_clks: + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); + return ret; +} + +static int th1520_gpu_domain_stop(struct device *dev) +{ + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain); + struct th1520_power_domain *pd = to_th1520_power_domain(genpd); + + reset_control_assert(pd->resets[TH1520_GPU_RESET_IDX].rstc); + reset_control_assert(pd->resets[TH1520_GPU_CLKGEN_RESET_IDX].rstc); + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); + + return 0; +} + static struct generic_pm_domain *th1520_pd_xlate(const struct of_phandle_args *spec, void *data) { @@ -99,6 +204,20 @@ th1520_add_pm_domain(struct device *dev, const struct th1520_power_info *pi) pd->genpd.power_off = th1520_pd_power_off; pd->genpd.name = pi->name; + /* there are special callbacks for the GPU */ + if (pi == &th1520_pd_ranges[TH1520_GPU_PD]) { + ret = th1520_gpu_init_clocks(dev, pd); + if (ret) + return ERR_PTR(ret); + + ret = th1520_gpu_init_resets(dev, pd); + if (ret) + return ERR_PTR(ret); + + pd->genpd.dev_ops.start = th1520_gpu_domain_start; + pd->genpd.dev_ops.stop = th1520_gpu_domain_stop; + } + ret = pm_genpd_init(&pd->genpd, NULL, true); if (ret) return ERR_PTR(ret);
Extend the TH1520 power domain driver to manage GPU related clocks and resets via generic PM domain start/stop callbacks. The TH1520 GPU requires a special sequence to correctly initialize: - Enable the GPU clocks - Deassert the GPU clkgen reset - Delay for a few cycles to satisfy hardware requirements - Deassert the GPU core reset This sequence is SoC-specific and needs to be abstracted away from the Imagination GPU driver, which expects a standard single reset line. Following discussions with kernel maintainers, this logic is placed inside a PM domain instead of polluting the clock or reset frameworks, or the GPU driver itself [1]. Managing this inside a generic power domain allows better coordination of clocks, resets, and power state, and aligns with the direction of treating PM domains as SoC-specific "power management drivers". [1] - https://lore.kernel.org/all/CAPDyKFqsJaTrF0tBSY-TjpqdVt5=6aPQHYfnDebtphfRZSU=-Q@mail.gmail.com/ Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/pmdomain/thead/th1520-pm-domains.c | 119 +++++++++++++++++++++ 1 file changed, 119 insertions(+)