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 <ezequ...@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
