From 4984979b9b9025a1cb9a9bea089d31a3e01ccff1 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 28 Aug 2013 15:02:37 +0200 Subject: drm/irq: simplify irq checks in drm_wait_vblank Checking for both an irq number _and_ whether it's enabled is redundant. Originally I've thought the drm_dev_to_irq call would break drivers which do their own irq checking, but those shouldn't have DRIVER_HAVE_IRQ set as Thierry Reding pointed out. But such drivers already need to set dev->irq_enabled for other reasons, so we might as well ditch that check, too. v2: Also drop the HAVE_IRQ check. Cc: Thierry Reding Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c2676b5908d..d4cc47690d6 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1160,9 +1160,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data, int ret; unsigned int flags, seq, crtc, high_crtc; - if (drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) - if ((!drm_dev_to_irq(dev)) || (!dev->irq_enabled)) - return -EINVAL; + if (!dev->irq_enabled) + return -EINVAL; if (vblwait->request.type & _DRM_VBLANK_SIGNAL) return -EINVAL; -- cgit v1.2.3-18-g5258 From eaaf8f0fc32468d4dedbb5b5f9c5bfb27744be4c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 28 Aug 2013 15:19:23 +0200 Subject: drm/pci: fold in irq_by_busid support This is a ums-only ioctl, and we've only ever supported ums (at least in upstream) on pci devices. So no point in keeping that piece of legacy logic abstracted within the drm bus driver. To keep things work without CONFIG_PCI also add a dummy ioctl. v2: Block the irq_by_busid ioctl for modeset drivers. v3: Spelling/whitespace polish (Thierry) Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 27 --------------------------- 1 file changed, 27 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index d4cc47690d6..e5920e7a439 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -56,33 +56,6 @@ */ #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 1000000 -/** - * Get interrupt from bus id. - * - * \param inode device inode. - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument, pointing to a drm_irq_busid structure. - * \return zero on success or a negative number on failure. - * - * Finds the PCI device with the specified bus id and gets its IRQ number. - * This IOCTL is deprecated, and will now return EINVAL for any busid not equal - * to that of the device that this DRM instance attached to. - */ -int drm_irq_by_busid(struct drm_device *dev, void *data, - struct drm_file *file_priv) -{ - struct drm_irq_busid *p = data; - - if (!dev->driver->bus->irq_by_busid) - return -EINVAL; - - if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) - return -EINVAL; - - return dev->driver->bus->irq_by_busid(dev, p); -} - /* * Clear vblank timestamp buffer for a crtc. */ -- cgit v1.2.3-18-g5258 From 22471cf638bab5d78ee1e6c465c9d7c1971ebbf5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 20:02:50 +0100 Subject: drm/irq: drm_control is a legacy ioctl, so pci devices only This just adds a correspdonding check, follow-up patches will exploit this. v2: Whitespace polish (Thierry) Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index e5920e7a439..580f9ed2dca 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -386,22 +386,21 @@ int drm_control(struct drm_device *dev, void *data, * this used to be a separate function in drm_dma.h */ + if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) + return 0; + if (drm_core_check_feature(dev, DRIVER_MODESET)) + return 0; + /* UMS was only ever support on pci devices. */ + if (WARN_ON(!dev->pdev)) + return -EINVAL; switch (ctl->func) { case DRM_INST_HANDLER: - if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) - return 0; - if (drm_core_check_feature(dev, DRIVER_MODESET)) - return 0; if (dev->if_version < DRM_IF_VERSION(1, 2) && ctl->irq != drm_dev_to_irq(dev)) return -EINVAL; return drm_irq_install(dev); case DRM_UNINST_HANDLER: - if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) - return 0; - if (drm_core_check_feature(dev, DRIVER_MODESET)) - return 0; return drm_irq_uninstall(dev); default: return -EINVAL; -- cgit v1.2.3-18-g5258 From e090c53b21e97864a25ada5c8437d9776978a050 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 20:27:05 +0100 Subject: drm/irq: remove cargo-culted locking from irq_install/uninstall The dev->struct_mutex locking in drm_irq.c only protects dev->irq_enabled. Which isn't really much at all and only prevents especially nasty ums userspace from concurrently installing the interrupt handling a few times. Or at least trying. There are tons of unlocked readers of dev->irqs_enabled in the vblank wait code (and by extension also in the pageflip code since that uses the same vblank timestamp engine). Real modesetting drivers should ensure that nothing can go haywire with a sane setup teardown sequence. So we only really need this for the drm_control ioctl, everywhere else this will just paper over nastiness. Note that drm/i915 is a bit specially due to the gem+ums combination. So there we also need to properly protect the entervt and leavevt ioctls. But it's definitely saner to do everything in one go than to drop the lock in-between. Finally there's the gpu reset code in drm/i915. That one's just race (concurrent userspace calls to for vblank waits of pageflips could spuriously fail). So wrap it up in with a nice comment since fixing this is more involved. v2: Rebase and fix commit message (Thierry) Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 580f9ed2dca..06a53f8b618 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -254,20 +254,13 @@ int drm_irq_install(struct drm_device *dev) if (drm_dev_to_irq(dev) == 0) return -EINVAL; - mutex_lock(&dev->struct_mutex); - /* Driver must have been initialized */ - if (!dev->dev_private) { - mutex_unlock(&dev->struct_mutex); + if (!dev->dev_private) return -EINVAL; - } - if (dev->irq_enabled) { - mutex_unlock(&dev->struct_mutex); + if (dev->irq_enabled) return -EBUSY; - } dev->irq_enabled = true; - mutex_unlock(&dev->struct_mutex); DRM_DEBUG("irq=%d\n", drm_dev_to_irq(dev)); @@ -288,9 +281,7 @@ int drm_irq_install(struct drm_device *dev) sh_flags, irqname, dev); if (ret < 0) { - mutex_lock(&dev->struct_mutex); dev->irq_enabled = false; - mutex_unlock(&dev->struct_mutex); return ret; } @@ -302,9 +293,7 @@ int drm_irq_install(struct drm_device *dev) ret = dev->driver->irq_postinstall(dev); if (ret < 0) { - mutex_lock(&dev->struct_mutex); dev->irq_enabled = false; - mutex_unlock(&dev->struct_mutex); if (!drm_core_check_feature(dev, DRIVER_MODESET)) vga_client_register(dev->pdev, NULL, NULL, NULL); free_irq(drm_dev_to_irq(dev), dev); @@ -330,10 +319,8 @@ int drm_irq_uninstall(struct drm_device *dev) if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) return -EINVAL; - mutex_lock(&dev->struct_mutex); irq_enabled = dev->irq_enabled; dev->irq_enabled = false; - mutex_unlock(&dev->struct_mutex); /* * Wake up any waiters so they don't hang. @@ -381,6 +368,7 @@ int drm_control(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_control *ctl = data; + int ret = 0; /* if we haven't irq we fallback for compatibility reasons - * this used to be a separate function in drm_dma.h @@ -399,9 +387,17 @@ int drm_control(struct drm_device *dev, void *data, if (dev->if_version < DRM_IF_VERSION(1, 2) && ctl->irq != drm_dev_to_irq(dev)) return -EINVAL; - return drm_irq_install(dev); + mutex_lock(&dev->struct_mutex); + ret = drm_irq_install(dev); + mutex_unlock(&dev->struct_mutex); + + return ret; case DRM_UNINST_HANDLER: - return drm_irq_uninstall(dev); + mutex_lock(&dev->struct_mutex); + ret = drm_irq_uninstall(dev); + mutex_unlock(&dev->struct_mutex); + + return ret; default: return -EINVAL; } -- cgit v1.2.3-18-g5258 From ebfa4324930618e72645d2eb7db1c9773228a868 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 20:27:09 +0100 Subject: drm: remove drm_dev_to_irq from drivers Only used in some legacy pci drivers, and dereferencing the PCI irq is actually shorter ... Since this removes all users for drm_dev_to_irq from the tree except in drm_irq.c, move the inline helper in there. It'll disappear soon, too. v2: Polish commit message (Thierry) Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 06a53f8b618..589e865832c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -233,6 +233,11 @@ static void drm_irq_vgaarb_nokms(void *cookie, bool state) } } +static inline int drm_dev_to_irq(struct drm_device *dev) +{ + return dev->driver->bus->get_irq(dev); +} + /** * Install IRQ handler. * -- cgit v1.2.3-18-g5258 From 7c1a38e391745cca72627979e5e02924bed5b43d Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 16 Dec 2013 11:21:15 +0100 Subject: drm/irq: track the irq installed in drm_irq_install in dev->irq To get rid of the dev->bus->get_irq callback we need to pass in the desired irq explicitly into drm_irq_install. To avoid having to do the same for drm_irq_unistall just track it internally. That leaves drivers with less room to botch things up. v2: Add the hunk lost in an earlier patch to this one (Thierry). v3: Fix up the totally fumbled logic in drm_irq_install and use the local variable consistently. Spotted by both Thierry and Laurent. Shame on me for failing to properly test the rebase version of this patch ... Cc: Thierry Reding Cc: Laurent Pinchart Reviewed-by: Thierry Reding Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 589e865832c..7cf407bbfed 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -249,14 +249,16 @@ static inline int drm_dev_to_irq(struct drm_device *dev) */ int drm_irq_install(struct drm_device *dev) { - int ret; + int ret, irq; unsigned long sh_flags = 0; char *irqname; + irq = drm_dev_to_irq(dev); + if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) return -EINVAL; - if (drm_dev_to_irq(dev) == 0) + if (irq == 0) return -EINVAL; /* Driver must have been initialized */ @@ -267,7 +269,7 @@ int drm_irq_install(struct drm_device *dev) return -EBUSY; dev->irq_enabled = true; - DRM_DEBUG("irq=%d\n", drm_dev_to_irq(dev)); + DRM_DEBUG("irq=%d\n", irq); /* Before installing handler */ if (dev->driver->irq_preinstall) @@ -282,7 +284,7 @@ int drm_irq_install(struct drm_device *dev) else irqname = dev->driver->name; - ret = request_irq(drm_dev_to_irq(dev), dev->driver->irq_handler, + ret = request_irq(irq, dev->driver->irq_handler, sh_flags, irqname, dev); if (ret < 0) { @@ -301,7 +303,9 @@ int drm_irq_install(struct drm_device *dev) dev->irq_enabled = false; if (!drm_core_check_feature(dev, DRIVER_MODESET)) vga_client_register(dev->pdev, NULL, NULL, NULL); - free_irq(drm_dev_to_irq(dev), dev); + free_irq(irq, dev); + } else { + dev->irq = irq; } return ret; @@ -344,7 +348,7 @@ int drm_irq_uninstall(struct drm_device *dev) if (!irq_enabled) return -EINVAL; - DRM_DEBUG("irq=%d\n", drm_dev_to_irq(dev)); + DRM_DEBUG("irq=%d\n", dev->irq); if (!drm_core_check_feature(dev, DRIVER_MODESET)) vga_client_register(dev->pdev, NULL, NULL, NULL); @@ -352,7 +356,7 @@ int drm_irq_uninstall(struct drm_device *dev) if (dev->driver->irq_uninstall) dev->driver->irq_uninstall(dev); - free_irq(drm_dev_to_irq(dev), dev); + free_irq(dev->irq, dev); return 0; } -- cgit v1.2.3-18-g5258 From a319c1a47855eef2e1789527688b8dfdcf101dba Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 21:09:15 +0100 Subject: drm/irq: Look up the pci irq directly in the drm_control ioctl It's only ever called for legacy drivers, which are all pci. Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 7cf407bbfed..cbf6f259bfe 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -377,7 +377,7 @@ int drm_control(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_control *ctl = data; - int ret = 0; + int ret = 0, irq; /* if we haven't irq we fallback for compatibility reasons - * this used to be a separate function in drm_dma.h @@ -393,8 +393,10 @@ int drm_control(struct drm_device *dev, void *data, switch (ctl->func) { case DRM_INST_HANDLER: + irq = dev->pdev->irq; + if (dev->if_version < DRM_IF_VERSION(1, 2) && - ctl->irq != drm_dev_to_irq(dev)) + ctl->irq != irq) return -EINVAL; mutex_lock(&dev->struct_mutex); ret = drm_irq_install(dev); -- cgit v1.2.3-18-g5258 From bb0f1b5c1695b4399cfd2359c114ae63edbb3ad8 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 21:09:27 +0100 Subject: drm: pass the irq explicitly to drm_irq_install Unfortunately this requires a drm-wide change, and I didn't see a sane way around that. Luckily it's fairly simple, we just need to inline the respective get_irq implementation from either drm_pci.c or drm_platform.c. With that we can now also remove drm_dev_to_irq from drm_irq.c. Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index cbf6f259bfe..de38bc9b658 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -233,11 +233,6 @@ static void drm_irq_vgaarb_nokms(void *cookie, bool state) } } -static inline int drm_dev_to_irq(struct drm_device *dev) -{ - return dev->driver->bus->get_irq(dev); -} - /** * Install IRQ handler. * @@ -247,14 +242,12 @@ static inline int drm_dev_to_irq(struct drm_device *dev) * \c irq_preinstall() and \c irq_postinstall() functions * before and after the installation. */ -int drm_irq_install(struct drm_device *dev) +int drm_irq_install(struct drm_device *dev, int irq) { - int ret, irq; + int ret; unsigned long sh_flags = 0; char *irqname; - irq = drm_dev_to_irq(dev); - if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) return -EINVAL; @@ -399,7 +392,7 @@ int drm_control(struct drm_device *dev, void *data, ctl->irq != irq) return -EINVAL; mutex_lock(&dev->struct_mutex); - ret = drm_irq_install(dev); + ret = drm_irq_install(dev, irq); mutex_unlock(&dev->struct_mutex); return ret; -- cgit v1.2.3-18-g5258 From 5829d1834e5486e83547f36576b160023c9609c2 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 3 Nov 2013 21:48:48 +0100 Subject: drm: rip out dev->devname This was only ever used to pretty-print the irq driver name. And on kms systems due to set_version bonghits we never set up the prettier name, ever. Which make this a bit pointless. Also, we can always dig out the driver-instance/irq relationship through other means, so this isn't that useful. So just rip it out to simplify the set_version/set_busid insanity a bit. Also delete the temporary busname from drm_pci_set_busid, it's now unused. v2: Rebase on top of the new host1x drm_bus for tegra. Reviewed-by: Thierry Reding Reviewed-by: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index de38bc9b658..7583767ec61 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -246,7 +246,6 @@ int drm_irq_install(struct drm_device *dev, int irq) { int ret; unsigned long sh_flags = 0; - char *irqname; if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) return -EINVAL; @@ -272,13 +271,8 @@ int drm_irq_install(struct drm_device *dev, int irq) if (drm_core_check_feature(dev, DRIVER_IRQ_SHARED)) sh_flags = IRQF_SHARED; - if (dev->devname) - irqname = dev->devname; - else - irqname = dev->driver->name; - ret = request_irq(irq, dev->driver->irq_handler, - sh_flags, irqname, dev); + sh_flags, dev->driver->name, dev); if (ret < 0) { dev->irq_enabled = false; -- cgit v1.2.3-18-g5258