[v4] Improve GPU reset sequence for Adreno GPU

[PATCH v4 3/5] drm/msm/a6xx: Vote for cx gdsc from gpu driver

Posted by Akhil P Oommen 2 years, 8 months ago

When a device has multiple power domains, dev->power_domain is left
empty during probe. That didn't cause any issue so far because we are
freeloading on smmu driver's vote on cx gdsc. Instead of that, create
a device_link between cx genpd device and gmu device to keep a vote from
gpu driver.

Before this patch:
localhost ~ # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
gx_gdsc		on		0
    /devices/genpd:1:3d6a000.gmu		active		0
cx_gdsc		on		0
    /devices/platform/soc@0/3da0000.iommu	active		0

After this patch:
localhost ~ # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
gx_gdsc		on		0
    /devices/genpd:1:3d6a000.gmu		active		0
cx_gdsc		on		0
    /devices/platform/soc@0/3da0000.iommu	active		0
    /devices/genpd:0:3d6a000.gmu		active		0

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---

(no changes since v1)

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 31 +++++++++++++++++++++++++++----
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 6484b97c5344..1580d0090f35 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1479,6 +1479,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
 
 	pm_runtime_force_suspend(gmu->dev);
 
+	/*
+	 * Since cxpd is a virt device, the devlink with gmu-dev will be removed
+	 * automatically when we do detach
+	 */
+	dev_pm_domain_detach(gmu->cxpd, false);
+
 	if (!IS_ERR_OR_NULL(gmu->gxpd)) {
 		pm_runtime_disable(gmu->gxpd);
 		dev_pm_domain_detach(gmu->gxpd, false);
@@ -1605,8 +1611,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 
 	if (adreno_is_a650_family(adreno_gpu)) {
 		gmu->rscc = a6xx_gmu_get_mmio(pdev, "rscc");
-		if (IS_ERR(gmu->rscc))
+		if (IS_ERR(gmu->rscc)) {
+			ret = -ENODEV;
 			goto err_mmio;
+		}
 	} else {
 		gmu->rscc = gmu->mmio + 0x23000;
 	}
@@ -1615,8 +1623,22 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	gmu->hfi_irq = a6xx_gmu_get_irq(gmu, pdev, "hfi", a6xx_hfi_irq);
 	gmu->gmu_irq = a6xx_gmu_get_irq(gmu, pdev, "gmu", a6xx_gmu_irq);
 
-	if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0)
+	if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0) {
+		ret = -ENODEV;
+		goto err_mmio;
+	}
+
+	gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
+	if (IS_ERR(gmu->cxpd)) {
+		ret = PTR_ERR(gmu->cxpd);
 		goto err_mmio;
+	}
+
+	if (!device_link_add(gmu->dev, gmu->cxpd,
+					DL_FLAG_PM_RUNTIME)) {
+		ret = -ENODEV;
+		goto detach_cxpd;
+	}
 
 	/*
 	 * Get a link to the GX power domain to reset the GPU in case of GMU
@@ -1634,6 +1656,9 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 
 	return 0;
 
+detach_cxpd:
+	dev_pm_domain_detach(gmu->cxpd, false);
+
 err_mmio:
 	iounmap(gmu->mmio);
 	if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "rscc"))
@@ -1641,8 +1666,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	free_irq(gmu->gmu_irq, gmu);
 	free_irq(gmu->hfi_irq, gmu);
 
-	ret = -ENODEV;
-
 err_memory:
 	a6xx_gmu_memory_free(gmu);
 err_put_device:
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index e034935b3986..5a42dd4dd31f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -56,6 +56,7 @@ struct a6xx_gmu {
 	int gmu_irq;
 
 	struct device *gxpd;
+	struct device *cxpd;
 
 	int idle_level;
 
-- 
2.7.4

Re: [PATCH v4 3/5] drm/msm/a6xx: Vote for cx gdsc from gpu driver

Posted by Ulf Hansson 2 years, 8 months ago

On Wed, 21 Dec 2022 at 18:14, Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
>
> When a device has multiple power domains, dev->power_domain is left
> empty during probe. That didn't cause any issue so far because we are
> freeloading on smmu driver's vote on cx gdsc. Instead of that, create
> a device_link between cx genpd device and gmu device to keep a vote from
> gpu driver.
>
> Before this patch:
> localhost ~ # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> gx_gdsc         on              0
>     /devices/genpd:1:3d6a000.gmu                active          0
> cx_gdsc         on              0
>     /devices/platform/soc@0/3da0000.iommu       active          0
>
> After this patch:
> localhost ~ # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> gx_gdsc         on              0
>     /devices/genpd:1:3d6a000.gmu                active          0
> cx_gdsc         on              0
>     /devices/platform/soc@0/3da0000.iommu       active          0
>     /devices/genpd:0:3d6a000.gmu                active          0
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>
> (no changes since v1)
>
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 31 +++++++++++++++++++++++++++----
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
>  2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 6484b97c5344..1580d0090f35 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -1479,6 +1479,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
>
>         pm_runtime_force_suspend(gmu->dev);
>
> +       /*
> +        * Since cxpd is a virt device, the devlink with gmu-dev will be removed
> +        * automatically when we do detach
> +        */
> +       dev_pm_domain_detach(gmu->cxpd, false);
> +
>         if (!IS_ERR_OR_NULL(gmu->gxpd)) {
>                 pm_runtime_disable(gmu->gxpd);
>                 dev_pm_domain_detach(gmu->gxpd, false);
> @@ -1605,8 +1611,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>
>         if (adreno_is_a650_family(adreno_gpu)) {
>                 gmu->rscc = a6xx_gmu_get_mmio(pdev, "rscc");
> -               if (IS_ERR(gmu->rscc))
> +               if (IS_ERR(gmu->rscc)) {
> +                       ret = -ENODEV;
>                         goto err_mmio;
> +               }
>         } else {
>                 gmu->rscc = gmu->mmio + 0x23000;
>         }
> @@ -1615,8 +1623,22 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>         gmu->hfi_irq = a6xx_gmu_get_irq(gmu, pdev, "hfi", a6xx_hfi_irq);
>         gmu->gmu_irq = a6xx_gmu_get_irq(gmu, pdev, "gmu", a6xx_gmu_irq);
>
> -       if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0)
> +       if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0) {
> +               ret = -ENODEV;
> +               goto err_mmio;
> +       }
> +
> +       gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
> +       if (IS_ERR(gmu->cxpd)) {
> +               ret = PTR_ERR(gmu->cxpd);
>                 goto err_mmio;
> +       }
> +
> +       if (!device_link_add(gmu->dev, gmu->cxpd,
> +                                       DL_FLAG_PM_RUNTIME)) {
> +               ret = -ENODEV;
> +               goto detach_cxpd;
> +       }
>
>         /*
>          * Get a link to the GX power domain to reset the GPU in case of GMU
> @@ -1634,6 +1656,9 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>
>         return 0;
>
> +detach_cxpd:
> +       dev_pm_domain_detach(gmu->cxpd, false);
> +
>  err_mmio:
>         iounmap(gmu->mmio);
>         if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "rscc"))
> @@ -1641,8 +1666,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>         free_irq(gmu->gmu_irq, gmu);
>         free_irq(gmu->hfi_irq, gmu);
>
> -       ret = -ENODEV;
> -
>  err_memory:
>         a6xx_gmu_memory_free(gmu);
>  err_put_device:
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index e034935b3986..5a42dd4dd31f 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -56,6 +56,7 @@ struct a6xx_gmu {
>         int gmu_irq;
>
>         struct device *gxpd;
> +       struct device *cxpd;
>
>         int idle_level;
>
> --
> 2.7.4
>

[PATCH v4 1/5] PM: domains: Allow a genpd consumer to require a synced power off
[PATCH v4 2/5] clk: qcom: gdsc: Support 'synced_poweroff' genpd flag
[PATCH v4 3/5] drm/msm/a6xx: Vote for cx gdsc from gpu driver
[PATCH v4 4/5] drm/msm/a6xx: Remove cx gdsc polling using 'reset'
[PATCH v4 5/5] drm/msm/a6xx: Use genpd notifier to ensure cx-gdsc collapse