On Thu, 10 Apr 2025 at 12:12, Matt Coster <matt.cos...@imgtec.com> wrote:
>
> The first supported GPU only used a single power domain so this was
> automatically handled by the device runtime.
>
> In order to support multiple power domains, they must be enumerated from
> devicetree and linked to both the GPU device and each other to ensure
> correct power sequencing at start time.
>
> For all Imagination Rogue GPUs, power domains are named "a", "b", etc. and
> the sequence A->B->... is always valid for startup with the reverse true
> for shutdown. Note this is not always the *only* valid sequence, but it's
> simple and does not require special-casing for different GPU power
> topologies.
>
> Signed-off-by: Matt Coster <matt.cos...@imgtec.com>
> Reviewed-by: Frank Binns <frank.bi...@imgtec.com>
Realize that I am a bit late to this, but I was recently pointed to
this commit [1] by Michal.
[...]
> +
> +int pvr_power_domains_init(struct pvr_device *pvr_dev)
> +{
> + struct device *dev = from_pvr_device(pvr_dev)->dev;
> +
> + struct device_link **domain_links __free(kfree) = NULL;
> + struct device **domain_devs __free(kfree) = NULL;
> + int domain_count;
> + int link_count;
> +
> + char dev_name[2] = "a";
It's certainly good to use a name to be future proof, but we should at
least try to pick a somewhat descriptive name for what this PM domain
is used for.
Moreover, this requires a corresponding update to the DT docs.
> + int err;
> + int i;
> +
> + domain_count = of_count_phandle_with_args(dev->of_node,
> "power-domains",
> + "#power-domain-cells");
> + if (domain_count < 0)
> + return domain_count;
> +
> + if (domain_count <= 1)
> + return 0;
> +
> + link_count = domain_count + (domain_count - 1);
> +
> + domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
> + if (!domain_devs)
> + return -ENOMEM;
> +
> + domain_links = kcalloc(link_count, sizeof(*domain_links), GFP_KERNEL);
> + if (!domain_links)
> + return -ENOMEM;
> +
> + for (i = 0; i < domain_count; i++) {
> + struct device *domain_dev;
> +
> + dev_name[0] = 'a' + i;
> + domain_dev = dev_pm_domain_attach_by_name(dev, dev_name);
> + if (IS_ERR_OR_NULL(domain_dev)) {
> + err = domain_dev ? PTR_ERR(domain_dev) : -ENODEV;
> + goto err_detach;
> + }
> +
> + domain_devs[i] = domain_dev;
> + }
> +
> + for (i = 0; i < domain_count; i++) {
> + struct device_link *link;
> +
> + link = device_link_add(dev, domain_devs[i], DL_FLAG_STATELESS
> | DL_FLAG_PM_RUNTIME);
> + if (!link) {
> + err = -ENODEV;
> + goto err_unlink;
> + }
> +
> + domain_links[i] = link;
> + }
> +
> + for (i = domain_count; i < link_count; i++) {
> + struct device_link *link;
> +
> + link = device_link_add(domain_devs[i - domain_count + 1],
> + domain_devs[i - domain_count],
> + DL_FLAG_STATELESS |
> DL_FLAG_PM_RUNTIME);
> + if (!link) {
> + err = -ENODEV;
> + goto err_unlink;
> + }
> +
> + domain_links[i] = link;
> + }
Most of the code above (and the error/remove-path) can be considered
as boiler-plate code and can be replaced by using
devm_pm_domain_attach_list() instead.
[...]
Kind regards
Uffe
[1]
https://lore.kernel.org/all/a68e3bee-f4ad-4d73-a5a8-e39772c41...@samsung.com/
