[PATCH 00/10] kms locking rework prep patches

[PATCH 00/10] kms locking rework prep patches

[Daniel Vetter]

Hi all,

So I've beaten on the series a bit more, written some evil testcases and things
seem to hold up. I'm rather happy with it now. I've also reordered patches a bit
to move all the prep stuff which doesn't introduce the new concepts, but just
adds shims/docs/reworks driver locking where required to the front.

So if (driver) maintainers want to squeeze anything of this into 3.7, might make
the merging a bit easier, but not required at all. The only bugfix which is
required for 3.7 imo is the ttm locking fix, already reviewed by Jerome. exynos
and radeon patches are already merged, so not included here.

I'll harrass everyone with the full patchset (and probably an added patch with a
short overview in the docs for this tacked on top) next year around again.

Cheers, Daniel

Daniel Vetter (10):
  drm: review locking rules in drm_crtc.c
  drm/doc: integrate drm_crtc.c kerneldoc
  drm/<drivers>: reorder framebuffer init sequence
  drm/vmwgfx: reorder framebuffer init sequence
  drm/gma500: move fbcon restore to lastclose
  drm/nouveau: protect evo_wait/evo_kick sections with a channel mutex
  drm/nouveau: try to protect nbo->pin_refcount
  drm/ttm: fix fence locking in ttm_buffer_object_transfer
  drm/<drivers>: Unified handling of unimplemented fb->create_handle
  drm: encapsulate crtc->set_config calls

 Documentation/DocBook/drm.tmpl            |    4 +
 drivers/gpu/drm/ast/ast_main.c            |   12 +-
 drivers/gpu/drm/cirrus/cirrus_main.c      |   12 +-
 drivers/gpu/drm/drm_crtc.c                |  219 ++++++++++-------------------
 drivers/gpu/drm/drm_fb_cma_helper.c       |   10 +-
 drivers/gpu/drm/drm_fb_helper.c           |    6 +-
 drivers/gpu/drm/gma500/framebuffer.c      |   28 +---
 drivers/gpu/drm/gma500/psb_drv.c          |   10 ++
 drivers/gpu/drm/i2c/ch7006_drv.c          |    2 +-
 drivers/gpu/drm/i915/intel_display.c      |    5 +-
 drivers/gpu/drm/mgag200/mgag200_main.c    |   16 +--
 drivers/gpu/drm/nouveau/nouveau_bo.c      |   22 +--
 drivers/gpu/drm/nouveau/nouveau_bo.h      |    2 +
 drivers/gpu/drm/nouveau/nouveau_display.c |   10 +-
 drivers/gpu/drm/nouveau/nv04_display.c    |    2 +-
 drivers/gpu/drm/nouveau/nv17_tv.c         |    2 +-
 drivers/gpu/drm/nouveau/nv50_display.c    |    8 ++
 drivers/gpu/drm/radeon/radeon_display.c   |    2 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c         |    4 +-
 drivers/gpu/drm/udl/udl_fb.c              |    3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c       |    2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c       |   42 +++---
 drivers/staging/omapdrm/omap_fb.c         |   16 +--
 include/drm/drmP.h                        |    5 +
 include/drm/drm_crtc.h                    |    1 +
 25 files changed, 177 insertions(+), 268 deletions(-)

-- 
1.7.10.4

[PATCH 01/10] drm: review locking rules in drm_crtc.c

[Daniel Vetter]

- config_cleanup was confused: It claimed that callers need to hold
  the modeset lock, but the connector|encoder_cleanup helpers grabbed
  that themselves (note that crtc_cleanup did _not_ grab the modeset
  lock). Which resulted in all drivers _not_ hodling the lock. Since
  this is for single-threaded cleanup code, drop the requirement from
  docs and also drop the lock_grabbing from all _cleanup functions.

- Kill the LOCKING section in the doctype, since clearly we're not
  good enough to keep them up-to-date. And misleading locking
  documentation is worse than useless (see e.g. the comment in the
  vmgfx driver about the cleanup mess). And since for most functions
  the very first line either grabs the lock or has a WARN_ON(!locked)
  the documentation doesn't really add anything.

- Instead put in some effort into explaining the only two special
  cases a bit better: config_init and config_cleanup are both called
  from single-threaded setup/teardown code, so don't do any locking.
  It's the driver's job though to enforce this.

- Where lacking, add a WARN_ON(!is_locked). Not many places though,
  since locking around fbdev setup/teardown is through-roughly screwed
  up, and so will break almost every single WARN annotation I've tried
  to add.

- Add a drm_modeset_is_locked helper - the Grate Modset Locking Rework
  will use the compiler to assist in the big reorg by renaming the
  mode lock, so start encapsulating things. Unfortunately this ended
  up in the "wrong" header file since it needs the definition of
  struct drm_device.

v2: Drop most WARNS again - we hit them all over the place, mostly in
the setup and teardown sequences. And trying to fix it up leads to
nice deadlocks, since the locking in the setup code is really
inconsistent.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c |  105 ++++++--------------------------------------
 include/drm/drmP.h         |    5 +++
 2 files changed, 18 insertions(+), 92 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f2d667b..b970e41 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -208,8 +208,6 @@ char *drm_get_connector_status_name(enum drm_connector_status status)
  * @ptr: object pointer, used to generate unique ID
  * @type: object type
  *
- * LOCKING:
- *
  * Create a unique identifier based on @ptr in @dev's identifier space.  Used
  * for tracking modes, CRTCs and connectors.
  *
@@ -247,9 +245,6 @@ again:
  * @dev: DRM device
  * @id: ID to free
  *
- * LOCKING:
- * Caller must hold DRM mode_config lock.
- *
  * Free @id from @dev's unique identifier pool.
  */
 static void drm_mode_object_put(struct drm_device *dev,
@@ -279,9 +274,6 @@ EXPORT_SYMBOL(drm_mode_object_find);
  * drm_framebuffer_init - initialize a framebuffer
  * @dev: DRM device
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Allocates an ID for the framebuffer's parent mode object, sets its mode
  * functions & device file and adds it to the master fd list.
  *
@@ -317,15 +309,12 @@ static void drm_framebuffer_free(struct kref *kref)
 
 /**
  * drm_framebuffer_unreference - unref a framebuffer
- *
- * LOCKING:
- * Caller must hold mode config lock.
  */
 void drm_framebuffer_unreference(struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = fb->dev;
 	DRM_DEBUG("FB ID: %d\n", fb->base.id);
-	WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+	WARN_ON(!drm_modeset_is_locked(dev));
 	kref_put(&fb->refcount, drm_framebuffer_free);
 }
 EXPORT_SYMBOL(drm_framebuffer_unreference);
@@ -344,15 +333,13 @@ EXPORT_SYMBOL(drm_framebuffer_reference);
  * drm_framebuffer_cleanup - remove a framebuffer object
  * @fb: framebuffer to remove
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Scans all the CRTCs in @dev's mode_config.  If they're using @fb, removes
  * it, setting it to NULL.
  */
 void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = fb->dev;
+
 	/*
 	 * This could be moved to drm_framebuffer_remove(), but for
 	 * debugging is nice to keep around the list of fb's that are
@@ -370,9 +357,6 @@ EXPORT_SYMBOL(drm_framebuffer_cleanup);
  * drm_framebuffer_remove - remove and unreference a framebuffer object
  * @fb: framebuffer to remove
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Scans all the CRTCs and planes in @dev's mode_config.  If they're
  * using @fb, removes it, setting it to NULL.
  */
@@ -384,6 +368,8 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 	struct drm_mode_set set;
 	int ret;
 
+	WARN_ON(!drm_modeset_is_locked(dev));
+
 	/* remove from any CRTC */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		if (crtc->fb == fb) {
@@ -421,9 +407,6 @@ EXPORT_SYMBOL(drm_framebuffer_remove);
  * @crtc: CRTC object to init
  * @funcs: callbacks for the new CRTC
  *
- * LOCKING:
- * Takes mode_config lock.
- *
  * Inits a new object created as base part of an driver crtc object.
  *
  * RETURNS:
@@ -460,9 +443,6 @@ EXPORT_SYMBOL(drm_crtc_init);
  * drm_crtc_cleanup - Cleans up the core crtc usage.
  * @crtc: CRTC to cleanup
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Cleanup @crtc. Removes from drm modesetting space
  * does NOT free object, caller does that.
  */
@@ -484,9 +464,6 @@ EXPORT_SYMBOL(drm_crtc_cleanup);
  * @connector: connector the new mode
  * @mode: mode data
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Add @mode to @connector's mode list for later use.
  */
 void drm_mode_probed_add(struct drm_connector *connector,
@@ -501,9 +478,6 @@ EXPORT_SYMBOL(drm_mode_probed_add);
  * @connector: connector list to modify
  * @mode: mode to remove
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Remove @mode from @connector's mode list, then free it.
  */
 void drm_mode_remove(struct drm_connector *connector,
@@ -521,9 +495,6 @@ EXPORT_SYMBOL(drm_mode_remove);
  * @funcs: callbacks for this connector
  * @name: user visible name of the connector
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Initialises a preallocated connector. Connectors should be
  * subclassed as part of driver connector objects.
  *
@@ -577,9 +548,6 @@ EXPORT_SYMBOL(drm_connector_init);
  * drm_connector_cleanup - cleans up an initialised connector
  * @connector: connector to cleanup
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Cleans up the connector but doesn't free the object.
  */
 void drm_connector_cleanup(struct drm_connector *connector)
@@ -596,11 +564,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
 	list_for_each_entry_safe(mode, t, &connector->user_modes, head)
 		drm_mode_remove(connector, mode);
 
-	mutex_lock(&dev->mode_config.mutex);
 	drm_mode_object_put(dev, &connector->base);
 	list_del(&connector->head);
 	dev->mode_config.num_connector--;
-	mutex_unlock(&dev->mode_config.mutex);
 }
 EXPORT_SYMBOL(drm_connector_cleanup);
 
@@ -721,9 +687,6 @@ EXPORT_SYMBOL(drm_plane_cleanup);
  * drm_mode_create - create a new display mode
  * @dev: DRM device
  *
- * LOCKING:
- * Caller must hold DRM mode_config lock.
- *
  * Create a new drm_display_mode, give it an ID, and return it.
  *
  * RETURNS:
@@ -751,9 +714,6 @@ EXPORT_SYMBOL(drm_mode_create);
  * @dev: DRM device
  * @mode: mode to remove
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Free @mode's unique identifier, then free it.
  */
 void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode)
@@ -978,11 +938,13 @@ EXPORT_SYMBOL(drm_mode_create_dirty_info_property);
  * drm_mode_config_init - initialize DRM mode_configuration structure
  * @dev: DRM device
  *
- * LOCKING:
- * None, should happen single threaded at init time.
- *
  * Initialize @dev's mode_config structure, used for tracking the graphics
  * configuration of @dev.
+ *
+ * Since this initializes the modeset locks, no locking is possible. Which is no
+ * problem, since this should happen single threaded at init time. It is the
+ * driver's problem to ensure this guarantee.
+ *
  */
 void drm_mode_config_init(struct drm_device *dev)
 {
@@ -1057,12 +1019,13 @@ EXPORT_SYMBOL(drm_mode_group_init_legacy_group);
  * drm_mode_config_cleanup - free up DRM mode_config info
  * @dev: DRM device
  *
- * LOCKING:
- * Caller must hold mode config lock.
- *
  * Free up all the connectors and CRTCs associated with this DRM device, then
  * free up the framebuffers and associated buffer objects.
  *
+ * Note that since this /should/ happen single-threaded at driver/device
+ * teardown time, no locking is required. It's the driver's job to ensure that
+ * this guarantee actually holds true.
+ *
  * FIXME: cleanup any dangling user buffer objects too
  */
 void drm_mode_config_cleanup(struct drm_device *dev)
@@ -1112,9 +1075,6 @@ EXPORT_SYMBOL(drm_mode_config_cleanup);
  * @out: drm_mode_modeinfo struct to return to the user
  * @in: drm_display_mode to use
  *
- * LOCKING:
- * None.
- *
  * Convert a drm_display_mode into a drm_mode_modeinfo structure to return to
  * the user.
  */
@@ -1151,9 +1111,6 @@ static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
  * @out: drm_display_mode to return to the user
  * @in: drm_mode_modeinfo to use
  *
- * LOCKING:
- * None.
- *
  * Convert a drm_mode_modeinfo into a drm_display_mode structure to return to
  * the caller.
  *
@@ -1193,9 +1150,6 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Construct a set of configuration description structures and return
  * them to the user, including CRTC, connector and framebuffer configuration.
  *
@@ -1381,9 +1335,6 @@ out:
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Construct a CRTC configuration structure to return to the user.
  *
  * Called by the user via ioctl.
@@ -1441,9 +1392,6 @@ out:
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Construct a connector configuration structure to return to the user.
  *
  * Called by the user via ioctl.
@@ -1618,9 +1566,6 @@ out:
  * @data: ioctl data
  * @file_priv: DRM file info
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Return an plane count and set of IDs.
  */
 int drm_mode_getplane_res(struct drm_device *dev, void *data,
@@ -1667,9 +1612,6 @@ out:
  * @data: ioctl data
  * @file_priv: DRM file info
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Return plane info, including formats supported, gamma size, any
  * current fb, etc.
  */
@@ -1735,9 +1677,6 @@ out:
  * @data: ioctl data*
  * @file_prive: DRM file info
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Set plane info, including placement, fb, scaling, and other factors.
  * Or pass a NULL fb to disable.
  */
@@ -1867,9 +1806,6 @@ out:
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Build a new CRTC configuration based on user request.
  *
  * Called by the user via ioctl.
@@ -2125,9 +2061,6 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format);
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Add a new FB to the specified CRTC, given a user request.
  *
  * Called by the user via ioctl.
@@ -2309,9 +2242,6 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Add a new FB to the specified CRTC, given a user request with format.
  *
  * Called by the user via ioctl.
@@ -2375,9 +2305,6 @@ out:
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Remove the FB specified by the user.
  *
  * Called by the user via ioctl.
@@ -2430,9 +2357,6 @@ out:
  * @cmd: cmd from ioctl
  * @arg: arg from ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Lookup the FB given its ID and return info about it.
  *
  * Called by the user via ioctl.
@@ -2549,9 +2473,6 @@ out_err1:
  * drm_fb_release - remove and free the FBs on this file
  * @filp: file * from the ioctl
  *
- * LOCKING:
- * Takes mode config lock.
- *
  * Destroy all the FBs associated with @filp.
  *
  * Called by the user via ioctl.
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..3c609ab 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1276,6 +1276,11 @@ static inline int drm_device_is_unplugged(struct drm_device *dev)
 	return ret;
 }
 
+static inline bool drm_modeset_is_locked(struct drm_device *dev)
+{
+	return mutex_is_locked(&dev->mode_config.mutex);
+}
+
 /******************************************************************/
 /** \name Internal function definitions */
 /*@{*/
-- 
1.7.10.4

[PATCH 02/10] drm/doc: integrate drm_crtc.c kerneldoc

[Daniel Vetter]

And do a quick pass to adjust them to the last few (years?) of changes
...

This time actually compile-tested ;-)

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl |    4 ++
 drivers/gpu/drm/drm_crtc.c     |   92 +++++++++++++++++++---------------------
 2 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 4ee2304..caab791 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -1609,6 +1609,10 @@ void intel_crt_init(struct drm_device *dev)
         make its properties available to applications.
       </para>
     </sect2>
+    <sect2>
+      <title>KMS API Functions</title>
+!Edrivers/gpu/drm/drm_crtc.c
+    </sect2>
   </sect1>
 
   <!-- Internals: kms helper functions -->
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b970e41..8154554 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -203,10 +203,10 @@ char *drm_get_connector_status_name(enum drm_connector_status status)
 }
 
 /**
- * drm_mode_object_get - allocate a new identifier
+ * drm_mode_object_get - allocate a new modeset identifier
  * @dev: DRM device
- * @ptr: object pointer, used to generate unique ID
- * @type: object type
+ * @obj: object pointer, used to generate unique ID
+ * @obj_type: object type
  *
  * Create a unique identifier based on @ptr in @dev's identifier space.  Used
  * for tracking modes, CRTCs and connectors.
@@ -241,9 +241,9 @@ again:
 }
 
 /**
- * drm_mode_object_put - free an identifer
+ * drm_mode_object_put - free a modeset identifer
  * @dev: DRM device
- * @id: ID to free
+ * @object: object to free
  *
  * Free @id from @dev's unique identifier pool.
  */
@@ -273,6 +273,8 @@ EXPORT_SYMBOL(drm_mode_object_find);
 /**
  * drm_framebuffer_init - initialize a framebuffer
  * @dev: DRM device
+ * @fb: framebuffer to be initialized
+ * @funcs: ... with these functions
  *
  * Allocates an ID for the framebuffer's parent mode object, sets its mode
  * functions & device file and adds it to the master fd list.
@@ -309,6 +311,9 @@ static void drm_framebuffer_free(struct kref *kref)
 
 /**
  * drm_framebuffer_unreference - unref a framebuffer
+ * @fb: framebuffer to unref
+ *
+ * This functions decrements the fb's refcount and frees it if it drops to zero.
  */
 void drm_framebuffer_unreference(struct drm_framebuffer *fb)
 {
@@ -321,6 +326,7 @@ EXPORT_SYMBOL(drm_framebuffer_unreference);
 
 /**
  * drm_framebuffer_reference - incr the fb refcnt
+ * @fb: framebuffer
  */
 void drm_framebuffer_reference(struct drm_framebuffer *fb)
 {
@@ -493,7 +499,7 @@ EXPORT_SYMBOL(drm_mode_remove);
  * @dev: DRM device
  * @connector: the connector to init
  * @funcs: callbacks for this connector
- * @name: user visible name of the connector
+ * @connector_type: user visible type of the connector
  *
  * Initialises a preallocated connector. Connectors should be
  * subclassed as part of driver connector objects.
@@ -1145,10 +1151,9 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
 
 /**
  * drm_mode_getresources - get graphics configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Construct a set of configuration description structures and return
  * them to the user, including CRTC, connector and framebuffer configuration.
@@ -1330,10 +1335,9 @@ out:
 
 /**
  * drm_mode_getcrtc - get CRTC configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Construct a CRTC configuration structure to return to the user.
  *
@@ -1387,10 +1391,9 @@ out:
 
 /**
  * drm_mode_getconnector - get connector configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Construct a connector configuration structure to return to the user.
  *
@@ -1675,7 +1678,7 @@ out:
  * drm_mode_setplane - set up or tear down an plane
  * @dev: DRM device
  * @data: ioctl data*
- * @file_prive: DRM file info
+ * @file_priv: DRM file info
  *
  * Set plane info, including placement, fb, scaling, and other factors.
  * Or pass a NULL fb to disable.
@@ -1801,10 +1804,9 @@ out:
 
 /**
  * drm_mode_setcrtc - set CRTC configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Build a new CRTC configuration based on user request.
  *
@@ -2056,10 +2058,9 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format);
 
 /**
  * drm_mode_addfb - add an FB to the graphics configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Add a new FB to the specified CRTC, given a user request.
  *
@@ -2237,10 +2238,9 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 
 /**
  * drm_mode_addfb2 - add an FB to the graphics configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Add a new FB to the specified CRTC, given a user request with format.
  *
@@ -2300,10 +2300,9 @@ out:
 
 /**
  * drm_mode_rmfb - remove an FB from the configuration
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Remove the FB specified by the user.
  *
@@ -2352,10 +2351,9 @@ out:
 
 /**
  * drm_mode_getfb - get FB info
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Lookup the FB given its ID and return info about it.
  *
@@ -2471,7 +2469,7 @@ out_err1:
 
 /**
  * drm_fb_release - remove and free the FBs on this file
- * @filp: file * from the ioctl
+ * @priv: drm file for the ioctl
  *
  * Destroy all the FBs associated with @filp.
  *
@@ -2581,10 +2579,9 @@ EXPORT_SYMBOL(drm_mode_detachmode_crtc);
 
 /**
  * drm_fb_attachmode - Attach a user mode to an connector
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * This attaches a user specified mode to an connector.
  * Called by the user via ioctl.
@@ -2636,10 +2633,9 @@ out:
 
 /**
  * drm_fb_detachmode - Detach a user specified mode from an connector
- * @inode: inode from the ioctl
- * @filp: file * from the ioctl
- * @cmd: cmd from ioctl
- * @arg: arg from ioctl
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
  *
  * Called by the user via ioctl.
  *
-- 
1.7.10.4

[PATCH 03/10] drm/<drivers>: reorder framebuffer init sequence

[Daniel Vetter]

With more fine-grained locking we can no longer rely on the big
mode_config lock to prevent concurrent access to mode resources
like framebuffers. Instead a framebuffer becomes accessible to
other threads as soon as it is added to the relevant lookup
structures. Hence it needs to be fully set up by the time drivers
call drm_framebuffer_init.

This patch here is the drivers part of that reorg. Nothing really fancy
going on safe for three special cases.

- exynos needs to be careful to properly unref all handles.
- nouveau gets a resource leak fixed for free: one of the error
  cases didn't cleanup the framebuffer, which is now moot since
  the framebuffer is only registered once it is fully set up.
- vmwgfx requires a slight reordering of operations, I'm hoping I didn't
  break anything (but it's refcount management only, so should be safe).

v2: Split out exynos, since it's a bit more hairy than expected.

v3: Drop bogus cirrus hunk noticed by Richard Wilbur.

v4: Split out vmwgfx since there's a small change in return values.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ast/ast_main.c            |    4 ++--
 drivers/gpu/drm/cirrus/cirrus_main.c      |    4 ++--
 drivers/gpu/drm/drm_fb_cma_helper.c       |   10 +++++-----
 drivers/gpu/drm/gma500/framebuffer.c      |    4 ++--
 drivers/gpu/drm/i915/intel_display.c      |    5 +++--
 drivers/gpu/drm/mgag200/mgag200_main.c    |    8 +++++---
 drivers/gpu/drm/nouveau/nouveau_display.c |   10 +++++-----
 drivers/gpu/drm/radeon/radeon_display.c   |    2 +-
 drivers/gpu/drm/udl/udl_fb.c              |    2 +-
 drivers/staging/omapdrm/omap_fb.c         |   16 ++++++++--------
 10 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f668e6c..d5ba709 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -266,13 +266,13 @@ int ast_framebuffer_init(struct drm_device *dev,
 {
 	int ret;
 
+	drm_helper_mode_fill_fb_struct(&ast_fb->base, mode_cmd);
+	ast_fb->obj = obj;
 	ret = drm_framebuffer_init(dev, &ast_fb->base, &ast_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&ast_fb->base, mode_cmd);
-	ast_fb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 6a9b12e..364474c 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -42,13 +42,13 @@ int cirrus_framebuffer_init(struct drm_device *dev,
 {
 	int ret;
 
+	drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd);
+	gfb->obj = obj;
 	ret = drm_framebuffer_init(dev, &gfb->base, &cirrus_fb_funcs);
 	if (ret) {
 		DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd);
-	gfb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index fd9d0af..e1e0cb0 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -85,6 +85,11 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 	if (!fb_cma)
 		return ERR_PTR(-ENOMEM);
 
+	drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd);
+
+	for (i = 0; i < num_planes; i++)
+		fb_cma->obj[i] = obj[i];
+
 	ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs);
 	if (ret) {
 		dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
@@ -92,11 +97,6 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 		return ERR_PTR(ret);
 	}
 
-	drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd);
-
-	for (i = 0; i < num_planes; i++)
-		fb_cma->obj[i] = obj[i];
-
 	return fb_cma;
 }
 
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index afded54..38e7e75 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -260,13 +260,13 @@ static int psb_framebuffer_init(struct drm_device *dev,
 	default:
 		return -EINVAL;
 	}
+	drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
+	fb->gtt = gt;
 	ret = drm_framebuffer_init(dev, &fb->base, &psb_fb_funcs);
 	if (ret) {
 		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
-	fb->gtt = gt;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 867cafe..c071161 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8367,14 +8367,15 @@ int intel_framebuffer_init(struct drm_device *dev,
 	if (mode_cmd->offsets[0] != 0)
 		return -EINVAL;
 
+	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
+	intel_fb->obj = obj;
+
 	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
 		return ret;
 	}
 
-	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
-	intel_fb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 70dd3c5..266438a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -40,13 +40,15 @@ int mgag200_framebuffer_init(struct drm_device *dev,
 			     struct drm_mode_fb_cmd2 *mode_cmd,
 			     struct drm_gem_object *obj)
 {
-	int ret = drm_framebuffer_init(dev, &gfb->base, &mga_fb_funcs);
+	int ret;
+	
+	drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd);
+	gfb->obj = obj;
+	ret = drm_framebuffer_init(dev, &gfb->base, &mga_fb_funcs);
 	if (ret) {
 		DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd);
-	gfb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index e4188f2..2591ff2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -78,11 +78,6 @@ nouveau_framebuffer_init(struct drm_device *dev,
 	struct drm_framebuffer *fb = &nv_fb->base;
 	int ret;
 
-	ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
-	if (ret) {
-		return ret;
-	}
-
 	drm_helper_mode_fill_fb_struct(fb, mode_cmd);
 	nv_fb->nvbo = nvbo;
 
@@ -125,6 +120,11 @@ nouveau_framebuffer_init(struct drm_device *dev,
 		}
 	}
 
+	ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
+	if (ret) {
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 310c0e5..069d5cc 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1084,12 +1084,12 @@ radeon_framebuffer_init(struct drm_device *dev,
 {
 	int ret;
 	rfb->obj = obj;
+	drm_helper_mode_fill_fb_struct(&rfb->base, mode_cmd);
 	ret = drm_framebuffer_init(dev, &rfb->base, &radeon_fb_funcs);
 	if (ret) {
 		rfb->obj = NULL;
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&rfb->base, mode_cmd);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d4ab3be..829c4a7 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -435,8 +435,8 @@ udl_framebuffer_init(struct drm_device *dev,
 	int ret;
 
 	ufb->obj = obj;
-	ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
 	drm_helper_mode_fill_fb_struct(&ufb->base, mode_cmd);
+	ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
 	return ret;
 }
 
diff --git a/drivers/staging/omapdrm/omap_fb.c b/drivers/staging/omapdrm/omap_fb.c
index 446801d..5090547 100644
--- a/drivers/staging/omapdrm/omap_fb.c
+++ b/drivers/staging/omapdrm/omap_fb.c
@@ -425,14 +425,6 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 	}
 
 	fb = &omap_fb->base;
-	ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
-	if (ret) {
-		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
-		goto fail;
-	}
-
-	DBG("create: FB ID: %d (%p)", fb->base.id, fb);
-
 	omap_fb->format = format;
 
 	for (i = 0; i < n; i++) {
@@ -463,6 +455,14 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 
 	drm_helper_mode_fill_fb_struct(fb, mode_cmd);
 
+	ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
+	if (ret) {
+		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
+		goto fail;
+	}
+
+	DBG("create: FB ID: %d (%p)", fb->base.id, fb);
+
 	return fb;
 
 fail:
-- 
1.7.10.4

[PATCH 04/10] drm/vmwgfx: reorder framebuffer init sequence

[Daniel Vetter]

vmwgfx has an oddity, when failing to reference the surface it'll
return 0, since that's what the successfull drm_framebuffer_init will
leave behind in ret. Fix this up by returning -EINVAL.

Split out from all the other driver updates due to the above tiny
semantic change. Shouldn't matter though since the reference grabbing
seemingly can't fail.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |   30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 5474394..edc9792 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -681,14 +681,10 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
 		goto out_err1;
 	}
 
-	ret = drm_framebuffer_init(dev, &vfbs->base.base,
-				   &vmw_framebuffer_surface_funcs);
-	if (ret)
-		goto out_err2;
-
 	if (!vmw_surface_reference(surface)) {
 		DRM_ERROR("failed to reference surface %p\n", surface);
-		goto out_err3;
+		ret = -EINVAL;
+		goto out_err2;
 	}
 
 	/* XXX get the first 3 from the surface info */
@@ -707,10 +703,15 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
 
 	*out = &vfbs->base;
 
+	ret = drm_framebuffer_init(dev, &vfbs->base.base,
+				   &vmw_framebuffer_surface_funcs);
+	if (ret)
+		goto out_err3;
+
 	return 0;
 
 out_err3:
-	drm_framebuffer_cleanup(&vfbs->base.base);
+	vmw_surface_unreference(&surface);
 out_err2:
 	kfree(vfbs);
 out_err1:
@@ -1053,14 +1054,10 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv,
 		goto out_err1;
 	}
 
-	ret = drm_framebuffer_init(dev, &vfbd->base.base,
-				   &vmw_framebuffer_dmabuf_funcs);
-	if (ret)
-		goto out_err2;
-
 	if (!vmw_dmabuf_reference(dmabuf)) {
 		DRM_ERROR("failed to reference dmabuf %p\n", dmabuf);
-		goto out_err3;
+		ret = -EINVAL;
+		goto out_err2;
 	}
 
 	vfbd->base.base.bits_per_pixel = mode_cmd->bpp;
@@ -1077,10 +1074,15 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv,
 	vfbd->base.user_handle = mode_cmd->handle;
 	*out = &vfbd->base;
 
+	ret = drm_framebuffer_init(dev, &vfbd->base.base,
+				   &vmw_framebuffer_dmabuf_funcs);
+	if (ret)
+		goto out_err3;
+
 	return 0;
 
 out_err3:
-	drm_framebuffer_cleanup(&vfbd->base.base);
+	vmw_dmabuf_unreference(&dmabuf);
 out_err2:
 	kfree(vfbd);
 out_err1:
-- 
1.7.10.4

[PATCH 05/10] drm/gma500: move fbcon restore to lastclose

[Daniel Vetter]

Doing this within the fb->destroy callback leads to a locking
nightmare. And all other drm drivers that restore the fbcon do
it in lastclose, too.

With this adjustments all fb->destroy callbacks optionally drop
references to any gem objects used as backing storage, call
drm_framebuffer_cleanup and then kfree the struct. Which nicely
simplifies the locking for framebuffer unreferencing and freeing,
since this doesn't require that we hold the mode_config lock. A
slight exception is the vmwgfx surface backed framebuffer, it also
calls drm_master_put and removes the object from a device-private
framebuffer list. Both seem to have solid locking in place already.

Conclusion is that now it is no longer required to hold the
mode_config lock while freeing a framebuffer.

v2: Drop the corresponding mutex_lock WARN check from
drm_framebuffer_unreference.

v3: Use just the mode_config lock not modeset_lock_all, due to patch
reordering.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c           |    2 --
 drivers/gpu/drm/gma500/framebuffer.c |   24 ------------------------
 drivers/gpu/drm/gma500/psb_drv.c     |   10 ++++++++++
 3 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8154554..8d665fa 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -317,9 +317,7 @@ static void drm_framebuffer_free(struct kref *kref)
  */
 void drm_framebuffer_unreference(struct drm_framebuffer *fb)
 {
-	struct drm_device *dev = fb->dev;
 	DRM_DEBUG("FB ID: %d\n", fb->base.id);
-	WARN_ON(!drm_modeset_is_locked(dev));
 	kref_put(&fb->refcount, drm_framebuffer_free);
 }
 EXPORT_SYMBOL(drm_framebuffer_unreference);
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 38e7e75..49800d2 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -668,30 +668,6 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
 {
 	struct psb_framebuffer *psbfb = to_psb_fb(fb);
 	struct gtt_range *r = psbfb->gtt;
-	struct drm_device *dev = fb->dev;
-	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct psb_fbdev *fbdev = dev_priv->fbdev;
-	struct drm_crtc *crtc;
-	int reset = 0;
-
-	/* Should never get stolen memory for a user fb */
-	WARN_ON(r->stolen);
-
-	/* Check if we are erroneously live */
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		if (crtc->fb == fb)
-			reset = 1;
-
-	if (reset)
-		/*
-		 * Now force a sane response before we permit the DRM CRTC
-		 * layer to do stupid things like blank the display. Instead
-		 * we reset this framebuffer as if the user had forced a reset.
-		 * We must do this before the cleanup so that the DRM layer
-		 * doesn't get a chance to stick its oar in where it isn't
-		 * wanted.
-		 */
-		drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper);
 
 	/* Let DRM do its clean up */
 	drm_framebuffer_cleanup(fb);
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index dd1fbfa..dbcefe9 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -149,6 +149,16 @@ static struct drm_ioctl_desc psb_ioctls[] = {
 
 static void psb_lastclose(struct drm_device *dev)
 {
+	int ret;
+	struct drm_psb_private *dev_priv = dev->dev_private;
+	struct psb_fbdev *fbdev = dev_priv->fbdev;
+
+	mutex_lock(&dev->mode_config.mutex);
+	ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper);
+	if (ret)
+		DRM_DEBUG("failed to restore crtc mode\n");
+	mutex_unlock(&dev->mode_config.mutex);
+
 	return;
 }
 
-- 
1.7.10.4

[PATCH 06/10] drm/nouveau: protect evo_wait/evo_kick sections with a channel mutex

[Daniel Vetter]

With per-crtc locks modeset operations can run in parallel, and the
cursor code uses the device-global evo master channel for hw frobbing.
But the pageflip code can also sync with the master under some
circumstances. Hence just wrap things up in a mutex to ensure that
pushbuf access doesn't intermingle.

The approach here is a bit overkill since the per-crtc channels used
to schedule the pageflips could probably be used without this pushbuf
locking, but I'm not familiar enough with the nouveau codebase to be
sure of that.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/nouveau/nv50_display.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 3587408..5751d63 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -128,6 +128,11 @@ struct nv50_dmac {
 	struct nv50_chan base;
 	dma_addr_t handle;
 	u32 *ptr;
+
+	/* Protects against concurrent pushbuf access to this channel, lock is
+	 * grabbed by evo_wait (if the pushbuf reservation is successful) and
+	 * dropped again by evo_kick. */
+	struct mutex lock;
 };
 
 static void
@@ -395,11 +400,13 @@ evo_wait(void *evoc, int nr)
 	struct nv50_dmac *dmac = evoc;
 	u32 put = nv_ro32(dmac->base.user, 0x0000) / 4;
 
+	mutex_lock(&dmac->lock);
 	if (put + nr >= (PAGE_SIZE / 4) - 8) {
 		dmac->ptr[put] = 0x20000000;
 
 		nv_wo32(dmac->base.user, 0x0000, 0x00000000);
 		if (!nv_wait(dmac->base.user, 0x0004, ~0, 0x00000000)) {
+			mutex_unlock(&dmac->lock);
 			NV_ERROR(dmac->base.user, "channel stalled\n");
 			return NULL;
 		}
@@ -415,6 +422,7 @@ evo_kick(u32 *push, void *evoc)
 {
 	struct nv50_dmac *dmac = evoc;
 	nv_wo32(dmac->base.user, 0x0000, (push - dmac->ptr) << 2);
+	mutex_unlock(&dmac->lock);
 }
 
 #define evo_mthd(p,m,s) *((p)++) = (((s) << 18) | (m))
-- 
1.7.10.4

[PATCH 07/10] drm/nouveau: try to protect nbo->pin_refcount

[Daniel Vetter]

... by moving the bo_pin/bo_unpin manipulation of the pin_refcount
under the protection of the ttm reservation lock. pin/unpin seems
to get called from all over the place, so atm this is completely racy.

After this patch there are only a few places in cleanup functions
left which access ->pin_refcount without locking. But I'm hoping that
those are safe and some other code invariant guarantees that this
won't blow up.

In any case, I only need to fix up pin/unpin to make ->pageflip work
safely, so let's keep it at that.

Add a comment to the header to explain the new locking rule.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c |   22 +++++++++++-----------
 drivers/gpu/drm/nouveau/nouveau_bo.h |    2 ++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 5614c89..1eb5c77 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -300,17 +300,18 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t memtype)
 	struct ttm_buffer_object *bo = &nvbo->bo;
 	int ret;
 
+	ret = ttm_bo_reserve(bo, false, false, false, 0);
+	if (ret)
+		goto out;
+
 	if (nvbo->pin_refcnt && !(memtype & (1 << bo->mem.mem_type))) {
 		NV_ERROR(drm, "bo %p pinned elsewhere: 0x%08x vs 0x%08x\n", bo,
 			 1 << bo->mem.mem_type, memtype);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	if (nvbo->pin_refcnt++)
-		return 0;
-
-	ret = ttm_bo_reserve(bo, false, false, false, 0);
-	if (ret)
 		goto out;
 
 	nouveau_bo_placement_set(nvbo, memtype, 0);
@@ -328,10 +329,8 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t memtype)
 			break;
 		}
 	}
-	ttm_bo_unreserve(bo);
 out:
-	if (unlikely(ret))
-		nvbo->pin_refcnt--;
+	ttm_bo_unreserve(bo);
 	return ret;
 }
 
@@ -342,13 +341,13 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
 	struct ttm_buffer_object *bo = &nvbo->bo;
 	int ret;
 
-	if (--nvbo->pin_refcnt)
-		return 0;
-
 	ret = ttm_bo_reserve(bo, false, false, false, 0);
 	if (ret)
 		return ret;
 
+	if (--nvbo->pin_refcnt)
+		goto out;
+
 	nouveau_bo_placement_set(nvbo, bo->mem.placement, 0);
 
 	ret = nouveau_bo_validate(nvbo, false, false);
@@ -365,6 +364,7 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
 		}
 	}
 
+out:
 	ttm_bo_unreserve(bo);
 	return ret;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h
index 25ca379..81d00fe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -28,6 +28,8 @@ struct nouveau_bo {
 	struct nouveau_drm_tile *tile;
 
 	struct drm_gem_object *gem;
+
+	/* protect by the ttm reservation lock */
 	int pin_refcnt;
 
 	struct ttm_bo_kmap_obj dma_buf_vmap;
-- 
1.7.10.4

[PATCH 08/10] drm/ttm: fix fence locking in ttm_buffer_object_transfer

[Daniel Vetter]

Noticed while reviewing the fence locking in the radeon pageflip
handler.

v2: Instead of grabbing the bdev->fence_lock in object_transfer just
move the single callsite of that function a few lines, so that it is
protected by the fence_lock. Suggested by Jerome Glisse.

v3: Fix typo in commit message.

Reviewed-by: Jerome Glisse <jglisse@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 9e9c5d2..d73d6e3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -654,11 +654,13 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		 */
 
 		set_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags);
+
+		/* ttm_buffer_object_transfer accesses bo->sync_obj */
+		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
 		spin_unlock(&bdev->fence_lock);
 		if (tmp_obj)
 			driver->sync_obj_unref(&tmp_obj);
 
-		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
 		if (ret)
 			return ret;
 
-- 
1.7.10.4

[PATCH 09/10] drm/<drivers>: Unified handling of unimplemented fb->create_handle

[Daniel Vetter]

Some drivers don't have real ->create_handle callbacks.

- cirrus/ast/mga200: Returns either 0 or -EINVAL.

- udl: Didn't even bother with a callback, leading to a nice
  userspace-triggerable OOPS.

- vmwgfx: This driver bothered with an implementation to return 0 as
  the handle (which is the canonical no-obj gem handle).

All have in common that ->create_handle doesn't really make too much
sense for them - that ioctl is used only for seamless fb takeover in
the radeon/nouveau/i915 ddx drivers. So allow drivers to not implement
this and return a consistent -ENODEV.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ast/ast_main.c         |    8 --------
 drivers/gpu/drm/cirrus/cirrus_main.c   |    8 --------
 drivers/gpu/drm/drm_crtc.c             |    5 ++++-
 drivers/gpu/drm/mgag200/mgag200_main.c |    8 --------
 drivers/gpu/drm/udl/udl_fb.c           |    1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |   12 ------------
 6 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d5ba709..f60fd7b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -246,16 +246,8 @@ static void ast_user_framebuffer_destroy(struct drm_framebuffer *fb)
 	kfree(fb);
 }
 
-static int ast_user_framebuffer_create_handle(struct drm_framebuffer *fb,
-					      struct drm_file *file,
-					      unsigned int *handle)
-{
-	return -EINVAL;
-}
-
 static const struct drm_framebuffer_funcs ast_fb_funcs = {
 	.destroy = ast_user_framebuffer_destroy,
-	.create_handle = ast_user_framebuffer_create_handle,
 };
 
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 364474c..35cbae8 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -23,16 +23,8 @@ static void cirrus_user_framebuffer_destroy(struct drm_framebuffer *fb)
 	kfree(fb);
 }
 
-static int cirrus_user_framebuffer_create_handle(struct drm_framebuffer *fb,
-						 struct drm_file *file_priv,
-						 unsigned int *handle)
-{
-	return 0;
-}
-
 static const struct drm_framebuffer_funcs cirrus_fb_funcs = {
 	.destroy = cirrus_user_framebuffer_destroy,
-	.create_handle = cirrus_user_framebuffer_create_handle,
 };
 
 int cirrus_framebuffer_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8d665fa..a9abf49 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2384,7 +2384,10 @@ int drm_mode_getfb(struct drm_device *dev,
 	r->depth = fb->depth;
 	r->bpp = fb->bits_per_pixel;
 	r->pitch = fb->pitches[0];
-	fb->funcs->create_handle(fb, file_priv, &r->handle);
+	if (fb->funcs->create_handle)
+		ret = fb->funcs->create_handle(fb, file_priv, &r->handle);
+	else
+		ret = -ENODEV;
 
 out:
 	mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 266438a..64297c7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -23,16 +23,8 @@ static void mga_user_framebuffer_destroy(struct drm_framebuffer *fb)
 	kfree(fb);
 }
 
-static int mga_user_framebuffer_create_handle(struct drm_framebuffer *fb,
-						 struct drm_file *file_priv,
-						 unsigned int *handle)
-{
-	return 0;
-}
-
 static const struct drm_framebuffer_funcs mga_fb_funcs = {
 	.destroy = mga_user_framebuffer_destroy,
-	.create_handle = mga_user_framebuffer_create_handle,
 };
 
 int mgag200_framebuffer_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 829c4a7..f8904b4 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -422,7 +422,6 @@ static void udl_user_framebuffer_destroy(struct drm_framebuffer *fb)
 static const struct drm_framebuffer_funcs udlfb_funcs = {
 	.destroy = udl_user_framebuffer_destroy,
 	.dirty = udl_user_framebuffer_dirty,
-	.create_handle = NULL,
 };
 
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index edc9792..9c0876b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -373,16 +373,6 @@ void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
  * Generic framebuffer code
  */
 
-int vmw_framebuffer_create_handle(struct drm_framebuffer *fb,
-				  struct drm_file *file_priv,
-				  unsigned int *handle)
-{
-	if (handle)
-		*handle = 0;
-
-	return 0;
-}
-
 /*
  * Surface framebuffer code
  */
@@ -610,7 +600,6 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer,
 static struct drm_framebuffer_funcs vmw_framebuffer_surface_funcs = {
 	.destroy = vmw_framebuffer_surface_destroy,
 	.dirty = vmw_framebuffer_surface_dirty,
-	.create_handle = vmw_framebuffer_create_handle,
 };
 
 static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
@@ -961,7 +950,6 @@ int vmw_framebuffer_dmabuf_dirty(struct drm_framebuffer *framebuffer,
 static struct drm_framebuffer_funcs vmw_framebuffer_dmabuf_funcs = {
 	.destroy = vmw_framebuffer_dmabuf_destroy,
 	.dirty = vmw_framebuffer_dmabuf_dirty,
-	.create_handle = vmw_framebuffer_create_handle,
 };
 
 /**
-- 
1.7.10.4

[PATCH 10/10] drm: encapsulate crtc->set_config calls

[Daniel Vetter]

With refcounting we need to adjust framebuffer refcounts at each
callsite - much easier to do if they all call the same little helper
function.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c             |   19 +++++++++++++++++--
 drivers/gpu/drm/drm_fb_helper.c        |    6 +++---
 drivers/gpu/drm/i2c/ch7006_drv.c       |    2 +-
 drivers/gpu/drm/nouveau/nv04_display.c |    2 +-
 drivers/gpu/drm/nouveau/nv17_tv.c      |    2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |    2 +-
 include/drm/drm_crtc.h                 |    1 +
 7 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a9abf49..7ca2f28 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -381,7 +381,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 			memset(&set, 0, sizeof(struct drm_mode_set));
 			set.crtc = crtc;
 			set.fb = NULL;
-			ret = crtc->funcs->set_config(&set);
+			ret = drm_mode_set_config_internal(&set);
 			if (ret)
 				DRM_ERROR("failed to reset crtc %p when fb was deleted\n", crtc);
 		}
@@ -1801,6 +1801,21 @@ out:
 }
 
 /**
+ * drm_mode_set_config_internal - helper to call ->set_config
+ * @set: modeset config to set
+ *
+ * This is a little helper to wrap internal calls to the ->set_config driver
+ * interface. The only thing it adds is correct refcounting dance.
+ */
+int drm_mode_set_config_internal(struct drm_mode_set *set)
+{
+	struct drm_crtc *crtc = set->crtc;
+
+	return crtc->funcs->set_config(set);
+}
+EXPORT_SYMBOL(drm_mode_set_config_internal);
+
+/**
  * drm_mode_setcrtc - set CRTC configuration
  * @dev: drm device for the ioctl
  * @data: data pointer for the ioctl
@@ -1963,7 +1978,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 	set.connectors = connector_set;
 	set.num_connectors = crtc_req->count_connectors;
 	set.fb = fb;
-	ret = crtc->funcs->set_config(&set);
+	ret = drm_mode_set_config_internal(&set);
 
 out:
 	kfree(connector_set);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 954d175..82c3a9f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -245,7 +245,7 @@ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 	int i, ret;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
-		ret = mode_set->crtc->funcs->set_config(mode_set);
+		ret = drm_mode_set_config_internal(mode_set);
 		if (ret)
 			error = true;
 	}
@@ -675,7 +675,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
 	mutex_lock(&dev->mode_config.mutex);
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		crtc = fb_helper->crtc_info[i].mode_set.crtc;
-		ret = crtc->funcs->set_config(&fb_helper->crtc_info[i].mode_set);
+		ret = drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set);
 		if (ret) {
 			mutex_unlock(&dev->mode_config.mutex);
 			return ret;
@@ -711,7 +711,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 		modeset->y = var->yoffset;
 
 		if (modeset->num_connectors) {
-			ret = crtc->funcs->set_config(modeset);
+			ret = drm_mode_set_config_internal(modeset);
 			if (!ret) {
 				info->var.xoffset = var->xoffset;
 				info->var.yoffset = var->yoffset;
diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index b865d07..51fa323 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -364,7 +364,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
 				.crtc = crtc,
 			};
 
-			crtc->funcs->set_config(&modeset);
+			drm_mode_set_config_internal(&modeset);
 		}
 	}
 
diff --git a/drivers/gpu/drm/nouveau/nv04_display.c b/drivers/gpu/drm/nouveau/nv04_display.c
index 2cd6fb8..4c6e9f8 100644
--- a/drivers/gpu/drm/nouveau/nv04_display.c
+++ b/drivers/gpu/drm/nouveau/nv04_display.c
@@ -140,7 +140,7 @@ nv04_display_destroy(struct drm_device *dev)
 			.crtc = crtc,
 		};
 
-		crtc->funcs->set_config(&modeset);
+		drm_mode_set_config_internal(&modeset);
 	}
 
 	/* Restore state */
diff --git a/drivers/gpu/drm/nouveau/nv17_tv.c b/drivers/gpu/drm/nouveau/nv17_tv.c
index 2ca276a..977e42b 100644
--- a/drivers/gpu/drm/nouveau/nv17_tv.c
+++ b/drivers/gpu/drm/nouveau/nv17_tv.c
@@ -768,7 +768,7 @@ static int nv17_tv_set_property(struct drm_encoder *encoder,
 				.crtc = crtc,
 			};
 
-			crtc->funcs->set_config(&modeset);
+			drm_mode_set_config_internal(&modeset);
 		}
 	}
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 161f8b2..07dfd82 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -829,7 +829,7 @@ static void vmw_lastclose(struct drm_device *dev)
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		set.crtc = crtc;
-		ret = crtc->funcs->set_config(&set);
+		ret = drm_mode_set_config_internal(&set);
 		WARN_ON(ret != 0);
 	}
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 00d78b5..665289a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -985,6 +985,7 @@ extern int drm_mode_getcrtc(struct drm_device *dev,
 			    void *data, struct drm_file *file_priv);
 extern int drm_mode_getconnector(struct drm_device *dev,
 			      void *data, struct drm_file *file_priv);
+extern int drm_mode_set_config_internal(struct drm_mode_set *set);
 extern int drm_mode_setcrtc(struct drm_device *dev,
 			    void *data, struct drm_file *file_priv);
 extern int drm_mode_getplane(struct drm_device *dev,
-- 
1.7.10.4

Re: [PATCH 02/10] drm/doc: integrate drm_crtc.c kerneldoc

[Ville Syrjälä]

On Tue, Dec 18, 2012 at 10:25:05PM +0100, Daniel Vetter wrote:
> And do a quick pass to adjust them to the last few (years?) of changes
> ...
> 
> This time actually compile-tested ;-)
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/DocBook/drm.tmpl |    4 ++
>  drivers/gpu/drm/drm_crtc.c     |   92 +++++++++++++++++++---------------------
>  2 files changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 4ee2304..caab791 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
<snip>
> @@ -321,6 +326,7 @@ EXPORT_SYMBOL(drm_framebuffer_unreference);
>  
>  /**
>   * drm_framebuffer_reference - incr the fb refcnt

Maybe improve the comment here while you're at it. The function name is
more verbose/readable than the comment :)

> + * @fb: framebuffer
>   */
>  void drm_framebuffer_reference(struct drm_framebuffer *fb)
>  {

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 02/10] drm/doc: integrate drm_crtc.c kerneldoc

[Daniel Vetter]

On Wed, Dec 19, 2012 at 7:19 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Dec 18, 2012 at 10:25:05PM +0100, Daniel Vetter wrote:
>> And do a quick pass to adjust them to the last few (years?) of changes
>> ...
>>
>> This time actually compile-tested ;-)
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  Documentation/DocBook/drm.tmpl |    4 ++
>>  drivers/gpu/drm/drm_crtc.c     |   92 +++++++++++++++++++---------------------
>>  2 files changed, 48 insertions(+), 48 deletions(-)
>>
>> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
>> index 4ee2304..caab791 100644
>> --- a/Documentation/DocBook/drm.tmpl
>> +++ b/Documentation/DocBook/drm.tmpl
> <snip>
>> @@ -321,6 +326,7 @@ EXPORT_SYMBOL(drm_framebuffer_unreference);
>>
>>  /**
>>   * drm_framebuffer_reference - incr the fb refcnt
>
> Maybe improve the comment here while you're at it. The function name is
> more verbose/readable than the comment :)

Imo better to make a pass over all the kerneldoc after the locking
rework has landed - the patches change it quite a lot ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/nouveau: protect evo_wait/evo_kick sections with a channel mutex

[Daniel Vetter]

With per-crtc locks modeset operations can run in parallel, and the
cursor code uses the device-global evo master channel for hw frobbing.
But the pageflip code can also sync with the master under some
circumstances. Hence just wrap things up in a mutex to ensure that
pushbuf access doesn't intermingle.

The approach here is a bit overkill since the per-crtc channels used
to schedule the pageflips could probably be used without this pushbuf
locking, but I'm not familiar enough with the nouveau codebase to be
sure of that.

v2: Add missing mutex_init to avoid angering lockdep.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/nouveau/nv50_display.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 3587408..d4cbea1 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -128,6 +128,11 @@ struct nv50_dmac {
 	struct nv50_chan base;
 	dma_addr_t handle;
 	u32 *ptr;
+
+	/* Protects against concurrent pushbuf access to this channel, lock is
+	 * grabbed by evo_wait (if the pushbuf reservation is successful) and
+	 * dropped again by evo_kick. */
+	struct mutex lock;
 };
 
 static void
@@ -271,6 +276,8 @@ nv50_dmac_create(struct nouveau_object *core, u32 bclass, u8 head,
 	u32 pushbuf = *(u32 *)data;
 	int ret;
 
+	mutex_init(&dmac->lock);
+
 	dmac->ptr = pci_alloc_consistent(nv_device(core)->pdev, PAGE_SIZE,
 					&dmac->handle);
 	if (!dmac->ptr)
@@ -395,11 +402,13 @@ evo_wait(void *evoc, int nr)
 	struct nv50_dmac *dmac = evoc;
 	u32 put = nv_ro32(dmac->base.user, 0x0000) / 4;
 
+	mutex_lock(&dmac->lock);
 	if (put + nr >= (PAGE_SIZE / 4) - 8) {
 		dmac->ptr[put] = 0x20000000;
 
 		nv_wo32(dmac->base.user, 0x0000, 0x00000000);
 		if (!nv_wait(dmac->base.user, 0x0004, ~0, 0x00000000)) {
+			mutex_unlock(&dmac->lock);
 			NV_ERROR(dmac->base.user, "channel stalled\n");
 			return NULL;
 		}
@@ -415,6 +424,7 @@ evo_kick(u32 *push, void *evoc)
 {
 	struct nv50_dmac *dmac = evoc;
 	nv_wo32(dmac->base.user, 0x0000, (push - dmac->ptr) << 2);
+	mutex_unlock(&dmac->lock);
 }
 
 #define evo_mthd(p,m,s) *((p)++) = (((s) << 18) | (m))
-- 
1.7.10.4