[PATCH] drm/doc: Update docs about device instance setup

[Daniel Vetter <daniel.vetter@ffwll.ch>]

->load is deprecated, bus functions are deprecated and everyone
should use drm_dev_alloc&register.

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 <laurent.pinchart@ideasonboard.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Acked-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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 @@
     <para>
       At the core of every DRM driver is a <structname>drm_driver</structname>
       structure. Drivers typically statically initialize a drm_driver structure,
-      and then pass it to one of the <function>drm_*_init()</function> functions
-      to register it with the DRM subsystem.
-    </para>
-    <para>
-      Newer drivers that no longer require a <structname>drm_bus</structname>
-      structure can alternatively use the low-level device initialization and
-      registration functions such as <function>drm_dev_alloc()</function> and
-      <function>drm_dev_register()</function> directly.
+      and then pass it to <function>drm_dev_alloc()</function> to allocate a
+      device instance. After the device instance is fully initialized it can be
+      registered (which makes it accessible from userspace) using
+      <function>drm_dev_register()</function>.
     </para>
     <para>
       The <structname>drm_driver</structname> structure contains static
@@ -296,83 +292,12 @@ char *date;</synopsis>
       </sect3>
     </sect2>
     <sect2>
-      <title>Device Registration</title>
-      <para>
-        A number of functions are provided to help with device registration.
-        The functions deal with PCI and platform devices, respectively.
-      </para>
-!Edrivers/gpu/drm/drm_pci.c
-!Edrivers/gpu/drm/drm_platform.c
-      <para>
-        New drivers that no longer rely on the services provided by the
-        <structname>drm_bus</structname> structure can call the low-level
-        device registration functions directly. The
-        <function>drm_dev_alloc()</function> function can be used to allocate
-        and initialize a new <structname>drm_device</structname> 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 <structfield>dev_private</structfield>
-        field. Drivers should also set the device's unique name using the
-        <function>drm_dev_set_unique()</function> function. After it has been
-        set up a device can be registered with the DRM subsystem by calling
-        <function>drm_dev_register()</function>. This will cause the device to
-        be exposed to userspace and will call the driver's
-        <structfield>.load()</structfield> implementation. When a device is
-        removed, the DRM device can safely be unregistered and freed by calling
-        <function>drm_dev_unregister()</function> followed by a call to
-        <function>drm_dev_unref()</function>.
-      </para>
+      <title>Device Instance and Driver Handling</title>
+!Pdrivers/gpu/drm/drm_drv.c driver instance overview
 !Edrivers/gpu/drm/drm_drv.c
     </sect2>
     <sect2>
       <title>Driver Load</title>
-      <para>
-        The <methodname>load</methodname> 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 (<xref linkend="drm-memory-management"/>), installing
-        the IRQ handler (<xref linkend="drm-irq-registration"/>), setting up
-        vertical blanking handling (<xref linkend="drm-vertical-blank"/>), mode
-	setting (<xref linkend="drm-mode-setting"/>) and initial output
-	configuration (<xref linkend="drm-kms-init"/>).
-      </para>
-      <note><para>
-        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
-        &amp; 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.
-      </para></note>
-      <synopsis>int (*load) (struct drm_device *, unsigned long flags);</synopsis>
-      <para>
-        The method takes two arguments, a pointer to the newly created
-	<structname>drm_device</structname> and flags. The flags are used to
-	pass the <structfield>driver_data</structfield> field of the device id
-	corresponding to the device passed to <function>drm_*_init()</function>.
-	Only PCI devices currently use this, USB and platform DRM drivers have
-	their <methodname>load</methodname> method called with flags to 0.
-      </para>
-      <sect3>
-        <title>Driver Private Data</title>
-        <para>
-          The driver private hangs off the main
-          <structname>drm_device</structname> 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
-          <structname>drm_device</structname>.<structfield>dev_priv</structfield>
-          appropriately; it should be freed and
-          <structname>drm_device</structname>.<structfield>dev_priv</structfield>
-          set to NULL when the driver is unloaded.
-        </para>
-      </sect3>
       <sect3 id="drm-irq-registration">
         <title>IRQ Registration</title>
         <para>
@@ -465,6 +390,18 @@ char *date;</synopsis>
         </para>
       </sect3>
     </sect2>
+    <sect2>
+      <title>Bus-specific Device Registration and PCI Support</title>
+      <para>
+        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.
+      </para>
+!Edrivers/gpu/drm/drm_pci.c
+!Edrivers/gpu/drm/drm_platform.c
+    </sect2>
   </sect1>
 
   <!-- Internals: memory management -->
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)
-- 
2.5.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel