Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a75a222 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm: Hide hw.lock cleanup in filp->release better 



A few things:
- Rename the cleanup function from drm_master_release to
  drm_legacy_lock_release. It doesn't relase any master stuff, but
  just the legacy hw lock.
- Hide it in drm_lock.c, which allows us to make a few more functions
  static in there. To avoid forward decl we need to shuffle the code a
  bit though.
- Push the check for ->master into the function itself.
- Only call this for !DRIVER_MODESET.

End result: Another place that takes struct_mutex gone for good for
modern drivers.

v2: Remove leftover comment.

Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarEmil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465930269-7883-3-git-send-email-daniel.vetter@ffwll.ch
parent e1b1cf2f

drivers/gpu/drm/drm_fops.c

+2 −15
Original line number Diff line number Diff line
@@ -338,18 +338,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) 
	return ret;

}



static void drm_master_release(struct drm_device *dev, struct file *filp)

{

	struct drm_file *file_priv = filp->private_data;



	if (drm_legacy_i_have_hw_lock(dev, file_priv)) {

		DRM_DEBUG("File %p released, freeing lock for context %d\n",

			  filp, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));

		drm_legacy_lock_free(&file_priv->master->lock,

				     _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));

	}

}



static void drm_events_release(struct drm_file *file_priv)

{

	struct drm_device *dev = file_priv->minor->dev;
@@ -468,9 +456,8 @@ int drm_release(struct inode *inode, struct file *filp) 
		  (long)old_encode_dev(file_priv->minor->kdev->devt),

		  dev->open_count);



	/* if the master has gone away we can't do anything with the lock */

	if (file_priv->minor->master)

		drm_master_release(dev, filp);

	if (!drm_core_check_feature(dev, DRIVER_MODESET))

		drm_legacy_lock_release(dev, filp);



	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))

		drm_legacy_reclaim_buffers(dev, file_priv);

drivers/gpu/drm/drm_legacy.h

+2 −6
Original line number Diff line number Diff line
@@ -88,14 +88,10 @@ struct drm_agp_mem { 
	struct list_head head;

};



/*

 * Generic Userspace Locking-API

 */



int drm_legacy_i_have_hw_lock(struct drm_device *d, struct drm_file *f);

/* drm_lock.c */

int drm_legacy_lock(struct drm_device *d, void *v, struct drm_file *f);

int drm_legacy_unlock(struct drm_device *d, void *v, struct drm_file *f);

int drm_legacy_lock_free(struct drm_lock_data *lock, unsigned int ctx);

void drm_legacy_lock_release(struct drm_device *dev, struct file *filp);



/* DMA support */

int drm_legacy_dma_setup(struct drm_device *dev);

drivers/gpu/drm/drm_lock.c

+122 −116
Original line number Diff line number Diff line
@@ -40,6 +40,110 @@ 


static int drm_lock_take(struct drm_lock_data *lock_data, unsigned int context);



/**

 * Take the heavyweight lock.

 *

 * \param lock lock pointer.

 * \param context locking context.

 * \return one if the lock is held, or zero otherwise.

 *

 * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction.

 */

static

int drm_lock_take(struct drm_lock_data *lock_data,

		  unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	spin_lock_bh(&lock_data->spinlock);

	do {

		old = *lock;

		if (old & _DRM_LOCK_HELD)

			new = old | _DRM_LOCK_CONT;

		else {

			new = context | _DRM_LOCK_HELD |

				((lock_data->user_waiters + lock_data->kernel_waiters > 1) ?

				 _DRM_LOCK_CONT : 0);

		}

		prev = cmpxchg(lock, old, new);

	} while (prev != old);

	spin_unlock_bh(&lock_data->spinlock);



	if (_DRM_LOCKING_CONTEXT(old) == context) {

		if (old & _DRM_LOCK_HELD) {

			if (context != DRM_KERNEL_CONTEXT) {

				DRM_ERROR("%d holds heavyweight lock\n",

					  context);

			}

			return 0;

		}

	}



	if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) {

		/* Have lock */

		return 1;

	}

	return 0;

}



/**

 * This takes a lock forcibly and hands it to context.	Should ONLY be used

 * inside *_unlock to give lock to kernel before calling *_dma_schedule.

 *

 * \param dev DRM device.

 * \param lock lock pointer.

 * \param context locking context.

 * \return always one.

 *

 * Resets the lock file pointer.

 * Marks the lock as held by the given context, via the \p cmpxchg instruction.

 */

static int drm_lock_transfer(struct drm_lock_data *lock_data,

			     unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	lock_data->file_priv = NULL;

	do {

		old = *lock;

		new = context | _DRM_LOCK_HELD;

		prev = cmpxchg(lock, old, new);

	} while (prev != old);

	return 1;

}



static int drm_legacy_lock_free(struct drm_lock_data *lock_data,

				unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	spin_lock_bh(&lock_data->spinlock);

	if (lock_data->kernel_waiters != 0) {

		drm_lock_transfer(lock_data, 0);

		lock_data->idle_has_lock = 1;

		spin_unlock_bh(&lock_data->spinlock);

		return 1;

	}

	spin_unlock_bh(&lock_data->spinlock);



	do {

		old = *lock;

		new = _DRM_LOCKING_CONTEXT(old);

		prev = cmpxchg(lock, old, new);

	} while (prev != old);



	if (_DRM_LOCK_IS_HELD(old) && _DRM_LOCKING_CONTEXT(old) != context) {

		DRM_ERROR("%d freed heavyweight lock held by %d\n",

			  context, _DRM_LOCKING_CONTEXT(old));

		return 1;

	}

	wake_up_interruptible(&lock_data->lock_queue);

	return 0;

}



/**

 * Lock ioctl.

 *
@@ -164,120 +268,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_ 
	return 0;

}



/**

 * Take the heavyweight lock.

 *

 * \param lock lock pointer.

 * \param context locking context.

 * \return one if the lock is held, or zero otherwise.

 *

 * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction.

 */

static

int drm_lock_take(struct drm_lock_data *lock_data,

		  unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	spin_lock_bh(&lock_data->spinlock);

	do {

		old = *lock;

		if (old & _DRM_LOCK_HELD)

			new = old | _DRM_LOCK_CONT;

		else {

			new = context | _DRM_LOCK_HELD |

				((lock_data->user_waiters + lock_data->kernel_waiters > 1) ?

				 _DRM_LOCK_CONT : 0);

		}

		prev = cmpxchg(lock, old, new);

	} while (prev != old);

	spin_unlock_bh(&lock_data->spinlock);



	if (_DRM_LOCKING_CONTEXT(old) == context) {

		if (old & _DRM_LOCK_HELD) {

			if (context != DRM_KERNEL_CONTEXT) {

				DRM_ERROR("%d holds heavyweight lock\n",

					  context);

			}

			return 0;

		}

	}



	if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) {

		/* Have lock */

		return 1;

	}

	return 0;

}



/**

 * This takes a lock forcibly and hands it to context.	Should ONLY be used

 * inside *_unlock to give lock to kernel before calling *_dma_schedule.

 *

 * \param dev DRM device.

 * \param lock lock pointer.

 * \param context locking context.

 * \return always one.

 *

 * Resets the lock file pointer.

 * Marks the lock as held by the given context, via the \p cmpxchg instruction.

 */

static int drm_lock_transfer(struct drm_lock_data *lock_data,

			     unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	lock_data->file_priv = NULL;

	do {

		old = *lock;

		new = context | _DRM_LOCK_HELD;

		prev = cmpxchg(lock, old, new);

	} while (prev != old);

	return 1;

}



/**

 * Free lock.

 *

 * \param dev DRM device.

 * \param lock lock.

 * \param context context.

 *

 * Resets the lock file pointer.

 * Marks the lock as not held, via the \p cmpxchg instruction. Wakes any task

 * waiting on the lock queue.

 */

int drm_legacy_lock_free(struct drm_lock_data *lock_data, unsigned int context)

{

	unsigned int old, new, prev;

	volatile unsigned int *lock = &lock_data->hw_lock->lock;



	spin_lock_bh(&lock_data->spinlock);

	if (lock_data->kernel_waiters != 0) {

		drm_lock_transfer(lock_data, 0);

		lock_data->idle_has_lock = 1;

		spin_unlock_bh(&lock_data->spinlock);

		return 1;

	}

	spin_unlock_bh(&lock_data->spinlock);



	do {

		old = *lock;

		new = _DRM_LOCKING_CONTEXT(old);

		prev = cmpxchg(lock, old, new);

	} while (prev != old);



	if (_DRM_LOCK_IS_HELD(old) && _DRM_LOCKING_CONTEXT(old) != context) {

		DRM_ERROR("%d freed heavyweight lock held by %d\n",

			  context, _DRM_LOCKING_CONTEXT(old));

		return 1;

	}

	wake_up_interruptible(&lock_data->lock_queue);

	return 0;

}



/**

 * This function returns immediately and takes the hw lock

 * with the kernel context if it is free, otherwise it gets the highest priority when and if
@@ -330,7 +320,7 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock_data) 
}

EXPORT_SYMBOL(drm_legacy_idlelock_release);



int drm_legacy_i_have_hw_lock(struct drm_device *dev,

static int drm_legacy_i_have_hw_lock(struct drm_device *dev,

				     struct drm_file *file_priv)

{

	struct drm_master *master = file_priv->master;
@@ -338,3 +328,19 @@ int drm_legacy_i_have_hw_lock(struct drm_device *dev, 
		_DRM_LOCK_IS_HELD(master->lock.hw_lock->lock) &&

		master->lock.file_priv == file_priv);

}



void drm_legacy_lock_release(struct drm_device *dev, struct file *filp)

{

	struct drm_file *file_priv = filp->private_data;



	/* if the master has gone away we can't do anything with the lock */

	if (!file_priv->minor->master)

		return;



	if (drm_legacy_i_have_hw_lock(dev, file_priv)) {

		DRM_DEBUG("File %p released, freeing lock for context %d\n",

			  filp, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));

		drm_legacy_lock_free(&file_priv->master->lock,

				     _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));

	}

}