* [PATCH v4 0/8] Add TH1520 GPU support with power sequencing [not found] <CGME20250614180906eucas1p116f8a13a4013edd3bbedfd2e4a8b0aa3@eucas1p1.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski [not found] ` <CGME20250614180907eucas1p13d341c30e495fb36598b1d7c10ec7070@eucas1p1.samsung.com> ` (8 more replies) 0 siblings, 9 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel, Krzysztof Kozlowski This patch series introduces support for the Imagination IMG BXM-4-64 GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is managing the GPU's complex power-up and power-down sequence, which involves multiple clocks and resets. The TH1520 GPU requires a specific sequence to be followed for its clocks and resets to ensure correct operation. Initial discussions and an earlier version of this series explored managing this via the generic power domain (genpd) framework. However, following further discussions with kernel maintainers [1], the approach has been reworked to utilize the dedicated power sequencing (pwrseq) framework. This revised series now employs a new pwrseq provider driver (pwrseq-thead-gpu.c) specifically for the TH1520 GPU. This driver encapsulates the SoC specific power sequence details. The Imagination GPU driver (pvr_device.c) is updated to act as a consumer of this power sequencer, requesting the "gpu-power" target. The sequencer driver, during its match phase with the GPU device, acquires the necessary clock and reset handles from the GPU device node to perform the full sequence. This approach aligns with the goal of abstracting SoC specific power management details away from generic device drivers and leverages the pwrseq framework as recommended. The series is structured as follows: Patch 1: Introduces the pwrseq-thead-gpu auxiliary driver to manage the GPU's power-on/off sequence. Patch 2: Adds device tree bindings for the gpu-clkgen reset to the existing thead,th1520-aon binding. Patch 3: Extends the pm-domains driver to detect the gpu-clkgen reset and spawn the pwrseq-thead-gpu auxiliary driver. Patch 4: Updates the Imagination DRM driver to utilize the pwrseq framework for TH1520 GPU power management. Patch 5: Adds the thead,th1520-gpu compatible string to the PowerVR GPU device tree bindings. Patch 6: Adds the gpu-clkgen reset property to the aon node in the TH1520 device tree source. Patch 7: Adds the device tree node for the IMG BXM-4-64 GPU and its required fixed-clock. Patch 8: Enables compilation of the Imagination PowerVR driver on the RISC-V architecture. This patchset finishes the work started in bigger series [2] by adding all remaining GPU power sequencing piece. After this patchset the GPU probes correctly. This series supersedes the previous genpd based approach. Testing on T-HEAD TH1520 SoC indicates the new pwrseq based solution works correctly. An open point in Patch 7/8 concerns the GPU memory clock (gpu_mem_clk), defined as a fixed-clock. The specific hardware frequency for this clock on the TH1520 could not be determined from available public documentation. Consequently, clock-frequency = <0>; has been used as a placeholder to enable driver functionality. Link to v3 of this series - [3]. v4: - the pwrseq driver is now an auxiliary driver with a robust match function based on the power-domains property, spawned from the AON node - Imagination DRM driver now uses of_device_id match data to conditionally probe for the pwrseq, solving the cross platform probe deferral issue - add Reviewed-by from Ulf for the entire series v3: - re-worked cover letter completely - complete architectural rework from using extended genpd callbacks to a dedicated pwrseq provider driver - introduced pwrseq-thead-gpu.c and associated DT bindings (thead,th1520-gpu-pwrseq) - the Imagination driver now calls devm_pwrseq_get() and uses pwrseq_power_on() / pwrseq_power_off() for the TH1520 GPU - removed the platform_resources_managed flag from dev_pm_info and associated logic - the new pwrseq driver's match() function now acquires consumer-specific resources (GPU clocks, GPU core reset) directly from the consumer device v2: Extended the series by adding two new commits: - introduced a new platform_resources_managed flag in dev_pm_info along with helper functions, allowing drivers to detect when clocks and resets are managed by the platform - updated the DRM Imagination driver to skip claiming clocks when platform_resources_managed is set Split the original bindings update: - the AON firmware bindings now only add the GPU clkgen reset (the GPU core reset remains handled by the GPU node) Reworked the TH1520 PM domain driver to: - acquire GPU clocks and reset dynamically using attach_dev/detach_dev callbacks - handle clkgen reset internally, while GPU core reset is obtained from the consumer device node - added a check to enforce that only a single device can be attached to the GPU PM domain [1] - https://lore.kernel.org/all/CAPDyKFpi6_CD++a9sbGBvJCuBSQS6YcpNttkRQhQMTWy1yyrRg@mail.gmail.com/ [2] - https://lore.kernel.org/all/20250219140239.1378758-1-m.wilczynski@samsung.com/ [3] - https://lore.kernel.org/all/20250530-apr_14_for_sending-v3-0-83d5744d997c@samsung.com/ --- Michal Wilczynski (8): power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver dt-bindings: firmware: thead,th1520: Add resets for GPU clkgen pmdomain: thead: Instantiate GPU power sequencer via auxiliary bus drm/imagination: Use pwrseq for TH1520 GPU power management dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible riscv: dts: thead: th1520: Add GPU clkgen reset to AON node riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node drm/imagination: Enable PowerVR driver for RISC-V .../bindings/firmware/thead,th1520-aon.yaml | 7 + .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 +- MAINTAINERS | 1 + arch/riscv/boot/dts/thead/th1520.dtsi | 25 +++ drivers/gpu/drm/imagination/Kconfig | 3 +- drivers/gpu/drm/imagination/pvr_device.c | 31 ++- drivers/gpu/drm/imagination/pvr_device.h | 17 ++ drivers/gpu/drm/imagination/pvr_drv.c | 6 + drivers/gpu/drm/imagination/pvr_power.c | 82 +++++--- drivers/pmdomain/thead/Kconfig | 1 + drivers/pmdomain/thead/th1520-pm-domains.c | 53 ++++++ drivers/power/sequencing/Kconfig | 8 + drivers/power/sequencing/Makefile | 1 + drivers/power/sequencing/pwrseq-thead-gpu.c | 208 +++++++++++++++++++++ 14 files changed, 417 insertions(+), 35 deletions(-) --- base-commit: 4774cfe3543abb8ee98089f535e28ebfd45b975a change-id: 20250414-apr_14_for_sending-5b3917817acc Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180907eucas1p13d341c30e495fb36598b1d7c10ec7070@eucas1p1.samsung.com>]
* [PATCH v4 1/8] power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver [not found] ` <CGME20250614180907eucas1p13d341c30e495fb36598b1d7c10ec7070@eucas1p1.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 2025-06-16 9:03 ` Bartosz Golaszewski 0 siblings, 1 reply; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel 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. 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> --- MAINTAINERS | 1 + drivers/power/sequencing/Kconfig | 8 ++ drivers/power/sequencing/Makefile | 1 + drivers/power/sequencing/pwrseq-thead-gpu.c | 208 ++++++++++++++++++++++++++++ 4 files changed, 218 insertions(+) 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"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v4 1/8] power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver 2025-06-14 18:06 ` [PATCH v4 1/8] power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver Michal Wilczynski @ 2025-06-16 9:03 ` Bartosz Golaszewski 0 siblings, 0 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2025-06-16 9:03 UTC (permalink / raw) To: Michal Wilczynski Cc: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180908eucas1p1f8e152bc86111d1fab4916e1737534e1@eucas1p1.samsung.com>]
* [PATCH v4 2/8] dt-bindings: firmware: thead,th1520: Add resets for GPU clkgen [not found] ` <CGME20250614180908eucas1p1f8e152bc86111d1fab4916e1737534e1@eucas1p1.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Extend the TH1520 AON to describe the GPU clkgen reset line, required for proper GPU clock and reset sequencing. The T-HEAD TH1520 GPU requires coordinated management of two clocks (core and sys) and two resets (GPU core reset and GPU clkgen reset). Only the clkgen reset is exposed at the AON level, to support SoC specific initialization handled through a dedicated auxiliary power sequencing driver. The GPU core reset remains described in the GPU device node, as from the GPU driver's perspective, there is only a single reset line [1]. This follows upstream maintainers' recommendations [2] to abstract SoC specific details into the PM domain layer rather than exposing them to drivers directly. Link: https://lore.kernel.org/all/816db99d-7088-4c1a-af03-b9a825ac09dc@imgtec.com/ - [1] Link: https://lore.kernel.org/all/38d9650fc11a674c8b689d6bab937acf@kernel.org/ - [2] Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml index bbc183200400de7aadbb21fea21911f6f4227b09..3365124c7fd4736922717bd31caa13272f4a4ea6 100644 --- a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml @@ -32,6 +32,13 @@ properties: items: - const: aon + resets: + maxItems: 1 + + reset-names: + items: + - const: gpu-clkgen + "#power-domain-cells": const: 1 -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180909eucas1p2a34e3242fb42f7fd25e4038c291276ff@eucas1p2.samsung.com>]
* [PATCH v4 3/8] pmdomain: thead: Instantiate GPU power sequencer via auxiliary bus [not found] ` <CGME20250614180909eucas1p2a34e3242fb42f7fd25e4038c291276ff@eucas1p2.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 2025-06-16 9:10 ` Bartosz Golaszewski 0 siblings, 1 reply; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel In order to support the complex power sequencing required by the TH1520 GPU, the AON power domain driver must be responsible for initiating the corresponding sequencer driver. This functionality is specific to platforms where the GPU power sequencing hardware is controlled by the AON block. Extend the AON power domain driver to check for the presence of the "gpu-clkgen" reset in its own device tree node. If the property is found, create and register a new auxiliary device. This device acts as a proxy that allows the dedicated `pwrseq-thead-gpu` auxiliary driver to bind and take control of the sequencing logic. Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/pmdomain/thead/Kconfig | 1 + drivers/pmdomain/thead/th1520-pm-domains.c | 53 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/drivers/pmdomain/thead/Kconfig b/drivers/pmdomain/thead/Kconfig index 7d52f8374b074167d508a80fd807929c53faef12..208828e0fa0dc91256bf808b905bea32bb84250d 100644 --- a/drivers/pmdomain/thead/Kconfig +++ b/drivers/pmdomain/thead/Kconfig @@ -4,6 +4,7 @@ config TH1520_PM_DOMAINS tristate "Support TH1520 Power Domains" depends on TH1520_AON_PROTOCOL select REGMAP_MMIO + select AUXILIARY_BUS help This driver enables power domain management for the T-HEAD TH-1520 SoC. On this SoC there are number of power domains, diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c index f702e20306f469aeb0ed15e54bd4f8309f28018c..9f2cd833e5f554d4a9154e276e5fe5720fc4d50f 100644 --- a/drivers/pmdomain/thead/th1520-pm-domains.c +++ b/drivers/pmdomain/thead/th1520-pm-domains.c @@ -5,8 +5,10 @@ * Author: Michal Wilczynski <m.wilczynski@samsung.com> */ +#include <linux/auxiliary_bus.h> #include <linux/firmware/thead/thead,th1520-aon.h> #include <linux/slab.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm_domain.h> @@ -128,6 +130,51 @@ static void th1520_pd_init_all_off(struct generic_pm_domain **domains, } } +static void th1520_pd_pwrseq_unregister_adev(void *adev) +{ + auxiliary_device_delete(adev); + auxiliary_device_uninit(adev); +} + +static int th1520_pd_pwrseq_gpu_init(struct device *dev) +{ + struct auxiliary_device *adev; + int ret; + + /* + * Correctly check only for the property's existence in the DT node. + * We don't need to get/claim the reset here; that is the job of + * the auxiliary driver that we are about to spawn. + */ + if (of_property_match_string(dev->of_node, "reset-names", + "gpu-clkgen") < 0) + /* + * This is not an error. It simply means the optional sequencer + * is not described in the device tree. + */ + return 0; + + adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL); + if (!adev) + return -ENOMEM; + + adev->name = "pwrseq-gpu"; + adev->dev.parent = dev; + + ret = auxiliary_device_init(adev); + if (ret) + return ret; + + ret = auxiliary_device_add(adev); + if (ret) { + auxiliary_device_uninit(adev); + return ret; + } + + return devm_add_action_or_reset(dev, th1520_pd_pwrseq_unregister_adev, + adev); +} + static int th1520_pd_probe(struct platform_device *pdev) { struct generic_pm_domain **domains; @@ -186,8 +233,14 @@ static int th1520_pd_probe(struct platform_device *pdev) if (ret) goto err_clean_genpd; + ret = th1520_pd_pwrseq_gpu_init(dev); + if (ret) + goto err_clean_provider; + return 0; +err_clean_provider: + of_genpd_del_provider(dev->of_node); err_clean_genpd: for (i--; i >= 0; i--) pm_genpd_remove(domains[i]); -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v4 3/8] pmdomain: thead: Instantiate GPU power sequencer via auxiliary bus 2025-06-14 18:06 ` [PATCH v4 3/8] pmdomain: thead: Instantiate GPU power sequencer via auxiliary bus Michal Wilczynski @ 2025-06-16 9:10 ` Bartosz Golaszewski 0 siblings, 0 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2025-06-16 9:10 UTC (permalink / raw) To: Michal Wilczynski Cc: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel On Sat, Jun 14, 2025 at 8:09 PM Michal Wilczynski <m.wilczynski@samsung.com> wrote: > > In order to support the complex power sequencing required by the TH1520 > GPU, the AON power domain driver must be responsible for initiating the > corresponding sequencer driver. This functionality is specific to > platforms where the GPU power sequencing hardware is controlled by the > AON block. > > Extend the AON power domain driver to check for the presence of the > "gpu-clkgen" reset in its own device tree node. > > If the property is found, create and register a new auxiliary device. > This device acts as a proxy that allows the dedicated `pwrseq-thead-gpu` > auxiliary driver to bind and take control of the sequencing logic. > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- [snip] > + > +static int th1520_pd_pwrseq_gpu_init(struct device *dev) > +{ > + struct auxiliary_device *adev; > + int ret; > + > + /* > + * Correctly check only for the property's existence in the DT node. > + * We don't need to get/claim the reset here; that is the job of > + * the auxiliary driver that we are about to spawn. > + */ > + if (of_property_match_string(dev->of_node, "reset-names", > + "gpu-clkgen") < 0) If you use device_property_match_string() here, you don't need to pull in of.h. It's typically preferred to use the top-level abstraction unless not possible. [snip] Thanks, Bartosz ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180911eucas1p16c9fb4a8160253c253f623bec2529f70@eucas1p1.samsung.com>]
* [PATCH v4 4/8] drm/imagination: Use pwrseq for TH1520 GPU power management [not found] ` <CGME20250614180911eucas1p16c9fb4a8160253c253f623bec2529f70@eucas1p1.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 2025-06-16 9:40 ` Bartosz Golaszewski 0 siblings, 1 reply; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Update the Imagination PVR DRM driver to leverage the pwrseq framework for managing the power sequence of the GPU on the T-HEAD TH1520 SoC. To cleanly handle the TH1520's specific power requirements in the generic driver, this patch implements the "driver match data" pattern. A has_pwrseq flag in a new pvr_soc_data struct is now associated with thead,th1520-gpu compatible string in the of_device_id table. At probe time, the driver checks this flag. If true, it calls devm_pwrseq_get("gpu-power"), requiring a valid sequencer and deferring probe on failure. In this mode, all power and reset control is delegated to the pwrseq provider. If the flag is false, the driver skips this logic and falls back to its standard manual power management. Clock handles are still acquired directly by this driver in both cases for other purposes like devfreq. The runtime PM callbacks, pvr_power_device_resume() and pvr_power_device_suspend(), are modified to call pwrseq_power_on() and pwrseq_power_off() respectively when the sequencer is present. A helper function, pvr_power_off_sequence_manual(), is introduced to encapsulate the manual power-down logic. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/gpu/drm/imagination/Kconfig | 1 + drivers/gpu/drm/imagination/pvr_device.c | 31 ++++++++++-- drivers/gpu/drm/imagination/pvr_device.h | 17 +++++++ drivers/gpu/drm/imagination/pvr_drv.c | 6 +++ drivers/gpu/drm/imagination/pvr_power.c | 82 +++++++++++++++++++++----------- 5 files changed, 104 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig index 3bfa2ac212dccb73c53bdc2bc259bcba636e7cfc..5f9fff43d6baadc42ebf48d91729bfbf27e06caa 100644 --- a/drivers/gpu/drm/imagination/Kconfig +++ b/drivers/gpu/drm/imagination/Kconfig @@ -11,6 +11,7 @@ config DRM_POWERVR select DRM_SCHED select DRM_GPUVM select FW_LOADER + select POWER_SEQUENCING help Choose this option if you have a system that has an Imagination Technologies PowerVR (Series 6 or later) or IMG GPU. diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..2f71c9501b157e6e14a6f3033e02db40ce5c74c7 100644 --- a/drivers/gpu/drm/imagination/pvr_device.c +++ b/drivers/gpu/drm/imagination/pvr_device.c @@ -23,8 +23,10 @@ #include <linux/firmware.h> #include <linux/gfp.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/pwrseq/consumer.h> #include <linux/reset.h> #include <linux/slab.h> #include <linux/stddef.h> @@ -618,6 +620,9 @@ pvr_device_init(struct pvr_device *pvr_dev) struct device *dev = drm_dev->dev; int err; + /* Get the platform-specific data based on the compatible string. */ + pvr_dev->soc_data = of_device_get_match_data(dev); + /* * Setup device parameters. We do this first in case other steps * depend on them. @@ -631,10 +636,28 @@ pvr_device_init(struct pvr_device *pvr_dev) if (err) return err; - /* Get the reset line for the GPU */ - err = pvr_device_reset_init(pvr_dev); - if (err) - return err; + /* + * For platforms that require it, get the power sequencer. + * For all others, perform manual reset initialization. + */ + if (pvr_dev->soc_data && pvr_dev->soc_data->has_pwrseq) { + pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power"); + if (IS_ERR(pvr_dev->pwrseq)) { + /* + * This platform requires a sequencer. If we can't get + * it, we must return the error (including -EPROBE_DEFER + * to wait for the provider to appear) + */ + return dev_err_probe( + dev, PTR_ERR(pvr_dev->pwrseq), + "Failed to get required power sequencer\n"); + } + } else { + /* This platform does not use a sequencer, init reset manually. */ + err = pvr_device_reset_init(pvr_dev); + if (err) + return err; + } /* Explicitly power the GPU so we can access control registers before the FW is booted. */ err = pm_runtime_resume_and_get(dev); diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..5182f06ca7e2071777bdaa9d07757df5ef869fe3 100644 --- a/drivers/gpu/drm/imagination/pvr_device.h +++ b/drivers/gpu/drm/imagination/pvr_device.h @@ -37,6 +37,9 @@ struct clk; /* Forward declaration from <linux/firmware.h>. */ struct firmware; +/* Forward declaration from <linux/pwrseq/consumer.h */ +struct pwrseq_desc; + /** * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device * @b: Branch ID. @@ -57,6 +60,14 @@ struct pvr_fw_version { u16 major, minor; }; +/* + * struct pvr_soc_data - Platform specific data associated with a compatible string. + * @has_pwrseq: True if the platform requires power management via the pwrseq framework. + */ +struct pvr_soc_data { + bool has_pwrseq; +}; + /** * struct pvr_device - powervr-specific wrapper for &struct drm_device */ @@ -98,6 +109,9 @@ struct pvr_device { /** @fw_version: Firmware version detected at runtime. */ struct pvr_fw_version fw_version; + /** @soc_data: Pointer to platform-specific quirk data. */ + const struct pvr_soc_data *soc_data; + /** @regs_resource: Resource representing device control registers. */ struct resource *regs_resource; @@ -148,6 +162,9 @@ struct pvr_device { */ struct reset_control *reset; + /** @pwrseq: Pointer to a power sequencer, if one is used. */ + struct pwrseq_desc *pwrseq; + /** @irq: IRQ number. */ int irq; diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..7756d85dcbf214279ae242c30b349d20eed7cab2 100644 --- a/drivers/gpu/drm/imagination/pvr_drv.c +++ b/drivers/gpu/drm/imagination/pvr_drv.c @@ -1481,6 +1481,11 @@ static void pvr_remove(struct platform_device *plat_dev) } static const struct of_device_id dt_match[] = { + { + .compatible = "thead,th1520-gpu", + .data = &(struct pvr_soc_data){ .has_pwrseq = true }, + }, + { .compatible = "img,img-rogue", .data = NULL }, /* @@ -1513,4 +1518,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC); MODULE_LICENSE("Dual MIT/GPL"); MODULE_IMPORT_NS("DMA_BUF"); MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw"); +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw"); MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw"); diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c index 41f5d89e78b854cf6993838868a4416a220b490a..7797bbae73c9264d2a490193138ae8636c144af0 100644 --- a/drivers/gpu/drm/imagination/pvr_power.c +++ b/drivers/gpu/drm/imagination/pvr_power.c @@ -18,6 +18,7 @@ #include <linux/platform_device.h> #include <linux/pm_domain.h> #include <linux/pm_runtime.h> +#include <linux/pwrseq/consumer.h> #include <linux/reset.h> #include <linux/timer.h> #include <linux/types.h> @@ -234,6 +235,19 @@ pvr_watchdog_init(struct pvr_device *pvr_dev) return 0; } +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev) +{ + int err; + + err = reset_control_assert(pvr_dev->reset); + + clk_disable_unprepare(pvr_dev->mem_clk); + clk_disable_unprepare(pvr_dev->sys_clk); + clk_disable_unprepare(pvr_dev->core_clk); + + return err; +} + int pvr_power_device_suspend(struct device *dev) { @@ -252,11 +266,10 @@ pvr_power_device_suspend(struct device *dev) goto err_drm_dev_exit; } - clk_disable_unprepare(pvr_dev->mem_clk); - clk_disable_unprepare(pvr_dev->sys_clk); - clk_disable_unprepare(pvr_dev->core_clk); - - err = reset_control_assert(pvr_dev->reset); + if (pvr_dev->pwrseq) + err = pwrseq_power_off(pvr_dev->pwrseq); + else + err = pvr_power_off_sequence_manual(pvr_dev); err_drm_dev_exit: drm_dev_exit(idx); @@ -276,44 +289,55 @@ pvr_power_device_resume(struct device *dev) if (!drm_dev_enter(drm_dev, &idx)) return -EIO; - err = clk_prepare_enable(pvr_dev->core_clk); - if (err) - goto err_drm_dev_exit; + if (pvr_dev->pwrseq) { + err = pwrseq_power_on(pvr_dev->pwrseq); + if (err) + goto err_drm_dev_exit; + } else { + err = clk_prepare_enable(pvr_dev->core_clk); + if (err) + goto err_drm_dev_exit; - err = clk_prepare_enable(pvr_dev->sys_clk); - if (err) - goto err_core_clk_disable; + err = clk_prepare_enable(pvr_dev->sys_clk); + if (err) + goto err_core_clk_disable; - err = clk_prepare_enable(pvr_dev->mem_clk); - if (err) - goto err_sys_clk_disable; + err = clk_prepare_enable(pvr_dev->mem_clk); + if (err) + goto err_sys_clk_disable; - /* - * 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); + /* + * 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); - err = reset_control_deassert(pvr_dev->reset); - if (err) - goto err_mem_clk_disable; + err = reset_control_deassert(pvr_dev->reset); + if (err) + goto err_mem_clk_disable; + } if (pvr_dev->fw_dev.booted) { err = pvr_power_fw_enable(pvr_dev); if (err) - goto err_reset_assert; + goto err_power_off; } drm_dev_exit(idx); return 0; -err_reset_assert: - reset_control_assert(pvr_dev->reset); +err_power_off: + if (pvr_dev->pwrseq) + pwrseq_power_off(pvr_dev->pwrseq); + else + pvr_power_off_sequence_manual(pvr_dev); + + goto err_drm_dev_exit; err_mem_clk_disable: clk_disable_unprepare(pvr_dev->mem_clk); -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v4 4/8] drm/imagination: Use pwrseq for TH1520 GPU power management 2025-06-14 18:06 ` [PATCH v4 4/8] drm/imagination: Use pwrseq for TH1520 GPU power management Michal Wilczynski @ 2025-06-16 9:40 ` Bartosz Golaszewski 2025-06-16 18:50 ` Michal Wilczynski 0 siblings, 1 reply; 18+ messages in thread From: Bartosz Golaszewski @ 2025-06-16 9:40 UTC (permalink / raw) To: Michal Wilczynski Cc: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel On Sat, Jun 14, 2025 at 8:09 PM Michal Wilczynski <m.wilczynski@samsung.com> wrote: > > Update the Imagination PVR DRM driver to leverage the pwrseq framework > for managing the power sequence of the GPU on the T-HEAD TH1520 SoC. > > To cleanly handle the TH1520's specific power requirements in the > generic driver, this patch implements the "driver match data" pattern. A > has_pwrseq flag in a new pvr_soc_data struct is now associated with > thead,th1520-gpu compatible string in the of_device_id table. > > At probe time, the driver checks this flag. If true, it calls > devm_pwrseq_get("gpu-power"), requiring a valid sequencer and deferring > probe on failure. In this mode, all power and reset control is delegated > to the pwrseq provider. If the flag is false, the driver skips this > logic and falls back to its standard manual power management. Clock > handles are still acquired directly by this driver in both cases for > other purposes like devfreq. > > The runtime PM callbacks, pvr_power_device_resume() and > pvr_power_device_suspend(), are modified to call pwrseq_power_on() and > pwrseq_power_off() respectively when the sequencer is present. A helper > function, pvr_power_off_sequence_manual(), is introduced to encapsulate > the manual power-down logic. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- [snip] > > +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev) > +{ > + int err; > + > + err = reset_control_assert(pvr_dev->reset); > + > + clk_disable_unprepare(pvr_dev->mem_clk); > + clk_disable_unprepare(pvr_dev->sys_clk); > + clk_disable_unprepare(pvr_dev->core_clk); > + > + return err; > +} > + > int > pvr_power_device_suspend(struct device *dev) > { > @@ -252,11 +266,10 @@ pvr_power_device_suspend(struct device *dev) > goto err_drm_dev_exit; > } > > - clk_disable_unprepare(pvr_dev->mem_clk); > - clk_disable_unprepare(pvr_dev->sys_clk); > - clk_disable_unprepare(pvr_dev->core_clk); > - > - err = reset_control_assert(pvr_dev->reset); > + if (pvr_dev->pwrseq) > + err = pwrseq_power_off(pvr_dev->pwrseq); > + else > + err = pvr_power_off_sequence_manual(pvr_dev); > > err_drm_dev_exit: > drm_dev_exit(idx); > @@ -276,44 +289,55 @@ pvr_power_device_resume(struct device *dev) > if (!drm_dev_enter(drm_dev, &idx)) > return -EIO; > > - err = clk_prepare_enable(pvr_dev->core_clk); > - if (err) > - goto err_drm_dev_exit; > + if (pvr_dev->pwrseq) { > + err = pwrseq_power_on(pvr_dev->pwrseq); > + if (err) > + goto err_drm_dev_exit; > + } else { > + err = clk_prepare_enable(pvr_dev->core_clk); > + if (err) > + goto err_drm_dev_exit; > > - err = clk_prepare_enable(pvr_dev->sys_clk); > - if (err) > - goto err_core_clk_disable; > + err = clk_prepare_enable(pvr_dev->sys_clk); > + if (err) > + goto err_core_clk_disable; > > - err = clk_prepare_enable(pvr_dev->mem_clk); > - if (err) > - goto err_sys_clk_disable; > + err = clk_prepare_enable(pvr_dev->mem_clk); > + if (err) > + goto err_sys_clk_disable; > In order to decrease the number of if-elses, would it make sense to put the "manual" and "pwrseq" operations into their own separate functions and then store addresses of these functions in the device match data struct as function pointers (instead of the has_pwrseq flag)? This way we'd just call them directly. Bart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 4/8] drm/imagination: Use pwrseq for TH1520 GPU power management 2025-06-16 9:40 ` Bartosz Golaszewski @ 2025-06-16 18:50 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-16 18:50 UTC (permalink / raw) To: Bartosz Golaszewski, Matt Coster, Frank Binns Cc: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel On 6/16/25 11:40, Bartosz Golaszewski wrote: > On Sat, Jun 14, 2025 at 8:09 PM Michal Wilczynski > <m.wilczynski@samsung.com> wrote: >> >> Update the Imagination PVR DRM driver to leverage the pwrseq framework >> for managing the power sequence of the GPU on the T-HEAD TH1520 SoC. >> >> To cleanly handle the TH1520's specific power requirements in the >> generic driver, this patch implements the "driver match data" pattern. A >> has_pwrseq flag in a new pvr_soc_data struct is now associated with >> thead,th1520-gpu compatible string in the of_device_id table. >> >> At probe time, the driver checks this flag. If true, it calls >> devm_pwrseq_get("gpu-power"), requiring a valid sequencer and deferring >> probe on failure. In this mode, all power and reset control is delegated >> to the pwrseq provider. If the flag is false, the driver skips this >> logic and falls back to its standard manual power management. Clock >> handles are still acquired directly by this driver in both cases for >> other purposes like devfreq. >> >> The runtime PM callbacks, pvr_power_device_resume() and >> pvr_power_device_suspend(), are modified to call pwrseq_power_on() and >> pwrseq_power_off() respectively when the sequencer is present. A helper >> function, pvr_power_off_sequence_manual(), is introduced to encapsulate >> the manual power-down logic. >> >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >> --- > > [snip] > >> >> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev) >> +{ >> + int err; >> + >> + err = reset_control_assert(pvr_dev->reset); >> + >> + clk_disable_unprepare(pvr_dev->mem_clk); >> + clk_disable_unprepare(pvr_dev->sys_clk); >> + clk_disable_unprepare(pvr_dev->core_clk); >> + >> + return err; >> +} >> + >> int >> pvr_power_device_suspend(struct device *dev) >> { >> @@ -252,11 +266,10 @@ pvr_power_device_suspend(struct device *dev) >> goto err_drm_dev_exit; >> } >> >> - clk_disable_unprepare(pvr_dev->mem_clk); >> - clk_disable_unprepare(pvr_dev->sys_clk); >> - clk_disable_unprepare(pvr_dev->core_clk); >> - >> - err = reset_control_assert(pvr_dev->reset); >> + if (pvr_dev->pwrseq) >> + err = pwrseq_power_off(pvr_dev->pwrseq); >> + else >> + err = pvr_power_off_sequence_manual(pvr_dev); >> >> err_drm_dev_exit: >> drm_dev_exit(idx); >> @@ -276,44 +289,55 @@ pvr_power_device_resume(struct device *dev) >> if (!drm_dev_enter(drm_dev, &idx)) >> return -EIO; >> >> - err = clk_prepare_enable(pvr_dev->core_clk); >> - if (err) >> - goto err_drm_dev_exit; >> + if (pvr_dev->pwrseq) { >> + err = pwrseq_power_on(pvr_dev->pwrseq); >> + if (err) >> + goto err_drm_dev_exit; >> + } else { >> + err = clk_prepare_enable(pvr_dev->core_clk); >> + if (err) >> + goto err_drm_dev_exit; >> >> - err = clk_prepare_enable(pvr_dev->sys_clk); >> - if (err) >> - goto err_core_clk_disable; >> + err = clk_prepare_enable(pvr_dev->sys_clk); >> + if (err) >> + goto err_core_clk_disable; >> >> - err = clk_prepare_enable(pvr_dev->mem_clk); >> - if (err) >> - goto err_sys_clk_disable; >> + err = clk_prepare_enable(pvr_dev->mem_clk); >> + if (err) >> + goto err_sys_clk_disable; >> > > In order to decrease the number of if-elses, would it make sense to > put the "manual" and "pwrseq" operations into their own separate > functions and then store addresses of these functions in the device > match data struct as function pointers (instead of the has_pwrseq > flag)? This way we'd just call them directly. Hi Bartosz, Thanks for the suggestion. That sounds good. I can rework the patch to use function pointers instead of the flag. Matt, as the maintainer of this code, do you have a preference on this? Let me know what you think. > > Bart > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180912eucas1p283681bf6a16249417e5e6d8eb25b969c@eucas1p2.samsung.com>]
* [PATCH v4 5/8] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible [not found] ` <CGME20250614180912eucas1p283681bf6a16249417e5e6d8eb25b969c@eucas1p2.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel, Krzysztof Kozlowski Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's specific GPU compatible string. The thead,th1520-gpu compatible, along with its full chain img,img-bxm-4-64, and img,img-rogue, is added to the list of recognized GPU types. The power-domains property requirement for img,img-bxm-4-64 is also ensured by adding it to the relevant allOf condition. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml @@ -21,6 +21,11 @@ properties: # work with newer dts. - const: img,img-axe - const: img,img-rogue + - items: + - enum: + - thead,th1520-gpu + - const: img,img-bxm-4-64 + - const: img,img-rogue - items: - enum: - ti,j721s2-gpu @@ -93,7 +98,9 @@ allOf: properties: compatible: contains: - const: img,img-axe-1-16m + enum: + - img,img-axe-1-16m + - img,img-bxm-4-64 then: properties: power-domains: -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180913eucas1p2554c9e8d5024c534565d3c1de58f2e61@eucas1p2.samsung.com>]
* [PATCH v4 6/8] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node [not found] ` <CGME20250614180913eucas1p2554c9e8d5024c534565d3c1de58f2e61@eucas1p2.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Add the "gpu-clkgen" reset property to the AON device tree node. This allows the AON power domain driver to detect the capability to power sequence the GPU and spawn the necessary pwrseq-thead-gpu auxiliary driver for managing the GPU's complex power sequence. This commit also adds the prerequisite dt-bindings/reset/thead,th1520-reset.h include to make the TH1520_RESET_ID_GPU_CLKGEN available. This include was previously dropped during a conflict resolution [1]. Link: https://lore.kernel.org/all/aAvfn2mq0Ksi8DF2@x1/ [1] Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- arch/riscv/boot/dts/thead/th1520.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi index 1db0054c4e093400e9dbebcee5fcfa5b5cae6e32..f3f5db0201ab8c0306d4d63072a1573431e51893 100644 --- a/arch/riscv/boot/dts/thead/th1520.dtsi +++ b/arch/riscv/boot/dts/thead/th1520.dtsi @@ -7,6 +7,7 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/clock/thead,th1520-clk-ap.h> #include <dt-bindings/power/thead,th1520-power.h> +#include <dt-bindings/reset/thead,th1520-reset.h> / { compatible = "thead,th1520"; @@ -234,6 +235,8 @@ aon: aon { compatible = "thead,th1520-aon"; mboxes = <&mbox_910t 1>; mbox-names = "aon"; + resets = <&rst TH1520_RESET_ID_GPU_CLKGEN>; + reset-names = "gpu-clkgen"; #power-domain-cells = <1>; }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180914eucas1p202074745dba308cf1e18d1b75a2f3cea@eucas1p2.samsung.com>]
* [PATCH v4 7/8] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node [not found] ` <CGME20250614180914eucas1p202074745dba308cf1e18d1b75a2f3cea@eucas1p2.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD TH1520 SoC used by the Lichee Pi 4A board. This node enables support for the GPU using the drm/imagination driver. By adding this node, the kernel can recognize and initialize the GPU, providing graphics acceleration capabilities on the Lichee Pi 4A and other boards based on the TH1520 SoC. Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be controlled programatically. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- arch/riscv/boot/dts/thead/th1520.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi index f3f5db0201ab8c0306d4d63072a1573431e51893..c8447eef36c3a6e92d768658b6b19dfeb59a47c4 100644 --- a/arch/riscv/boot/dts/thead/th1520.dtsi +++ b/arch/riscv/boot/dts/thead/th1520.dtsi @@ -225,6 +225,13 @@ aonsys_clk: clock-73728000 { #clock-cells = <0>; }; + gpu_mem_clk: mem-clk { + compatible = "fixed-clock"; + clock-frequency = <0>; + clock-output-names = "gpu_mem_clk"; + #clock-cells = <0>; + }; + stmmac_axi_config: stmmac-axi-config { snps,wr_osr_lmt = <15>; snps,rd_osr_lmt = <15>; @@ -500,6 +507,21 @@ clk: clock-controller@ffef010000 { #clock-cells = <1>; }; + gpu: gpu@ffef400000 { + compatible = "thead,th1520-gpu", "img,img-bxm-4-64", + "img,img-rogue"; + reg = <0xff 0xef400000 0x0 0x100000>; + interrupt-parent = <&plic>; + interrupts = <102 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_vo CLK_GPU_CORE>, + <&gpu_mem_clk>, + <&clk_vo CLK_GPU_CFG_ACLK>; + clock-names = "core", "mem", "sys"; + power-domains = <&aon TH1520_GPU_PD>; + power-domain-names = "a"; + resets = <&rst TH1520_RESET_ID_GPU>; + }; + rst: reset-controller@ffef528000 { compatible = "thead,th1520-reset"; reg = <0xff 0xef528000 0x0 0x4f>; -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <CGME20250614180916eucas1p1ee5f075be14d88aa828ec4e55c26bae9@eucas1p1.samsung.com>]
* [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V [not found] ` <CGME20250614180916eucas1p1ee5f075be14d88aa828ec4e55c26bae9@eucas1p1.samsung.com> @ 2025-06-14 18:06 ` Michal Wilczynski 2025-06-15 10:51 ` kernel test robot 0 siblings, 1 reply; 18+ messages in thread From: Michal Wilczynski @ 2025-06-14 18:06 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Several RISC-V boards feature Imagination GPUs that are compatible with the PowerVR driver. An example is the IMG BXM-4-64 GPU on the Lichee Pi 4A board. This commit adjusts the driver's Kconfig dependencies to allow the PowerVR driver to be compiled on the RISC-V architecture. By enabling compilation on RISC-V, we expand support for these GPUs, providing graphics acceleration capabilities and enhancing hardware compatibility on RISC-V platforms. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/gpu/drm/imagination/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig index 5f9fff43d6baadc42ebf48d91729bfbf27e06caa..d1e5a18e8e84dc57452cd4bf0fe89dfb90a7bb29 100644 --- a/drivers/gpu/drm/imagination/Kconfig +++ b/drivers/gpu/drm/imagination/Kconfig @@ -3,7 +3,7 @@ config DRM_POWERVR tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics" - depends on ARM64 + depends on (ARM64 || RISCV) depends on DRM depends on PM select DRM_EXEC -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V 2025-06-14 18:06 ` [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V Michal Wilczynski @ 2025-06-15 10:51 ` kernel test robot 2025-06-16 9:09 ` Michal Wilczynski 0 siblings, 1 reply; 18+ messages in thread From: kernel test robot @ 2025-06-15 10:51 UTC (permalink / raw) To: Michal Wilczynski, Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel Hi Michal, kernel test robot noticed the following build warnings: [auto build test WARNING on 4774cfe3543abb8ee98089f535e28ebfd45b975a] url: https://github.com/intel-lab-lkp/linux/commits/Michal-Wilczynski/power-sequencing-Add-T-HEAD-TH1520-GPU-power-sequencer-driver/20250615-021142 base: 4774cfe3543abb8ee98089f535e28ebfd45b975a patch link: https://lore.kernel.org/r/20250614-apr_14_for_sending-v4-8-8e3945c819cd%40samsung.com patch subject: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V config: riscv-kismet-CONFIG_DRM_GEM_SHMEM_HELPER-CONFIG_DRM_POWERVR-0-0 (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/config) reproduce: (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202506151839.IKkZs0Z0-lkp@intel.com/ kismet warnings: (new ones prefixed by >>) >> kismet: WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER when selected by DRM_POWERVR WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n] Selected by [y]: - DRM_POWERVR [=y] && HAS_IOMEM [=y] && (ARM64 || RISCV [=y]) && DRM [=y] && PM [=y] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V 2025-06-15 10:51 ` kernel test robot @ 2025-06-16 9:09 ` Michal Wilczynski 2025-06-16 9:22 ` Bartosz Golaszewski 0 siblings, 1 reply; 18+ messages in thread From: Michal Wilczynski @ 2025-06-16 9:09 UTC (permalink / raw) To: kernel test robot, Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel On 6/15/25 12:51, kernel test robot wrote: > Hi Michal, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on 4774cfe3543abb8ee98089f535e28ebfd45b975a] > > url: https://protect2.fireeye.com/v1/url?k=6c3bc994-0cd954c9-6c3a42db-000babd9f1ba-30c2378fa012fc4a&q=1&e=c39c960c-4d5f-44d7-aed7-0097394dfc81&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FMichal-Wilczynski%2Fpower-sequencing-Add-T-HEAD-TH1520-GPU-power-sequencer-driver%2F20250615-021142 > base: 4774cfe3543abb8ee98089f535e28ebfd45b975a > patch link: https://lore.kernel.org/r/20250614-apr_14_for_sending-v4-8-8e3945c819cd%40samsung.com > patch subject: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V > config: riscv-kismet-CONFIG_DRM_GEM_SHMEM_HELPER-CONFIG_DRM_POWERVR-0-0 (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/config) > reproduce: (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202506151839.IKkZs0Z0-lkp@intel.com/ > > kismet warnings: (new ones prefixed by >>) >>> kismet: WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER when selected by DRM_POWERVR > WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER > Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n] I believe this is triggered because RISC-V can be compiled without MMU support, while MMU support is mandatory for ARM64. Would an acceptable fix be to require an explicit dependency on the MMU, like so? depends on (ARM64 || RISCV) && MMU > Selected by [y]: > - DRM_POWERVR [=y] && HAS_IOMEM [=y] && (ARM64 || RISCV [=y]) && DRM [=y] && PM [=y] > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V 2025-06-16 9:09 ` Michal Wilczynski @ 2025-06-16 9:22 ` Bartosz Golaszewski 0 siblings, 0 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2025-06-16 9:22 UTC (permalink / raw) To: Michal Wilczynski Cc: kernel test robot, Drew Fustini, Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel On Mon, Jun 16, 2025 at 11:09 AM Michal Wilczynski <m.wilczynski@samsung.com> wrote: > > > > On 6/15/25 12:51, kernel test robot wrote: > > Hi Michal, > > > > kernel test robot noticed the following build warnings: > > > > [auto build test WARNING on 4774cfe3543abb8ee98089f535e28ebfd45b975a] > > > > url: https://protect2.fireeye.com/v1/url?k=6c3bc994-0cd954c9-6c3a42db-000babd9f1ba-30c2378fa012fc4a&q=1&e=c39c960c-4d5f-44d7-aed7-0097394dfc81&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FMichal-Wilczynski%2Fpower-sequencing-Add-T-HEAD-TH1520-GPU-power-sequencer-driver%2F20250615-021142 > > base: 4774cfe3543abb8ee98089f535e28ebfd45b975a > > patch link: https://lore.kernel.org/r/20250614-apr_14_for_sending-v4-8-8e3945c819cd%40samsung.com > > patch subject: [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V > > config: riscv-kismet-CONFIG_DRM_GEM_SHMEM_HELPER-CONFIG_DRM_POWERVR-0-0 (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/config) > > reproduce: (https://download.01.org/0day-ci/archive/20250615/202506151839.IKkZs0Z0-lkp@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202506151839.IKkZs0Z0-lkp@intel.com/ > > > > kismet warnings: (new ones prefixed by >>) > >>> kismet: WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER when selected by DRM_POWERVR > > WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER > > Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n] > > I believe this is triggered because RISC-V can be compiled without MMU > support, while MMU support is mandatory for ARM64. > > Would an acceptable fix be to require an explicit dependency on the MMU, > like so? > > depends on (ARM64 || RISCV) && MMU > I'd put them on separate lines. While at it: how about enabling build with COMPILE_TEST to extend build coverage too? Bart > > Selected by [y]: > > - DRM_POWERVR [=y] && HAS_IOMEM [=y] && (ARM64 || RISCV [=y]) && DRM [=y] && PM [=y] > > > > Best regards, > -- > Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 0/8] Add TH1520 GPU support with power sequencing 2025-06-14 18:06 ` [PATCH v4 0/8] Add TH1520 GPU support with power sequencing Michal Wilczynski ` (7 preceding siblings ...) [not found] ` <CGME20250614180916eucas1p1ee5f075be14d88aa828ec4e55c26bae9@eucas1p1.samsung.com> @ 2025-06-17 22:14 ` Drew Fustini 2025-06-18 10:02 ` Michal Wilczynski 8 siblings, 1 reply; 18+ messages in thread From: Drew Fustini @ 2025-06-17 22:14 UTC (permalink / raw) To: Michal Wilczynski Cc: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel, Krzysztof Kozlowski On Sat, Jun 14, 2025 at 08:06:06PM +0200, Michal Wilczynski wrote: > This patch series introduces support for the Imagination IMG BXM-4-64 > GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is > managing the GPU's complex power-up and power-down sequence, which > involves multiple clocks and resets. > > The TH1520 GPU requires a specific sequence to be followed for its > clocks and resets to ensure correct operation. Initial discussions and > an earlier version of this series explored managing this via the generic > power domain (genpd) framework. However, following further discussions > with kernel maintainers [1], the approach has been reworked to utilize > the dedicated power sequencing (pwrseq) framework. > > This revised series now employs a new pwrseq provider driver > (pwrseq-thead-gpu.c) specifically for the TH1520 GPU. This driver > encapsulates the SoC specific power sequence details. The Imagination > GPU driver (pvr_device.c) is updated to act as a consumer of this power > sequencer, requesting the "gpu-power" target. The sequencer driver, > during its match phase with the GPU device, acquires the necessary clock > and reset handles from the GPU device node to perform the full sequence. > > This approach aligns with the goal of abstracting SoC specific power > management details away from generic device drivers and leverages the > pwrseq framework as recommended. > > The series is structured as follows: > > Patch 1: Introduces the pwrseq-thead-gpu auxiliary driver to manage the > GPU's power-on/off sequence. > Patch 2: Adds device tree bindings for the gpu-clkgen reset to the > existing thead,th1520-aon binding. > Patch 3: Extends the pm-domains driver to detect the gpu-clkgen reset > and spawn the pwrseq-thead-gpu auxiliary driver. > Patch 4: Updates the Imagination DRM driver to utilize the pwrseq > framework for TH1520 GPU power management. > Patch 5: Adds the thead,th1520-gpu compatible string to the PowerVR GPU > device tree bindings. > Patch 6: Adds the gpu-clkgen reset property to the aon node in the > TH1520 device tree source. > Patch 7: Adds the device tree node for the IMG BXM-4-64 GPU and its > required fixed-clock. > Patch 8: Enables compilation of the Imagination PowerVR driver on the > RISC-V architecture. > > This patchset finishes the work started in bigger series [2] by adding > all remaining GPU power sequencing piece. After this patchset the GPU > probes correctly. > > This series supersedes the previous genpd based approach. Testing on > T-HEAD TH1520 SoC indicates the new pwrseq based solution works > correctly. > > An open point in Patch 7/8 concerns the GPU memory clock (gpu_mem_clk), > defined as a fixed-clock. The specific hardware frequency for this clock > on the TH1520 could not be determined from available public > documentation. Consequently, clock-frequency = <0>; has been used as a > placeholder to enable driver functionality. > I don't have any more information that what is in the public PDFs [1], so I think it is okay to have a placeholder frequency. Is it the case that the frequency doesn't really matter from the perspective of the driver? Thanks, Drew [1] https://git.beagleboard.org/beaglev-ahead/beaglev-ahead/-/tree/main/docs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 0/8] Add TH1520 GPU support with power sequencing 2025-06-17 22:14 ` [PATCH v4 0/8] Add TH1520 GPU support with power sequencing Drew Fustini @ 2025-06-18 10:02 ` Michal Wilczynski 0 siblings, 0 replies; 18+ messages in thread From: Michal Wilczynski @ 2025-06-18 10:02 UTC (permalink / raw) To: Drew Fustini Cc: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel, Krzysztof Kozlowski On 6/18/25 00:14, Drew Fustini wrote: > On Sat, Jun 14, 2025 at 08:06:06PM +0200, Michal Wilczynski wrote: >> This patch series introduces support for the Imagination IMG BXM-4-64 >> GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is >> managing the GPU's complex power-up and power-down sequence, which >> involves multiple clocks and resets. >> >> The TH1520 GPU requires a specific sequence to be followed for its >> clocks and resets to ensure correct operation. Initial discussions and >> an earlier version of this series explored managing this via the generic >> power domain (genpd) framework. However, following further discussions >> with kernel maintainers [1], the approach has been reworked to utilize >> the dedicated power sequencing (pwrseq) framework. >> >> This revised series now employs a new pwrseq provider driver >> (pwrseq-thead-gpu.c) specifically for the TH1520 GPU. This driver >> encapsulates the SoC specific power sequence details. The Imagination >> GPU driver (pvr_device.c) is updated to act as a consumer of this power >> sequencer, requesting the "gpu-power" target. The sequencer driver, >> during its match phase with the GPU device, acquires the necessary clock >> and reset handles from the GPU device node to perform the full sequence. >> >> This approach aligns with the goal of abstracting SoC specific power >> management details away from generic device drivers and leverages the >> pwrseq framework as recommended. >> >> The series is structured as follows: >> >> Patch 1: Introduces the pwrseq-thead-gpu auxiliary driver to manage the >> GPU's power-on/off sequence. >> Patch 2: Adds device tree bindings for the gpu-clkgen reset to the >> existing thead,th1520-aon binding. >> Patch 3: Extends the pm-domains driver to detect the gpu-clkgen reset >> and spawn the pwrseq-thead-gpu auxiliary driver. >> Patch 4: Updates the Imagination DRM driver to utilize the pwrseq >> framework for TH1520 GPU power management. >> Patch 5: Adds the thead,th1520-gpu compatible string to the PowerVR GPU >> device tree bindings. >> Patch 6: Adds the gpu-clkgen reset property to the aon node in the >> TH1520 device tree source. >> Patch 7: Adds the device tree node for the IMG BXM-4-64 GPU and its >> required fixed-clock. >> Patch 8: Enables compilation of the Imagination PowerVR driver on the >> RISC-V architecture. >> >> This patchset finishes the work started in bigger series [2] by adding >> all remaining GPU power sequencing piece. After this patchset the GPU >> probes correctly. >> >> This series supersedes the previous genpd based approach. Testing on >> T-HEAD TH1520 SoC indicates the new pwrseq based solution works >> correctly. >> >> An open point in Patch 7/8 concerns the GPU memory clock (gpu_mem_clk), >> defined as a fixed-clock. The specific hardware frequency for this clock >> on the TH1520 could not be determined from available public >> documentation. Consequently, clock-frequency = <0>; has been used as a >> placeholder to enable driver functionality. >> > > I don't have any more information that what is in the public PDFs [1], > so I think it is okay to have a placeholder frequency. > > Is it the case that the frequency doesn't really matter from the > perspective of the driver? Yeah it doesn't matter, I asked simply because it would be better in the DT to accurately describe the HW. I would omit the 'clock-frequency' altogether, but doing that makes the driver probe fail. > > Thanks, > Drew > > [1] https://protect2.fireeye.com/v1/url?k=260051e8-477bfb60-2601daa7-74fe4860018a-782a548f971ff58f&q=1&e=7e973bd1-ed36-4a12-af1c-1cf44bea2e5c&u=https%3A%2F%2Fgit.beagleboard.org%2Fbeaglev-ahead%2Fbeaglev-ahead%2F-%2Ftree%2Fmain%2Fdocs > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-06-18 10:02 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <CGME20250614180906eucas1p116f8a13a4013edd3bbedfd2e4a8b0aa3@eucas1p1.samsung.com> 2025-06-14 18:06 ` [PATCH v4 0/8] Add TH1520 GPU support with power sequencing Michal Wilczynski [not found] ` <CGME20250614180907eucas1p13d341c30e495fb36598b1d7c10ec7070@eucas1p1.samsung.com> 2025-06-14 18:06 ` [PATCH v4 1/8] power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver Michal Wilczynski 2025-06-16 9:03 ` Bartosz Golaszewski [not found] ` <CGME20250614180908eucas1p1f8e152bc86111d1fab4916e1737534e1@eucas1p1.samsung.com> 2025-06-14 18:06 ` [PATCH v4 2/8] dt-bindings: firmware: thead,th1520: Add resets for GPU clkgen Michal Wilczynski [not found] ` <CGME20250614180909eucas1p2a34e3242fb42f7fd25e4038c291276ff@eucas1p2.samsung.com> 2025-06-14 18:06 ` [PATCH v4 3/8] pmdomain: thead: Instantiate GPU power sequencer via auxiliary bus Michal Wilczynski 2025-06-16 9:10 ` Bartosz Golaszewski [not found] ` <CGME20250614180911eucas1p16c9fb4a8160253c253f623bec2529f70@eucas1p1.samsung.com> 2025-06-14 18:06 ` [PATCH v4 4/8] drm/imagination: Use pwrseq for TH1520 GPU power management Michal Wilczynski 2025-06-16 9:40 ` Bartosz Golaszewski 2025-06-16 18:50 ` Michal Wilczynski [not found] ` <CGME20250614180912eucas1p283681bf6a16249417e5e6d8eb25b969c@eucas1p2.samsung.com> 2025-06-14 18:06 ` [PATCH v4 5/8] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible Michal Wilczynski [not found] ` <CGME20250614180913eucas1p2554c9e8d5024c534565d3c1de58f2e61@eucas1p2.samsung.com> 2025-06-14 18:06 ` [PATCH v4 6/8] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node Michal Wilczynski [not found] ` <CGME20250614180914eucas1p202074745dba308cf1e18d1b75a2f3cea@eucas1p2.samsung.com> 2025-06-14 18:06 ` [PATCH v4 7/8] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node Michal Wilczynski [not found] ` <CGME20250614180916eucas1p1ee5f075be14d88aa828ec4e55c26bae9@eucas1p1.samsung.com> 2025-06-14 18:06 ` [PATCH v4 8/8] drm/imagination: Enable PowerVR driver for RISC-V Michal Wilczynski 2025-06-15 10:51 ` kernel test robot 2025-06-16 9:09 ` Michal Wilczynski 2025-06-16 9:22 ` Bartosz Golaszewski 2025-06-17 22:14 ` [PATCH v4 0/8] Add TH1520 GPU support with power sequencing Drew Fustini 2025-06-18 10:02 ` Michal Wilczynski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).