[PATCH 0/2] drm/lima: two driver cleanups

[PATCH 0/2] drm/lima: two driver cleanups

[Erico Nunes]

Patch 1 is a fix for a crash which triggers on removing the module on
kernels with CONFIG_DEBUG_SHIRQ enabled, such as the Fedora kernel.

Patch 2 is a fix to this warning:
  drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer
  type 'enum lima_gpu_id' from 'const void *'
  [-Werror,-Wvoid-pointer-to-enum-cast]
which we have received as a repeated report from the kernel test bot to
the lima mailing list.
The warning only reproduces with recent clang on aarch64, but the patch
does get rid of it and there seem to be no more warnings for W=1.

Erico Nunes (2):
  drm/lima: fix shared irq handling on driver remove
  drm/lima: fix void pointer to enum lima_gpu_id cast warning

 drivers/gpu/drm/lima/lima_drv.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/lima/lima_drv.h |  5 +++++
 drivers/gpu/drm/lima/lima_gp.c  |  2 ++
 drivers/gpu/drm/lima/lima_mmu.c |  5 +++++
 drivers/gpu/drm/lima/lima_pp.c  |  4 ++++
 5 files changed, 34 insertions(+), 3 deletions(-)

-- 
2.44.0

[PATCH 1/2] drm/lima: fix shared irq handling on driver remove

[Erico Nunes]

lima uses a shared interrupt, so the interrupt handlers must be prepared
to be called at any time. At driver removal time, the clocks are
disabled early and the interrupts stay registered until the very end of
the remove process due to the devm usage.
This is potentially a bug as the interrupts access device registers
which assumes clocks are enabled. A crash can be triggered by removing
the driver in a kernel with CONFIG_DEBUG_SHIRQ enabled.
This patch frees the interrupts at each lima device finishing callback
so that the handlers are already unregistered by the time we fully
disable clocks.

Signed-off-by: Erico Nunes <nunes.erico@gmail.com>
---
 drivers/gpu/drm/lima/lima_gp.c  | 2 ++
 drivers/gpu/drm/lima/lima_mmu.c | 5 +++++
 drivers/gpu/drm/lima/lima_pp.c  | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/lima/lima_gp.c b/drivers/gpu/drm/lima/lima_gp.c
index 6b354e2fb61d..14c39be8da90 100644
--- a/drivers/gpu/drm/lima/lima_gp.c
+++ b/drivers/gpu/drm/lima/lima_gp.c
@@ -338,7 +338,9 @@ int lima_gp_init(struct lima_ip *ip)
 
 void lima_gp_fini(struct lima_ip *ip)
 {
+	struct lima_device *dev = ip->dev;
 
+	devm_free_irq(dev->dev, ip->irq, ip);
 }
 
 int lima_gp_pipe_init(struct lima_device *dev)
diff --git a/drivers/gpu/drm/lima/lima_mmu.c b/drivers/gpu/drm/lima/lima_mmu.c
index e18317c5ca8c..6611e2836bf0 100644
--- a/drivers/gpu/drm/lima/lima_mmu.c
+++ b/drivers/gpu/drm/lima/lima_mmu.c
@@ -118,7 +118,12 @@ int lima_mmu_init(struct lima_ip *ip)
 
 void lima_mmu_fini(struct lima_ip *ip)
 {
+	struct lima_device *dev = ip->dev;
+
+	if (ip->id == lima_ip_ppmmu_bcast)
+		return;
 
+	devm_free_irq(dev->dev, ip->irq, ip);
 }
 
 void lima_mmu_flush_tlb(struct lima_ip *ip)
diff --git a/drivers/gpu/drm/lima/lima_pp.c b/drivers/gpu/drm/lima/lima_pp.c
index d0d2db0ef1ce..84bec163c9ed 100644
--- a/drivers/gpu/drm/lima/lima_pp.c
+++ b/drivers/gpu/drm/lima/lima_pp.c
@@ -286,7 +286,9 @@ int lima_pp_init(struct lima_ip *ip)
 
 void lima_pp_fini(struct lima_ip *ip)
 {
+	struct lima_device *dev = ip->dev;
 
+	devm_free_irq(dev->dev, ip->irq, ip);
 }
 
 int lima_pp_bcast_resume(struct lima_ip *ip)
@@ -319,7 +321,9 @@ int lima_pp_bcast_init(struct lima_ip *ip)
 
 void lima_pp_bcast_fini(struct lima_ip *ip)
 {
+	struct lima_device *dev = ip->dev;
 
+	devm_free_irq(dev->dev, ip->irq, ip);
 }
 
 static int lima_pp_task_validate(struct lima_sched_pipe *pipe,
-- 
2.44.0

[PATCH 2/2] drm/lima: fix void pointer to enum lima_gpu_id cast warning

[Erico Nunes]

Create a simple data struct to hold compatible data so that we don't
have to do the casts to void pointer to hold data.
Fixes the following warning:
  drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer
  type 'enum lima_gpu_id' from 'const void *'

Signed-off-by: Erico Nunes <nunes.erico@gmail.com>
---
 drivers/gpu/drm/lima/lima_drv.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/lima/lima_drv.h |  5 +++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
index 10fd9154cc46..739c865b556f 100644
--- a/drivers/gpu/drm/lima/lima_drv.c
+++ b/drivers/gpu/drm/lima/lima_drv.c
@@ -371,6 +371,7 @@ static int lima_pdev_probe(struct platform_device *pdev)
 {
 	struct lima_device *ldev;
 	struct drm_device *ddev;
+	const struct lima_compatible *comp;
 	int err;
 
 	err = lima_sched_slab_init();
@@ -384,7 +385,13 @@ static int lima_pdev_probe(struct platform_device *pdev)
 	}
 
 	ldev->dev = &pdev->dev;
-	ldev->id = (enum lima_gpu_id)of_device_get_match_data(&pdev->dev);
+	comp = of_device_get_match_data(&pdev->dev);
+	if (!comp) {
+		err = -ENODEV;
+		goto err_out0;
+	}
+
+	ldev->id = comp->id;
 
 	platform_set_drvdata(pdev, ldev);
 
@@ -459,9 +466,17 @@ static void lima_pdev_remove(struct platform_device *pdev)
 	lima_sched_slab_fini();
 }
 
+static const struct lima_compatible lima_mali400_data = {
+	.id = lima_gpu_mali400,
+};
+
+static const struct lima_compatible lima_mali450_data = {
+	.id = lima_gpu_mali450,
+};
+
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "arm,mali-400", .data = (void *)lima_gpu_mali400 },
-	{ .compatible = "arm,mali-450", .data = (void *)lima_gpu_mali450 },
+	{ .compatible = "arm,mali-400", .data = &lima_mali400_data },
+	{ .compatible = "arm,mali-450", .data = &lima_mali450_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
diff --git a/drivers/gpu/drm/lima/lima_drv.h b/drivers/gpu/drm/lima/lima_drv.h
index c738d288547b..6706c19b166e 100644
--- a/drivers/gpu/drm/lima/lima_drv.h
+++ b/drivers/gpu/drm/lima/lima_drv.h
@@ -7,6 +7,7 @@
 #include <drm/drm_file.h>
 
 #include "lima_ctx.h"
+#include "lima_device.h"
 
 extern int lima_sched_timeout_ms;
 extern uint lima_heap_init_nr_pages;
@@ -39,6 +40,10 @@ struct lima_submit {
 	struct lima_sched_task *task;
 };
 
+struct lima_compatible {
+	enum lima_gpu_id id;
+};
+
 static inline struct lima_drm_priv *
 to_lima_drm_priv(struct drm_file *file)
 {
-- 
2.44.0

Re: [PATCH 0/2] drm/lima: two driver cleanups

[Qiang Yu]

Serial is Reviewed-by: Qiang Yu <yuq825@gmail.com>

On Tue, Apr 2, 2024 at 6:43 AM Erico Nunes <nunes.erico@gmail.com> wrote:
>
> Patch 1 is a fix for a crash which triggers on removing the module on
> kernels with CONFIG_DEBUG_SHIRQ enabled, such as the Fedora kernel.
>
> Patch 2 is a fix to this warning:
>   drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer
>   type 'enum lima_gpu_id' from 'const void *'
>   [-Werror,-Wvoid-pointer-to-enum-cast]
> which we have received as a repeated report from the kernel test bot to
> the lima mailing list.
> The warning only reproduces with recent clang on aarch64, but the patch
> does get rid of it and there seem to be no more warnings for W=1.
>
> Erico Nunes (2):
>   drm/lima: fix shared irq handling on driver remove
>   drm/lima: fix void pointer to enum lima_gpu_id cast warning
>
>  drivers/gpu/drm/lima/lima_drv.c | 21 ++++++++++++++++++---
>  drivers/gpu/drm/lima/lima_drv.h |  5 +++++
>  drivers/gpu/drm/lima/lima_gp.c  |  2 ++
>  drivers/gpu/drm/lima/lima_mmu.c |  5 +++++
>  drivers/gpu/drm/lima/lima_pp.c  |  4 ++++
>  5 files changed, 34 insertions(+), 3 deletions(-)
>
> --
> 2.44.0
>

Re: [PATCH 0/2] drm/lima: two driver cleanups

[Qiang Yu]

applied to drm-misc-next

On Thu, Apr 4, 2024 at 8:51 PM Qiang Yu <yuq825@gmail.com> wrote:
>
> Serial is Reviewed-by: Qiang Yu <yuq825@gmail.com>
>
> On Tue, Apr 2, 2024 at 6:43 AM Erico Nunes <nunes.erico@gmail.com> wrote:
> >
> > Patch 1 is a fix for a crash which triggers on removing the module on
> > kernels with CONFIG_DEBUG_SHIRQ enabled, such as the Fedora kernel.
> >
> > Patch 2 is a fix to this warning:
> >   drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer
> >   type 'enum lima_gpu_id' from 'const void *'
> >   [-Werror,-Wvoid-pointer-to-enum-cast]
> > which we have received as a repeated report from the kernel test bot to
> > the lima mailing list.
> > The warning only reproduces with recent clang on aarch64, but the patch
> > does get rid of it and there seem to be no more warnings for W=1.
> >
> > Erico Nunes (2):
> >   drm/lima: fix shared irq handling on driver remove
> >   drm/lima: fix void pointer to enum lima_gpu_id cast warning
> >
> >  drivers/gpu/drm/lima/lima_drv.c | 21 ++++++++++++++++++---
> >  drivers/gpu/drm/lima/lima_drv.h |  5 +++++
> >  drivers/gpu/drm/lima/lima_gp.c  |  2 ++
> >  drivers/gpu/drm/lima/lima_mmu.c |  5 +++++
> >  drivers/gpu/drm/lima/lima_pp.c  |  4 ++++
> >  5 files changed, 34 insertions(+), 3 deletions(-)
> >
> > --
> > 2.44.0
> >