* [PATCH] drm/panfrost: Prefix interrupt handlers' names @ 2019-12-13 12:39 Ezequiel Garcia 2019-12-13 13:18 ` Neil Armstrong 0 siblings, 1 reply; 8+ messages in thread From: Ezequiel Garcia @ 2019-12-13 12:39 UTC (permalink / raw) To: Rob Herring, Tomeu Vizoso, dri-devel Cc: kernel, Ezequiel Garcia, Alyssa Rosenzweig, Steven Price Currently, the interrupt lines requested by Panfrost use ambiguous names, which adds some obscurity to interrupt introspection (i.e. any tool based on procfs' interrupts file). In order to improve this, prefix each requested interrupt with either the module name or the device name, where possible. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index f67ed925c0ef..0355c4a78eaa 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -348,7 +348,7 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) return -ENODEV; err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler, - IRQF_SHARED, "gpu", pfdev); + IRQF_SHARED, dev_name(pfdev->dev), pfdev); if (err) { dev_err(pfdev->dev, "failed to request gpu irq"); return err; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 05c85f45a0de..3bd79ebb6c40 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -480,7 +480,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) return -ENODEV; ret = devm_request_irq(pfdev->dev, irq, panfrost_job_irq_handler, - IRQF_SHARED, "job", pfdev); + IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); if (ret) { dev_err(pfdev->dev, "failed to request job irq"); return ret; diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..806958434726 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -612,9 +612,11 @@ int panfrost_mmu_init(struct panfrost_device *pfdev) if (irq <= 0) return -ENODEV; - err = devm_request_threaded_irq(pfdev->dev, irq, panfrost_mmu_irq_handler, + err = devm_request_threaded_irq(pfdev->dev, irq, + panfrost_mmu_irq_handler, panfrost_mmu_irq_handler_thread, - IRQF_SHARED, "mmu", pfdev); + IRQF_SHARED, KBUILD_MODNAME "-mmu", + pfdev); if (err) { dev_err(pfdev->dev, "failed to request mmu irq"); -- 2.22.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 12:39 [PATCH] drm/panfrost: Prefix interrupt handlers' names Ezequiel Garcia @ 2019-12-13 13:18 ` Neil Armstrong 2019-12-13 13:46 ` Robin Murphy 0 siblings, 1 reply; 8+ messages in thread From: Neil Armstrong @ 2019-12-13 13:18 UTC (permalink / raw) To: Ezequiel Garcia, Rob Herring, Tomeu Vizoso, dri-devel Cc: kernel, Alyssa Rosenzweig, Steven Price Hi, On 13/12/2019 13:39, Ezequiel Garcia wrote: > Currently, the interrupt lines requested by Panfrost > use ambiguous names, which adds some obscurity > to interrupt introspection (i.e. any tool based > on procfs' interrupts file). > > In order to improve this, prefix each requested > interrupt with either the module name or the device > name, where possible. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++++-- > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c > index f67ed925c0ef..0355c4a78eaa 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c > @@ -348,7 +348,7 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) > return -ENODEV; > > err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler, > - IRQF_SHARED, "gpu", pfdev); > + IRQF_SHARED, dev_name(pfdev->dev), pfdev); > if (err) { > dev_err(pfdev->dev, "failed to request gpu irq"); > return err; > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index 05c85f45a0de..3bd79ebb6c40 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -480,7 +480,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) > return -ENODEV; > > ret = devm_request_irq(pfdev->dev, irq, panfrost_job_irq_handler, > - IRQF_SHARED, "job", pfdev); > + IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); > if (ret) { > dev_err(pfdev->dev, "failed to request job irq"); > return ret; > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 842bdd7cf6be..806958434726 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -612,9 +612,11 @@ int panfrost_mmu_init(struct panfrost_device *pfdev) > if (irq <= 0) > return -ENODEV; > > - err = devm_request_threaded_irq(pfdev->dev, irq, panfrost_mmu_irq_handler, > + err = devm_request_threaded_irq(pfdev->dev, irq, > + panfrost_mmu_irq_handler, > panfrost_mmu_irq_handler_thread, > - IRQF_SHARED, "mmu", pfdev); > + IRQF_SHARED, KBUILD_MODNAME "-mmu", > + pfdev); > > if (err) { > dev_err(pfdev->dev, "failed to request mmu irq"); > Why don't you use dev_name(pfdev->dev) everywhere ? Neil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 13:18 ` Neil Armstrong @ 2019-12-13 13:46 ` Robin Murphy 2019-12-13 14:32 ` Alyssa Rosenzweig 2019-12-13 15:46 ` Ezequiel Garcia 0 siblings, 2 replies; 8+ messages in thread From: Robin Murphy @ 2019-12-13 13:46 UTC (permalink / raw) To: Neil Armstrong, Ezequiel Garcia, Rob Herring, Tomeu Vizoso, dri-devel Cc: kernel, Alyssa Rosenzweig, Steven Price On 13/12/2019 1:18 pm, Neil Armstrong wrote: > Hi, > > On 13/12/2019 13:39, Ezequiel Garcia wrote: >> Currently, the interrupt lines requested by Panfrost >> use ambiguous names, which adds some obscurity >> to interrupt introspection (i.e. any tool based >> on procfs' interrupts file). >> >> In order to improve this, prefix each requested >> interrupt with either the module name or the device >> name, where possible. >> >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> >> --- >> drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +- >> drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- >> drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++++-- >> 3 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c >> index f67ed925c0ef..0355c4a78eaa 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c >> @@ -348,7 +348,7 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) >> return -ENODEV; >> >> err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler, >> - IRQF_SHARED, "gpu", pfdev); >> + IRQF_SHARED, dev_name(pfdev->dev), pfdev); >> if (err) { >> dev_err(pfdev->dev, "failed to request gpu irq"); >> return err; >> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c >> index 05c85f45a0de..3bd79ebb6c40 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_job.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c >> @@ -480,7 +480,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) >> return -ENODEV; >> >> ret = devm_request_irq(pfdev->dev, irq, panfrost_job_irq_handler, >> - IRQF_SHARED, "job", pfdev); >> + IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); >> if (ret) { >> dev_err(pfdev->dev, "failed to request job irq"); >> return ret; >> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c >> index 842bdd7cf6be..806958434726 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c >> @@ -612,9 +612,11 @@ int panfrost_mmu_init(struct panfrost_device *pfdev) >> if (irq <= 0) >> return -ENODEV; >> >> - err = devm_request_threaded_irq(pfdev->dev, irq, panfrost_mmu_irq_handler, >> + err = devm_request_threaded_irq(pfdev->dev, irq, >> + panfrost_mmu_irq_handler, >> panfrost_mmu_irq_handler_thread, >> - IRQF_SHARED, "mmu", pfdev); >> + IRQF_SHARED, KBUILD_MODNAME "-mmu", >> + pfdev); >> >> if (err) { >> dev_err(pfdev->dev, "failed to request mmu irq"); >> > > Why don't you use dev_name(pfdev->dev) everywhere ? Agreed, while the current implementation may be confusing it is at least self-consistent. TBH it would probably be sufficient to save the bother of allocating strings and just settle on "panfrost-{gpu,job,mmu}", since upstream users are unlikely to ever come across a system with more than one Mali in it ;) And FWIW note that "GPU" really is the specific hardware name of that IRQ output; it's not just a generic fill-in for "the one that isn't the Job or MMU IRQ". Thanks, Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 13:46 ` Robin Murphy @ 2019-12-13 14:32 ` Alyssa Rosenzweig 2019-12-13 15:31 ` Robin Murphy 2019-12-13 15:46 ` Ezequiel Garcia 1 sibling, 1 reply; 8+ messages in thread From: Alyssa Rosenzweig @ 2019-12-13 14:32 UTC (permalink / raw) To: Robin Murphy Cc: Tomeu Vizoso, Neil Armstrong, dri-devel, Steven Price, kernel, Ezequiel Garcia [-- Attachment #1.1: Type: text/plain, Size: 288 bytes --] > TBH it would probably be sufficient to save the bother of allocating > strings and just settle on "panfrost-{gpu,job,mmu}", since upstream > users are unlikely to ever come across a system with more than one > Mali in it ;) Agreed. ----Wait, you said *upstream*? Are there .... oh no [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 14:32 ` Alyssa Rosenzweig @ 2019-12-13 15:31 ` Robin Murphy 2019-12-13 15:49 ` Alyssa Rosenzweig 0 siblings, 1 reply; 8+ messages in thread From: Robin Murphy @ 2019-12-13 15:31 UTC (permalink / raw) To: Alyssa Rosenzweig Cc: Tomeu Vizoso, Neil Armstrong, dri-devel, Steven Price, kernel, Ezequiel Garcia On 13/12/2019 2:32 pm, Alyssa Rosenzweig wrote: >> TBH it would probably be sufficient to save the bother of allocating >> strings and just settle on "panfrost-{gpu,job,mmu}", since upstream >> users are unlikely to ever come across a system with more than one >> Mali in it ;) > > Agreed. > > ----Wait, you said *upstream*? Are there .... oh no Heh, fear not - I'm only thinking of the "Juno board with FPGA prototyping stack" setup, and the people using those in anger are all working on some other driver anyway ;) Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 15:31 ` Robin Murphy @ 2019-12-13 15:49 ` Alyssa Rosenzweig 0 siblings, 0 replies; 8+ messages in thread From: Alyssa Rosenzweig @ 2019-12-13 15:49 UTC (permalink / raw) To: Robin Murphy Cc: Tomeu Vizoso, Neil Armstrong, dri-devel, Steven Price, kernel, Ezequiel Garcia [-- Attachment #1.1: Type: text/plain, Size: 640 bytes --] On Fri, Dec 13, 2019 at 03:31:45PM +0000, Robin Murphy wrote: > On 13/12/2019 2:32 pm, Alyssa Rosenzweig wrote: > > > TBH it would probably be sufficient to save the bother of allocating > > > strings and just settle on "panfrost-{gpu,job,mmu}", since upstream > > > users are unlikely to ever come across a system with more than one > > > Mali in it ;) > > > > Agreed. > > > > ----Wait, you said *upstream*? Are there .... oh no > > Heh, fear not - I'm only thinking of the "Juno board with FPGA prototyping > stack" setup, and the people using those in anger are all working on some > other driver anyway ;) Gotcha :) [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 13:46 ` Robin Murphy 2019-12-13 14:32 ` Alyssa Rosenzweig @ 2019-12-13 15:46 ` Ezequiel Garcia 2019-12-13 15:51 ` Alyssa Rosenzweig 1 sibling, 1 reply; 8+ messages in thread From: Ezequiel Garcia @ 2019-12-13 15:46 UTC (permalink / raw) To: Robin Murphy, Neil Armstrong, Rob Herring, Tomeu Vizoso, dri-devel Cc: kernel, Alyssa Rosenzweig, Steven Price Hey everyone, Thanks for the quick comments. (Feedback for kernel patches on the same day, am I dreaming??) On Fri, 2019-12-13 at 13:46 +0000, Robin Murphy wrote: > On 13/12/2019 1:18 pm, Neil Armstrong wrote: > > Hi, > > > > On 13/12/2019 13:39, Ezequiel Garcia wrote: > > > Currently, the interrupt lines requested by Panfrost > > > use ambiguous names, which adds some obscurity > > > to interrupt introspection (i.e. any tool based > > > on procfs' interrupts file). > > > > > > In order to improve this, prefix each requested > > > interrupt with either the module name or the device > > > name, where possible. > > > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +- > > > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- > > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++++-- > > > 3 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c > > > index f67ed925c0ef..0355c4a78eaa 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c > > > @@ -348,7 +348,7 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) > > > return -ENODEV; > > > > > > err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler, > > > - IRQF_SHARED, "gpu", pfdev); > > > + IRQF_SHARED, dev_name(pfdev->dev), pfdev); > > > if (err) { > > > dev_err(pfdev->dev, "failed to request gpu irq"); > > > return err; > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > > > index 05c85f45a0de..3bd79ebb6c40 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > > > @@ -480,7 +480,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) > > > return -ENODEV; > > > > > > ret = devm_request_irq(pfdev->dev, irq, panfrost_job_irq_handler, > > > - IRQF_SHARED, "job", pfdev); > > > + IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); > > > if (ret) { > > > dev_err(pfdev->dev, "failed to request job irq"); > > > return ret; > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > index 842bdd7cf6be..806958434726 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > @@ -612,9 +612,11 @@ int panfrost_mmu_init(struct panfrost_device *pfdev) > > > if (irq <= 0) > > > return -ENODEV; > > > > > > - err = devm_request_threaded_irq(pfdev->dev, irq, panfrost_mmu_irq_handler, > > > + err = devm_request_threaded_irq(pfdev->dev, irq, > > > + panfrost_mmu_irq_handler, > > > panfrost_mmu_irq_handler_thread, > > > - IRQF_SHARED, "mmu", pfdev); > > > + IRQF_SHARED, KBUILD_MODNAME "-mmu", > > > + pfdev); > > > > > > if (err) { > > > dev_err(pfdev->dev, "failed to request mmu irq"); > > > > > > > Why don't you use dev_name(pfdev->dev) everywhere ? > > Agreed, while the current implementation may be confusing it is at least > self-consistent. TBH it would probably be sufficient to save the bother > of allocating strings and just settle on "panfrost-{gpu,job,mmu}", since > upstream users are unlikely to ever come across a system with more than > one Mali in it ;) > > And FWIW note that "GPU" really is the specific hardware name of that > IRQ output; it's not just a generic fill-in for "the one that isn't the > Job or MMU IRQ". > Yeah, that makese sense. So how about we go for "panfrost-{job,mmu}" and leave the "gpu" one? Or "panfrost-{gpu,job,mmu}" for consistency? Thanks, Eze _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/panfrost: Prefix interrupt handlers' names 2019-12-13 15:46 ` Ezequiel Garcia @ 2019-12-13 15:51 ` Alyssa Rosenzweig 0 siblings, 0 replies; 8+ messages in thread From: Alyssa Rosenzweig @ 2019-12-13 15:51 UTC (permalink / raw) To: Ezequiel Garcia Cc: Tomeu Vizoso, Neil Armstrong, dri-devel, Steven Price, kernel, Robin Murphy [-- Attachment #1.1: Type: text/plain, Size: 838 bytes --] > (Feedback for kernel patches on the same day, am I dreaming??) That's panfrost! > > Agreed, while the current implementation may be confusing it is at least > > self-consistent. TBH it would probably be sufficient to save the bother > > of allocating strings and just settle on "panfrost-{gpu,job,mmu}", since > > upstream users are unlikely to ever come across a system with more than > > one Mali in it ;) > > > > And FWIW note that "GPU" really is the specific hardware name of that > > IRQ output; it's not just a generic fill-in for "the one that isn't the > > Job or MMU IRQ". > > > > Yeah, that makese sense. So how about we go for "panfrost-{job,mmu}" > and leave the "gpu" one? > > Or "panfrost-{gpu,job,mmu}" for consistency? I would prefer "panfrost-{gpu,job,mmu}" for consistency, I think. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-14 14:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-13 12:39 [PATCH] drm/panfrost: Prefix interrupt handlers' names Ezequiel Garcia 2019-12-13 13:18 ` Neil Armstrong 2019-12-13 13:46 ` Robin Murphy 2019-12-13 14:32 ` Alyssa Rosenzweig 2019-12-13 15:31 ` Robin Murphy 2019-12-13 15:49 ` Alyssa Rosenzweig 2019-12-13 15:46 ` Ezequiel Garcia 2019-12-13 15:51 ` Alyssa Rosenzweig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.