| Message ID | 20250614-apr_14_for_sending-v4-1-8e3945c819cd@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add TH1520 GPU support with power sequencing | expand |
On Sat, Jun 14, 2025 at 8:09 PM Michal Wilczynski <m.wilczynski@samsung.com> wrote: > > Introduce the pwrseq-thead-gpu driver, a power sequencer provider for > the Imagination BXM-4-64 GPU on the T-HEAD TH1520 SoC. This driver is > an auxiliary driver instantiated by the AON power domain driver. Just a technicality: this driver controls an auxiliary *device* instantiated by the AON power domain driver. > > The TH1520 GPU requires a specific sequence to correctly initialize and > power down its resources: > - Enable GPU clocks (core and sys). > - De-assert the GPU clock generator reset (clkgen_reset). > - Introduce a short hardware-required delay. > - De-assert the GPU core reset. The power-down sequence performs these > steps in reverse. > > Implement this sequence via the pwrseq_power_on and pwrseq_power_off > callbacks. > > Crucially, the driver's match function is called when a consumer (the > Imagination GPU driver) requests the "gpu-power" target. During this > match, the sequencer uses devm_clk_bulk_get() and > devm_reset_control_get_exclusive() on the consumer's device to obtain > handles to the GPU's "core" and "sys" clocks, and the GPU core reset. > These, along with clkgen_reset obtained from parent aon node, allow it > to perform the complete sequence. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- [snip] > + > +static int pwrseq_thead_gpu_power_on(struct pwrseq_device *pwrseq) Please follow the naming convention of the callbacks: this should be pwrseq_thead_gpu_enable(). [snip] > + > +static int pwrseq_thead_gpu_power_off(struct pwrseq_device *pwrseq) Same here. [snip] > +static int pwrseq_thead_gpu_match(struct pwrseq_device *pwrseq, > + struct device *dev) > +{ > + struct pwrseq_thead_gpu_ctx *ctx = pwrseq_device_get_drvdata(pwrseq); > + static const char *const clk_names[] = { "core", "sys" }; > + struct of_phandle_args pwr_spec; > + int i, ret; > + > + /* We only match the specific T-HEAD TH1520 GPU compatible */ > + if (!of_device_is_compatible(dev->of_node, "thead,th1520-gpu")) > + return 0; > + > + ret = of_parse_phandle_with_args(dev->of_node, "power-domains", > + "#power-domain-cells", 0, &pwr_spec); > + if (ret) > + return 0; > + > + /* Additionally verify consumer device has AON as power-domain */ > + if (pwr_spec.np != ctx->aon_node || pwr_spec.args[0] != TH1520_GPU_PD) { > + of_node_put(pwr_spec.np); > + return 0; > + } > + > + of_node_put(pwr_spec.np); > + > + /* Prevent multiple consumers from attaching */ > + if (ctx->gpu_reset || ctx->clks) > + return -EBUSY; Isn't it the whole point of pwrseq - to allow multiple consumers to seamlessly attach to the provider and control the underlying resources in a safe way? I think you should just not request the relevant resources for the second time (really only applies to the exclusive reset and even then it's not clear why it needs to be exclusive) but still return 1 for a valid consumer and let pwrseq handle the refcount? Also: can this even happen at all? > + > + ctx->num_clks = ARRAY_SIZE(clk_names); > + ctx->clks = devm_kcalloc(dev, ctx->num_clks, sizeof(*ctx->clks), > + GFP_KERNEL); > + if (!ctx->clks) > + return -ENOMEM; > + > + for (i = 0; i < ctx->num_clks; i++) > + ctx->clks[i].id = clk_names[i]; > + > + ret = devm_clk_bulk_get(dev, ctx->num_clks, ctx->clks); This is interesting. I admit I had not considered the pwrseq provider being able to acquire the resources from the consumer node at the time of writing the subsystem. As the pwrseq framework aims at being as flexible as possible, this is definitely something that we should allow but the usage of devres here is problematic on at least two levels. First: you're acquiring the resources from the struct device of the consumer and so the devres entries are added to its devres list. They will get released when the consumer device is detached and the pwrseq provider may end up accessing them afterwards. Second: if .match() fails or even returns 0, the resource is still acquired. Call .match() enough times and you have the devres list needlessly clobbered with unused resources. You should stick to non-devres variants and make sure they are all cleaned-up unless returning 1. (Note to self: these shouldn't be magic values really). You can then release them in this driver's remove callback. > + if (ret) > + return ret; > + > + ctx->gpu_reset = devm_reset_control_get_exclusive(dev, NULL); > + if (IS_ERR(ctx->gpu_reset)) > + return PTR_ERR(ctx->gpu_reset); > + > + return 1; > +} > + > +static int pwrseq_thead_gpu_probe(struct auxiliary_device *adev, > + const struct auxiliary_device_id *id) > +{ > + struct device *dev = &adev->dev; > + struct device *parent_dev = dev->parent; > + struct pwrseq_thead_gpu_ctx *ctx; > + struct pwrseq_config config = {}; > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + ctx->aon_node = parent_dev->of_node; > + > + ctx->clkgen_reset = > + devm_reset_control_get_exclusive(parent_dev, "gpu-clkgen"); > + if (IS_ERR(ctx->clkgen_reset)) > + return dev_err_probe( > + dev, PTR_ERR(ctx->clkgen_reset), > + "Failed to get GPU clkgen reset from parent\n"); > + > + config.parent = dev; > + config.owner = THIS_MODULE; > + config.drvdata = ctx; > + config.match = pwrseq_thead_gpu_match; > + config.targets = pwrseq_thead_gpu_targets; > + > + ctx->pwrseq = devm_pwrseq_device_register(dev, &config); > + if (IS_ERR(ctx->pwrseq)) > + return dev_err_probe(dev, PTR_ERR(ctx->pwrseq), > + "Failed to register power sequencer\n"); > + > + return 0; > +} > + > +static const struct auxiliary_device_id pwrseq_thead_gpu_id_table[] = { > + { .name = "th1520_pm_domains.pwrseq-gpu" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(auxiliary, pwrseq_thead_gpu_id_table); > + > +static struct auxiliary_driver pwrseq_thead_gpu_driver = { > + .driver = { > + .name = "pwrseq-thead-gpu", > + }, > + .probe = pwrseq_thead_gpu_probe, > + .id_table = pwrseq_thead_gpu_id_table, > +}; > +module_auxiliary_driver(pwrseq_thead_gpu_driver); > + > +MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>"); > +MODULE_DESCRIPTION("T-HEAD TH1520 GPU power sequencer driver"); > +MODULE_LICENSE("GPL"); > > -- > 2.34.1 > Thanks! Bartosz
diff --git a/MAINTAINERS b/MAINTAINERS index 0183c028fa18c397d30ec82fd419894f1f50a448..3283ff592215249bcf702dbb4ab710477243477e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -21395,6 +21395,7 @@ F: drivers/mailbox/mailbox-th1520.c F: drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c F: drivers/pinctrl/pinctrl-th1520.c F: drivers/pmdomain/thead/ +F: drivers/power/sequencing/pwrseq-thead-gpu.c F: drivers/reset/reset-th1520.c F: include/dt-bindings/clock/thead,th1520-clk-ap.h F: include/dt-bindings/power/thead,th1520-power.h diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig index ddcc42a984921c55667c46ac586d259625e1f1a7..7fa912c9af2479cdce909467c29fe335788f0bd7 100644 --- a/drivers/power/sequencing/Kconfig +++ b/drivers/power/sequencing/Kconfig @@ -27,4 +27,12 @@ config POWER_SEQUENCING_QCOM_WCN this driver is needed for correct power control or else we'd risk not respecting the required delays between enabling Bluetooth and WLAN. +config POWER_SEQUENCING_THEAD_GPU + tristate "T-HEAD TH1520 GPU power sequencing driver" + depends on ARCH_THEAD && AUXILIARY_BUS + help + Say Y here to enable the power sequencing driver for the TH1520 SoC + GPU. This driver handles the complex clock and reset sequence + required to power on the Imagination BXM GPU on this platform. + endif diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile index 2eec2df7912d11827f9ba914177dd2c882e44bce..647f81f4013ab825630f069d2e0f6d22159f1f56 100644 --- a/drivers/power/sequencing/Makefile +++ b/drivers/power/sequencing/Makefile @@ -4,3 +4,4 @@ obj-$(CONFIG_POWER_SEQUENCING) += pwrseq-core.o pwrseq-core-y := core.o obj-$(CONFIG_POWER_SEQUENCING_QCOM_WCN) += pwrseq-qcom-wcn.o +obj-$(CONFIG_POWER_SEQUENCING_THEAD_GPU) += pwrseq-thead-gpu.o diff --git a/drivers/power/sequencing/pwrseq-thead-gpu.c b/drivers/power/sequencing/pwrseq-thead-gpu.c new file mode 100644 index 0000000000000000000000000000000000000000..bb77aba59a031471fe00c919fcc4a5f2564e0cb6 --- /dev/null +++ b/drivers/power/sequencing/pwrseq-thead-gpu.c @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * T-HEAD TH1520 GPU Power Sequencer Driver + * + * Copyright (c) 2025 Samsung Electronics Co., Ltd. + * Author: Michal Wilczynski <m.wilczynski@samsung.com> + * + * This driver implements the power sequence for the Imagination BXM-4-64 + * GPU on the T-HEAD TH1520 SoC. The sequence requires coordinating resources + * from both the sequencer's parent device node (clkgen_reset) and the GPU's + * device node (clocks and core reset). + * + * The `match` function is used to acquire the GPU's resources when the + * GPU driver requests the "gpu-power" sequence target. + */ + +#include <linux/auxiliary_bus.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pwrseq/provider.h> +#include <linux/reset.h> + +#include <dt-bindings/power/thead,th1520-power.h> + +struct pwrseq_thead_gpu_ctx { + struct pwrseq_device *pwrseq; + struct reset_control *clkgen_reset; + struct device_node *aon_node; + + /* Consumer resources */ + struct clk_bulk_data *clks; + int num_clks; + struct reset_control *gpu_reset; +}; + +static int pwrseq_thead_gpu_power_on(struct pwrseq_device *pwrseq) +{ + struct pwrseq_thead_gpu_ctx *ctx = pwrseq_device_get_drvdata(pwrseq); + int ret; + + if (!ctx->clks || !ctx->gpu_reset) + return -ENODEV; + + ret = clk_bulk_prepare_enable(ctx->num_clks, ctx->clks); + if (ret) + return ret; + + ret = reset_control_deassert(ctx->clkgen_reset); + 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(ctx->gpu_reset); + if (ret) + goto err_assert_clkgen; + + return 0; + +err_assert_clkgen: + reset_control_assert(ctx->clkgen_reset); +err_disable_clks: + clk_bulk_disable_unprepare(ctx->num_clks, ctx->clks); + return ret; +} + +static int pwrseq_thead_gpu_power_off(struct pwrseq_device *pwrseq) +{ + struct pwrseq_thead_gpu_ctx *ctx = pwrseq_device_get_drvdata(pwrseq); + + if (!ctx->clks || !ctx->gpu_reset) + return -ENODEV; + + reset_control_assert(ctx->gpu_reset); + reset_control_assert(ctx->clkgen_reset); + clk_bulk_disable_unprepare(ctx->num_clks, ctx->clks); + + return 0; +} + +static const struct pwrseq_unit_data pwrseq_thead_gpu_unit = { + .name = "gpu-power-sequence", + .enable = pwrseq_thead_gpu_power_on, + .disable = pwrseq_thead_gpu_power_off, +}; + +static const struct pwrseq_target_data pwrseq_thead_gpu_target = { + .name = "gpu-power", + .unit = &pwrseq_thead_gpu_unit, +}; + +static const struct pwrseq_target_data *pwrseq_thead_gpu_targets[] = { + &pwrseq_thead_gpu_target, + NULL +}; + +static int pwrseq_thead_gpu_match(struct pwrseq_device *pwrseq, + struct device *dev) +{ + struct pwrseq_thead_gpu_ctx *ctx = pwrseq_device_get_drvdata(pwrseq); + static const char *const clk_names[] = { "core", "sys" }; + struct of_phandle_args pwr_spec; + int i, ret; + + /* We only match the specific T-HEAD TH1520 GPU compatible */ + if (!of_device_is_compatible(dev->of_node, "thead,th1520-gpu")) + return 0; + + ret = of_parse_phandle_with_args(dev->of_node, "power-domains", + "#power-domain-cells", 0, &pwr_spec); + if (ret) + return 0; + + /* Additionally verify consumer device has AON as power-domain */ + if (pwr_spec.np != ctx->aon_node || pwr_spec.args[0] != TH1520_GPU_PD) { + of_node_put(pwr_spec.np); + return 0; + } + + of_node_put(pwr_spec.np); + + /* Prevent multiple consumers from attaching */ + if (ctx->gpu_reset || ctx->clks) + return -EBUSY; + + ctx->num_clks = ARRAY_SIZE(clk_names); + ctx->clks = devm_kcalloc(dev, ctx->num_clks, sizeof(*ctx->clks), + GFP_KERNEL); + if (!ctx->clks) + return -ENOMEM; + + for (i = 0; i < ctx->num_clks; i++) + ctx->clks[i].id = clk_names[i]; + + ret = devm_clk_bulk_get(dev, ctx->num_clks, ctx->clks); + if (ret) + return ret; + + ctx->gpu_reset = devm_reset_control_get_exclusive(dev, NULL); + if (IS_ERR(ctx->gpu_reset)) + return PTR_ERR(ctx->gpu_reset); + + return 1; +} + +static int pwrseq_thead_gpu_probe(struct auxiliary_device *adev, + const struct auxiliary_device_id *id) +{ + struct device *dev = &adev->dev; + struct device *parent_dev = dev->parent; + struct pwrseq_thead_gpu_ctx *ctx; + struct pwrseq_config config = {}; + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->aon_node = parent_dev->of_node; + + ctx->clkgen_reset = + devm_reset_control_get_exclusive(parent_dev, "gpu-clkgen"); + if (IS_ERR(ctx->clkgen_reset)) + return dev_err_probe( + dev, PTR_ERR(ctx->clkgen_reset), + "Failed to get GPU clkgen reset from parent\n"); + + config.parent = dev; + config.owner = THIS_MODULE; + config.drvdata = ctx; + config.match = pwrseq_thead_gpu_match; + config.targets = pwrseq_thead_gpu_targets; + + ctx->pwrseq = devm_pwrseq_device_register(dev, &config); + if (IS_ERR(ctx->pwrseq)) + return dev_err_probe(dev, PTR_ERR(ctx->pwrseq), + "Failed to register power sequencer\n"); + + return 0; +} + +static const struct auxiliary_device_id pwrseq_thead_gpu_id_table[] = { + { .name = "th1520_pm_domains.pwrseq-gpu" }, + {}, +}; +MODULE_DEVICE_TABLE(auxiliary, pwrseq_thead_gpu_id_table); + +static struct auxiliary_driver pwrseq_thead_gpu_driver = { + .driver = { + .name = "pwrseq-thead-gpu", + }, + .probe = pwrseq_thead_gpu_probe, + .id_table = pwrseq_thead_gpu_id_table, +}; +module_auxiliary_driver(pwrseq_thead_gpu_driver); + +MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>"); +MODULE_DESCRIPTION("T-HEAD TH1520 GPU power sequencer driver"); +MODULE_LICENSE("GPL");