linux/drivers/gpu/drm/drm_drv.c

1071 lines
29 KiB
C
Raw Normal View History

/*
* Created: Fri Jan 19 10:48:35 2001 by faith@acm.org
*
* Copyright 2001 VA Linux Systems, Inc., Sunnyvale, California.
* All Rights Reserved.
*
* Author Rickard E. (Rik) Faith <faith@valinux.com>
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice (including the next
* paragraph) shall be included in all copies or substantial portions of the
* Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*/
#include <linux/debugfs.h>
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
#include <linux/fs.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
#include <linux/mount.h>
#include <linux/pseudo_fs.h>
include cleanup: Update gfp.h and slab.h includes to prepare for breaking implicit slab.h inclusion from percpu.h percpu.h is included by sched.h and module.h and thus ends up being included when building most .c files. percpu.h includes slab.h which in turn includes gfp.h making everything defined by the two files universally available and complicating inclusion dependencies. percpu.h -> slab.h dependency is about to be removed. Prepare for this change by updating users of gfp and slab facilities include those headers directly instead of assuming availability. As this conversion needs to touch large number of source files, the following script is used as the basis of conversion. http://userweb.kernel.org/~tj/misc/slabh-sweep.py The script does the followings. * Scan files for gfp and slab usages and update includes such that only the necessary includes are there. ie. if only gfp is used, gfp.h, if slab is used, slab.h. * When the script inserts a new include, it looks at the include blocks and try to put the new include such that its order conforms to its surrounding. It's put in the include block which contains core kernel includes, in the same order that the rest are ordered - alphabetical, Christmas tree, rev-Xmas-tree or at the end if there doesn't seem to be any matching order. * If the script can't find a place to put a new include (mostly because the file doesn't have fitting include block), it prints out an error message indicating which .h file needs to be added to the file. The conversion was done in the following steps. 1. The initial automatic conversion of all .c files updated slightly over 4000 files, deleting around 700 includes and adding ~480 gfp.h and ~3000 slab.h inclusions. The script emitted errors for ~400 files. 2. Each error was manually checked. Some didn't need the inclusion, some needed manual addition while adding it to implementation .h or embedding .c file was more appropriate for others. This step added inclusions to around 150 files. 3. The script was run again and the output was compared to the edits from #2 to make sure no file was left behind. 4. Several build tests were done and a couple of problems were fixed. e.g. lib/decompress_*.c used malloc/free() wrappers around slab APIs requiring slab.h to be added manually. 5. The script was run on all .h files but without automatically editing them as sprinkling gfp.h and slab.h inclusions around .h files could easily lead to inclusion dependency hell. Most gfp.h inclusion directives were ignored as stuff from gfp.h was usually wildly available and often used in preprocessor macros. Each slab.h inclusion directive was examined and added manually as necessary. 6. percpu.h was updated not to include slab.h. 7. Build test were done on the following configurations and failures were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my distributed build env didn't work with gcov compiles) and a few more options had to be turned off depending on archs to make things build (like ipr on powerpc/64 which failed due to missing writeq). * x86 and x86_64 UP and SMP allmodconfig and a custom test config. * powerpc and powerpc64 SMP allmodconfig * sparc and sparc64 SMP allmodconfig * ia64 SMP allmodconfig * s390 SMP allmodconfig * alpha SMP allmodconfig * um on x86_64 SMP allmodconfig 8. percpu.h modifications were reverted so that it could be applied as a separate patch and serve as bisection point. Given the fact that I had only a couple of failures from tests on step 6, I'm fairly confident about the coverage of this conversion patch. If there is a breakage, it's likely to be something in one of the arch headers which should be easily discoverable easily on most builds of the specific arch. Signed-off-by: Tejun Heo <tj@kernel.org> Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
2010-03-24 08:04:11 +00:00
#include <linux/slab.h>
#include <linux/srcu.h>
#include <drm/drm_client.h>
#include <drm/drm_color_mgmt.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
drm: Set final_kfree in drm_dev_alloc I also did a full review of all callers, and only the xen driver forgot to call drm_dev_put in the failure path. Fix that up too. v2: I noticed that xen has a drm_driver.release hook, and uses drm_dev_alloc(). We need to remove the kfree from xen_drm_drv_release(). bochs also has a release hook, but leaked the drm_device ever since commit 0a6659bdc5e8221da99eebb176fd9591435e38de Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Dec 17 18:04:46 2013 +0100 drm/bochs: new driver This patch here fixes that leak. Same for virtio, started leaking with commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Feb 11 14:58:04 2020 +0100 drm/virtio: add drm_driver.release callback. Acked-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: xen-devel@lists.xenproject.org Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: xen-devel@lists.xenproject.org Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-5-daniel.vetter@ffwll.ch
2020-03-23 14:49:03 +00:00
#include <drm/drm_managed.h>
#include <drm/drm_mode_object.h>
#include <drm/drm_print.h>
#include "drm_crtc_internal.h"
#include "drm_internal.h"
#include "drm_legacy.h"
drm: drop obsolete drm_core.h The drm_core.h header contains a set of constants meant to be used throughout DRM. However, as it turns out, they're each used just once and don't bring any benefit. They're also grossly mis-named and lack name-spacing. This patch inlines them, or moves them into drm_internal.h as appropriate: - CORE_AUTHOR and CORE_DESC are inlined into corresponding MODULE_*() macros. It's just confusing having to follow 2 pointers when trying to find the definition of these fields. Grep'ping for MODULE_AUTHOR() should reveal the full information, if there's no strong reason not to. - CORE_NAME, CORE_DATE, CORE_MAJOR, CORE_MINOR, and CORE_PATCHLEVEL are inlined into the sysfs 'version' attribute. They're stripped everywhere else (which is just some printk() statements). CORE_NAME just doesn't make *any* sense, as we hard-code it in many places, anyway. The other constants are outdated and just serve binary-compatibility purposes. Hence, inline them in 'version' sysfs attribute (we might even try dropping it..). - DRM_IF_MAJOR and DRM_IF_MINOR are moved into drm_internal.h as they're only used by the global ioctl handlers. Furthermore, versioning interfaces breaks backports and as such is deprecated, anyway. We just keep them for historic reasons. I doubt anyone will ever modify them again. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-6-dh.herrmann@gmail.com
2016-09-01 12:48:36 +00:00
MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl");
MODULE_DESCRIPTION("DRM shared core routines");
MODULE_LICENSE("GPL and additional rights");
static DEFINE_SPINLOCK(drm_minor_lock);
static struct idr drm_minors_idr;
/*
* If the drm core fails to init for whatever reason,
* we should prevent any drivers from registering with it.
* It's best to check this at drm_dev_init(), as some drivers
* prefer to embed struct drm_device into their own device
* structure and call drm_dev_init() themselves.
*/
static bool drm_core_init_complete = false;
static struct dentry *drm_debugfs_root;
DEFINE_STATIC_SRCU(drm_unplug_srcu);
/*
* DRM Minors
* A DRM device can provide several char-dev interfaces on the DRM-Major. Each
* of them is represented by a drm_minor object. Depending on the capabilities
* of the device-driver, different interfaces are registered.
*
* Minors can be accessed via dev->$minor_name. This pointer is either
* NULL or a valid drm_minor pointer and stays valid as long as the device is
* valid. This means, DRM minors have the same life-time as the underlying
* device. However, this doesn't mean that the minor is active. Minors are
* registered and unregistered dynamically according to device-state.
*/
static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
unsigned int type)
{
switch (type) {
case DRM_MINOR_PRIMARY:
return &dev->primary;
case DRM_MINOR_RENDER:
return &dev->render;
default:
BUG();
}
}
static void drm_minor_alloc_release(struct drm_device *dev, void *data)
{
struct drm_minor *minor = data;
unsigned long flags;
drm: Manage drm_mode_config_init with drmm_ drm_mode_config_cleanup is idempotent, so no harm in calling this twice. This allows us to gradually switch drivers over by removing explicit drm_mode_config_cleanup calls. With this step it's now also possible that (at least for simple drivers) automatic resource cleanup can be done correctly without a drm_driver->release hook. Therefore allow this now in devm_drm_dev_init(). Also with drmm_ explicit drm_driver->release hooks are kinda not the best option: Drivers can always just register their current release hook with drmm_add_action, but even better they could split them up to simplify the unwinding for the driver load failure case. So deprecate that hook to discourage future users. v2: Fixup the example in the kerneldoc too. v3: - For paranoia, double check that minor->dev == dev in the release hook, because I botched the pointer math in the drmm library. - Call drm_mode_config_cleanup when drmm_add_action fails, we'd be missing some mutex_destroy and ida_cleanup otherwise (Laurent) v4: Add a drmm_add_action_or_reset (like devm_ has) to encapsulate this pattern (Noralf). v5: Fix oversight in the new drmm_add_action_or_reset macro (Noralf) v4: Review from Sam: - drmm_mode_config_init wrapper (also suggested by Thomas) - improve commit message, explain better why ->relase is deprecated v5: - Make drmm_ the main function, with the old one as compat wrapper (Sam) - Add FIXME comments to drm_mode_config_cleanup/init() that drivers shouldn't use these anymore. - Move drmm_add_action_or_reset helper to an earlier patch. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: "Noralf Trønnes" <noralf@tronnes.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: Noralf Trønnes <noralf@tronnes.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-27-daniel.vetter@ffwll.ch
2020-03-23 14:49:25 +00:00
WARN_ON(dev != minor->dev);
put_device(minor->kdev);
spin_lock_irqsave(&drm_minor_lock, flags);
idr_remove(&drm_minors_idr, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
}
static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
unsigned long flags;
int r;
minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
if (!minor)
return -ENOMEM;
minor->type = type;
minor->dev = dev;
idr_preload(GFP_KERNEL);
spin_lock_irqsave(&drm_minor_lock, flags);
r = idr_alloc(&drm_minors_idr,
NULL,
64 * type,
64 * (type + 1),
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
idr_preload_end();
if (r < 0)
return r;
minor->index = r;
r = drmm_add_action_or_reset(dev, drm_minor_alloc_release, minor);
if (r)
return r;
minor->kdev = drm_sysfs_minor_alloc(minor);
if (IS_ERR(minor->kdev))
return PTR_ERR(minor->kdev);
*drm_minor_get_slot(dev, type) = minor;
return 0;
}
static int drm_minor_register(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
unsigned long flags;
int ret;
DRM_DEBUG("\n");
minor = *drm_minor_get_slot(dev, type);
if (!minor)
return 0;
ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
if (ret) {
DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
goto err_debugfs;
}
ret = device_add(minor->kdev);
if (ret)
drm: remove procfs code, take 2 So almost two years ago I've tried to nuke the procfs code already once before: http://lists.freedesktop.org/archives/dri-devel/2011-October/015707.html The conclusion was that userspace drivers (specifically libdrm device node detection) stopped relying on procfs in 2001. But after some digging it turned out that the drmstat tool in libdrm is still using those files (but only when certain options are set). So we've decided to keep profcs. But I when I've started to dig around again what exactly this tool does I've noticed that it tries to read the "mem", "vm", and "vma" files from procfs. Now as far my git history digging shows "mem" never did anything useful (at least in the version that first showed up in upstream history in 2004) and the file was remove in commit 955b12def42e83287c1bdb1411d99451753c1391 Author: Ben Gamari <bgamari@gmail.com> Date: Tue Feb 17 20:08:49 2009 -0500 drm: Convert proc files to seq_file and introduce debugfs Which means that for over 4 years drmstat has been broken, and no one cared. In my opinion that's proof enough that no one is actually using drmstat, and so that we can savely nuke the procfs support from drm. While at it fix up the error case cleanup for debugfs in drm_get_minor. v2: Fix dates, libdrm stopped relying on procfs for drm node detection in 2001. v3: fixup compilation warning for !CONFIG_DEBUG_FS, reported by Fengguang Wu. Cc: kbuild test robot <fengguang.wu@intel.com> Cc: Dave Airlie <airlied@linux.ie> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Dave Airlie <airlied@redhat.com>
2013-08-08 13:41:34 +00:00
goto err_debugfs;
/* replace NULL with @minor so lookups will succeed from now on */
spin_lock_irqsave(&drm_minor_lock, flags);
idr_replace(&drm_minors_idr, minor, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
DRM_DEBUG("new minor registered %d\n", minor->index);
return 0;
drm: remove procfs code, take 2 So almost two years ago I've tried to nuke the procfs code already once before: http://lists.freedesktop.org/archives/dri-devel/2011-October/015707.html The conclusion was that userspace drivers (specifically libdrm device node detection) stopped relying on procfs in 2001. But after some digging it turned out that the drmstat tool in libdrm is still using those files (but only when certain options are set). So we've decided to keep profcs. But I when I've started to dig around again what exactly this tool does I've noticed that it tries to read the "mem", "vm", and "vma" files from procfs. Now as far my git history digging shows "mem" never did anything useful (at least in the version that first showed up in upstream history in 2004) and the file was remove in commit 955b12def42e83287c1bdb1411d99451753c1391 Author: Ben Gamari <bgamari@gmail.com> Date: Tue Feb 17 20:08:49 2009 -0500 drm: Convert proc files to seq_file and introduce debugfs Which means that for over 4 years drmstat has been broken, and no one cared. In my opinion that's proof enough that no one is actually using drmstat, and so that we can savely nuke the procfs support from drm. While at it fix up the error case cleanup for debugfs in drm_get_minor. v2: Fix dates, libdrm stopped relying on procfs for drm node detection in 2001. v3: fixup compilation warning for !CONFIG_DEBUG_FS, reported by Fengguang Wu. Cc: kbuild test robot <fengguang.wu@intel.com> Cc: Dave Airlie <airlied@linux.ie> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Dave Airlie <airlied@redhat.com>
2013-08-08 13:41:34 +00:00
err_debugfs:
drm_debugfs_cleanup(minor);
return ret;
}
static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
unsigned long flags;
minor = *drm_minor_get_slot(dev, type);
if (!minor || !device_is_registered(minor->kdev))
return;
/* replace @minor with NULL so lookups will fail from now on */
spin_lock_irqsave(&drm_minor_lock, flags);
idr_replace(&drm_minors_idr, NULL, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
drm_debugfs_cleanup(minor);
}
/*
* Looks up the given minor-ID and returns the respective DRM-minor object. The
* refence-count of the underlying device is increased so you must release this
* object with drm_minor_release().
*
* As long as you hold this minor, it is guaranteed that the object and the
* minor->dev pointer will stay valid! However, the device may get unplugged and
* unregistered while you hold the minor.
*/
struct drm_minor *drm_minor_acquire(unsigned int minor_id)
{
struct drm_minor *minor;
unsigned long flags;
spin_lock_irqsave(&drm_minor_lock, flags);
minor = idr_find(&drm_minors_idr, minor_id);
if (minor)
drm_dev_get(minor->dev);
spin_unlock_irqrestore(&drm_minor_lock, flags);
if (!minor) {
return ERR_PTR(-ENODEV);
} else if (drm_dev_is_unplugged(minor->dev)) {
drm_dev_put(minor->dev);
return ERR_PTR(-ENODEV);
}
return minor;
}
void drm_minor_release(struct drm_minor *minor)
{
drm_dev_put(minor->dev);
}
/**
* DOC: driver instance overview
*
* A device instance for a drm driver is represented by &struct drm_device. This
* is allocated and initialized with devm_drm_dev_alloc(), usually from
* bus-specific ->probe() callbacks implemented by the driver. The driver then
* needs to initialize all the various subsystems for the drm device like memory
* management, vblank handling, modesetting support and initial output
* configuration plus obviously initialize all the corresponding hardware bits.
* Finally when everything is up and running and ready for userspace the device
* instance can be published using drm_dev_register().
*
* There is also deprecated support for initalizing device instances using
* bus-specific helpers and the &drm_driver.load callback. But due to
* backwards-compatibility needs the device instance have to be published too
* early, which requires unpretty global locking to make safe and is therefore
* only support for existing drivers not yet converted to the new scheme.
*
* When cleaning up a device instance everything needs to be done in reverse:
* First unpublish the device instance with drm_dev_unregister(). Then clean up
* any other resources allocated at device initialization and drop the driver's
* reference to &drm_device using drm_dev_put().
*
drm: Add docs for managed resources All collected together to provide a consistent story in one patch, instead of the somewhat bumpy refactor-evolution leading to this. Also some thoughts on what the next steps could be: - Create a macro called devm_drm_dev_alloc() which essentially wraps the kzalloc(); devm_drm_dev_init(); drmm_add_final_kfree() combo. Needs to be a macro since we'll have to do some typeof trickery and casting to make this fully generic for all drivers that embed struct drm_device into their own thing. - A lot of the simple drivers now have essentially just drm_dev_unplug(); drm_atomic_helper_shutdown(); as their $bus_driver->remove hook. We could create a devm_mode_config_reset which sets drm_atomic_helper_shutdown as it's cleanup action, and a devm_drm_dev_register with drm_dev_unplug as it's cleanup action, and simple drivers wouldn't have a need for a ->remove function at all, and we could delete them. - For more complicated drivers we need drmm_ versions of a _lot_ more things. All the userspace visible objects (crtc, plane, encoder, crtc), anything else hanging of those (maybe a drmm_get_edid, at least for panels and other built-in stuff). Also some more thoughts on why we're not reusing devm_ with maybe a fake struct device embedded into the drm_device (we can't use the kdev, since that's in each drm_minor). - Code review gets extremely tricky, since every time you see a devm_ you need to carefully check whether the fake device (with the drm_device lifetim) or the real device (with the lifetim of the underlying physical device and driver binding) are used. That's not going to help at all, and we have enormous amounts of drivers who use devm_ where they really shouldn't. Having different types makes sure the compiler type checks this for us and ensures correctness. - The set of functions are very much non-overlapping. E.g. devm_ioremap makes total sense, drmm_ioremap has the wrong lifetime, since hw resources need to be cleaned out at driver unbind and wont outlive that like a drm_device. Similar, but other way round for drmm_connector_init (which is the only correct version, devm_ for drm_connector is just buggy). Simply not having the wrong version again prevents bugs. Finally I guess this opens a huge todo for all the drivers. I'm semi-tempted to do a tree-wide s/devm_kzalloc/drmm_kzalloc/ since most likely that'll fix an enormous amount of bugs and most likely not cause any issues at all (aside from maybe holding onto memory slightly too long). v2: - Doc improvements from Laurent. - Also add kerneldoc for the new drmm_add_action_or_reset. v3: - Remove kerneldoc for drmm_remove_action. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: linux-doc@vger.kernel.org Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> fixup docs Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-52-daniel.vetter@ffwll.ch
2020-03-23 14:49:50 +00:00
* Note that any allocation or resource which is visible to userspace must be
* released only when the final drm_dev_put() is called, and not when the
* driver is unbound from the underlying physical struct &device. Best to use
* &drm_device managed resources with drmm_add_action(), drmm_kmalloc() and
* related functions.
*
* devres managed resources like devm_kmalloc() can only be used for resources
* directly related to the underlying hardware device, and only used in code
* paths fully protected by drm_dev_enter() and drm_dev_exit().
*
* Display driver example
* ~~~~~~~~~~~~~~~~~~~~~~
*
* The following example shows a typical structure of a DRM display driver.
* The example focus on the probe() function and the other functions that is
* almost always present and serves as a demonstration of devm_drm_dev_alloc().
*
* .. code-block:: c
*
* struct driver_device {
* struct drm_device drm;
* void *userspace_facing;
* struct clk *pclk;
* };
*
* static const struct drm_driver driver_drm_driver = {
* [...]
* };
*
* static int driver_probe(struct platform_device *pdev)
* {
* struct driver_device *priv;
* struct drm_device *drm;
* int ret;
*
* priv = devm_drm_dev_alloc(&pdev->dev, &driver_drm_driver,
* struct driver_device, drm);
* if (IS_ERR(priv))
* return PTR_ERR(priv);
* drm = &priv->drm;
*
drm: Manage drm_mode_config_init with drmm_ drm_mode_config_cleanup is idempotent, so no harm in calling this twice. This allows us to gradually switch drivers over by removing explicit drm_mode_config_cleanup calls. With this step it's now also possible that (at least for simple drivers) automatic resource cleanup can be done correctly without a drm_driver->release hook. Therefore allow this now in devm_drm_dev_init(). Also with drmm_ explicit drm_driver->release hooks are kinda not the best option: Drivers can always just register their current release hook with drmm_add_action, but even better they could split them up to simplify the unwinding for the driver load failure case. So deprecate that hook to discourage future users. v2: Fixup the example in the kerneldoc too. v3: - For paranoia, double check that minor->dev == dev in the release hook, because I botched the pointer math in the drmm library. - Call drm_mode_config_cleanup when drmm_add_action fails, we'd be missing some mutex_destroy and ida_cleanup otherwise (Laurent) v4: Add a drmm_add_action_or_reset (like devm_ has) to encapsulate this pattern (Noralf). v5: Fix oversight in the new drmm_add_action_or_reset macro (Noralf) v4: Review from Sam: - drmm_mode_config_init wrapper (also suggested by Thomas) - improve commit message, explain better why ->relase is deprecated v5: - Make drmm_ the main function, with the old one as compat wrapper (Sam) - Add FIXME comments to drm_mode_config_cleanup/init() that drivers shouldn't use these anymore. - Move drmm_add_action_or_reset helper to an earlier patch. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: "Noralf Trønnes" <noralf@tronnes.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: Noralf Trønnes <noralf@tronnes.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-27-daniel.vetter@ffwll.ch
2020-03-23 14:49:25 +00:00
* ret = drmm_mode_config_init(drm);
* if (ret)
* return ret;
*
* priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
* if (!priv->userspace_facing)
* return -ENOMEM;
*
* priv->pclk = devm_clk_get(dev, "PCLK");
* if (IS_ERR(priv->pclk))
* return PTR_ERR(priv->pclk);
*
* // Further setup, display pipeline etc
*
* platform_set_drvdata(pdev, drm);
*
* drm_mode_config_reset(drm);
*
* ret = drm_dev_register(drm);
* if (ret)
* return ret;
*
* drm_fbdev_generic_setup(drm, 32);
*
* return 0;
* }
*
* // This function is called before the devm_ resources are released
* static int driver_remove(struct platform_device *pdev)
* {
* struct drm_device *drm = platform_get_drvdata(pdev);
*
* drm_dev_unregister(drm);
* drm_atomic_helper_shutdown(drm)
*
* return 0;
* }
*
* // This function is called on kernel restart and shutdown
* static void driver_shutdown(struct platform_device *pdev)
* {
* drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
* }
*
* static int __maybe_unused driver_pm_suspend(struct device *dev)
* {
* return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
* }
*
* static int __maybe_unused driver_pm_resume(struct device *dev)
* {
* drm_mode_config_helper_resume(dev_get_drvdata(dev));
*
* return 0;
* }
*
* static const struct dev_pm_ops driver_pm_ops = {
* SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
* };
*
* static struct platform_driver driver_driver = {
* .driver = {
* [...]
* .pm = &driver_pm_ops,
* },
* .probe = driver_probe,
* .remove = driver_remove,
* .shutdown = driver_shutdown,
* };
* module_platform_driver(driver_driver);
*
* Drivers that want to support device unplugging (USB, DT overlay unload) should
* use drm_dev_unplug() instead of drm_dev_unregister(). The driver must protect
* regions that is accessing device resources to prevent use after they're
* released. This is done using drm_dev_enter() and drm_dev_exit(). There is one
* shortcoming however, drm_dev_unplug() marks the drm_device as unplugged before
* drm_atomic_helper_shutdown() is called. This means that if the disable code
* paths are protected, they will not run on regular driver module unload,
* possibily leaving the hardware enabled.
*/
/**
* drm_put_dev - Unregister and release a DRM device
* @dev: DRM device
*
* Called at module unload time or when a PCI device is unplugged.
*
* Cleans up all DRM device, calling drm_lastclose().
*
* Note: Use of this function is deprecated. It will eventually go away
* completely. Please use drm_dev_unregister() and drm_dev_put() explicitly
* instead to make sure that the device isn't userspace accessible any more
* while teardown is in progress, ensuring that userspace can't access an
* inconsistent state.
*/
void drm_put_dev(struct drm_device *dev)
{
DRM_DEBUG("\n");
if (!dev) {
DRM_ERROR("cleanup called no dev\n");
return;
}
drm_dev_unregister(dev);
drm_dev_put(dev);
}
EXPORT_SYMBOL(drm_put_dev);
/**
* drm_dev_enter - Enter device critical section
* @dev: DRM device
* @idx: Pointer to index that will be passed to the matching drm_dev_exit()
*
* This function marks and protects the beginning of a section that should not
* be entered after the device has been unplugged. The section end is marked
* with drm_dev_exit(). Calls to this function can be nested.
*
* Returns:
* True if it is OK to enter the section, false otherwise.
*/
bool drm_dev_enter(struct drm_device *dev, int *idx)
{
*idx = srcu_read_lock(&drm_unplug_srcu);
if (dev->unplugged) {
srcu_read_unlock(&drm_unplug_srcu, *idx);
return false;
}
return true;
}
EXPORT_SYMBOL(drm_dev_enter);
/**
* drm_dev_exit - Exit device critical section
* @idx: index returned from drm_dev_enter()
*
* This function marks the end of a section that should not be entered after
* the device has been unplugged.
*/
void drm_dev_exit(int idx)
{
srcu_read_unlock(&drm_unplug_srcu, idx);
}
EXPORT_SYMBOL(drm_dev_exit);
/**
* drm_dev_unplug - unplug a DRM device
* @dev: DRM device
*
* This unplugs a hotpluggable DRM device, which makes it inaccessible to
* userspace operations. Entry-points can use drm_dev_enter() and
* drm_dev_exit() to protect device resources in a race free manner. This
* essentially unregisters the device like drm_dev_unregister(), but can be
* called while there are still open users of @dev.
*/
void drm_dev_unplug(struct drm_device *dev)
{
/*
* After synchronizing any critical read section is guaranteed to see
* the new value of ->unplugged, and any critical section which might
* still have seen the old value of ->unplugged is guaranteed to have
* finished.
*/
dev->unplugged = true;
synchronize_srcu(&drm_unplug_srcu);
drm_dev_unregister(dev);
/* Clear all CPU mappings pointing to this device */
unmap_mapping_range(dev->anon_inode->i_mapping, 0, 0, 1);
}
EXPORT_SYMBOL(drm_dev_unplug);
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
/*
* DRM internal mount
* We want to be able to allocate our own "struct address_space" to control
* memory-mappings in VRAM (or stolen RAM, ...). However, core MM does not allow
* stand-alone address_space objects, so we need an underlying inode. As there
* is no way to allocate an independent inode easily, we need a fake internal
* VFS mount-point.
*
* The drm_fs_inode_new() function allocates a new inode, drm_fs_inode_free()
* frees it again. You are allowed to use iget() and iput() to get references to
* the inode. But each drm_fs_inode_new() call must be paired with exactly one
* drm_fs_inode_free() call (which does not have to be the last iput()).
* We use drm_fs_inode_*() to manage our internal VFS mount-point and share it
* between multiple inode-users. You could, technically, call
* iget() + drm_fs_inode_free() directly after alloc and sometime later do an
* iput(), but this way you'd end up with a new vfsmount for each inode.
*/
static int drm_fs_cnt;
static struct vfsmount *drm_fs_mnt;
static int drm_fs_init_fs_context(struct fs_context *fc)
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
{
return init_pseudo(fc, 0x010203ff) ? 0 : -ENOMEM;
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
}
static struct file_system_type drm_fs_type = {
.name = "drm",
.owner = THIS_MODULE,
.init_fs_context = drm_fs_init_fs_context,
drm: add pseudo filesystem for shared inodes Our current DRM design uses a single address_space for all users of the same DRM device. However, there is no way to create an anonymous address_space without an underlying inode. Therefore, we wait for the first ->open() callback on a registered char-dev and take-over the inode of the char-dev. This worked well so far, but has several drawbacks: - We screw with FS internals and rely on some non-obvious invariants like inode->i_mapping being the same as inode->i_data for char-devs. - We don't have any address_space prior to the first ->open() from user-space. This leads to ugly fallback code and we cannot allocate global objects early. As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by drivers for anonymous inode-allocation. Therefore, this patch follows the proposed alternative solution and adds a pseudo filesystem mount-point to DRM. We can then allocate private inodes including a private address_space for each DRM device at initialization time. Note that we could use: sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd); to get access to the underlying sysfs-inode of a "struct device" object. However, most of this information is currently hidden and it's not clear whether this address_space is suitable for driver access. Thus, unless linux allows anonymous address_space objects or driver-core provides a public inode per device, we're left with our own private internal mount point. Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
2014-01-03 13:09:47 +00:00
.kill_sb = kill_anon_super,
};
static struct inode *drm_fs_inode_new(void)
{
struct inode *inode;
int r;
r = simple_pin_fs(&drm_fs_type, &drm_fs_mnt, &drm_fs_cnt);
if (r < 0) {
DRM_ERROR("Cannot mount pseudo fs: %d\n", r);
return ERR_PTR(r);
}
inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
if (IS_ERR(inode))
simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
return inode;
}
static void drm_fs_inode_free(struct inode *inode)
{
if (inode) {
iput(inode);
simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
}
}
/**
* DOC: component helper usage recommendations
*
* DRM drivers that drive hardware where a logical device consists of a pile of
* independent hardware blocks are recommended to use the :ref:`component helper
* library<component>`. For consistency and better options for code reuse the
* following guidelines apply:
*
* - The entire device initialization procedure should be run from the
* &component_master_ops.master_bind callback, starting with
* devm_drm_dev_alloc(), then binding all components with
* component_bind_all() and finishing with drm_dev_register().
*
* - The opaque pointer passed to all components through component_bind_all()
* should point at &struct drm_device of the device instance, not some driver
* specific private structure.
*
* - The component helper fills the niche where further standardization of
* interfaces is not practical. When there already is, or will be, a
* standardized interface like &drm_bridge or &drm_panel, providing its own
* functions to find such components at driver load time, like
* drm_of_find_panel_or_bridge(), then the component helper should not be
* used.
*/
static void drm_dev_init_release(struct drm_device *dev, void *res)
{
drm_legacy_ctxbitmap_cleanup(dev);
drm_legacy_remove_map_hash(dev);
drm_fs_inode_free(dev->anon_inode);
put_device(dev->dev);
/* Prevent use-after-free in drm_managed_release when debugging is
* enabled. Slightly awkward, but can't really be helped. */
dev->dev = NULL;
mutex_destroy(&dev->master_mutex);
mutex_destroy(&dev->clientlist_mutex);
mutex_destroy(&dev->filelist_mutex);
mutex_destroy(&dev->struct_mutex);
drm_legacy_destroy_members(dev);
}
static int drm_dev_init(struct drm_device *dev,
const struct drm_driver *driver,
struct device *parent)
{
int ret;
if (!drm_core_init_complete) {
DRM_ERROR("DRM core is not initialized\n");
return -ENODEV;
}
if (WARN_ON(!parent))
return -EINVAL;
kref_init(&dev->ref);
dev->dev = get_device(parent);
dev->driver = driver;
drm: add managed resources tied to drm_device We have lots of these. And the cleanup code tends to be of dubious quality. The biggest wrong pattern is that developers use devm_, which ties the release action to the underlying struct device, whereas all the userspace visible stuff attached to a drm_device can long outlive that one (e.g. after a hotunplug while userspace has open files and mmap'ed buffers). Give people what they want, but with more correctness. Mostly copied from devres.c, with types adjusted to fit drm_device and a few simplifications - I didn't (yet) copy over everything. Since the types don't match code sharing looked like a hopeless endeavour. For now it's only super simplified, no groups, you can't remove actions (but kfree exists, we'll need that soon). Plus all specific to drm_device ofc, including the logging. Which I didn't bother to make compile-time optional, since none of the other drm logging is compile time optional either. One tricky bit here is the chicken&egg between allocating your drm_device structure and initiliazing it with drm_dev_init. For perfect onion unwinding we'd need to have the action to kfree the allocation registered before drm_dev_init registers any of its own release handlers. But drm_dev_init doesn't know where exactly the drm_device is emebedded into the overall structure, and by the time it returns it'll all be too late. And forcing drivers to be able clean up everything except the one kzalloc is silly. Work around this by having a very special final_kfree pointer. This also avoids troubles with the list head possibly disappearing from underneath us when we release all resources attached to the drm_device. v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless shuffling while getting everything into shape. v3: Add static to add/del_dr (Neil) Move typo fix to the right patch (Neil) v4: Enforce contract for drmm_add_final_kfree: Use ksize() to check that the drm_device is indeed contained somewhere in the final kfree(). Because we need that or the entire managed release logic blows up in a pile of use-after-frees. Motivated by a discussion with Laurent. v5: Review from Laurent: - %zu instead of casting size_t - header guards - sorting of includes - guarding of data assignment if we didn't allocate it for a NULL pointer - delete spurious newline - cast void* data parameter correctly in ->release call, no idea how this even worked before v6: Review from Sam - Add the kerneldoc for the managed sub-struct back in, even if it doesn't show up in the generated html somehow. - Explain why __always_inline. - Fix bisectability around the final kfree() in drm_dev_relase(). This is just interim code which will disappear again. - Some whitespace polish. - Add debug output when drmm_add_action or drmm_kmalloc fail. v7: My bisectability fix wasn't up to par as noticed by smatch. v8: Remove unecessary {} around if else v9: Use kstrdup_const, which requires kfree_const and introducing a free_dr() helper (Thomas). v10: kfree_const goes boom on the plain "kmalloc" assignment, somehow we need to wrap that in kstrdup_const() too!! Also renumber revision log, I somehow reset it midway thruh. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Neil Armstrong <narmstrong@baylibre.com Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200324124540.3227396-1-daniel.vetter@ffwll.ch
2020-03-24 12:45:40 +00:00
INIT_LIST_HEAD(&dev->managed.resources);
spin_lock_init(&dev->managed.lock);
/* no per-device feature limits by default */
dev->driver_features = ~0u;
drm_legacy_init_members(dev);
INIT_LIST_HEAD(&dev->filelist);
INIT_LIST_HEAD(&dev->filelist_internal);
INIT_LIST_HEAD(&dev->clientlist);
INIT_LIST_HEAD(&dev->vblank_event_list);
spin_lock_init(&dev->event_lock);
mutex_init(&dev->struct_mutex);
mutex_init(&dev->filelist_mutex);
mutex_init(&dev->clientlist_mutex);
mutex_init(&dev->master_mutex);
ret = drmm_add_action(dev, drm_dev_init_release, NULL);
if (ret)
return ret;
dev->anon_inode = drm_fs_inode_new();
if (IS_ERR(dev->anon_inode)) {
ret = PTR_ERR(dev->anon_inode);
DRM_ERROR("Cannot allocate anonymous inode: %d\n", ret);
goto err;
}
if (drm_core_check_feature(dev, DRIVER_RENDER)) {
ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
if (ret)
goto err;
}
ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
if (ret)
goto err;
ret = drm_legacy_create_map_hash(dev);
if (ret)
goto err;
drm_legacy_ctxbitmap_init(dev);
if (drm_core_check_feature(dev, DRIVER_GEM)) {
ret = drm_gem_init(dev);
if (ret) {
DRM_ERROR("Cannot initialize graphics execution manager (GEM)\n");
goto err;
}
}
ret = drm_dev_set_unique(dev, dev_name(parent));
if (ret)
goto err;
return 0;
err:
drm_managed_release(dev);
return ret;
}
static void devm_drm_dev_init_release(void *data)
{
drm_dev_put(data);
}
static int devm_drm_dev_init(struct device *parent,
struct drm_device *dev,
const struct drm_driver *driver)
{
int ret;
ret = drm_dev_init(dev, driver, parent);
if (ret)
return ret;
return devm_add_action_or_reset(parent,
devm_drm_dev_init_release, dev);
}
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset)
{
void *container;
struct drm_device *drm;
int ret;
container = kzalloc(size, GFP_KERNEL);
if (!container)
return ERR_PTR(-ENOMEM);
drm = container + offset;
ret = devm_drm_dev_init(parent, drm, driver);
if (ret) {
kfree(container);
return ERR_PTR(ret);
}
drmm_add_final_kfree(drm, container);
return container;
}
EXPORT_SYMBOL(__devm_drm_dev_alloc);
/**
* drm_dev_alloc - Allocate new DRM device
* @driver: DRM driver to allocate device for
* @parent: Parent device object
*
* This is the deprecated version of devm_drm_dev_alloc(), which does not support
* subclassing through embedding the struct &drm_device in a driver private
* structure, and which does not support automatic cleanup through devres.
*
* RETURNS:
* Pointer to new DRM device, or ERR_PTR on failure.
*/
struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
struct device *parent)
{
struct drm_device *dev;
int ret;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
return ERR_PTR(-ENOMEM);
ret = drm_dev_init(dev, driver, parent);
if (ret) {
kfree(dev);
return ERR_PTR(ret);
}
drm: Set final_kfree in drm_dev_alloc I also did a full review of all callers, and only the xen driver forgot to call drm_dev_put in the failure path. Fix that up too. v2: I noticed that xen has a drm_driver.release hook, and uses drm_dev_alloc(). We need to remove the kfree from xen_drm_drv_release(). bochs also has a release hook, but leaked the drm_device ever since commit 0a6659bdc5e8221da99eebb176fd9591435e38de Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Dec 17 18:04:46 2013 +0100 drm/bochs: new driver This patch here fixes that leak. Same for virtio, started leaking with commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Feb 11 14:58:04 2020 +0100 drm/virtio: add drm_driver.release callback. Acked-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: xen-devel@lists.xenproject.org Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: xen-devel@lists.xenproject.org Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-5-daniel.vetter@ffwll.ch
2020-03-23 14:49:03 +00:00
drmm_add_final_kfree(dev, dev);
return dev;
}
EXPORT_SYMBOL(drm_dev_alloc);
static void drm_dev_release(struct kref *ref)
{
struct drm_device *dev = container_of(ref, struct drm_device, ref);
drm: add managed resources tied to drm_device We have lots of these. And the cleanup code tends to be of dubious quality. The biggest wrong pattern is that developers use devm_, which ties the release action to the underlying struct device, whereas all the userspace visible stuff attached to a drm_device can long outlive that one (e.g. after a hotunplug while userspace has open files and mmap'ed buffers). Give people what they want, but with more correctness. Mostly copied from devres.c, with types adjusted to fit drm_device and a few simplifications - I didn't (yet) copy over everything. Since the types don't match code sharing looked like a hopeless endeavour. For now it's only super simplified, no groups, you can't remove actions (but kfree exists, we'll need that soon). Plus all specific to drm_device ofc, including the logging. Which I didn't bother to make compile-time optional, since none of the other drm logging is compile time optional either. One tricky bit here is the chicken&egg between allocating your drm_device structure and initiliazing it with drm_dev_init. For perfect onion unwinding we'd need to have the action to kfree the allocation registered before drm_dev_init registers any of its own release handlers. But drm_dev_init doesn't know where exactly the drm_device is emebedded into the overall structure, and by the time it returns it'll all be too late. And forcing drivers to be able clean up everything except the one kzalloc is silly. Work around this by having a very special final_kfree pointer. This also avoids troubles with the list head possibly disappearing from underneath us when we release all resources attached to the drm_device. v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless shuffling while getting everything into shape. v3: Add static to add/del_dr (Neil) Move typo fix to the right patch (Neil) v4: Enforce contract for drmm_add_final_kfree: Use ksize() to check that the drm_device is indeed contained somewhere in the final kfree(). Because we need that or the entire managed release logic blows up in a pile of use-after-frees. Motivated by a discussion with Laurent. v5: Review from Laurent: - %zu instead of casting size_t - header guards - sorting of includes - guarding of data assignment if we didn't allocate it for a NULL pointer - delete spurious newline - cast void* data parameter correctly in ->release call, no idea how this even worked before v6: Review from Sam - Add the kerneldoc for the managed sub-struct back in, even if it doesn't show up in the generated html somehow. - Explain why __always_inline. - Fix bisectability around the final kfree() in drm_dev_relase(). This is just interim code which will disappear again. - Some whitespace polish. - Add debug output when drmm_add_action or drmm_kmalloc fail. v7: My bisectability fix wasn't up to par as noticed by smatch. v8: Remove unecessary {} around if else v9: Use kstrdup_const, which requires kfree_const and introducing a free_dr() helper (Thomas). v10: kfree_const goes boom on the plain "kmalloc" assignment, somehow we need to wrap that in kstrdup_const() too!! Also renumber revision log, I somehow reset it midway thruh. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Neil Armstrong <narmstrong@baylibre.com Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200324124540.3227396-1-daniel.vetter@ffwll.ch
2020-03-24 12:45:40 +00:00
if (dev->driver->release)
dev->driver->release(dev);
drm: add managed resources tied to drm_device We have lots of these. And the cleanup code tends to be of dubious quality. The biggest wrong pattern is that developers use devm_, which ties the release action to the underlying struct device, whereas all the userspace visible stuff attached to a drm_device can long outlive that one (e.g. after a hotunplug while userspace has open files and mmap'ed buffers). Give people what they want, but with more correctness. Mostly copied from devres.c, with types adjusted to fit drm_device and a few simplifications - I didn't (yet) copy over everything. Since the types don't match code sharing looked like a hopeless endeavour. For now it's only super simplified, no groups, you can't remove actions (but kfree exists, we'll need that soon). Plus all specific to drm_device ofc, including the logging. Which I didn't bother to make compile-time optional, since none of the other drm logging is compile time optional either. One tricky bit here is the chicken&egg between allocating your drm_device structure and initiliazing it with drm_dev_init. For perfect onion unwinding we'd need to have the action to kfree the allocation registered before drm_dev_init registers any of its own release handlers. But drm_dev_init doesn't know where exactly the drm_device is emebedded into the overall structure, and by the time it returns it'll all be too late. And forcing drivers to be able clean up everything except the one kzalloc is silly. Work around this by having a very special final_kfree pointer. This also avoids troubles with the list head possibly disappearing from underneath us when we release all resources attached to the drm_device. v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless shuffling while getting everything into shape. v3: Add static to add/del_dr (Neil) Move typo fix to the right patch (Neil) v4: Enforce contract for drmm_add_final_kfree: Use ksize() to check that the drm_device is indeed contained somewhere in the final kfree(). Because we need that or the entire managed release logic blows up in a pile of use-after-frees. Motivated by a discussion with Laurent. v5: Review from Laurent: - %zu instead of casting size_t - header guards - sorting of includes - guarding of data assignment if we didn't allocate it for a NULL pointer - delete spurious newline - cast void* data parameter correctly in ->release call, no idea how this even worked before v6: Review from Sam - Add the kerneldoc for the managed sub-struct back in, even if it doesn't show up in the generated html somehow. - Explain why __always_inline. - Fix bisectability around the final kfree() in drm_dev_relase(). This is just interim code which will disappear again. - Some whitespace polish. - Add debug output when drmm_add_action or drmm_kmalloc fail. v7: My bisectability fix wasn't up to par as noticed by smatch. v8: Remove unecessary {} around if else v9: Use kstrdup_const, which requires kfree_const and introducing a free_dr() helper (Thomas). v10: kfree_const goes boom on the plain "kmalloc" assignment, somehow we need to wrap that in kstrdup_const() too!! Also renumber revision log, I somehow reset it midway thruh. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Neil Armstrong <narmstrong@baylibre.com Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200324124540.3227396-1-daniel.vetter@ffwll.ch
2020-03-24 12:45:40 +00:00
drm_managed_release(dev);
kfree(dev->managed.final_kfree);
}
/**
* drm_dev_get - Take reference of a DRM device
* @dev: device to take reference of or NULL
*
* This increases the ref-count of @dev by one. You *must* already own a
* reference when calling this. Use drm_dev_put() to drop this reference
* again.
*
* This function never fails. However, this function does not provide *any*
* guarantee whether the device is alive or running. It only provides a
* reference to the object and the memory associated with it.
*/
void drm_dev_get(struct drm_device *dev)
{
if (dev)
kref_get(&dev->ref);
}
EXPORT_SYMBOL(drm_dev_get);
/**
* drm_dev_put - Drop reference of a DRM device
* @dev: device to drop reference of or NULL
*
* This decreases the ref-count of @dev by one. The device is destroyed if the
* ref-count drops to zero.
*/
void drm_dev_put(struct drm_device *dev)
{
if (dev)
kref_put(&dev->ref, drm_dev_release);
}
EXPORT_SYMBOL(drm_dev_put);
drm: Add fake controlD* symlinks for backwards compat We thought that no userspace is using them, but oops libdrm is using them to figure out whether a driver supports modesetting. Check out drmCheckModesettingSupported but maybe don't because it's horrible and totally runs counter to where we want to go with libdrm device handling. The function looks in the device hierarchy for whether controlD* exist using the following format string: /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d The "/drm" subdirectory is the glue directory from the sysfs class stuff, and the only way to get at it seems to through kdev->kobj.parent (when kdev is represents e.g. the card0 chardev instance in sysfs). Git grep says we're not the only ones touching that, so I hope it's ok we dig into such internals - I couldn't find a proper interface for getting at the glue directory. Quick git grep shows that at least -amdgpu, -ati are using this. -modesetting do not, and on -intel it's only about the 4th fallback path for device lookup, which is why this didn't blow up earlier. Oh well, we need to keep it working, and the simplest way is to add a symlink at the right place in sysfs from controlD* to card*. v2: - Fix error path handling by adding if (!minor) return checks (David) - Fix the controlD* numbers to match what's been there (David) - Add a comment what exactly userspace minimally needs. - Correct the analysis for -intel (Chris). Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") Cc: Dave Airlie <airlied@gmail.com> Reported-and-tested-by: Alex Deucher <alexander.deucher@amd.com> Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161209135656.14881-1-daniel.vetter@ffwll.ch
2016-12-09 13:56:56 +00:00
static int create_compat_control_link(struct drm_device *dev)
{
struct drm_minor *minor;
char *name;
int ret;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return 0;
minor = *drm_minor_get_slot(dev, DRM_MINOR_PRIMARY);
if (!minor)
return 0;
/*
* Some existing userspace out there uses the existing of the controlD*
* sysfs files to figure out whether it's a modeset driver. It only does
* readdir, hence a symlink is sufficient (and the least confusing
* option). Otherwise controlD* is entirely unused.
*
* Old controlD chardev have been allocated in the range
* 64-127.
*/
name = kasprintf(GFP_KERNEL, "controlD%d", minor->index + 64);
if (!name)
return -ENOMEM;
ret = sysfs_create_link(minor->kdev->kobj.parent,
&minor->kdev->kobj,
name);
kfree(name);
return ret;
}
static void remove_compat_control_link(struct drm_device *dev)
{
struct drm_minor *minor;
char *name;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return;
minor = *drm_minor_get_slot(dev, DRM_MINOR_PRIMARY);
if (!minor)
return;
name = kasprintf(GFP_KERNEL, "controlD%d", minor->index + 64);
drm: Add fake controlD* symlinks for backwards compat We thought that no userspace is using them, but oops libdrm is using them to figure out whether a driver supports modesetting. Check out drmCheckModesettingSupported but maybe don't because it's horrible and totally runs counter to where we want to go with libdrm device handling. The function looks in the device hierarchy for whether controlD* exist using the following format string: /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d The "/drm" subdirectory is the glue directory from the sysfs class stuff, and the only way to get at it seems to through kdev->kobj.parent (when kdev is represents e.g. the card0 chardev instance in sysfs). Git grep says we're not the only ones touching that, so I hope it's ok we dig into such internals - I couldn't find a proper interface for getting at the glue directory. Quick git grep shows that at least -amdgpu, -ati are using this. -modesetting do not, and on -intel it's only about the 4th fallback path for device lookup, which is why this didn't blow up earlier. Oh well, we need to keep it working, and the simplest way is to add a symlink at the right place in sysfs from controlD* to card*. v2: - Fix error path handling by adding if (!minor) return checks (David) - Fix the controlD* numbers to match what's been there (David) - Add a comment what exactly userspace minimally needs. - Correct the analysis for -intel (Chris). Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") Cc: Dave Airlie <airlied@gmail.com> Reported-and-tested-by: Alex Deucher <alexander.deucher@amd.com> Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161209135656.14881-1-daniel.vetter@ffwll.ch
2016-12-09 13:56:56 +00:00
if (!name)
return;
sysfs_remove_link(minor->kdev->kobj.parent, name);
kfree(name);
}
/**
* drm_dev_register - Register DRM device
* @dev: Device to register
* @flags: Flags passed to the driver's .load() function
*
* Register the DRM device @dev with the system, advertise device to user-space
* and start normal device operation. @dev must be initialized via drm_dev_init()
* previously.
*
* Never call this twice on any device!
*
* NOTE: To ensure backward compatibility with existing drivers method this
* function calls the &drm_driver.load method after registering the device
* nodes, creating race conditions. Usage of the &drm_driver.load methods is
* therefore deprecated, drivers must perform all initialization before calling
* drm_dev_register().
*
* RETURNS:
* 0 on success, negative error code on failure.
*/
int drm_dev_register(struct drm_device *dev, unsigned long flags)
{
const struct drm_driver *driver = dev->driver;
int ret;
if (!driver->load)
drm_mode_config_validate(dev);
WARN_ON(!dev->managed.final_kfree);
if (drm_dev_needs_global_mutex(dev))
mutex_lock(&drm_global_mutex);
ret = drm_minor_register(dev, DRM_MINOR_RENDER);
if (ret)
goto err_minors;
ret = drm_minor_register(dev, DRM_MINOR_PRIMARY);
if (ret)
goto err_minors;
drm: Add fake controlD* symlinks for backwards compat We thought that no userspace is using them, but oops libdrm is using them to figure out whether a driver supports modesetting. Check out drmCheckModesettingSupported but maybe don't because it's horrible and totally runs counter to where we want to go with libdrm device handling. The function looks in the device hierarchy for whether controlD* exist using the following format string: /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d The "/drm" subdirectory is the glue directory from the sysfs class stuff, and the only way to get at it seems to through kdev->kobj.parent (when kdev is represents e.g. the card0 chardev instance in sysfs). Git grep says we're not the only ones touching that, so I hope it's ok we dig into such internals - I couldn't find a proper interface for getting at the glue directory. Quick git grep shows that at least -amdgpu, -ati are using this. -modesetting do not, and on -intel it's only about the 4th fallback path for device lookup, which is why this didn't blow up earlier. Oh well, we need to keep it working, and the simplest way is to add a symlink at the right place in sysfs from controlD* to card*. v2: - Fix error path handling by adding if (!minor) return checks (David) - Fix the controlD* numbers to match what's been there (David) - Add a comment what exactly userspace minimally needs. - Correct the analysis for -intel (Chris). Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") Cc: Dave Airlie <airlied@gmail.com> Reported-and-tested-by: Alex Deucher <alexander.deucher@amd.com> Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161209135656.14881-1-daniel.vetter@ffwll.ch
2016-12-09 13:56:56 +00:00
ret = create_compat_control_link(dev);
if (ret)
goto err_minors;
drm/kms: Duct-tape for mode object lifetime checks commit 4f5368b5541a902f6596558b05f5c21a9770dd32 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Fri Jun 14 08:17:23 2019 +0200 drm/kms: Catch mode_object lifetime errors uncovered a bit a mess in dp drivers. Most drivers (from a quick look, all except i915) register all the dp stuff in their init code, which is too early. With CONFIG_DRM_DP_AUX_CHARDEV this will blow up, because drm_dp_aux_register tries to add a child to a device in sysfs (the connector) which doesn't even exist yet. No one seems to have cared thus far. But with the above change I also moved the setting of dev->registered after the ->load callback, in an attempt to keep old drivers from hitting any WARN_ON backtraces. But that moved radeon.ko from the "working, by accident" to "now also broken" category. Since this is a huge mess I figured a revert would be simplest. But this check has already caught issues in i915: commit 1b9bd09630d4db4827cc04d358a41a16a6bc2cb0 Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Tue Aug 20 19:16:57 2019 +0300 drm/i915: Do not create a new max_bpc prop for MST connectors Hence I'd like to retain it. Fix the radeon regression by moving the setting of dev->registered back to were it was, and stop the backtraces with an explicit check for dev->driver->load. Everyone else will stay as broken with CONFIG_DRM_DP_AUX_CHARDEV. The next patch will improve the kerneldoc and add a todo entry for this. Fixes: 4f5368b5541a ("drm/kms: Catch mode_object lifetime errors") Cc: Sean Paul <sean@poorly.run> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reported-by: Michel Dänzer <michel@daenzer.net> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com> Tested-by: Michel Dänzer <mdaenzer@redhat.com> Cc: Michel Dänzer <michel@daenzer.net> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190917120936.7501-1-daniel.vetter@ffwll.ch
2019-09-17 12:09:35 +00:00
dev->registered = true;
if (dev->driver->load) {
ret = dev->driver->load(dev, flags);
if (ret)
goto err_minors;
}
drm: Protect drm_connector_register_all() under DRIVER_MODESET 0-day kbuilder found [ 1.360244] BUG: unable to handle kernel NULL pointer dereference at (null) [ 1.360972] IP: [<c14db9ad>] mutex_lock_nested+0x11f/0x2c3 [ 1.361512] *pde = 00000000 [ 1.361827] Oops: 0002 [#1] [ 1.362123] Modules linked in: [ 1.362451] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc2-00564-ge28cd4d #1 [ 1.363202] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 1.364105] task: c03d0000 ti: d28da000 task.ti: d28da000 [ 1.364636] EIP: 0060:[<c14db9ad>] EFLAGS: 00210096 CPU: 0 [ 1.365215] EIP is at mutex_lock_nested+0x11f/0x2c3 [ 1.365703] EAX: 00000000 EBX: d39e8ae8 ECX: d39e8b14 EDX: c1361cf9 [ 1.366351] ESI: c03d0000 EDI: d28dbed0 EBP: d28dbeec ESP: d28dbec0 [ 1.367010] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 [ 1.367534] CR0: 80050033 CR2: 00000000 CR3: 019a9000 CR4: 00000690 [ 1.368152] Stack: [ 1.368356] d39e8b14 d39e8b24 c1361cf9 00200246 d39e8b14 00000000 11111111 d28dbed0 [ 1.369235] d39e8800 d39e8ae8 00000000 d28dbf08 c1361cf9 d28dbf0c c10b25be d39e8800 [ 1.370087] 00000000 00000000 d28dbf1c c135e37d fffffff4 ffffffff 00000000 d28dbf28 [ 1.371012] Call Trace: [ 1.371272] [<c1361cf9>] ? drm_connector_register_all+0x1a/0x92 [ 1.371847] [<c1361cf9>] drm_connector_register_all+0x1a/0x92 [ 1.372421] [<c10b25be>] ? kstrdup+0x25/0x3a [ 1.372863] [<c135e37d>] drm_dev_register+0x59/0x99 [ 1.373358] [<c195ea3e>] vgem_init+0x34/0x49 [ 1.373770] [<c195ea0a>] ? mipi_dsi_bus_init+0xf/0xf [ 1.374257] [<c100048f>] do_one_initcall+0x7c/0xfd [ 1.374754] [<c104b409>] ? parse_args+0x1fd/0x314 [ 1.375259] [<c1939c10>] ? kernel_init_freeable+0xd0/0x179 [ 1.375837] [<c1939c2c>] kernel_init_freeable+0xec/0x179 [ 1.376371] [<c14d66ea>] kernel_init+0x8/0xcb [ 1.376806] [<c14debce>] ret_from_kernel_thread+0xe/0x30 [ 1.377322] [<c14d66e2>] ? rest_init+0x10e/0x10e [ 1.377754] Code: 89 fa e8 71 c5 b7 ff 8b 4e 04 89 fa 89 d8 e8 8e c6 b7 ff 8d 43 2c 89 45 d4 8b 43 30 8d 4b 2c 89 45 e8 89 7b 30 89 4d e4 8b 55 dc <89> 38 8d 43 3c 89 75 ec e8 c9 dd b7 ff eb 0c 31 c0 87 03 48 +75 [ 1.380442] EIP: [<c14db9ad>] mutex_lock_nested+0x11f/0x2c3 SS:ESP 0068:d28dbec0 [ 1.381174] CR2: 0000000000000000 when loading the non-modesetting vGEM module. To prevent use of the uninitialised dev->mode_config from drm_dev_register() we move the drm_connector_register_all() under a DRIVER_MODESET guard. Longer term, we probably want to initialise the embedded dev->mode_config automatically from drm_dev_init() for all DRIVER_MODESET drivers. v2: Also protect drm_dev_unregister. Fixes: e28cd4d0a223 ("drm: Automatically register/unregister all connectors") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Cc: dri-devel@lists.freedesktop.org Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Testcase: igt/vgem_reload_basic Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1466257601-5656-1-git-send-email-chris@chris-wilson.co.uk
2016-06-18 13:46:41 +00:00
if (drm_core_check_feature(dev, DRIVER_MODESET))
drm_modeset_register_all(dev);
DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
driver->name, driver->major, driver->minor,
driver->patchlevel, driver->date,
dev->dev ? dev_name(dev->dev) : "virtual device",
dev->primary->index);
goto out_unlock;
err_minors:
drm: Add fake controlD* symlinks for backwards compat We thought that no userspace is using them, but oops libdrm is using them to figure out whether a driver supports modesetting. Check out drmCheckModesettingSupported but maybe don't because it's horrible and totally runs counter to where we want to go with libdrm device handling. The function looks in the device hierarchy for whether controlD* exist using the following format string: /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d The "/drm" subdirectory is the glue directory from the sysfs class stuff, and the only way to get at it seems to through kdev->kobj.parent (when kdev is represents e.g. the card0 chardev instance in sysfs). Git grep says we're not the only ones touching that, so I hope it's ok we dig into such internals - I couldn't find a proper interface for getting at the glue directory. Quick git grep shows that at least -amdgpu, -ati are using this. -modesetting do not, and on -intel it's only about the 4th fallback path for device lookup, which is why this didn't blow up earlier. Oh well, we need to keep it working, and the simplest way is to add a symlink at the right place in sysfs from controlD* to card*. v2: - Fix error path handling by adding if (!minor) return checks (David) - Fix the controlD* numbers to match what's been there (David) - Add a comment what exactly userspace minimally needs. - Correct the analysis for -intel (Chris). Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") Cc: Dave Airlie <airlied@gmail.com> Reported-and-tested-by: Alex Deucher <alexander.deucher@amd.com> Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161209135656.14881-1-daniel.vetter@ffwll.ch
2016-12-09 13:56:56 +00:00
remove_compat_control_link(dev);
drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
out_unlock:
if (drm_dev_needs_global_mutex(dev))
mutex_unlock(&drm_global_mutex);
return ret;
}
EXPORT_SYMBOL(drm_dev_register);
/**
* drm_dev_unregister - Unregister DRM device
* @dev: Device to unregister
*
* Unregister the DRM device from the system. This does the reverse of
* drm_dev_register() but does not deallocate the device. The caller must call
* drm_dev_put() to drop their final reference.
*
* A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
* which can be called while there are still open users of @dev.
*
* This should be called first in the device teardown code to make sure
* userspace can't access the device instance any more.
*/
void drm_dev_unregister(struct drm_device *dev)
{
drm: Only lastclose on unload for legacy drivers The only thing modern drivers are supposed to do in lastclose is restore the fb emulation state. Which is entirely optional, and there's really no reason to do that. So restrict it to legacy drivers (where the driver cleanup essentially happens in lastclose). This will also allow us to share the unregister function with drm_dev_unplug(). Quoting my reply to Alex on dri-devel: On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >>> The only thing modern drivers are supposed to do in lastclose is >>> restore the fb emulation state. Which is entirely optional, and >>> there's really no reason to do that. So restrict it to legacy drivers >>> (where the driver cleanup essentially happens in lastclose). >> >> vga_switcheroo_process_delayed_switch() gets called in lastclose. >> Won't that need to get moved elsewhere for this to work? > > Hm right, I forgot the lazy way to do runtime pm by keeping the device > alive as long as anyone has an open fd for it ... This shouldn't be a > problem, since you need to unregister from vgaswitcheroo anyway on > unload. Maybe that blows up, I'll check the code and augment the patch > as needed. So I think there's 3 cases: - Trying to unload the module. You can't do that while anyone has the fd still open, so lastclose is guaranteeed to run. - Forcefully unbinding the driver through sysfs. Not any better, since the can_switch stuff checks for the open count, and so will delay the delayed switch no matter what. - Actual hotremoval: a) not implemented since none of the drivers taking part in vgaswitcheroo correctly unplug the drm device and b) you can't hotremove a chip from a laptop. v2: Extend commit message with m-l discussion. Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170802115604.12734-4-daniel.vetter@ffwll.ch
2017-08-02 11:56:03 +00:00
if (drm_core_check_feature(dev, DRIVER_LEGACY))
drm_lastclose(dev);
dev->registered = false;
drm_client_dev_unregister(dev);
drm: Protect drm_connector_register_all() under DRIVER_MODESET 0-day kbuilder found [ 1.360244] BUG: unable to handle kernel NULL pointer dereference at (null) [ 1.360972] IP: [<c14db9ad>] mutex_lock_nested+0x11f/0x2c3 [ 1.361512] *pde = 00000000 [ 1.361827] Oops: 0002 [#1] [ 1.362123] Modules linked in: [ 1.362451] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc2-00564-ge28cd4d #1 [ 1.363202] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 1.364105] task: c03d0000 ti: d28da000 task.ti: d28da000 [ 1.364636] EIP: 0060:[<c14db9ad>] EFLAGS: 00210096 CPU: 0 [ 1.365215] EIP is at mutex_lock_nested+0x11f/0x2c3 [ 1.365703] EAX: 00000000 EBX: d39e8ae8 ECX: d39e8b14 EDX: c1361cf9 [ 1.366351] ESI: c03d0000 EDI: d28dbed0 EBP: d28dbeec ESP: d28dbec0 [ 1.367010] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 [ 1.367534] CR0: 80050033 CR2: 00000000 CR3: 019a9000 CR4: 00000690 [ 1.368152] Stack: [ 1.368356] d39e8b14 d39e8b24 c1361cf9 00200246 d39e8b14 00000000 11111111 d28dbed0 [ 1.369235] d39e8800 d39e8ae8 00000000 d28dbf08 c1361cf9 d28dbf0c c10b25be d39e8800 [ 1.370087] 00000000 00000000 d28dbf1c c135e37d fffffff4 ffffffff 00000000 d28dbf28 [ 1.371012] Call Trace: [ 1.371272] [<c1361cf9>] ? drm_connector_register_all+0x1a/0x92 [ 1.371847] [<c1361cf9>] drm_connector_register_all+0x1a/0x92 [ 1.372421] [<c10b25be>] ? kstrdup+0x25/0x3a [ 1.372863] [<c135e37d>] drm_dev_register+0x59/0x99 [ 1.373358] [<c195ea3e>] vgem_init+0x34/0x49 [ 1.373770] [<c195ea0a>] ? mipi_dsi_bus_init+0xf/0xf [ 1.374257] [<c100048f>] do_one_initcall+0x7c/0xfd [ 1.374754] [<c104b409>] ? parse_args+0x1fd/0x314 [ 1.375259] [<c1939c10>] ? kernel_init_freeable+0xd0/0x179 [ 1.375837] [<c1939c2c>] kernel_init_freeable+0xec/0x179 [ 1.376371] [<c14d66ea>] kernel_init+0x8/0xcb [ 1.376806] [<c14debce>] ret_from_kernel_thread+0xe/0x30 [ 1.377322] [<c14d66e2>] ? rest_init+0x10e/0x10e [ 1.377754] Code: 89 fa e8 71 c5 b7 ff 8b 4e 04 89 fa 89 d8 e8 8e c6 b7 ff 8d 43 2c 89 45 d4 8b 43 30 8d 4b 2c 89 45 e8 89 7b 30 89 4d e4 8b 55 dc <89> 38 8d 43 3c 89 75 ec e8 c9 dd b7 ff eb 0c 31 c0 87 03 48 +75 [ 1.380442] EIP: [<c14db9ad>] mutex_lock_nested+0x11f/0x2c3 SS:ESP 0068:d28dbec0 [ 1.381174] CR2: 0000000000000000 when loading the non-modesetting vGEM module. To prevent use of the uninitialised dev->mode_config from drm_dev_register() we move the drm_connector_register_all() under a DRIVER_MODESET guard. Longer term, we probably want to initialise the embedded dev->mode_config automatically from drm_dev_init() for all DRIVER_MODESET drivers. v2: Also protect drm_dev_unregister. Fixes: e28cd4d0a223 ("drm: Automatically register/unregister all connectors") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Cc: dri-devel@lists.freedesktop.org Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Testcase: igt/vgem_reload_basic Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1466257601-5656-1-git-send-email-chris@chris-wilson.co.uk
2016-06-18 13:46:41 +00:00
if (drm_core_check_feature(dev, DRIVER_MODESET))
drm_modeset_unregister_all(dev);
if (dev->driver->unload)
dev->driver->unload(dev);
if (dev->agp)
drm_pci_agp_destroy(dev);
drm_legacy_rmmaps(dev);
drm: Add fake controlD* symlinks for backwards compat We thought that no userspace is using them, but oops libdrm is using them to figure out whether a driver supports modesetting. Check out drmCheckModesettingSupported but maybe don't because it's horrible and totally runs counter to where we want to go with libdrm device handling. The function looks in the device hierarchy for whether controlD* exist using the following format string: /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d The "/drm" subdirectory is the glue directory from the sysfs class stuff, and the only way to get at it seems to through kdev->kobj.parent (when kdev is represents e.g. the card0 chardev instance in sysfs). Git grep says we're not the only ones touching that, so I hope it's ok we dig into such internals - I couldn't find a proper interface for getting at the glue directory. Quick git grep shows that at least -amdgpu, -ati are using this. -modesetting do not, and on -intel it's only about the 4th fallback path for device lookup, which is why this didn't blow up earlier. Oh well, we need to keep it working, and the simplest way is to add a symlink at the right place in sysfs from controlD* to card*. v2: - Fix error path handling by adding if (!minor) return checks (David) - Fix the controlD* numbers to match what's been there (David) - Add a comment what exactly userspace minimally needs. - Correct the analysis for -intel (Chris). Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") Cc: Dave Airlie <airlied@gmail.com> Reported-and-tested-by: Alex Deucher <alexander.deucher@amd.com> Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: David Herrmann <dh.herrmann@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161209135656.14881-1-daniel.vetter@ffwll.ch
2016-12-09 13:56:56 +00:00
remove_compat_control_link(dev);
drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
}
EXPORT_SYMBOL(drm_dev_unregister);
/**
* drm_dev_set_unique - Set the unique name of a DRM device
* @dev: device of which to set the unique name
* @name: unique name
*
* Sets the unique name of a DRM device using the specified string. This is
* already done by drm_dev_init(), drivers should only override the default
* unique name for backwards compatibility reasons.
*
* Return: 0 on success or a negative error code on failure.
*/
int drm_dev_set_unique(struct drm_device *dev, const char *name)
{
drmm_kfree(dev, dev->unique);
dev->unique = drmm_kstrdup(dev, name, GFP_KERNEL);
return dev->unique ? 0 : -ENOMEM;
}
EXPORT_SYMBOL(drm_dev_set_unique);
/*
* DRM Core
* The DRM core module initializes all global DRM objects and makes them
* available to drivers. Once setup, drivers can probe their respective
* devices.
* Currently, core management includes:
* - The "DRM-Global" key/value database
* - Global ID management for connectors
* - DRM major number allocation
* - DRM minor management
* - DRM sysfs class
* - DRM debugfs root
*
* Furthermore, the DRM core provides dynamic char-dev lookups. For each
* interface registered on a DRM device, you can request minor numbers from DRM
* core. DRM core takes care of major-number management and char-dev
* registration. A stub ->open() callback forwards any open() requests to the
* registered minor.
*/
static int drm_stub_open(struct inode *inode, struct file *filp)
{
const struct file_operations *new_fops;
struct drm_minor *minor;
int err;
DRM_DEBUG("\n");
minor = drm_minor_acquire(iminor(inode));
if (IS_ERR(minor))
return PTR_ERR(minor);
new_fops = fops_get(minor->dev->driver->fops);
if (!new_fops) {
err = -ENODEV;
goto out;
}
replace_fops(filp, new_fops);
if (filp->f_op->open)
err = filp->f_op->open(inode, filp);
else
err = 0;
out:
drm_minor_release(minor);
return err;
}
static const struct file_operations drm_stub_fops = {
.owner = THIS_MODULE,
.open = drm_stub_open,
.llseek = noop_llseek,
};
drm: cleanup drm_core_{init,exit}() Various cleanups to the DRM core initialization and exit handlers: - Register chrdev last: Once register_chrdev() returns, open() will succeed on the given chrdevs. This is usually not an issue, as no chardevs are registered, yet. However, nodes can be created by user-space via mknod(2), even though such major/minor combinations are unknown to the kernel. Avoid calling into drm_stub_open() in those cases. Again, drm_stub_open() would just bail out as the inode is unknown, but it's really non-obvious if you hack on drm_stub_open(). - Unify error-paths into just one label. All the error-path helpers can be called even though the constructors were not called yet, or failed. Hence, just call all cleanups unconditionally. - Call into drm_global_release(). This is a no-op, but provides debugging helpers in case there're GLOBALS left on module unload. This function was unused until now. - Use DRM_ERROR() instead of printk(), and also print the error-code on failure (even if it is static!). - Don't throw away error-codes of register_chrdev()! - Don't hardcode -1 as errno. This is just plain wrong. - Order exit-handlers in the exact reverse order of initialization (except if the order actually matters for syncing-reasons, which is not the case here, though). v2: - Call drm_core_exit() directly from the init-error-handler. Requires to drop __exit annotation, though. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-7-dh.herrmann@gmail.com
2016-09-01 12:48:37 +00:00
static void drm_core_exit(void)
{
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
drm_sysfs_destroy();
idr_destroy(&drm_minors_idr);
drm_connector_ida_destroy();
}
static int __init drm_core_init(void)
{
drm: cleanup drm_core_{init,exit}() Various cleanups to the DRM core initialization and exit handlers: - Register chrdev last: Once register_chrdev() returns, open() will succeed on the given chrdevs. This is usually not an issue, as no chardevs are registered, yet. However, nodes can be created by user-space via mknod(2), even though such major/minor combinations are unknown to the kernel. Avoid calling into drm_stub_open() in those cases. Again, drm_stub_open() would just bail out as the inode is unknown, but it's really non-obvious if you hack on drm_stub_open(). - Unify error-paths into just one label. All the error-path helpers can be called even though the constructors were not called yet, or failed. Hence, just call all cleanups unconditionally. - Call into drm_global_release(). This is a no-op, but provides debugging helpers in case there're GLOBALS left on module unload. This function was unused until now. - Use DRM_ERROR() instead of printk(), and also print the error-code on failure (even if it is static!). - Don't throw away error-codes of register_chrdev()! - Don't hardcode -1 as errno. This is just plain wrong. - Order exit-handlers in the exact reverse order of initialization (except if the order actually matters for syncing-reasons, which is not the case here, though). v2: - Call drm_core_exit() directly from the init-error-handler. Requires to drop __exit annotation, though. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-7-dh.herrmann@gmail.com
2016-09-01 12:48:37 +00:00
int ret;
drm_connector_ida_init();
idr_init(&drm_minors_idr);
ret = drm_sysfs_init();
if (ret < 0) {
drm: cleanup drm_core_{init,exit}() Various cleanups to the DRM core initialization and exit handlers: - Register chrdev last: Once register_chrdev() returns, open() will succeed on the given chrdevs. This is usually not an issue, as no chardevs are registered, yet. However, nodes can be created by user-space via mknod(2), even though such major/minor combinations are unknown to the kernel. Avoid calling into drm_stub_open() in those cases. Again, drm_stub_open() would just bail out as the inode is unknown, but it's really non-obvious if you hack on drm_stub_open(). - Unify error-paths into just one label. All the error-path helpers can be called even though the constructors were not called yet, or failed. Hence, just call all cleanups unconditionally. - Call into drm_global_release(). This is a no-op, but provides debugging helpers in case there're GLOBALS left on module unload. This function was unused until now. - Use DRM_ERROR() instead of printk(), and also print the error-code on failure (even if it is static!). - Don't throw away error-codes of register_chrdev()! - Don't hardcode -1 as errno. This is just plain wrong. - Order exit-handlers in the exact reverse order of initialization (except if the order actually matters for syncing-reasons, which is not the case here, though). v2: - Call drm_core_exit() directly from the init-error-handler. Requires to drop __exit annotation, though. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-7-dh.herrmann@gmail.com
2016-09-01 12:48:37 +00:00
DRM_ERROR("Cannot create DRM class: %d\n", ret);
goto error;
}
drm_debugfs_root = debugfs_create_dir("dri", NULL);
drm: cleanup drm_core_{init,exit}() Various cleanups to the DRM core initialization and exit handlers: - Register chrdev last: Once register_chrdev() returns, open() will succeed on the given chrdevs. This is usually not an issue, as no chardevs are registered, yet. However, nodes can be created by user-space via mknod(2), even though such major/minor combinations are unknown to the kernel. Avoid calling into drm_stub_open() in those cases. Again, drm_stub_open() would just bail out as the inode is unknown, but it's really non-obvious if you hack on drm_stub_open(). - Unify error-paths into just one label. All the error-path helpers can be called even though the constructors were not called yet, or failed. Hence, just call all cleanups unconditionally. - Call into drm_global_release(). This is a no-op, but provides debugging helpers in case there're GLOBALS left on module unload. This function was unused until now. - Use DRM_ERROR() instead of printk(), and also print the error-code on failure (even if it is static!). - Don't throw away error-codes of register_chrdev()! - Don't hardcode -1 as errno. This is just plain wrong. - Order exit-handlers in the exact reverse order of initialization (except if the order actually matters for syncing-reasons, which is not the case here, though). v2: - Call drm_core_exit() directly from the init-error-handler. Requires to drop __exit annotation, though. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-7-dh.herrmann@gmail.com
2016-09-01 12:48:37 +00:00
ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
if (ret < 0)
goto error;
drm_core_init_complete = true;
DRM_DEBUG("Initialized\n");
return 0;
drm: cleanup drm_core_{init,exit}() Various cleanups to the DRM core initialization and exit handlers: - Register chrdev last: Once register_chrdev() returns, open() will succeed on the given chrdevs. This is usually not an issue, as no chardevs are registered, yet. However, nodes can be created by user-space via mknod(2), even though such major/minor combinations are unknown to the kernel. Avoid calling into drm_stub_open() in those cases. Again, drm_stub_open() would just bail out as the inode is unknown, but it's really non-obvious if you hack on drm_stub_open(). - Unify error-paths into just one label. All the error-path helpers can be called even though the constructors were not called yet, or failed. Hence, just call all cleanups unconditionally. - Call into drm_global_release(). This is a no-op, but provides debugging helpers in case there're GLOBALS left on module unload. This function was unused until now. - Use DRM_ERROR() instead of printk(), and also print the error-code on failure (even if it is static!). - Don't throw away error-codes of register_chrdev()! - Don't hardcode -1 as errno. This is just plain wrong. - Order exit-handlers in the exact reverse order of initialization (except if the order actually matters for syncing-reasons, which is not the case here, though). v2: - Call drm_core_exit() directly from the init-error-handler. Requires to drop __exit annotation, though. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20160901124837.680-7-dh.herrmann@gmail.com
2016-09-01 12:48:37 +00:00
error:
drm_core_exit();
return ret;
}
module_init(drm_core_init);
module_exit(drm_core_exit);