2024-05-12 05:50:45

by Kiarash Hajian

[permalink] [raw]
Subject: [PATCH v3 0/2] drm/msm/a6xx: request memory region

Signed-off-by: Kiarash Hajian <[email protected]>
---
Changes in v3:
- Remove redundant devm_iounmap calls, relying on devres for automatic resource cleanup.

Changes in v2:
- update the subject prefix to "drm/msm/a6xx:", to match the majority of other changes to this file.

---
Kiarash Hajian (2):
drm/msm/a6xx: request memory region
drm/msm/a6xx: request memory region

drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
---
base-commit: cf87f46fd34d6c19283d9625a7822f20d90b64a4
change-id: 20240511-msm-adreno-memory-region-2bcb1c958621

Best regards,
--
Kiarash Hajian <[email protected]>



2024-05-12 05:50:59

by Kiarash Hajian

[permalink] [raw]
Subject: [PATCH v3 1/2] drm/msm/a6xx: request memory region

The driver's memory regions are currently just ioremap()ed, but not
reserved through a request. That's not a bug, but having the request is
a little more robust.

Implement the region-request through the corresponding managed
devres-function.

Signed-off-by: Kiarash Hajian <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 8bea8ef26f77..aa83cb461a75 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -636,9 +636,9 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)

err:
if (!IS_ERR_OR_NULL(pdcptr))
- iounmap(pdcptr);
+ devm_iounmap(&pdev->dev,pdcptr);
if (!IS_ERR_OR_NULL(seqptr))
- iounmap(seqptr);
+ devm_iounmap(&pdev->dev,seqptr);
}

/*
@@ -1503,7 +1503,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
return ERR_PTR(-EINVAL);
}

- ret = ioremap(res->start, resource_size(res));
+ ret = devm_ioremap_resource(&pdev->dev, res);
if (!ret) {
DRM_DEV_ERROR(&pdev->dev, "Unable to map the %s registers\n", name);
return ERR_PTR(-EINVAL);
@@ -1646,7 +1646,7 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
dev_pm_domain_detach(gmu->cxpd, false);

err_mmio:
- iounmap(gmu->mmio);
+ devm_iounmap(gmu->dev ,gmu->mmio);

/* Drop reference taken in of_find_device_by_node */
put_device(gmu->dev);
@@ -1825,9 +1825,9 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
dev_pm_domain_detach(gmu->cxpd, false);

err_mmio:
- iounmap(gmu->mmio);
+ devm_iounmap(gmu->dev ,gmu->mmio);
if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "rscc"))
- iounmap(gmu->rscc);
+ devm_iounmap(gmu->dev ,gmu->rscc);
free_irq(gmu->gmu_irq, gmu);
free_irq(gmu->hfi_irq, gmu);


--
2.43.0


2024-05-12 05:51:15

by Kiarash Hajian

[permalink] [raw]
Subject: [PATCH v3 2/2] drm/msm/a6xx: request memory region

The devm_iounmap function is being used unnecessarily,
managed resource mechanisms (devres) are handling resource cleanup automatically

This commit removes the calls to devm_iounmap and relies on devres

Signed-off-by: Kiarash Hajian <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 18 ------------------
1 file changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index aa83cb461a75..d64bf6212d6f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -524,9 +524,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
uint32_t pdc_address_offset;
bool pdc_in_aop = false;

- if (IS_ERR(pdcptr))
- goto err;
-
if (adreno_is_a650(adreno_gpu) ||
adreno_is_a660_family(adreno_gpu) ||
adreno_is_a7xx(adreno_gpu))
@@ -540,8 +537,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)

if (!pdc_in_aop) {
seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq");
- if (IS_ERR(seqptr))
- goto err;
}

/* Disable SDE clock gating */
@@ -633,12 +628,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
wmb();

a6xx_rpmh_stop(gmu);
-
-err:
- if (!IS_ERR_OR_NULL(pdcptr))
- devm_iounmap(&pdev->dev,pdcptr);
- if (!IS_ERR_OR_NULL(seqptr))
- devm_iounmap(&pdev->dev,seqptr);
}

/*
@@ -1635,7 +1624,6 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
if (IS_ERR(gmu->gxpd)) {
ret = PTR_ERR(gmu->gxpd);
- goto err_mmio;
}

gmu->initialized = true;
@@ -1645,9 +1633,6 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
detach_cxpd:
dev_pm_domain_detach(gmu->cxpd, false);

-err_mmio:
- devm_iounmap(gmu->dev ,gmu->mmio);
-
/* Drop reference taken in of_find_device_by_node */
put_device(gmu->dev);

@@ -1825,9 +1810,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
dev_pm_domain_detach(gmu->cxpd, false);

err_mmio:
- devm_iounmap(gmu->dev ,gmu->mmio);
- if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "rscc"))
- devm_iounmap(gmu->dev ,gmu->rscc);
free_irq(gmu->gmu_irq, gmu);
free_irq(gmu->hfi_irq, gmu);


--
2.43.0


2024-05-12 07:18:35

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] drm/msm/a6xx: request memory region

On Sun, May 12, 2024 at 01:49:39AM -0400, Kiarash Hajian wrote:
> The devm_iounmap function is being used unnecessarily,
> managed resource mechanisms (devres) are handling resource cleanup automatically
>
> This commit removes the calls to devm_iounmap and relies on devres
>
> Signed-off-by: Kiarash Hajian <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 18 ------------------
> 1 file changed, 18 deletions(-)

In my opinion, this patch is better be squashed into the first patch.
Though I'd leave a final word here to Rob and Konrad.

BTW: for some reason your patches don't appear on freedreno's patchwork,
although they definitely hit the list and appear on lore.kernel.org.

--
With best wishes
Dmitry

2024-05-12 22:25:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] drm/msm/a6xx: request memory region

Hi Kiarash,

kernel test robot noticed the following build errors:

[auto build test ERROR on cf87f46fd34d6c19283d9625a7822f20d90b64a4]

url: https://github.com/intel-lab-lkp/linux/commits/Kiarash-Hajian/drm-msm-a6xx-request-memory-region/20240512-135215
base: cf87f46fd34d6c19283d9625a7822f20d90b64a4
patch link: https://lore.kernel.org/r/20240512-msm-adreno-memory-region-v3-2-0a728ad45010%40gmail.com
patch subject: [PATCH v3 2/2] drm/msm/a6xx: request memory region
config: i386-buildonly-randconfig-001-20240513 (https://download.01.org/0day-ci/archive/20240513/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240513/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:1605:8: error: use of undeclared label 'err_mmio'
1605 | goto err_mmio;
| ^
1 error generated.


vim +/err_mmio +1605 drivers/gpu/drm/msm/adreno/a6xx_gmu.c

c11fa1204fe940 Akhil P Oommen 2023-01-02 1582
5a903a44a98471 Konrad Dybcio 2023-06-16 1583 int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
5a903a44a98471 Konrad Dybcio 2023-06-16 1584 {
5a903a44a98471 Konrad Dybcio 2023-06-16 1585 struct platform_device *pdev = of_find_device_by_node(node);
5a903a44a98471 Konrad Dybcio 2023-06-16 1586 struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
5a903a44a98471 Konrad Dybcio 2023-06-16 1587 int ret;
5a903a44a98471 Konrad Dybcio 2023-06-16 1588
5a903a44a98471 Konrad Dybcio 2023-06-16 1589 if (!pdev)
5a903a44a98471 Konrad Dybcio 2023-06-16 1590 return -ENODEV;
5a903a44a98471 Konrad Dybcio 2023-06-16 1591
5a903a44a98471 Konrad Dybcio 2023-06-16 1592 gmu->dev = &pdev->dev;
5a903a44a98471 Konrad Dybcio 2023-06-16 1593
5a903a44a98471 Konrad Dybcio 2023-06-16 1594 of_dma_configure(gmu->dev, node, true);
5a903a44a98471 Konrad Dybcio 2023-06-16 1595
5a903a44a98471 Konrad Dybcio 2023-06-16 1596 pm_runtime_enable(gmu->dev);
5a903a44a98471 Konrad Dybcio 2023-06-16 1597
5a903a44a98471 Konrad Dybcio 2023-06-16 1598 /* Mark legacy for manual SPTPRAC control */
5a903a44a98471 Konrad Dybcio 2023-06-16 1599 gmu->legacy = true;
5a903a44a98471 Konrad Dybcio 2023-06-16 1600
5a903a44a98471 Konrad Dybcio 2023-06-16 1601 /* Map the GMU registers */
5a903a44a98471 Konrad Dybcio 2023-06-16 1602 gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
5a903a44a98471 Konrad Dybcio 2023-06-16 1603 if (IS_ERR(gmu->mmio)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1604 ret = PTR_ERR(gmu->mmio);
5a903a44a98471 Konrad Dybcio 2023-06-16 @1605 goto err_mmio;
5a903a44a98471 Konrad Dybcio 2023-06-16 1606 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1607
5a903a44a98471 Konrad Dybcio 2023-06-16 1608 gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
5a903a44a98471 Konrad Dybcio 2023-06-16 1609 if (IS_ERR(gmu->cxpd)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1610 ret = PTR_ERR(gmu->cxpd);
5a903a44a98471 Konrad Dybcio 2023-06-16 1611 goto err_mmio;
5a903a44a98471 Konrad Dybcio 2023-06-16 1612 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1613
5a903a44a98471 Konrad Dybcio 2023-06-16 1614 if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1615 ret = -ENODEV;
5a903a44a98471 Konrad Dybcio 2023-06-16 1616 goto detach_cxpd;
5a903a44a98471 Konrad Dybcio 2023-06-16 1617 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1618
5a903a44a98471 Konrad Dybcio 2023-06-16 1619 init_completion(&gmu->pd_gate);
5a903a44a98471 Konrad Dybcio 2023-06-16 1620 complete_all(&gmu->pd_gate);
5a903a44a98471 Konrad Dybcio 2023-06-16 1621 gmu->pd_nb.notifier_call = cxpd_notifier_cb;
5a903a44a98471 Konrad Dybcio 2023-06-16 1622
5a903a44a98471 Konrad Dybcio 2023-06-16 1623 /* Get a link to the GX power domain to reset the GPU */
5a903a44a98471 Konrad Dybcio 2023-06-16 1624 gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
5a903a44a98471 Konrad Dybcio 2023-06-16 1625 if (IS_ERR(gmu->gxpd)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1626 ret = PTR_ERR(gmu->gxpd);
5a903a44a98471 Konrad Dybcio 2023-06-16 1627 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1628
5a903a44a98471 Konrad Dybcio 2023-06-16 1629 gmu->initialized = true;
5a903a44a98471 Konrad Dybcio 2023-06-16 1630
5a903a44a98471 Konrad Dybcio 2023-06-16 1631 return 0;
5a903a44a98471 Konrad Dybcio 2023-06-16 1632
5a903a44a98471 Konrad Dybcio 2023-06-16 1633 detach_cxpd:
5a903a44a98471 Konrad Dybcio 2023-06-16 1634 dev_pm_domain_detach(gmu->cxpd, false);
5a903a44a98471 Konrad Dybcio 2023-06-16 1635
5a903a44a98471 Konrad Dybcio 2023-06-16 1636 /* Drop reference taken in of_find_device_by_node */
5a903a44a98471 Konrad Dybcio 2023-06-16 1637 put_device(gmu->dev);
5a903a44a98471 Konrad Dybcio 2023-06-16 1638
5a903a44a98471 Konrad Dybcio 2023-06-16 1639 return ret;
5a903a44a98471 Konrad Dybcio 2023-06-16 1640 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1641

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-12 23:07:08

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] drm/msm/a6xx: request memory region

Hi Kiarash,

kernel test robot noticed the following build errors:

[auto build test ERROR on cf87f46fd34d6c19283d9625a7822f20d90b64a4]

url: https://github.com/intel-lab-lkp/linux/commits/Kiarash-Hajian/drm-msm-a6xx-request-memory-region/20240512-135215
base: cf87f46fd34d6c19283d9625a7822f20d90b64a4
patch link: https://lore.kernel.org/r/20240512-msm-adreno-memory-region-v3-2-0a728ad45010%40gmail.com
patch subject: [PATCH v3 2/2] drm/msm/a6xx: request memory region
config: i386-buildonly-randconfig-003-20240513 (https://download.01.org/0day-ci/archive/20240513/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240513/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/gpu/drm/msm/adreno/a6xx_gmu.c: In function 'a6xx_gmu_wrapper_init':
>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:1611:17: error: label 'err_mmio' used but not defined
1611 | goto err_mmio;
| ^~~~


vim +/err_mmio +1611 drivers/gpu/drm/msm/adreno/a6xx_gmu.c

c11fa1204fe940 Akhil P Oommen 2023-01-02 1582
5a903a44a98471 Konrad Dybcio 2023-06-16 1583 int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
5a903a44a98471 Konrad Dybcio 2023-06-16 1584 {
5a903a44a98471 Konrad Dybcio 2023-06-16 1585 struct platform_device *pdev = of_find_device_by_node(node);
5a903a44a98471 Konrad Dybcio 2023-06-16 1586 struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
5a903a44a98471 Konrad Dybcio 2023-06-16 1587 int ret;
5a903a44a98471 Konrad Dybcio 2023-06-16 1588
5a903a44a98471 Konrad Dybcio 2023-06-16 1589 if (!pdev)
5a903a44a98471 Konrad Dybcio 2023-06-16 1590 return -ENODEV;
5a903a44a98471 Konrad Dybcio 2023-06-16 1591
5a903a44a98471 Konrad Dybcio 2023-06-16 1592 gmu->dev = &pdev->dev;
5a903a44a98471 Konrad Dybcio 2023-06-16 1593
5a903a44a98471 Konrad Dybcio 2023-06-16 1594 of_dma_configure(gmu->dev, node, true);
5a903a44a98471 Konrad Dybcio 2023-06-16 1595
5a903a44a98471 Konrad Dybcio 2023-06-16 1596 pm_runtime_enable(gmu->dev);
5a903a44a98471 Konrad Dybcio 2023-06-16 1597
5a903a44a98471 Konrad Dybcio 2023-06-16 1598 /* Mark legacy for manual SPTPRAC control */
5a903a44a98471 Konrad Dybcio 2023-06-16 1599 gmu->legacy = true;
5a903a44a98471 Konrad Dybcio 2023-06-16 1600
5a903a44a98471 Konrad Dybcio 2023-06-16 1601 /* Map the GMU registers */
5a903a44a98471 Konrad Dybcio 2023-06-16 1602 gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
5a903a44a98471 Konrad Dybcio 2023-06-16 1603 if (IS_ERR(gmu->mmio)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1604 ret = PTR_ERR(gmu->mmio);
5a903a44a98471 Konrad Dybcio 2023-06-16 1605 goto err_mmio;
5a903a44a98471 Konrad Dybcio 2023-06-16 1606 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1607
5a903a44a98471 Konrad Dybcio 2023-06-16 1608 gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
5a903a44a98471 Konrad Dybcio 2023-06-16 1609 if (IS_ERR(gmu->cxpd)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1610 ret = PTR_ERR(gmu->cxpd);
5a903a44a98471 Konrad Dybcio 2023-06-16 @1611 goto err_mmio;
5a903a44a98471 Konrad Dybcio 2023-06-16 1612 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1613
5a903a44a98471 Konrad Dybcio 2023-06-16 1614 if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1615 ret = -ENODEV;
5a903a44a98471 Konrad Dybcio 2023-06-16 1616 goto detach_cxpd;
5a903a44a98471 Konrad Dybcio 2023-06-16 1617 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1618
5a903a44a98471 Konrad Dybcio 2023-06-16 1619 init_completion(&gmu->pd_gate);
5a903a44a98471 Konrad Dybcio 2023-06-16 1620 complete_all(&gmu->pd_gate);
5a903a44a98471 Konrad Dybcio 2023-06-16 1621 gmu->pd_nb.notifier_call = cxpd_notifier_cb;
5a903a44a98471 Konrad Dybcio 2023-06-16 1622
5a903a44a98471 Konrad Dybcio 2023-06-16 1623 /* Get a link to the GX power domain to reset the GPU */
5a903a44a98471 Konrad Dybcio 2023-06-16 1624 gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
5a903a44a98471 Konrad Dybcio 2023-06-16 1625 if (IS_ERR(gmu->gxpd)) {
5a903a44a98471 Konrad Dybcio 2023-06-16 1626 ret = PTR_ERR(gmu->gxpd);
5a903a44a98471 Konrad Dybcio 2023-06-16 1627 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1628
5a903a44a98471 Konrad Dybcio 2023-06-16 1629 gmu->initialized = true;
5a903a44a98471 Konrad Dybcio 2023-06-16 1630
5a903a44a98471 Konrad Dybcio 2023-06-16 1631 return 0;
5a903a44a98471 Konrad Dybcio 2023-06-16 1632
5a903a44a98471 Konrad Dybcio 2023-06-16 1633 detach_cxpd:
5a903a44a98471 Konrad Dybcio 2023-06-16 1634 dev_pm_domain_detach(gmu->cxpd, false);
5a903a44a98471 Konrad Dybcio 2023-06-16 1635
5a903a44a98471 Konrad Dybcio 2023-06-16 1636 /* Drop reference taken in of_find_device_by_node */
5a903a44a98471 Konrad Dybcio 2023-06-16 1637 put_device(gmu->dev);
5a903a44a98471 Konrad Dybcio 2023-06-16 1638
5a903a44a98471 Konrad Dybcio 2023-06-16 1639 return ret;
5a903a44a98471 Konrad Dybcio 2023-06-16 1640 }
5a903a44a98471 Konrad Dybcio 2023-06-16 1641

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki