[13/20] drm: inline "struct drm_sigdata"

Commit Message

David Herrmann Aug. 29, 2014, 10:12 a.m. UTC

The sigdata structure is only used to group two fields in drm_device.
Inline it and make it an unnamed object.

Signed-off-by: David Herrmann <[e-mail]>
---
 drivers/gpu/drm/drm_lock.c | 15 ++++++++-------
 include/drm/drmP.h         | 12 +++++-------
 2 files changed, 13 insertions(+), 14 deletions(-)

Comments

Thierry Reding Aug. 29, 2014, 12:21 p.m. UTC | #1

On Fri, Aug 29, 2014 at 12:12:39PM +0200, David Herrmann wrote:
> The sigdata structure is only used to group two fields in drm_device.
> Inline it and make it an unnamed object.

I'm not sure what unnamed object means.

> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> @@ -1035,9 +1029,13 @@ struct drm_device {
>  
>  	struct drm_sg_mem *sg;	/**< Scatter gather memory */
>  	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
> -	struct drm_sigdata sigdata;	   /**< For block_all_signals */
>  	sigset_t sigmask;
>  
> +	struct {
> +		int context;

Your patch doesn't introduce this, but shouldn't context here be
unsigned given that it's compared to drm_hw_lock's .lock field?

I guess struct drm_lock could have that same change. I suppose it might
not be all that important since presumably the code works, but it's
still inconsistent use of types.

Anyway, if you could clarify the "unnamed object" in the commit
description, this patch:

Reviewed-by: Thierry Reding <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index e26b59e..60f1481 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -120,7 +120,7 @@  int drm_lock(struct drm_device *dev, void *data, struct drm_file *file_priv)
 		sigaddset(&dev->sigmask, SIGTTOU);
 		dev->sigdata.context = lock->context;
 		dev->sigdata.lock = master->lock.hw_lock;
-		block_all_signals(drm_notifier, &dev->sigdata, &dev->sigmask);
+		block_all_signals(drm_notifier, dev, &dev->sigmask);
 	}
 
 	if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT))
@@ -286,26 +286,27 @@  int drm_lock_free(struct drm_lock_data *lock_data, unsigned int context)
  * If the lock is not held, then let the signal proceed as usual.  If the lock
  * is held, then set the contended flag and keep the signal blocked.
  *
- * \param priv pointer to a drm_sigdata structure.
+ * \param priv pointer to a drm_device structure.
  * \return one if the signal should be delivered normally, or zero if the
  * signal should be blocked.
  */
 static int drm_notifier(void *priv)
 {
-	struct drm_sigdata *s = (struct drm_sigdata *) priv;
+	struct drm_device *dev = priv;
+	struct drm_hw_lock *lock = dev->sigdata.lock;
 	unsigned int old, new, prev;
 
 	/* Allow signal delivery if lock isn't held */
-	if (!s->lock || !_DRM_LOCK_IS_HELD(s->lock->lock)
-	    || _DRM_LOCKING_CONTEXT(s->lock->lock) != s->context)
+	if (!lock || !_DRM_LOCK_IS_HELD(lock->lock)
+	    || _DRM_LOCKING_CONTEXT(lock->lock) != dev->sigdata.context)
 		return 1;
 
 	/* Otherwise, set flag to force call to
 	   drmUnlock */
 	do {
-		old = s->lock->lock;
+		old = lock->lock;
 		new = old | _DRM_LOCK_CONT;
-		prev = cmpxchg(&s->lock->lock, old, new);
+		prev = cmpxchg(&lock->lock, old, new);
 	} while (prev != old);
 	return 0;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8f55875..840a373 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -432,12 +432,6 @@  struct drm_sg_mem {
 	dma_addr_t *busaddr;
 };
 
-struct drm_sigdata {
-	int context;
-	struct drm_hw_lock *lock;
-};
-
-
 /**
  * Kernel side of a mapping
  */
@@ -1035,9 +1029,13 @@  struct drm_device {
 
 	struct drm_sg_mem *sg;	/**< Scatter gather memory */
 	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
-	struct drm_sigdata sigdata;	   /**< For block_all_signals */
 	sigset_t sigmask;
 
+	struct {
+		int context;
+		struct drm_hw_lock *lock;
+	} sigdata;
+
 	struct drm_local_map *agp_buffer_map;
 	unsigned int agp_buffer_token;