Hi Devarsh,
On Mon, Oct 09, 2023 at 01:20:18PM +0530, Devarsh Thakkar wrote:
> Some SoC's such as AM62P have dedicated power domains
> for OLDI which need to be powered on separetely along
> with display controller.
>
> So during driver probe, power up all attached PM domains
> enumerated in devicetree node for DSS.
>
> This also prepares base to add display support for AM62P.
>
> Signed-off-by: Devarsh Thakkar <devar...@ti.com>
> ---
> drivers/gpu/drm/tidss/tidss_drv.c | 76 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/tidss/tidss_drv.h | 5 ++
> 2 files changed, 81 insertions(+)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c
> b/drivers/gpu/drm/tidss/tidss_drv.c
> index 4d063eb9cd0b..a703a27d17bf 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -8,6 +8,7 @@
> #include <linux/of.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/pm_domain.h>
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> @@ -114,6 +115,72 @@ static const struct drm_driver tidss_driver = {
> .minor = 0,
> };
>
> +static int tidss_detach_pm_domains(struct tidss_device *tidss)
> +{
> + int i;
> +
> + if (tidss->num_domains <= 1)
> + return 0;
> +
> + for (i = 0; i < tidss->num_domains; i++) {
> + if (tidss->pd_link[i] && !IS_ERR(tidss->pd_link[i]))
> + device_link_del(tidss->pd_link[i]);
> + if (tidss->pd_dev[i] && !IS_ERR(tidss->pd_dev[i]))
> + dev_pm_domain_detach(tidss->pd_dev[i], true);
> + tidss->pd_dev[i] = NULL;
> + tidss->pd_link[i] = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static int tidss_attach_pm_domains(struct tidss_device *tidss)
> +{
> + struct device *dev = tidss->dev;
> + int i;
> + int ret;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct device_node *np = pdev->dev.of_node;
> +
> + tidss->num_domains = of_count_phandle_with_args(np, "power-domains",
> + "#power-domain-cells");
> + if (tidss->num_domains <= 1) {
> + dev_dbg(dev, "One or less power domains, no need to do attach
> domains\n");
> + return 0;
> + }
> +
> + tidss->pd_dev = devm_kmalloc_array(dev, tidss->num_domains,
> + sizeof(*tidss->pd_dev), GFP_KERNEL);
> + if (!tidss->pd_dev)
> + return -ENOMEM;
> +
> + tidss->pd_link = devm_kmalloc_array(dev, tidss->num_domains,
> + sizeof(*tidss->pd_link),
> GFP_KERNEL);
> + if (!tidss->pd_link)
> + return -ENOMEM;
> +
> + for (i = 0; i < tidss->num_domains; i++) {
> + tidss->pd_dev[i] = dev_pm_domain_attach_by_id(dev, i);
> + if (IS_ERR(tidss->pd_dev[i])) {
> + ret = PTR_ERR(tidss->pd_dev[i]);
> + goto fail;
> + }
> +
> + tidss->pd_link[i] = device_link_add(dev, tidss->pd_dev[i],
> + DL_FLAG_STATELESS |
> + DL_FLAG_PM_RUNTIME |
> DL_FLAG_RPM_ACTIVE);
> + if (!tidss->pd_link[i]) {
> + ret = -EINVAL;
> + goto fail;
> + }
> + }
> +
> + return 0;
> +fail:
> + tidss_detach_pm_domains(tidss);
> + return ret;
> +}My understanding is that this will be done automatically at probe time. Why do we need to roll our own there? A comment on top of the function and the commit log would help. Thanks! Maxime
signature.asc
Description: PGP signature
