From patchwork Mon Sep 28 19:46:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 7279651 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id EDAB8BEEA4 for ; Mon, 28 Sep 2015 19:43:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 88F5E2076C for ; Mon, 28 Sep 2015 19:43:49 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 1365720767 for ; Mon, 28 Sep 2015 19:43:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1CCF26E320; Mon, 28 Sep 2015 12:43:46 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1F5256E320 for ; Mon, 28 Sep 2015 12:43:45 -0700 (PDT) Received: by wicfx3 with SMTP id fx3so120239991wic.1 for ; Mon, 28 Sep 2015 12:43:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=BEgyWXq/tIEn0MjGpvKH18b8LKwOLAhS1XPigO6rhwk=; b=X9B/DGjgj1hn4q6pHBgzB9nH5c01NsepPmcujB0BbqqTA/Oj0OlstFqxXORckRndtr RjKH1NDE6rBJGq8KCTE9a/Bv6C2MHXl9bmjMkksXQHfLbJnOcjxMXg5n1Om/Q2s2T2+J 02UaeO6gGTj77u8FbMcF6c3E8MN72phay8Q1w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=BEgyWXq/tIEn0MjGpvKH18b8LKwOLAhS1XPigO6rhwk=; b=gKr4z9dxOs3yH8R4V3wtLtCZZfmUZDEZt69LBx6VLKGd6r2JasT3WtWNXc0rqH6FuM MyO9KuOkkSa0uF/EoJs5Z233sA2XHSgwh3d2Ou47eZgXeIJ9nUNpgyxW3ztlJbnlZbOC DGFCyWRhRikS0zgUrTzwino8wjekHEKDkxUFt+sjrQ2LIhkW495JQdurjp5hlhhddhUL CUWibxIT8AHr4EgEL7CIgzEjQAhkvasBXBvzSPB4ohe5DCApzYlP+EhhH3Tn8dHZOtKj O5TXM72eJIdjYPPPYGbM6vZBxbh5C7LUwWjXKjcjd0jUHllJVMK0y+ocz8xJNzPZMaRM kcCw== X-Gm-Message-State: ALoCoQlxaFwJmMqLsaSyqGDuhKJvnuOoSrjPZy1Pq5zcs5g2RFcFFBpB7eEYHb+kaMmFFJe6odf9 X-Received: by 10.180.107.102 with SMTP id hb6mr18939750wib.37.1443469423668; Mon, 28 Sep 2015 12:43:43 -0700 (PDT) Received: from phenom.ffwll.local (212-51-149-109.fiber7.init7.net. [212.51.149.109]) by smtp.gmail.com with ESMTPSA id o10sm19977965wia.4.2015.09.28.12.43.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 28 Sep 2015 12:43:43 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH] drm/doc: Update docs about device instance setup Date: Mon, 28 Sep 2015 21:46:35 +0200 Message-Id: <1443469595-24601-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.5.1 In-Reply-To: <1439200538-25233-2-git-send-email-daniel.vetter@ffwll.ch> References: <1439200538-25233-2-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Daniel Vetter , Intel Graphics Development , Laurent Pinchart X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP ->load is deprecated, bus functions are deprecated and everyone should use drm_dev_alloc®ister. So update the .tmpl (and pull a bunch of the overview docs into the sourcecode to increase chances that it'll stay in sync in the future) and add notes to functions which are deprecated. I didn't bother to clean up and document the unload sequence similarly since that one is still a bit a mess: drm_dev_unregister does way too much, drm_unplug_dev does what _unregister should be doing but then has the complication of promising something it doesn't actually do (it doesn't unplug existing open fds for instance, only prevents new ones). Motivated since I don't want to hunt every new driver for usage of drm_platform_init any more ;-) v2: Reword the deprecation note for ->load a bit, using Laurent's suggestion as an example (but making the wording a bit stronger even). Fix spelling in commit message. Cc: Laurent Pinchart Cc: David Herrmann Acked-by: David Herrmann Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 99 ++++++++---------------------------------- drivers/gpu/drm/drm_drv.c | 55 +++++++++++++++++++++-- drivers/gpu/drm/drm_pci.c | 11 +++++ drivers/gpu/drm/drm_platform.c | 3 ++ 4 files changed, 83 insertions(+), 85 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 5395cde6905e..6d24ebb97cda 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -138,14 +138,10 @@ At the core of every DRM driver is a drm_driver structure. Drivers typically statically initialize a drm_driver structure, - and then pass it to one of the drm_*_init() functions - to register it with the DRM subsystem. - - - Newer drivers that no longer require a drm_bus - structure can alternatively use the low-level device initialization and - registration functions such as drm_dev_alloc() and - drm_dev_register() directly. + and then pass it to drm_dev_alloc() to allocate a + device instance. After the device instance is fully initialized it can be + registered (which makes it accessible from userspace) using + drm_dev_register(). The drm_driver structure contains static @@ -296,83 +292,12 @@ char *date; - Device Registration - - A number of functions are provided to help with device registration. - The functions deal with PCI and platform devices, respectively. - -!Edrivers/gpu/drm/drm_pci.c -!Edrivers/gpu/drm/drm_platform.c - - New drivers that no longer rely on the services provided by the - drm_bus structure can call the low-level - device registration functions directly. The - drm_dev_alloc() function can be used to allocate - and initialize a new drm_device structure. - Drivers will typically want to perform some additional setup on this - structure, such as allocating driver-specific data and storing a - pointer to it in the DRM device's dev_private - field. Drivers should also set the device's unique name using the - drm_dev_set_unique() function. After it has been - set up a device can be registered with the DRM subsystem by calling - drm_dev_register(). This will cause the device to - be exposed to userspace and will call the driver's - .load() implementation. When a device is - removed, the DRM device can safely be unregistered and freed by calling - drm_dev_unregister() followed by a call to - drm_dev_unref(). - + Device Instance and Driver Handling +!Pdrivers/gpu/drm/drm_drv.c driver instance overview !Edrivers/gpu/drm/drm_drv.c Driver Load - - The load method is the driver and device - initialization entry point. The method is responsible for allocating and - initializing driver private data, performing resource allocation and - mapping (e.g. acquiring - clocks, mapping registers or allocating command buffers), initializing - the memory manager (), installing - the IRQ handler (), setting up - vertical blanking handling (), mode - setting () and initial output - configuration (). - - - If compatibility is a concern (e.g. with drivers converted over from - User Mode Setting to Kernel Mode Setting), care must be taken to prevent - device initialization and control that is incompatible with currently - active userspace drivers. For instance, if user level mode setting - drivers are in use, it would be problematic to perform output discovery - & configuration at load time. Likewise, if user-level drivers - unaware of memory management are in use, memory management and command - buffer setup may need to be omitted. These requirements are - driver-specific, and care needs to be taken to keep both old and new - applications and libraries working. - - int (*load) (struct drm_device *, unsigned long flags); - - The method takes two arguments, a pointer to the newly created - drm_device and flags. The flags are used to - pass the driver_data field of the device id - corresponding to the device passed to drm_*_init(). - Only PCI devices currently use this, USB and platform DRM drivers have - their load method called with flags to 0. - - - Driver Private Data - - The driver private hangs off the main - drm_device structure and can be used for - tracking various device-specific bits of information, like register - offsets, command buffer status, register state for suspend/resume, etc. - At load time, a driver may simply allocate one and set - drm_device.dev_priv - appropriately; it should be freed and - drm_device.dev_priv - set to NULL when the driver is unloaded. - - IRQ Registration @@ -465,6 +390,18 @@ char *date; + + Bus-specific Device Registration and PCI Support + + A number of functions are provided to help with device registration. + The functions deal with PCI and platform devices respectively and are + only provided for historical reasons. These are all deprecated and + shouldn't be used in new drivers. Besides that there's a few + helpers for pci drivers. + +!Edrivers/gpu/drm/drm_pci.c +!Edrivers/gpu/drm/drm_platform.c + diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 9ad823fcde87..031c69b71547 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -397,15 +397,51 @@ void drm_minor_release(struct drm_minor *minor) } /** + * DOC: driver instance overview + * + * A device instance for a drm driver is represented by struct &drm_device. This + * is allocated with 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 intial output configuration plus obviously + * initialize all the corresponding hardware bits. An important part of this is + * also calling drm_dev_set_unique() to set the userspace-visible unique name of + * this device instance. 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 ->load() callback. But due to + * backwards-compatibility needs the device instance has 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 revers: + * 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_unref(). + * + * Note that the lifetime rules for &drm_device instance has still a lot of + * historical baggage. Hence use the reference counting provided by + * drm_dev_ref() and drm_dev_unref() only carefully. + * + * Also note that embedding of &drm_device is currently not (yet) supported (but + * it would be easy to add). Drivers can store driver-private data in the + * dev_priv field of &drm_device. + */ + +/** * drm_put_dev - Unregister and release a DRM device * @dev: DRM device * * Called at module unload time or when a PCI device is unplugged. * - * Use of this function is discouraged. It will eventually go away completely. - * Please use drm_dev_unregister() and drm_dev_unref() explicitly instead. - * * 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_unref() explicitly + * instead to make sure that the device isn't userspace accessible any more + * while teardown is in progress, to make sure userspace can't access an + * inconsisten state. */ void drm_put_dev(struct drm_device *dev) { @@ -518,7 +554,9 @@ static void drm_fs_inode_free(struct inode *inode) * * Allocate and initialize a new DRM device. No device registration is done. * Call drm_dev_register() to advertice the device to user space and register it - * with other core subsystems. + * with other core subsystems. This should be done last in the device + * initialization sequence to make sure userspace can't access an inconsistent + * state. * * The initial ref-count of the object is 1. Use drm_dev_ref() and * drm_dev_unref() to take and drop further ref-counts. @@ -673,6 +711,12 @@ EXPORT_SYMBOL(drm_dev_unref); * * Never call this twice on any device! * + * NOTE: To ensure backward compatibility with existing drivers method this + * function calls the ->load() method after registering the device nodes, + * creating race conditions. Usage of the ->load() methods is therefore + * deprecated, drivers must perform all initialization before calling + * drm_dev_register(). + * * RETURNS: * 0 on success, negative error code on failure. */ @@ -720,6 +764,9 @@ EXPORT_SYMBOL(drm_dev_register); * 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_unref() to drop their final reference. + * + * 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) { diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 1b1bd42b0368..fcd2a86acd2c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -266,6 +266,9 @@ void drm_pci_agp_destroy(struct drm_device *dev) * then register the character device and inter module information. * Try and register, if we fail to register, backout previous work. * + * NOTE: This function is deprecated, please use drm_dev_alloc() and + * drm_dev_register() instead and remove your ->load() callback. + * * Return: 0 on success or a negative error code on failure. */ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, @@ -326,6 +329,10 @@ EXPORT_SYMBOL(drm_get_pci_dev); * Initializes a drm_device structures, registering the stubs and initializing * the AGP device. * + * NOTE: This function is deprecated. Modern modesetting drm drivers should use + * pci_register_driver() directly, this function only provides shadow-binding + * support for old legacy drivers on top of that core pci function. + * * Return: 0 on success or a negative error code on failure. */ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) @@ -435,6 +442,10 @@ EXPORT_SYMBOL(drm_pci_init); * * Unregisters one or more devices matched by a PCI driver from the DRM * subsystem. + * + * NOTE: This function is deprecated. Modern modesetting drm drivers should use + * pci_unregister_driver() directly, this function only provides shadow-binding + * support for old legacy drivers on top of that core pci function. */ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) { diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index 5314c9d5fef4..644169e1a029 100644 --- a/drivers/gpu/drm/drm_platform.c +++ b/drivers/gpu/drm/drm_platform.c @@ -95,6 +95,9 @@ EXPORT_SYMBOL(drm_platform_set_busid); * subsystem, initializing a drm_device structure and calling the driver's * .load() function. * + * NOTE: This function is deprecated, please use drm_dev_alloc() and + * drm_dev_register() instead and remove your ->load() callback. + * * Return: 0 on success or a negative error code on failure. */ int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device)