[PATCH 00/21] drm_event cleanup

[PATCH 00/21] drm_event cleanup

[Daniel Vetter]

Hi all,

This patch series is inspired by a WIP patch from Rob Clark to consolidate the
drm_event handling a bit. I've went a bit further and also moved the pending
event handling and unlinking into the core, which allows us to nuke a bunch of
code from drivers who all copypasted this themselves. Plus fix up all the others
who failed to handle this correctly.

Net -500 lines of code, plus kerneldoc for drm_fops.c and all the new functions
as bonus.

Comments and review highly welcome as usual.

Cheers, Daniel

Daniel Vetter (21):
  drm: kerneldoc for drm_fops.c
  drm: Add functions to setup/tear down drm_events.
  drm/exynos: Use the new event init/free functions
  drm/vmwgfx: Use the new event init/free functions
  drm: Create drm_send_event helpers
  drm/fsl: Remove preclose hook
  drm/armada: Remove NULL open/pre/postclose hooks
  drm/gma500: Remove empty preclose hook
  drm: Clean up pending events in the core
  drm/i915: Nuke intel_modeset_preclose
  drm/atmel: Nuke preclose
  drm/exynos: Remove event cancelling from postclose
  drm/imx: Unconfuse preclose logic
  drm/msm: Nuke preclose hooks
  drm/omap: Nuke close hooks
  drm/rcar: Nuke preclose hook
  drm/shmob: Nuke preclose hook
  drm/tegra: Stop cancelling page flip events
  drm/tilcdc: Nuke preclose hook
  drm/vc4: Nuke preclose hook
  drm/vmwgfx: Nuke preclose hook

 Documentation/DocBook/gpu.tmpl                 |  48 +----
 drivers/gpu/drm/armada/armada_drv.c            |   3 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  18 --
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  10 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |   3 -
 drivers/gpu/drm/drm_atomic.c                   |  44 ++---
 drivers/gpu/drm/drm_crtc.c                     |  36 +---
 drivers/gpu/drm/drm_fops.c                     | 259 ++++++++++++++++++++++---
 drivers/gpu/drm/drm_irq.c                      |   7 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c        |  14 --
 drivers/gpu/drm/exynos/exynos_drm_g2d.c        |  36 +---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c        |  28 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c      |   5 -
 drivers/gpu/drm/gma500/psb_drv.c               |   9 -
 drivers/gpu/drm/i915/i915_dma.c                |   2 -
 drivers/gpu/drm/i915/intel_display.c           |  21 --
 drivers/gpu/drm/i915/intel_drv.h               |   1 -
 drivers/gpu/drm/imx/imx-drm-core.c             |  13 --
 drivers/gpu/drm/imx/ipuv3-crtc.c               |   4 -
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c       |   7 -
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c        |  11 --
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h        |   1 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c       |   6 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c        |  11 --
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h        |   1 -
 drivers/gpu/drm/omapdrm/omap_drv.c             |  29 ---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c         |  20 --
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h         |   2 -
 drivers/gpu/drm/rcar-du/rcar_du_drv.c          |  10 -
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c      |  20 --
 drivers/gpu/drm/shmobile/shmob_drm_crtc.h      |   2 -
 drivers/gpu/drm/shmobile/shmob_drm_drv.c       |   8 -
 drivers/gpu/drm/tegra/dc.c                     |  17 --
 drivers/gpu/drm/tegra/drm.c                    |   3 -
 drivers/gpu/drm/tegra/drm.h                    |   1 -
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c           |  20 --
 drivers/gpu/drm/tilcdc/tilcdc_drv.c            |   8 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.h            |   1 -
 drivers/gpu/drm/vc4/vc4_crtc.c                 |  20 --
 drivers/gpu/drm/vc4/vc4_drv.c                  |  10 -
 drivers/gpu/drm/vc4/vc4_drv.h                  |   1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c            |  10 -
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c          |  73 +------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.h          |   2 -
 include/drm/drmP.h                             |  26 ++-
 45 files changed, 299 insertions(+), 582 deletions(-)

-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/21] drm: kerneldoc for drm_fops.c

[Daniel Vetter]

Just prep work before I throw more drm_event refactorings on top.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl |  48 +--------------
 drivers/gpu/drm/drm_fops.c     | 129 ++++++++++++++++++++++++++++++++++-------
 include/drm/drmP.h             |  17 +++---
 3 files changed, 117 insertions(+), 77 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 03535a7712eb..c434501c6868 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2886,52 +2886,8 @@ void (*postclose) (struct drm_device *, struct drm_file *);</synopsis>
     </sect2>
     <sect2>
       <title>File Operations</title>
-      <synopsis>const struct file_operations *fops</synopsis>
-      <abstract>File operations for the DRM device node.</abstract>
-      <para>
-        Drivers must define the file operations structure that forms the DRM
-	userspace API entry point, even though most of those operations are
-	implemented in the DRM core. The <methodname>open</methodname>,
-	<methodname>release</methodname> and <methodname>ioctl</methodname>
-	operations are handled by
-	<programlisting>
-	.owner = THIS_MODULE,
-	.open = drm_open,
-	.release = drm_release,
-	.unlocked_ioctl = drm_ioctl,
-  #ifdef CONFIG_COMPAT
-	.compat_ioctl = drm_compat_ioctl,
-  #endif
-        </programlisting>
-      </para>
-      <para>
-        Drivers that implement private ioctls that requires 32/64bit
-	compatibility support must provide their own
-	<methodname>compat_ioctl</methodname> handler that processes private
-	ioctls and calls <function>drm_compat_ioctl</function> for core ioctls.
-      </para>
-      <para>
-        The <methodname>read</methodname> and <methodname>poll</methodname>
-	operations provide support for reading DRM events and polling them. They
-	are implemented by
-	<programlisting>
-	.poll = drm_poll,
-	.read = drm_read,
-	.llseek = no_llseek,
-	</programlisting>
-      </para>
-      <para>
-        The memory mapping implementation varies depending on how the driver
-	manages memory. Pre-GEM drivers will use <function>drm_mmap</function>,
-	while GEM-aware drivers will use <function>drm_gem_mmap</function>. See
-	<xref linkend="drm-gem"/>.
-	<programlisting>
-	.mmap = drm_gem_mmap,
-	</programlisting>
-      </para>
-      <para>
-        No other file operation is supported by the DRM API.
-      </para>
+!Pdrivers/gpu/drm/drm_fops.c file operations
+!Edrivers/gpu/drm/drm_fops.c
     </sect2>
     <sect2>
       <title>IOCTLs</title>
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 359ec13cdf9d..73075a1fa380 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -1,4 +1,4 @@
-/**
+/*
  * \file drm_fops.c
  * File operations for DRM
  *
@@ -44,6 +44,46 @@
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
+/**
+ * DOC: file operations
+ *
+ * Drivers must define the file operations structure that forms the DRM
+ * userspace API entry point, even though most of those operations are
+ * implemented in the DRM core. The mandatory functions are drm_open(),
+ * drm_read(), drm_ioctl() and drm_compat_ioctl if CONFIG_COMPAT is enabled.
+ * Drivers which implement private ioctls that require 32/64 bit compatibility
+ * support must provided their onw .compat_ioctl() handler that processes
+ * private ioctls and calls drm_compat_ioctl() for core ioctls.
+ *
+ * In addition drm_read() and drm_poll() provide support for DRM events. DRM
+ * events are a generic and extensible means to send asynchronous events to
+ * userspace through the file descriptor. They are used to send vblank event and
+ * page flip completions by the KMS API. But drivers can also use it for their
+ * own needs, e.g. to signal completion of rendering.
+ *
+ * The memory mapping implementation will vary depending on how the driver
+ * manages memory. Legacy drivers will use the deprecated drm_legacy_mmap()
+ * function, modern drivers should use one of the provided memory-manager
+ * specific implementations. For GEM-based drivers this is drm_gem_mmap().
+ *
+ * No other file operations are supported by the DRM userspace API. Overall the
+ * following is an example #file_operations structure:
+ *
+ *     static const example_drm_fops = {
+ *             .owner = THIS_MODULE,
+ *             .open = drm_open,
+ *             .release = drm_release,
+ *             .unlocked_ioctl = drm_ioctl,
+ *     #ifdef CONFIG_COMPAT
+ *             .compat_ioctl = drm_compat_ioctl,
+ *     #endif
+ *             .poll = drm_poll,
+ *             .read = drm_read,
+ *             .llseek = no_llseek,
+ *             .mmap = drm_gem_mmap,
+ *     };
+ */
+
 static int drm_open_helper(struct file *filp, struct drm_minor *minor);
 
 static int drm_setup(struct drm_device * dev)
@@ -67,15 +107,17 @@ static int drm_setup(struct drm_device * dev)
 }
 
 /**
- * Open file.
+ * drm_open - open method for DRM file
+ * @inode: device inode
+ * @filp: file pointer.
  *
- * \param inode device inode
- * \param filp file pointer.
- * \return zero on success or a negative number on failure.
+ * This function must be used by drivers as their .open() #file_operations
+ * method. It looks up the correct DRM device and instantiates all the per-file
+ * resources for it.
+ *
+ * RETURNS:
  *
- * Searches the DRM device with the same minor number, calls open_helper(), and
- * increments the device open count. If the open count was previous at zero,
- * i.e., it's the first that the device is open, then calls setup().
+ * 0 on success or negative errno value on falure.
  */
 int drm_open(struct inode *inode, struct file *filp)
 {
@@ -112,7 +154,7 @@ err_undo:
 }
 EXPORT_SYMBOL(drm_open);
 
-/**
+/*
  * Check whether DRI will run on this CPU.
  *
  * \return non-zero if the DRI will run on this CPU, or zero otherwise.
@@ -125,7 +167,7 @@ static int drm_cpu_valid(void)
 	return 1;
 }
 
-/**
+/*
  * drm_new_set_master - Allocate a new master object and become master for the
  * associated master realm.
  *
@@ -179,7 +221,7 @@ out_err:
 	return ret;
 }
 
-/**
+/*
  * Called whenever a process opens /dev/drm.
  *
  * \param filp file pointer.
@@ -333,7 +375,7 @@ static void drm_events_release(struct drm_file *file_priv)
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
-/**
+/*
  * drm_legacy_dev_reinit
  *
  * Reinitializes a legacy/ums drm device in it's lastclose function.
@@ -362,7 +404,7 @@ static void drm_legacy_dev_reinit(struct drm_device *dev)
 	DRM_DEBUG("lastclose completed\n");
 }
 
-/**
+/*
  * Take down the DRM device.
  *
  * \param dev DRM device structure.
@@ -384,16 +426,17 @@ void drm_lastclose(struct drm_device * dev)
 }
 
 /**
- * Release file.
+ * drm_release - release method for DRM file
+ * @inode: device inode
+ * @filp: file pointer.
  *
- * \param inode device inode
- * \param file_priv DRM file private.
- * \return zero on success or a negative number on failure.
+ * This function must be used by drivers as their .release() #file_operations
+ * method. It frees any resources associated with the open file, and if this is
+ * the last open file for the DRM device also proceeds to call drm_lastclose().
  *
- * If the hardware lock is held then free it, and take it again for the kernel
- * context since it's necessary to reclaim buffers. Unlink the file private
- * data from its list and free it. Decreases the open count and if it reaches
- * zero calls drm_lastclose().
+ * RETURNS:
+ *
+ * Always succeeds and returns 0.
  */
 int drm_release(struct inode *inode, struct file *filp)
 {
@@ -447,7 +490,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	if (file_priv->is_master) {
 		struct drm_master *master = file_priv->master;
 
-		/**
+		/*
 		 * Since the master is disappearing, so is the
 		 * possibility to lock.
 		 */
@@ -504,6 +547,32 @@ int drm_release(struct inode *inode, struct file *filp)
 }
 EXPORT_SYMBOL(drm_release);
 
+/**
+ * drm_read - read method for DRM file
+ * @filp: file pointer
+ * @buffer: userspace destination pointer for the read
+ * @count: count in bytes to read
+ * @offset: offset to read
+ *
+ * This function must be used by drivers as their .read() #file_operations
+ * method iff they use DRM events for asynchronous signalling to userspace.
+ * Since events are used by the KMS API for vblank and page flip completion this
+ * means all modern display drivers must use it.
+ *
+ * @offset is ignore, DRM events are read like a pipe. Therefore drivers also
+ * must set the .llseek() #file_operation to no_llseek(). Polling support is
+ * provided by drm_poll().
+ *
+ * This function will only ever read a full event. Therefore userspace must
+ * supply a big enough buffer to fit any event to ensure forward progress. Since
+ * the maximum event space is currently 4K it's recommended to just use that for
+ * safety.
+ *
+ * RETURNS:
+ *
+ * Number of bytes read (always aligned to full events, and can be 0) or a
+ * negative error code on failure.
+ */
 ssize_t drm_read(struct file *filp, char __user *buffer,
 		 size_t count, loff_t *offset)
 {
@@ -574,6 +643,22 @@ put_back_event:
 }
 EXPORT_SYMBOL(drm_read);
 
+/**
+ * drm_poll - poll method for DRM file
+ * @filp: file pointer
+ * @wait: poll waiter table
+ *
+ * This function must be used by drivers as their .read() #file_operations
+ * method iff they use DRM events for asynchronous signalling to userspace.
+ * Since events are used by the KMS API for vblank and page flip completion this
+ * means all modern display drivers must use it.
+ *
+ * See also drm_read().
+ *
+ * RETURNS:
+ *
+ * Mask of POLL flags indicating the current status of the file.
+ */
 unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait)
 {
 	struct drm_file *file_priv = filp->private_data;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c2d253e9b6f7..fc9b9ad5b089 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -919,15 +919,14 @@ extern long drm_compat_ioctl(struct file *filp,
 			     unsigned int cmd, unsigned long arg);
 extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
 
-				/* Device support (drm_fops.h) */
-extern int drm_open(struct inode *inode, struct file *filp);
-extern ssize_t drm_read(struct file *filp, char __user *buffer,
-			size_t count, loff_t *offset);
-extern int drm_release(struct inode *inode, struct file *filp);
-extern int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
-
-				/* Mapping support (drm_vm.h) */
-extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
+/* File Operations (drm_fops.c) */
+int drm_open(struct inode *inode, struct file *filp);
+ssize_t drm_read(struct file *filp, char __user *buffer,
+		 size_t count, loff_t *offset);
+int drm_release(struct inode *inode, struct file *filp);
+int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
+
+unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
 
 /* Misc. IOCTL support (drm_ioctl.c) */
 int drm_noop(struct drm_device *dev, void *data,
-- 
2.6.4

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

[PATCH 02/21] drm: Add functions to setup/tear down drm_events.

[Daniel Vetter]

An attempt at not spreading out the file_priv->event_space stuff out
quite so far and wide.  And I think fixes something in ipp_get_event()
that is broken (or if they are doing something more weird/subtle, then
breaks it in a fun way).

Based upon a patch from Rob Clark, rebased and polished.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 44 ++++++++---------------------
 drivers/gpu/drm/drm_crtc.c   | 36 +++++++-----------------
 drivers/gpu/drm/drm_fops.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h           |  7 ++++-
 4 files changed, 94 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f74193885f1..8fb469c4e4b8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1347,44 +1347,23 @@ static struct drm_pending_vblank_event *create_vblank_event(
 		struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data)
 {
 	struct drm_pending_vblank_event *e = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-	if (file_priv->event_space < sizeof e->event) {
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		goto out;
-	}
-	file_priv->event_space -= sizeof e->event;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
+	int ret;
 
 	e = kzalloc(sizeof *e, GFP_KERNEL);
-	if (e == NULL) {
-		spin_lock_irqsave(&dev->event_lock, flags);
-		file_priv->event_space += sizeof e->event;
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		goto out;
-	}
+	if (!e)
+		return NULL;
 
 	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
-	e->event.base.length = sizeof e->event;
+	e->event.base.length = sizeof(e->event);
 	e->event.user_data = user_data;
-	e->base.event = &e->event.base;
-	e->base.file_priv = file_priv;
-	e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
-
-out:
-	return e;
-}
 
-static void destroy_vblank_event(struct drm_device *dev,
-		struct drm_file *file_priv, struct drm_pending_vblank_event *e)
-{
-	unsigned long flags;
+	ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
+	if (ret) {
+		kfree(e);
+		return NULL;
+	}
 
-	spin_lock_irqsave(&dev->event_lock, flags);
-	file_priv->event_space += sizeof e->event;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-	kfree(e);
+	return e;
 }
 
 static int atomic_set_prop(struct drm_atomic_state *state,
@@ -1646,8 +1625,7 @@ out:
 			if (!crtc_state->event)
 				continue;
 
-			destroy_vblank_event(dev, file_priv,
-					     crtc_state->event);
+			drm_event_cancel_free(dev, &crtc_state->event->base);
 		}
 	}
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1e75a145834a..60a4184d41b7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5264,7 +5264,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb = NULL;
 	struct drm_pending_vblank_event *e = NULL;
-	unsigned long flags;
 	int ret = -EINVAL;
 
 	if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
@@ -5315,41 +5314,26 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	}
 
 	if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
-		ret = -ENOMEM;
-		spin_lock_irqsave(&dev->event_lock, flags);
-		if (file_priv->event_space < sizeof(e->event)) {
-			spin_unlock_irqrestore(&dev->event_lock, flags);
-			goto out;
-		}
-		file_priv->event_space -= sizeof(e->event);
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-
-		e = kzalloc(sizeof(*e), GFP_KERNEL);
-		if (e == NULL) {
-			spin_lock_irqsave(&dev->event_lock, flags);
-			file_priv->event_space += sizeof(e->event);
-			spin_unlock_irqrestore(&dev->event_lock, flags);
+		e = kzalloc(sizeof *e, GFP_KERNEL);
+		if (!e) {
+			ret = -ENOMEM;
 			goto out;
 		}
-
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
 		e->event.user_data = page_flip->user_data;
-		e->base.event = &e->event.base;
-		e->base.file_priv = file_priv;
-		e->base.destroy =
-			(void (*) (struct drm_pending_event *)) kfree;
+		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
+		if (ret) {
+			kfree(e);
+			goto out;
+		}
 	}
 
 	crtc->primary->old_fb = crtc->primary->fb;
 	ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
 	if (ret) {
-		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
-			spin_lock_irqsave(&dev->event_lock, flags);
-			file_priv->event_space += sizeof(e->event);
-			spin_unlock_irqrestore(&dev->event_lock, flags);
-			kfree(e);
-		}
+		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT)
+			drm_event_cancel_free(dev, &e->base);
 		/* Keep the old fb, don't unref it. */
 		crtc->primary->old_fb = NULL;
 	} else {
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 73075a1fa380..d6542864bd95 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -672,3 +672,70 @@ unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait)
 	return mask;
 }
 EXPORT_SYMBOL(drm_poll);
+
+/**
+ * drm_event_reserve_init - init a DRM event and reserve space for it
+ * @dev: DRM device
+ * @file_priv: DRM file private data
+ * @p: tracking structure for the pending event
+ * @e: actual event data to deliver to userspace
+ *
+ * This function prepares the passed in even for eventual deliver. If the event
+ * doesn't get delivered (because the IOCTL fails later on, before queuing up
+ * anything) then the even must be cancelled and freed using
+ * drm_event_cancel_free().
+ *
+ * If callers embedded @p into a larger structure it must be allocated with
+ * kmalloc and @p must be the first member element.
+ *
+ * RETURNS:
+ *
+ * 0 on success or a negative error code on failure.
+ */
+int drm_event_reserve_init(struct drm_device *dev,
+			   struct drm_file *file_priv,
+			   struct drm_pending_event *p,
+			   struct drm_event *e)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	if (file_priv->event_space < e->length) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	file_priv->event_space -= e->length;
+
+	p->event = e;
+	p->file_priv = file_priv;
+
+	/* we *could* pass this in as arg, but everyone uses kfree: */
+	p->destroy = (void (*) (struct drm_pending_event *)) kfree;
+
+out:
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(drm_event_reserve_init);
+
+/**
+ * drm_event_cancel_free - free a DRM event and release it's space
+ * @dev: DRM device
+ * @p: tracking structure for the pending event
+ *
+ * This function frees the event @p initialized with drm_event_reserve_init()
+ * and releases any allocated space.
+ */
+void drm_event_cancel_free(struct drm_device *dev,
+			   struct drm_pending_event *p)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&dev->event_lock, flags);
+	p->file_priv->event_space += p->event->length;
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+	p->destroy(p);
+}
+EXPORT_SYMBOL(drm_event_cancel_free);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fc9b9ad5b089..ad4d0a31294d 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -925,8 +925,13 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
 		 size_t count, loff_t *offset);
 int drm_release(struct inode *inode, struct file *filp);
 int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
-
 unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
+int drm_event_reserve_init(struct drm_device *dev,
+			   struct drm_file *file_priv,
+			   struct drm_pending_event *p,
+			   struct drm_event *e);
+void drm_event_cancel_free(struct drm_device *dev,
+			   struct drm_pending_event *p);
 
 /* Misc. IOCTL support (drm_ioctl.c) */
 int drm_noop(struct drm_device *dev, void *data,
-- 
2.6.4

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

[PATCH 03/21] drm/exynos: Use the new event init/free functions

[Daniel Vetter]

Also fixes a bug in IPP with not correctly checking/allocating for
space in the event space. Not a too serious bug since it's not a
real ringbuffer, just a limit to avoid too much kernel allocations.

Cc: Rob Clark <robdclark@gmail.com>
Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 31 ++++++++-----------------------
 drivers/gpu/drm/exynos/exynos_drm_ipp.c | 23 +++++++++--------------
 2 files changed, 17 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index c17efdb238a6..82e7f95dfed9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1072,7 +1072,6 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 	struct drm_exynos_pending_g2d_event *e;
 	struct g2d_cmdlist_node *node;
 	struct g2d_cmdlist *cmdlist;
-	unsigned long flags;
 	int size;
 	int ret;
 
@@ -1094,21 +1093,8 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 	node->event = NULL;
 
 	if (req->event_type != G2D_EVENT_NOT) {
-		spin_lock_irqsave(&drm_dev->event_lock, flags);
-		if (file->event_space < sizeof(e->event)) {
-			spin_unlock_irqrestore(&drm_dev->event_lock, flags);
-			ret = -ENOMEM;
-			goto err;
-		}
-		file->event_space -= sizeof(e->event);
-		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
-
 		e = kzalloc(sizeof(*node->event), GFP_KERNEL);
 		if (!e) {
-			spin_lock_irqsave(&drm_dev->event_lock, flags);
-			file->event_space += sizeof(e->event);
-			spin_unlock_irqrestore(&drm_dev->event_lock, flags);
-
 			ret = -ENOMEM;
 			goto err;
 		}
@@ -1116,9 +1102,12 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 		e->event.base.type = DRM_EXYNOS_G2D_EVENT;
 		e->event.base.length = sizeof(e->event);
 		e->event.user_data = req->user_data;
-		e->base.event = &e->event.base;
-		e->base.file_priv = file;
-		e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
+
+		ret = drm_event_reserve_init(drm_dev, file, &e->base, &e->event.base);
+		if (ret) {
+			kfree(e);
+			goto err;
+		}
 
 		node->event = e;
 	}
@@ -1219,12 +1208,8 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 err_unmap:
 	g2d_unmap_cmdlist_gem(g2d, node, file);
 err_free_event:
-	if (node->event) {
-		spin_lock_irqsave(&drm_dev->event_lock, flags);
-		file->event_space += sizeof(e->event);
-		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
-		kfree(node->event);
-	}
+	if (node->event)
+		drm_event_cancel_free(drm_dev, &node->event->base);
 err:
 	g2d_put_cmdlist(g2d, node);
 	return ret;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 67d24236e745..c8819c05e2dd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -618,27 +618,18 @@ static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
 	mutex_unlock(&c_node->mem_lock);
 }
 
-static void ipp_free_event(struct drm_pending_event *event)
-{
-	kfree(event);
-}
-
 static int ipp_get_event(struct drm_device *drm_dev,
 		struct drm_exynos_ipp_cmd_node *c_node,
 		struct drm_exynos_ipp_queue_buf *qbuf)
 {
 	struct drm_exynos_ipp_send_event *e;
-	unsigned long flags;
+	int ret;
 
 	DRM_DEBUG_KMS("ops_id[%d]buf_id[%d]\n", qbuf->ops_id, qbuf->buf_id);
 
 	e = kzalloc(sizeof(*e), GFP_KERNEL);
-	if (!e) {
-		spin_lock_irqsave(&drm_dev->event_lock, flags);
-		c_node->filp->event_space += sizeof(e->event);
-		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+	if (!e)
 		return -ENOMEM;
-	}
 
 	/* make event */
 	e->event.base.type = DRM_EXYNOS_IPP_EVENT;
@@ -646,9 +637,13 @@ static int ipp_get_event(struct drm_device *drm_dev,
 	e->event.user_data = qbuf->user_data;
 	e->event.prop_id = qbuf->prop_id;
 	e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
-	e->base.event = &e->event.base;
-	e->base.file_priv = c_node->filp;
-	e->base.destroy = ipp_free_event;
+
+	ret = drm_event_reserve_init(drm_dev, c_node->filp, &e->base, &e->event.base);
+	if (ret) {
+		kfree(e);
+		return ret;
+	}
+
 	mutex_lock(&c_node->event_lock);
 	list_add_tail(&e->base.link, &c_node->event_list);
 	mutex_unlock(&c_node->event_lock);
-- 
2.6.4

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

[PATCH 04/21] drm/vmwgfx: Use the new event init/free functions

[Daniel Vetter]

Cc: Rob Clark <robdclark@gmail.com
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index 8e689b439890..eda93bf52a6e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -1025,38 +1025,26 @@ static int vmw_event_fence_action_create(struct drm_file *file_priv,
 	struct vmw_event_fence_pending *event;
 	struct vmw_fence_manager *fman = fman_from_fence(fence);
 	struct drm_device *dev = fman->dev_priv->dev;
-	unsigned long irq_flags;
 	int ret;
 
-	spin_lock_irqsave(&dev->event_lock, irq_flags);
-
-	ret = (file_priv->event_space < sizeof(event->event)) ? -EBUSY : 0;
-	if (likely(ret == 0))
-		file_priv->event_space -= sizeof(event->event);
-
-	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
-
-	if (unlikely(ret != 0)) {
-		DRM_ERROR("Failed to allocate event space for this file.\n");
-		goto out_no_space;
-	}
-
-
 	event = kzalloc(sizeof(*event), GFP_KERNEL);
 	if (unlikely(event == NULL)) {
 		DRM_ERROR("Failed to allocate an event.\n");
 		ret = -ENOMEM;
-		goto out_no_event;
+		goto out_no_space;
 	}
 
 	event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
 	event->event.base.length = sizeof(*event);
 	event->event.user_data = user_data;
 
-	event->base.event = &event->event.base;
-	event->base.file_priv = file_priv;
-	event->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
+	ret = drm_event_reserve_init(dev, file_priv, &event->base, &event->event.base);
 
+	if (unlikely(ret != 0)) {
+		DRM_ERROR("Failed to allocate event space for this file.\n");
+		kfree(event);
+		goto out_no_space;
+	}
 
 	if (flags & DRM_VMW_FE_FLAG_REQ_TIME)
 		ret = vmw_event_fence_action_queue(file_priv, fence,
@@ -1076,11 +1064,7 @@ static int vmw_event_fence_action_create(struct drm_file *file_priv,
 	return 0;
 
 out_no_queue:
-	event->base.destroy(&event->base);
-out_no_event:
-	spin_lock_irqsave(&dev->event_lock, irq_flags);
-	file_priv->event_space += sizeof(*event);
-	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
+	drm_event_cancel_free(dev, &event->base);
 out_no_space:
 	return ret;
 }
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/21] drm: Create drm_send_event helpers

[Daniel Vetter]

Use them in the core vblank code and exynos/vmwgfx drivers.

Note that the difference between wake_up_all and _interruptible in
vmwgfx doesn't matter since the only waiter is the core code in
drm_fops.c. And that is interruptible.

Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fops.c              | 38 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_irq.c               |  7 ++----
 drivers/gpu/drm/exynos/exynos_drm_g2d.c |  5 +----
 drivers/gpu/drm/exynos/exynos_drm_ipp.c |  5 +----
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c   |  3 +--
 include/drm/drmP.h                      |  2 ++
 6 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index d6542864bd95..3d338075113c 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -739,3 +739,41 @@ void drm_event_cancel_free(struct drm_device *dev,
 	p->destroy(p);
 }
 EXPORT_SYMBOL(drm_event_cancel_free);
+
+/**
+ * drm_send_event_locked - send DRM event to file descriptor
+ * @dev: DRM device
+ * @e: DRM event to deliver
+ *
+ * This function sends the event @e, initialized with drm_event_reserve_init(),
+ * to its associated userspace DRM file. Callers must already hold
+ * dev->event_lock, see drm_send_event() for the unlocked version.
+ */
+void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
+{
+	assert_spin_locked(&dev->event_lock);
+
+	list_add_tail(&e->link,
+		      &e->file_priv->event_list);
+	wake_up_interruptible(&e->file_priv->event_wait);
+}
+EXPORT_SYMBOL(drm_send_event_locked);
+
+/**
+ * drm_send_event - send DRM event to file descriptor
+ * @dev: DRM device
+ * @e: DRM event to deliver
+ *
+ * This function sends the event @e, initialized with drm_event_reserve_init(),
+ * to its associated userspace DRM file. This function acquires dev->event_lock,
+ * see drm_send_event_locked() for callers which already hold this lock.
+ */
+void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev->event_lock, irqflags);
+	drm_send_event_locked(dev, e);
+	spin_unlock_irqrestore(&dev->event_lock, irqflags);
+}
+EXPORT_SYMBOL(drm_send_event);
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a70b29909974..3fe8dbff6058 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -983,15 +983,12 @@ static void send_vblank_event(struct drm_device *dev,
 		struct drm_pending_vblank_event *e,
 		unsigned long seq, struct timeval *now)
 {
-	assert_spin_locked(&dev->event_lock);
-
 	e->event.sequence = seq;
 	e->event.tv_sec = now->tv_sec;
 	e->event.tv_usec = now->tv_usec;
 
-	list_add_tail(&e->base.link,
-		      &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
+	drm_send_event_locked(dev, &e->base);
+
 	trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
 					 e->event.sequence);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 82e7f95dfed9..db56c8259f18 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -893,10 +893,7 @@ static void g2d_finish_event(struct g2d_data *g2d, u32 cmdlist_no)
 	e->event.tv_usec = now.tv_usec;
 	e->event.cmdlist_no = cmdlist_no;
 
-	spin_lock_irqsave(&drm_dev->event_lock, flags);
-	list_move_tail(&e->base.link, &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
-	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+	drm_send_event(dev, &e->base);
 }
 
 static irqreturn_t g2d_irq_handler(int irq, void *dev_id)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c8819c05e2dd..1f6a6c1881d6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1520,10 +1520,7 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv,
 	for_each_ipp_ops(i)
 		e->event.buf_id[i] = tbuf_id[i];
 
-	spin_lock_irqsave(&drm_dev->event_lock, flags);
-	list_move_tail(&e->base.link, &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
-	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+	drm_send_event(dev, &e->base);
 	mutex_unlock(&c_node->event_lock);
 
 	DRM_DEBUG_KMS("done cmd[%d]prop_id[%d]buf_id[%d]\n",
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index eda93bf52a6e..e0edf149d9d5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -880,9 +880,8 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
 	}
 
 	list_del_init(&eaction->fpriv_head);
-	list_add_tail(&eaction->event->link, &file_priv->event_list);
 	eaction->event = NULL;
-	wake_up_all(&file_priv->event_wait);
+	drm_send_event_locked(dev, eaction->event);
 	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
 }
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ad4d0a31294d..ae73abf5c2cf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -932,6 +932,8 @@ int drm_event_reserve_init(struct drm_device *dev,
 			   struct drm_event *e);
 void drm_event_cancel_free(struct drm_device *dev,
 			   struct drm_pending_event *p);
+void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e);
+void drm_send_event(struct drm_device *dev, struct drm_pending_event *e);
 
 /* Misc. IOCTL support (drm_ioctl.c) */
 int drm_noop(struct drm_device *dev, void *data,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/21] drm/fsl: Remove preclose hook

[Daniel Vetter]

Doesn't do anything, but annoys when auditing them all.

Cc: Jianwei Wang <jianwei.wang.chn@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index fca97d3fc846..9648b7f9a31c 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -112,10 +112,6 @@ static int fsl_dcu_unload(struct drm_device *dev)
 	return 0;
 }
 
-static void fsl_dcu_drm_preclose(struct drm_device *dev, struct drm_file *file)
-{
-}
-
 static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -191,7 +187,6 @@ static struct drm_driver fsl_dcu_drm_driver = {
 				| DRIVER_PRIME | DRIVER_ATOMIC,
 	.load			= fsl_dcu_load,
 	.unload			= fsl_dcu_unload,
-	.preclose		= fsl_dcu_drm_preclose,
 	.irq_handler		= fsl_dcu_drm_irq,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= fsl_dcu_drm_enable_vblank,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/21] drm/armada: Remove NULL open/pre/postclose hooks

[Daniel Vetter]

The compiler will do this, but the void hits when grepping all the
hooks for a subsystem wide audit are slightly annoying. So remove them
for next time around.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/armada/armada_drv.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 3bd7e1cde99e..82043c204b76 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -188,9 +188,6 @@ static const struct file_operations armada_drm_fops = {
 
 static struct drm_driver armada_drm_driver = {
 	.load			= armada_drm_load,
-	.open			= NULL,
-	.preclose		= NULL,
-	.postclose		= NULL,
 	.lastclose		= armada_drm_lastclose,
 	.unload			= armada_drm_unload,
 	.set_busid		= drm_platform_set_busid,
-- 
2.6.4

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

[PATCH 08/21] drm/gma500: Remove empty preclose hook

[Daniel Vetter]

I'm auditing them all, empty ones just confuse ...

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/gma500/psb_drv.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 92e7e5795398..4e1c6850520e 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -442,14 +442,6 @@ static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd,
 	/* FIXME: do we need to wrap the other side of this */
 }
 
-/*
- * When a client dies:
- *    - Check for and clean up flipped page state
- */
-static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv)
-{
-}
-
 static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	return drm_get_pci_dev(pdev, ent, &driver);
@@ -495,7 +487,6 @@ static struct drm_driver driver = {
 	.load = psb_driver_load,
 	.unload = psb_driver_unload,
 	.lastclose = psb_driver_lastclose,
-	.preclose = psb_driver_preclose,
 	.set_busid = drm_pci_set_busid,
 
 	.num_ioctls = ARRAY_SIZE(psb_ioctls),
-- 
2.6.4

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

[PATCH 09/21] drm: Clean up pending events in the core

[Daniel Vetter]

There's really no reason to not do so, instead of replicating this
for every use-case and every driver. Now we can't just nuke the events,
since that would still mean that all drm_event users would need to know
when that has happened, since calling e.g. drm_send_event isn't allowed
any more. Instead just unlink them from the file, and detect this case
and handle it appropriately in all functions.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fops.c | 27 ++++++++++++++++++---------
 include/drm/drmP.h         |  2 ++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 3d338075113c..e831537dc931 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -264,6 +264,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 	INIT_LIST_HEAD(&priv->fbs);
 	mutex_init(&priv->fbs_lock);
 	INIT_LIST_HEAD(&priv->blobs);
+	INIT_LIST_HEAD(&priv->pending_event_list);
 	INIT_LIST_HEAD(&priv->event_list);
 	init_waitqueue_head(&priv->event_wait);
 	priv->event_space = 4096; /* set aside 4k for event buffer */
@@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file *file_priv)
 {
 	struct drm_device *dev = file_priv->minor->dev;
 	struct drm_pending_event *e, *et;
-	struct drm_pending_vblank_event *v, *vt;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	/* Remove pending flips */
-	list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
-		if (v->base.file_priv == file_priv) {
-			list_del(&v->base.link);
-			drm_vblank_put(dev, v->pipe);
-			v->base.destroy(&v->base);
-		}
+	/* Unlink pending events */
+	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
+				 pending_link) {
+		list_del(&e->pending_link);
+		e->file_priv = NULL;
+	}
 
 	/* Remove unconsumed events */
 	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
@@ -710,6 +709,7 @@ int drm_event_reserve_init(struct drm_device *dev,
 	file_priv->event_space -= e->length;
 
 	p->event = e;
+	list_add(&p->pending_link, &file_priv->pending_event_list);
 	p->file_priv = file_priv;
 
 	/* we *could* pass this in as arg, but everyone uses kfree: */
@@ -734,7 +734,10 @@ void drm_event_cancel_free(struct drm_device *dev,
 {
 	unsigned long flags;
 	spin_lock_irqsave(&dev->event_lock, flags);
-	p->file_priv->event_space += p->event->length;
+	if (p->file_priv) {
+		p->file_priv->event_space += p->event->length;
+		list_del(&p->pending_link);
+	}
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 	p->destroy(p);
 }
@@ -753,6 +756,12 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
 {
 	assert_spin_locked(&dev->event_lock);
 
+	if (!e->file_priv) {
+		e->destroy(e);
+		return;
+	}
+
+	list_del(&e->pending_link);
 	list_add_tail(&e->link,
 		      &e->file_priv->event_list);
 	wake_up_interruptible(&e->file_priv->event_wait);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ae73abf5c2cf..3d78a7406d54 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -283,6 +283,7 @@ struct drm_ioctl_desc {
 struct drm_pending_event {
 	struct drm_event *event;
 	struct list_head link;
+	struct list_head pending_link;
 	struct drm_file *file_priv;
 	pid_t pid; /* pid of requester, no guarantee it's valid by the time
 		      we deliver the event, for tracing only */
@@ -346,6 +347,7 @@ struct drm_file {
 	struct list_head blobs;
 
 	wait_queue_head_t event_wait;
+	struct list_head pending_event_list;
 	struct list_head event_list;
 	int event_space;
 
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/21] drm/i915: Nuke intel_modeset_preclose

[Daniel Vetter]

Now that the drm core unlinks/disarms events there's no need to do so
ourselves anymore. Nuke the code.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  2 --
 drivers/gpu/drm/i915/intel_display.c | 21 ---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 3 files changed, 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a0f5659032fc..4bfa72e5a217 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1262,8 +1262,6 @@ void i915_driver_preclose(struct drm_device *dev, struct drm_file *file)
 	i915_gem_context_close(dev, file);
 	i915_gem_release(dev, file);
 	mutex_unlock(&dev->struct_mutex);
-
-	intel_modeset_preclose(dev, file);
 }
 
 void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b0928b1413e2..aa7e9c04a923 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16355,24 +16355,3 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
 		err_printf(m, "  VSYNC: %08x\n", error->transcoder[i].vsync);
 	}
 }
-
-void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct intel_crtc *crtc;
-
-	for_each_intel_crtc(dev, crtc) {
-		struct intel_unpin_work *work;
-
-		spin_lock_irq(&dev->event_lock);
-
-		work = crtc->unpin_work;
-
-		if (work && work->event &&
-		    work->event->base.file_priv == file) {
-			kfree(work->event);
-			work->event = NULL;
-		}
-
-		spin_unlock_irq(&dev->event_lock);
-	}
-}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e88050aee0c1..5e7422085162 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1235,7 +1235,6 @@ enum intel_display_power_domain
 intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
-void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
-- 
2.6.4

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

[PATCH 11/21] drm/atmel: Nuke preclose

[Daniel Vetter]

The only thing this did was cancle pending flip events, and the core
takes care of that now.

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 18 ------------------
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 10 ----------
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |  3 ---
 3 files changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 468a14f266a7..9863291a9a54 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -280,24 +280,6 @@ static void atmel_hlcdc_crtc_destroy(struct drm_crtc *c)
 	kfree(crtc);
 }
 
-void atmel_hlcdc_crtc_cancel_page_flip(struct drm_crtc *c,
-				       struct drm_file *file)
-{
-	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = c->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = crtc->event;
-	if (event && event->base.file_priv == file) {
-		event->base.destroy(&event->base);
-		drm_vblank_put(dev, crtc->id);
-		crtc->event = NULL;
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 static void atmel_hlcdc_crtc_finish_page_flip(struct atmel_hlcdc_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index a45b32ba029e..3d8d16402d07 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -619,15 +619,6 @@ static void atmel_hlcdc_dc_connector_unplug_all(struct drm_device *dev)
 	mutex_unlock(&dev->mode_config.mutex);
 }
 
-static void atmel_hlcdc_dc_preclose(struct drm_device *dev,
-				    struct drm_file *file)
-{
-	struct drm_crtc *crtc;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		atmel_hlcdc_crtc_cancel_page_flip(crtc, file);
-}
-
 static void atmel_hlcdc_dc_lastclose(struct drm_device *dev)
 {
 	struct atmel_hlcdc_dc *dc = dev->dev_private;
@@ -698,7 +689,6 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
 	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
 			   DRIVER_MODESET | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.preclose = atmel_hlcdc_dc_preclose,
 	.lastclose = atmel_hlcdc_dc_lastclose,
 	.irq_handler = atmel_hlcdc_dc_irq_handler,
 	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index cf6b375bc38d..fed517f297da 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -152,9 +152,6 @@ int atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state);
 
 void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
 
-void atmel_hlcdc_crtc_cancel_page_flip(struct drm_crtc *crtc,
-				       struct drm_file *file);
-
 void atmel_hlcdc_crtc_suspend(struct drm_crtc *crtc);
 void atmel_hlcdc_crtc_resume(struct drm_crtc *crtc);
 
-- 
2.6.4

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

[PATCH 12/21] drm/exynos: Remove event cancelling from postclose

[Daniel Vetter]

The core takes care of this now. And since kfree(NULL) is ok we can
simplify the function even further now.

Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 9756797a15a5..868ab9f54f17 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -335,20 +335,6 @@ static void exynos_drm_preclose(struct drm_device *dev,
 
 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
 {
-	struct drm_pending_event *e, *et;
-	unsigned long flags;
-
-	if (!file->driver_priv)
-		return;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-	/* Release all events handled by page flip handler but not freed. */
-	list_for_each_entry_safe(e, et, &file->event_list, link) {
-		list_del(&e->link);
-		e->destroy(e);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
 	kfree(file->driver_priv);
 	file->driver_priv = NULL;
 }
-- 
2.6.4

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

[PATCH 13/21] drm/imx: Unconfuse preclose logic

[Daniel Vetter]

So this one is special, since it tries to prevent races when userspace
crashes simply by disabling the vblank machinery. Well except that imx
always has vblanks enabled, and the disable_vblank hook actually just
tries to cancel a pending pageflip. Without any locking whatsoever. Of
course this is wrong, since it'll result in the hw not actually
displaying what drm thinks is the current frontbuffer.

Well since the core takes care of the disappearing DRM fd now. So we
can nuke all this confused code without ill side-effects.

Someone else needs to audit the locking for ->newfb and
->page_flip_event and fix it up. Common approach is to reuse
dev->event_lock for this.

Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/imx/imx-drm-core.c | 13 -------------
 drivers/gpu/drm/imx/ipuv3-crtc.c   |  4 ----
 2 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 7be7ac808304..ff94ca4a2c1e 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -171,18 +171,6 @@ static void imx_drm_disable_vblank(struct drm_device *drm, unsigned int pipe)
 	imx_drm_crtc->imx_drm_helper_funcs.disable_vblank(imx_drm_crtc->crtc);
 }
 
-static void imx_drm_driver_preclose(struct drm_device *drm,
-		struct drm_file *file)
-{
-	int i;
-
-	if (!file->is_master)
-		return;
-
-	for (i = 0; i < MAX_CRTC; i++)
-		imx_drm_disable_vblank(drm, i);
-}
-
 static const struct file_operations imx_drm_driver_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
@@ -462,7 +450,6 @@ static struct drm_driver imx_drm_driver = {
 	.load			= imx_drm_driver_load,
 	.unload			= imx_drm_driver_unload,
 	.lastclose		= imx_drm_driver_lastclose,
-	.preclose		= imx_drm_driver_preclose,
 	.set_busid		= drm_platform_set_busid,
 	.gem_free_object	= drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 30a57185bdb4..846b5f558897 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -285,10 +285,6 @@ static int ipu_enable_vblank(struct drm_crtc *crtc)
 
 static void ipu_disable_vblank(struct drm_crtc *crtc)
 {
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-
-	ipu_crtc->page_flip_event = NULL;
-	ipu_crtc->newfb = NULL;
 }
 
 static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc,
-- 
2.6.4

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

[PATCH 14/21] drm/msm: Nuke preclose hooks

[Daniel Vetter]

They only complete the page flip events to avoid oops when the drm
file closes. The core takes care of that now and we can remove this
code.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c |  7 -------
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c  | 11 -----------
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h  |  1 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c |  6 ------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c  | 11 -----------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h  |  1 -
 6 files changed, 37 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
index 28df397c3b04..909d74250de7 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
@@ -575,13 +575,6 @@ uint32_t mdp4_crtc_vblank(struct drm_crtc *crtc)
 	return mdp4_crtc->vblank.irqmask;
 }
 
-void mdp4_crtc_cancel_pending_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
-	DBG("%s: cancel: %p", mdp4_crtc->name, file);
-	complete_flip(crtc, file);
-}
-
 /* set dma config, ie. the format the encoder wants. */
 void mdp4_crtc_set_config(struct drm_crtc *crtc, uint32_t config)
 {
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index 5a8e3d6bcbff..1c8e330f8d98 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -179,16 +179,6 @@ static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
 	}
 }
 
-static void mdp4_preclose(struct msm_kms *kms, struct drm_file *file)
-{
-	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
-	struct msm_drm_private *priv = mdp4_kms->dev->dev_private;
-	unsigned i;
-
-	for (i = 0; i < priv->num_crtcs; i++)
-		mdp4_crtc_cancel_pending_flip(priv->crtcs[i], file);
-}
-
 static void mdp4_destroy(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
@@ -213,7 +203,6 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.wait_for_crtc_commit_done = mdp4_wait_for_crtc_commit_done,
 		.get_format      = mdp_get_format,
 		.round_pixclk    = mdp4_round_pixclk,
-		.preclose        = mdp4_preclose,
 		.destroy         = mdp4_destroy,
 	},
 	.set_irqmask         = mdp4_set_irqmask,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h
index d2c96ef431f4..9ec53b464662 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h
@@ -199,7 +199,6 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
 		enum mdp4_pipe pipe_id, bool private_plane);
 
 uint32_t mdp4_crtc_vblank(struct drm_crtc *crtc);
-void mdp4_crtc_cancel_pending_flip(struct drm_crtc *crtc, struct drm_file *file);
 void mdp4_crtc_set_config(struct drm_crtc *crtc, uint32_t config);
 void mdp4_crtc_set_intf(struct drm_crtc *crtc, enum mdp4_intf intf, int mixer);
 void mdp4_crtc_wait_for_commit_done(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index 20cee5ce4071..46682aa8870c 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -721,12 +721,6 @@ uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc)
 	return mdp5_crtc->vblank.irqmask;
 }
 
-void mdp5_crtc_cancel_pending_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	DBG("cancel: %p", file);
-	complete_flip(crtc, file);
-}
-
 void mdp5_crtc_set_pipeline(struct drm_crtc *crtc,
 		struct mdp5_interface *intf, struct mdp5_ctl *ctl)
 {
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index e115318402bd..5e4d16b399c7 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -117,16 +117,6 @@ static int mdp5_set_split_display(struct msm_kms *kms,
 		return mdp5_encoder_set_split_display(encoder, slave_encoder);
 }
 
-static void mdp5_preclose(struct msm_kms *kms, struct drm_file *file)
-{
-	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
-	struct msm_drm_private *priv = mdp5_kms->dev->dev_private;
-	unsigned i;
-
-	for (i = 0; i < priv->num_crtcs; i++)
-		mdp5_crtc_cancel_pending_flip(priv->crtcs[i], file);
-}
-
 static void mdp5_destroy(struct msm_kms *kms)
 {
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
@@ -164,7 +154,6 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.get_format      = mdp_get_format,
 		.round_pixclk    = mdp5_round_pixclk,
 		.set_split_display = mdp5_set_split_display,
-		.preclose        = mdp5_preclose,
 		.destroy         = mdp5_destroy,
 	},
 	.set_irqmask         = mdp5_set_irqmask,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index 00730ba08a60..9a25898239d3 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -211,7 +211,6 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
 uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
 
 int mdp5_crtc_get_lm(struct drm_crtc *crtc);
-void mdp5_crtc_cancel_pending_flip(struct drm_crtc *crtc, struct drm_file *file);
 void mdp5_crtc_set_pipeline(struct drm_crtc *crtc,
 		struct mdp5_interface *intf, struct mdp5_ctl *ctl);
 void mdp5_crtc_wait_for_commit_done(struct drm_crtc *crtc);
-- 
2.6.4

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

[PATCH 15/21] drm/omap: Nuke close hooks

[Daniel Vetter]

Again since the core takes care of this we can remove them. While at
it also remove the postclose hook, it's empty.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index dfafdb602ad2..e8b5080f6e2d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -787,33 +787,6 @@ static void dev_lastclose(struct drm_device *dev)
 	}
 }
 
-static void dev_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	struct drm_pending_event *event;
-	unsigned long flags;
-
-	DBG("preclose: dev=%p", dev);
-
-	/*
-	 * Unlink all pending CRTC events to make sure they won't be queued up
-	 * by a pending asynchronous commit.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	list_for_each_entry(event, &priv->commit.events, link) {
-		if (event->file_priv == file) {
-			file->event_space += event->event->length;
-			event->file_priv = NULL;
-		}
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-static void dev_postclose(struct drm_device *dev, struct drm_file *file)
-{
-	DBG("postclose: dev=%p, file=%p", dev, file);
-}
-
 static const struct vm_operations_struct omap_gem_vm_ops = {
 	.fault = omap_gem_fault,
 	.open = drm_gem_vm_open,
@@ -838,8 +811,6 @@ static struct drm_driver omap_drm_driver = {
 	.unload = dev_unload,
 	.open = dev_open,
 	.lastclose = dev_lastclose,
-	.preclose = dev_preclose,
-	.postclose = dev_postclose,
 	.set_busid = drm_platform_set_busid,
 	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = omap_irq_enable_vblank,
-- 
2.6.4

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

[PATCH 16/21] drm/rcar: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 20 --------------------
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  2 --
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 10 ----------
 3 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 88a4b706be16..4ec80ae1fa99 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -282,26 +282,6 @@ static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc)
  * Page Flip
  */
 
-void rcar_du_crtc_cancel_page_flip(struct rcar_du_crtc *rcrtc,
-				   struct drm_file *file)
-{
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = rcrtc->crtc.dev;
-	unsigned long flags;
-
-	/* Destroy the pending vertical blanking event associated with the
-	 * pending page flip, if any, and disable vertical blanking interrupts.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = rcrtc->event;
-	if (event && event->base.file_priv == file) {
-		rcrtc->event = NULL;
-		event->base.destroy(&event->base);
-		drm_crtc_vblank_put(&rcrtc->crtc);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 static void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc)
 {
 	struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 4b95d9d08c49..2bbe3f5aab65 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -67,8 +67,6 @@ enum rcar_du_output {
 
 int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index);
 void rcar_du_crtc_enable_vblank(struct rcar_du_crtc *rcrtc, bool enable);
-void rcar_du_crtc_cancel_page_flip(struct rcar_du_crtc *rcrtc,
-				   struct drm_file *file);
 void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
 void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 40422f6b645e..0bb2b31555bf 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -220,15 +220,6 @@ done:
 	return ret;
 }
 
-static void rcar_du_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct rcar_du_device *rcdu = dev->dev_private;
-	unsigned int i;
-
-	for (i = 0; i < rcdu->num_crtcs; ++i)
-		rcar_du_crtc_cancel_page_flip(&rcdu->crtcs[i], file);
-}
-
 static void rcar_du_lastclose(struct drm_device *dev)
 {
 	struct rcar_du_device *rcdu = dev->dev_private;
@@ -271,7 +262,6 @@ static struct drm_driver rcar_du_driver = {
 				| DRIVER_ATOMIC,
 	.load			= rcar_du_load,
 	.unload			= rcar_du_unload,
-	.preclose		= rcar_du_preclose,
 	.lastclose		= rcar_du_lastclose,
 	.set_busid		= drm_platform_set_busid,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 17/21] drm/shmob: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 20 --------------------
 drivers/gpu/drm/shmobile/shmob_drm_drv.c  |  8 --------
 2 files changed, 28 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index b80802f55143..de7959a60774 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -438,26 +438,6 @@ static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
 	.mode_set_base = shmob_drm_crtc_mode_set_base,
 };
 
-void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc *scrtc,
-				     struct drm_file *file)
-{
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = scrtc->crtc.dev;
-	unsigned long flags;
-
-	/* Destroy the pending vertical blanking event associated with the
-	 * pending page flip, if any, and disable vertical blanking interrupts.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = scrtc->event;
-	if (event && event->base.file_priv == file) {
-		scrtc->event = NULL;
-		event->base.destroy(&event->base);
-		drm_vblank_put(dev, 0);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)
 {
 	struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 04e66e3751b4..7700ff172079 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -200,13 +200,6 @@ done:
 	return ret;
 }
 
-static void shmob_drm_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct shmob_drm_device *sdev = dev->dev_private;
-
-	shmob_drm_crtc_cancel_page_flip(&sdev->crtc, file);
-}
-
 static irqreturn_t shmob_drm_irq(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -266,7 +259,6 @@ static struct drm_driver shmob_drm_driver = {
 				| DRIVER_PRIME,
 	.load			= shmob_drm_load,
 	.unload			= shmob_drm_unload,
-	.preclose		= shmob_drm_preclose,
 	.set_busid		= drm_platform_set_busid,
 	.irq_handler		= shmob_drm_irq,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
-- 
2.6.4

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

[PATCH 18/21] drm/tegra: Stop cancelling page flip events

[Daniel Vetter]

The core takes care of that now.

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Terje Bergström <tbergstrom@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/tegra/dc.c  | 17 -----------------
 drivers/gpu/drm/tegra/drm.c |  3 ---
 2 files changed, 20 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index dde6f208c347..fb2b4b0271a2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -988,23 +988,6 @@ static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 	spin_unlock_irqrestore(&drm->event_lock, flags);
 }
 
-void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct tegra_dc *dc = to_tegra_dc(crtc);
-	struct drm_device *drm = crtc->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&drm->event_lock, flags);
-
-	if (dc->event && dc->event->base.file_priv == file) {
-		dc->event->base.destroy(&dc->event->base);
-		drm_crtc_vblank_put(crtc);
-		dc->event = NULL;
-	}
-
-	spin_unlock_irqrestore(&drm->event_lock, flags);
-}
-
 static void tegra_dc_destroy(struct drm_crtc *crtc)
 {
 	drm_crtc_cleanup(crtc);
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index c5c856a0879d..021d0e1398fb 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -860,9 +860,6 @@ static void tegra_drm_preclose(struct drm_device *drm, struct drm_file *file)
 	struct tegra_drm_context *context, *tmp;
 	struct drm_crtc *crtc;
 
-	list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
-		tegra_dc_cancel_page_flip(crtc, file);
-
 	list_for_each_entry_safe(context, tmp, &fpriv->contexts, list)
 		tegra_drm_context_free(context);
 
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 19/21] drm/tilcdc: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/shmobile/shmob_drm_crtc.h |  2 --
 drivers/gpu/drm/tegra/drm.h               |  1 -
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c      | 20 --------------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c       |  8 --------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h       |  1 -
 5 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
index eddad6dcc88a..38ed4ff8aaf2 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
@@ -47,8 +47,6 @@ struct shmob_drm_connector {
 
 int shmob_drm_crtc_create(struct shmob_drm_device *sdev);
 void shmob_drm_crtc_enable_vblank(struct shmob_drm_device *sdev, bool enable);
-void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc *scrtc,
-				     struct drm_file *file);
 void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc);
 void shmob_drm_crtc_suspend(struct shmob_drm_crtc *scrtc);
 void shmob_drm_crtc_resume(struct shmob_drm_crtc *scrtc);
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index c088f2f67eda..8a10f5b7d9dc 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -195,7 +195,6 @@ struct tegra_dc_window {
 u32 tegra_dc_get_vblank_counter(struct tegra_dc *dc);
 void tegra_dc_enable_vblank(struct tegra_dc *dc);
 void tegra_dc_disable_vblank(struct tegra_dc *dc);
-void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 void tegra_dc_commit(struct tegra_dc *dc);
 int tegra_dc_state_setup_clock(struct tegra_dc *dc,
 			       struct drm_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 7d07733bdc86..4802da8e6d6f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -662,26 +662,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 	return IRQ_HANDLED;
 }
 
-void tilcdc_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = crtc->dev;
-	unsigned long flags;
-
-	/* Destroy the pending vertical blanking event associated with the
-	 * pending page flip, if any, and disable vertical blanking interrupts.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = tilcdc_crtc->event;
-	if (event && event->base.file_priv == file) {
-		tilcdc_crtc->event = NULL;
-		event->base.destroy(&event->base);
-		drm_vblank_put(dev, 0);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 {
 	struct tilcdc_crtc *tilcdc_crtc;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4ddb21e7f52f..b39ba213e303 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -349,13 +349,6 @@ fail_free_priv:
 	return ret;
 }
 
-static void tilcdc_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct tilcdc_drm_private *priv = dev->dev_private;
-
-	tilcdc_crtc_cancel_page_flip(priv->crtc, file);
-}
-
 static void tilcdc_lastclose(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
@@ -556,7 +549,6 @@ static struct drm_driver tilcdc_driver = {
 	.driver_features    = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET,
 	.load               = tilcdc_load,
 	.unload             = tilcdc_unload,
-	.preclose           = tilcdc_preclose,
 	.lastclose          = tilcdc_lastclose,
 	.set_busid          = drm_platform_set_busid,
 	.irq_handler        = tilcdc_irq,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index e863ad0d26fe..66105d8dc620 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -163,7 +163,6 @@ struct tilcdc_panel_info {
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
 struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev);
-void tilcdc_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc);
 void tilcdc_crtc_update_clk(struct drm_crtc *crtc);
 void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 20/21] drm/vc4: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>enter the commit message for your changes. Lines starting
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 20 --------------------
 drivers/gpu/drm/vc4/vc4_drv.c  | 10 ----------
 2 files changed, 30 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 018145e0b87d..937409792b97 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -593,26 +593,6 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {
 	.atomic_flush = vc4_crtc_atomic_flush,
 };
 
-/* Frees the page flip event when the DRM device is closed with the
- * event still outstanding.
- */
-void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	if (vc4_crtc->event && vc4_crtc->event->base.file_priv == file) {
-		vc4_crtc->event->base.destroy(&vc4_crtc->event->base);
-		drm_crtc_vblank_put(crtc);
-		vc4_crtc->event = NULL;
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 static const struct vc4_crtc_data pv0_data = {
 	.hvs_channel = 0,
 	.encoder0_type = VC4_ENCODER_TYPE_DSI0,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index f1655fff8425..b7d2ff0e6e1f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -43,14 +43,6 @@ void __iomem *vc4_ioremap_regs(struct platform_device *dev, int index)
 	return map;
 }
 
-static void vc4_drm_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct drm_crtc *crtc;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		vc4_cancel_page_flip(crtc, file);
-}
-
 static void vc4_lastclose(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -91,8 +83,6 @@ static struct drm_driver vc4_drm_driver = {
 			    DRIVER_HAVE_IRQ |
 			    DRIVER_PRIME),
 	.lastclose = vc4_lastclose,
-	.preclose = vc4_drm_preclose,
-
 	.irq_handler = vc4_irq,
 	.irq_preinstall = vc4_irq_preinstall,
 	.irq_postinstall = vc4_irq_postinstall,
-- 
2.6.4

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

[PATCH 21/21] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

Cc: Thomas Hellström <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h         |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 10 ---------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 38 -----------------------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
 4 files changed, 51 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 080865ec2bae..4c734d087d7f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
 extern struct platform_driver vc4_crtc_driver;
 int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
 void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
-void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
 
 /* vc4_debugfs.c */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c49812b80dd0..c04c4d1e2f3e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
 	return 0;
 }
 
-static void vmw_preclose(struct drm_device *dev,
-			 struct drm_file *file_priv)
-{
-	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
-	struct vmw_private *dev_priv = vmw_priv(dev);
-
-	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
-}
-
 static void vmw_postclose(struct drm_device *dev,
 			 struct drm_file *file_priv)
 {
@@ -1500,7 +1491,6 @@ static struct drm_driver driver = {
 	.master_set = vmw_master_set,
 	.master_drop = vmw_master_drop,
 	.open = vmw_driver_open,
-	.preclose = vmw_preclose,
 	.postclose = vmw_postclose,
 	.set_busid = drm_pci_set_busid,
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index e0edf149d9d5..55dc3e4754df 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -808,44 +808,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 }
 
 /**
- * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
- *
- * @fman: Pointer to a struct vmw_fence_manager
- * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
- * with pointers to a struct drm_file object about to be closed.
- *
- * This function removes all pending fence events with references to a
- * specific struct drm_file object about to be closed. The caller is required
- * to pass a list of all struct vmw_event_fence_action objects with such
- * events attached. This function is typically called before the
- * struct drm_file object's event management is taken down.
- */
-void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				struct list_head *event_list)
-{
-	struct vmw_event_fence_action *eaction;
-	struct drm_pending_event *event;
-	unsigned long irq_flags;
-
-	while (1) {
-		spin_lock_irqsave(&fman->lock, irq_flags);
-		if (list_empty(event_list))
-			goto out_unlock;
-		eaction = list_first_entry(event_list,
-					   struct vmw_event_fence_action,
-					   fpriv_head);
-		list_del_init(&eaction->fpriv_head);
-		event = eaction->event;
-		eaction->event = NULL;
-		spin_unlock_irqrestore(&fman->lock, irq_flags);
-		event->destroy(event);
-	}
-out_unlock:
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-}
-
-
-/**
  * vmw_event_fence_action_seq_passed
  *
  * @action: The struct vmw_fence_action embedded in a struct
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
index 8be6c29f5eb5..83ae301ee141 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
@@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 				     struct drm_file *file_priv);
 extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv);
-extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				       struct list_head *event_list);
 extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
 					struct vmw_fence_obj *fence,
 					struct drm_pending_event *event,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 21/21] drm/vmwgfx: Nuke preclose hook

[Thomas Hellstrom]

On 01/08/2016 09:36 PM, Daniel Vetter wrote:
> Again since the drm core takes care of event unlinking/disarming this
> is now just needless code.
>
> Cc: Thomas Hellström <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Hmm,

IIRC this is actually a list of events that core drm is not aware of
yet. They sit on this list waiting for a fence to pass and are then
transferred to core drm....

/Thomas


> ---
>  drivers/gpu/drm/vc4/vc4_drv.h         |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 10 ---------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 38 -----------------------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
>  4 files changed, 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 080865ec2bae..4c734d087d7f 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
>  extern struct platform_driver vc4_crtc_driver;
>  int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
>  void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
> -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
>  int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
>  
>  /* vc4_debugfs.c */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index c49812b80dd0..c04c4d1e2f3e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void vmw_preclose(struct drm_device *dev,
> -			 struct drm_file *file_priv)
> -{
> -	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
> -	struct vmw_private *dev_priv = vmw_priv(dev);
> -
> -	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
> -}
> -
>  static void vmw_postclose(struct drm_device *dev,
>  			 struct drm_file *file_priv)
>  {
> @@ -1500,7 +1491,6 @@ static struct drm_driver driver = {
>  	.master_set = vmw_master_set,
>  	.master_drop = vmw_master_drop,
>  	.open = vmw_driver_open,
> -	.preclose = vmw_preclose,
>  	.postclose = vmw_postclose,
>  	.set_busid = drm_pci_set_busid,
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index e0edf149d9d5..55dc3e4754df 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -808,44 +808,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  }
>  
>  /**
> - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
> - *
> - * @fman: Pointer to a struct vmw_fence_manager
> - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
> - * with pointers to a struct drm_file object about to be closed.
> - *
> - * This function removes all pending fence events with references to a
> - * specific struct drm_file object about to be closed. The caller is required
> - * to pass a list of all struct vmw_event_fence_action objects with such
> - * events attached. This function is typically called before the
> - * struct drm_file object's event management is taken down.
> - */
> -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				struct list_head *event_list)
> -{
> -	struct vmw_event_fence_action *eaction;
> -	struct drm_pending_event *event;
> -	unsigned long irq_flags;
> -
> -	while (1) {
> -		spin_lock_irqsave(&fman->lock, irq_flags);
> -		if (list_empty(event_list))
> -			goto out_unlock;
> -		eaction = list_first_entry(event_list,
> -					   struct vmw_event_fence_action,
> -					   fpriv_head);
> -		list_del_init(&eaction->fpriv_head);
> -		event = eaction->event;
> -		eaction->event = NULL;
> -		spin_unlock_irqrestore(&fman->lock, irq_flags);
> -		event->destroy(event);
> -	}
> -out_unlock:
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -}
> -
> -
> -/**
>   * vmw_event_fence_action_seq_passed
>   *
>   * @action: The struct vmw_fence_action embedded in a struct
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> index 8be6c29f5eb5..83ae301ee141 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  				     struct drm_file *file_priv);
>  extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
>  				 struct drm_file *file_priv);
> -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				       struct list_head *event_list);
>  extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
>  					struct vmw_fence_obj *fence,
>  					struct drm_pending_event *event,

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/21] drm: Add functions to setup/tear down drm_events.

[Alex Deucher]

On Fri, Jan 8, 2016 at 3:36 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> An attempt at not spreading out the file_priv->event_space stuff out
> quite so far and wide.  And I think fixes something in ipp_get_event()
> that is broken (or if they are doing something more weird/subtle, then
> breaks it in a fun way).
>
> Based upon a patch from Rob Clark, rebased and polished.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 44 ++++++++---------------------
>  drivers/gpu/drm/drm_crtc.c   | 36 +++++++-----------------
>  drivers/gpu/drm/drm_fops.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drmP.h           |  7 ++++-
>  4 files changed, 94 insertions(+), 60 deletions(-)
>
<snip>
> +
> +/**
> + * drm_event_reserve_init - init a DRM event and reserve space for it
> + * @dev: DRM device
> + * @file_priv: DRM file private data
> + * @p: tracking structure for the pending event
> + * @e: actual event data to deliver to userspace
> + *
> + * This function prepares the passed in even for eventual deliver. If the event

Typo even -> event, deliver -> delivery

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

Re: [PATCH 00/21] drm_event cleanup

[Alex Deucher]

On Fri, Jan 8, 2016 at 3:36 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> This patch series is inspired by a WIP patch from Rob Clark to consolidate the
> drm_event handling a bit. I've went a bit further and also moved the pending
> event handling and unlinking into the core, which allows us to nuke a bunch of
> code from drivers who all copypasted this themselves. Plus fix up all the others
> who failed to handle this correctly.
>
> Net -500 lines of code, plus kerneldoc for drm_fops.c and all the new functions
> as bonus.
>
> Comments and review highly welcome as usual.

Other than the typo in the comments in patch 2, this series looks good
to me.  With that fixed, the series is:

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> Cheers, Daniel
>
> Daniel Vetter (21):
>   drm: kerneldoc for drm_fops.c
>   drm: Add functions to setup/tear down drm_events.
>   drm/exynos: Use the new event init/free functions
>   drm/vmwgfx: Use the new event init/free functions
>   drm: Create drm_send_event helpers
>   drm/fsl: Remove preclose hook
>   drm/armada: Remove NULL open/pre/postclose hooks
>   drm/gma500: Remove empty preclose hook
>   drm: Clean up pending events in the core
>   drm/i915: Nuke intel_modeset_preclose
>   drm/atmel: Nuke preclose
>   drm/exynos: Remove event cancelling from postclose
>   drm/imx: Unconfuse preclose logic
>   drm/msm: Nuke preclose hooks
>   drm/omap: Nuke close hooks
>   drm/rcar: Nuke preclose hook
>   drm/shmob: Nuke preclose hook
>   drm/tegra: Stop cancelling page flip events
>   drm/tilcdc: Nuke preclose hook
>   drm/vc4: Nuke preclose hook
>   drm/vmwgfx: Nuke preclose hook
>
>  Documentation/DocBook/gpu.tmpl                 |  48 +----
>  drivers/gpu/drm/armada/armada_drv.c            |   3 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  18 --
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  10 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |   3 -
>  drivers/gpu/drm/drm_atomic.c                   |  44 ++---
>  drivers/gpu/drm/drm_crtc.c                     |  36 +---
>  drivers/gpu/drm/drm_fops.c                     | 259 ++++++++++++++++++++++---
>  drivers/gpu/drm/drm_irq.c                      |   7 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c        |  14 --
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c        |  36 +---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c        |  28 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c      |   5 -
>  drivers/gpu/drm/gma500/psb_drv.c               |   9 -
>  drivers/gpu/drm/i915/i915_dma.c                |   2 -
>  drivers/gpu/drm/i915/intel_display.c           |  21 --
>  drivers/gpu/drm/i915/intel_drv.h               |   1 -
>  drivers/gpu/drm/imx/imx-drm-core.c             |  13 --
>  drivers/gpu/drm/imx/ipuv3-crtc.c               |   4 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c       |   7 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c        |  11 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h        |   1 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c       |   6 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c        |  11 --
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h        |   1 -
>  drivers/gpu/drm/omapdrm/omap_drv.c             |  29 ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c         |  20 --
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h         |   2 -
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c          |  10 -
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c      |  20 --
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.h      |   2 -
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c       |   8 -
>  drivers/gpu/drm/tegra/dc.c                     |  17 --
>  drivers/gpu/drm/tegra/drm.c                    |   3 -
>  drivers/gpu/drm/tegra/drm.h                    |   1 -
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c           |  20 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c            |   8 -
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h            |   1 -
>  drivers/gpu/drm/vc4/vc4_crtc.c                 |  20 --
>  drivers/gpu/drm/vc4/vc4_drv.c                  |  10 -
>  drivers/gpu/drm/vc4/vc4_drv.h                  |   1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c            |  10 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c          |  73 +------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h          |   2 -
>  include/drm/drmP.h                             |  26 ++-
>  45 files changed, 299 insertions(+), 582 deletions(-)
>
> --
> 2.6.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 21/21] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

On Fri, Jan 8, 2016 at 9:53 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> On 01/08/2016 09:36 PM, Daniel Vetter wrote:
>> Again since the drm core takes care of event unlinking/disarming this
>> is now just needless code.
>>
>> Cc: Thomas Hellström <thellstrom@vmware.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Hmm,
>
> IIRC this is actually a list of events that core drm is not aware of
> yet. They sit on this list waiting for a fence to pass and are then
> transferred to core drm....

Yes I know. Earlier patches in the series extract new core functions
to setup/tear down such events and send them out, which is what's
needed to make this trick possible. Exynos similarly uses events
similarly, and is also converted. Same for nouveau it seems, but there
the code doesn't use the reserve/send split, so I'm unclear
how/whether at all it correctly handles this race.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Add functions to setup/tear down drm_events.

[Daniel Vetter]

An attempt at not spreading out the file_priv->event_space stuff out
quite so far and wide.  And I think fixes something in ipp_get_event()
that is broken (or if they are doing something more weird/subtle, then
breaks it in a fun way).

Based upon a patch from Rob Clark, rebased and polished.

v2: Spelling fixes (Alex).

Cc: Alex Deucher <alexdeucher@gmail.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 44 ++++++++---------------------
 drivers/gpu/drm/drm_crtc.c   | 36 +++++++-----------------
 drivers/gpu/drm/drm_fops.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h           |  7 ++++-
 4 files changed, 94 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f74193885f1..8fb469c4e4b8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1347,44 +1347,23 @@ static struct drm_pending_vblank_event *create_vblank_event(
 		struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data)
 {
 	struct drm_pending_vblank_event *e = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-	if (file_priv->event_space < sizeof e->event) {
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		goto out;
-	}
-	file_priv->event_space -= sizeof e->event;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
+	int ret;
 
 	e = kzalloc(sizeof *e, GFP_KERNEL);
-	if (e == NULL) {
-		spin_lock_irqsave(&dev->event_lock, flags);
-		file_priv->event_space += sizeof e->event;
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		goto out;
-	}
+	if (!e)
+		return NULL;
 
 	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
-	e->event.base.length = sizeof e->event;
+	e->event.base.length = sizeof(e->event);
 	e->event.user_data = user_data;
-	e->base.event = &e->event.base;
-	e->base.file_priv = file_priv;
-	e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
-
-out:
-	return e;
-}
 
-static void destroy_vblank_event(struct drm_device *dev,
-		struct drm_file *file_priv, struct drm_pending_vblank_event *e)
-{
-	unsigned long flags;
+	ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
+	if (ret) {
+		kfree(e);
+		return NULL;
+	}
 
-	spin_lock_irqsave(&dev->event_lock, flags);
-	file_priv->event_space += sizeof e->event;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-	kfree(e);
+	return e;
 }
 
 static int atomic_set_prop(struct drm_atomic_state *state,
@@ -1646,8 +1625,7 @@ out:
 			if (!crtc_state->event)
 				continue;
 
-			destroy_vblank_event(dev, file_priv,
-					     crtc_state->event);
+			drm_event_cancel_free(dev, &crtc_state->event->base);
 		}
 	}
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1e75a145834a..60a4184d41b7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5264,7 +5264,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb = NULL;
 	struct drm_pending_vblank_event *e = NULL;
-	unsigned long flags;
 	int ret = -EINVAL;
 
 	if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
@@ -5315,41 +5314,26 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	}
 
 	if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
-		ret = -ENOMEM;
-		spin_lock_irqsave(&dev->event_lock, flags);
-		if (file_priv->event_space < sizeof(e->event)) {
-			spin_unlock_irqrestore(&dev->event_lock, flags);
-			goto out;
-		}
-		file_priv->event_space -= sizeof(e->event);
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-
-		e = kzalloc(sizeof(*e), GFP_KERNEL);
-		if (e == NULL) {
-			spin_lock_irqsave(&dev->event_lock, flags);
-			file_priv->event_space += sizeof(e->event);
-			spin_unlock_irqrestore(&dev->event_lock, flags);
+		e = kzalloc(sizeof *e, GFP_KERNEL);
+		if (!e) {
+			ret = -ENOMEM;
 			goto out;
 		}
-
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
 		e->event.user_data = page_flip->user_data;
-		e->base.event = &e->event.base;
-		e->base.file_priv = file_priv;
-		e->base.destroy =
-			(void (*) (struct drm_pending_event *)) kfree;
+		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
+		if (ret) {
+			kfree(e);
+			goto out;
+		}
 	}
 
 	crtc->primary->old_fb = crtc->primary->fb;
 	ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
 	if (ret) {
-		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
-			spin_lock_irqsave(&dev->event_lock, flags);
-			file_priv->event_space += sizeof(e->event);
-			spin_unlock_irqrestore(&dev->event_lock, flags);
-			kfree(e);
-		}
+		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT)
+			drm_event_cancel_free(dev, &e->base);
 		/* Keep the old fb, don't unref it. */
 		crtc->primary->old_fb = NULL;
 	} else {
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 73075a1fa380..476408b638e3 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -672,3 +672,70 @@ unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait)
 	return mask;
 }
 EXPORT_SYMBOL(drm_poll);
+
+/**
+ * drm_event_reserve_init - init a DRM event and reserve space for it
+ * @dev: DRM device
+ * @file_priv: DRM file private data
+ * @p: tracking structure for the pending event
+ * @e: actual event data to deliver to userspace
+ *
+ * This function prepares the passed in event for eventual delivery. If the event
+ * doesn't get delivered (because the IOCTL fails later on, before queuing up
+ * anything) then the even must be cancelled and freed using
+ * drm_event_cancel_free().
+ *
+ * If callers embedded @p into a larger structure it must be allocated with
+ * kmalloc and @p must be the first member element.
+ *
+ * RETURNS:
+ *
+ * 0 on success or a negative error code on failure.
+ */
+int drm_event_reserve_init(struct drm_device *dev,
+			   struct drm_file *file_priv,
+			   struct drm_pending_event *p,
+			   struct drm_event *e)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	if (file_priv->event_space < e->length) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	file_priv->event_space -= e->length;
+
+	p->event = e;
+	p->file_priv = file_priv;
+
+	/* we *could* pass this in as arg, but everyone uses kfree: */
+	p->destroy = (void (*) (struct drm_pending_event *)) kfree;
+
+out:
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(drm_event_reserve_init);
+
+/**
+ * drm_event_cancel_free - free a DRM event and release it's space
+ * @dev: DRM device
+ * @p: tracking structure for the pending event
+ *
+ * This function frees the event @p initialized with drm_event_reserve_init()
+ * and releases any allocated space.
+ */
+void drm_event_cancel_free(struct drm_device *dev,
+			   struct drm_pending_event *p)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&dev->event_lock, flags);
+	p->file_priv->event_space += p->event->length;
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+	p->destroy(p);
+}
+EXPORT_SYMBOL(drm_event_cancel_free);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fc9b9ad5b089..ad4d0a31294d 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -925,8 +925,13 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
 		 size_t count, loff_t *offset);
 int drm_release(struct inode *inode, struct file *filp);
 int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
-
 unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
+int drm_event_reserve_init(struct drm_device *dev,
+			   struct drm_file *file_priv,
+			   struct drm_pending_event *p,
+			   struct drm_event *e);
+void drm_event_cancel_free(struct drm_device *dev,
+			   struct drm_pending_event *p);
 
 /* Misc. IOCTL support (drm_ioctl.c) */
 int drm_noop(struct drm_device *dev, void *data,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm: Create drm_send_event helpers

[Daniel Vetter]

Use them in the core vblank code and exynos/vmwgfx drivers.

Note that the difference between wake_up_all and _interruptible in
vmwgfx doesn't matter since the only waiter is the core code in
drm_fops.c. And that is interruptible.

v2: Adjust existing kerneldoc too.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com> (v1)
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fops.c              | 42 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_irq.c               |  7 ++----
 drivers/gpu/drm/exynos/exynos_drm_g2d.c |  5 +---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c |  5 +---
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c   |  3 +--
 include/drm/drmP.h                      |  2 ++
 6 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 476408b638e3..d85af1b2a238 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -683,7 +683,9 @@ EXPORT_SYMBOL(drm_poll);
  * This function prepares the passed in event for eventual delivery. If the event
  * doesn't get delivered (because the IOCTL fails later on, before queuing up
  * anything) then the even must be cancelled and freed using
- * drm_event_cancel_free().
+ * drm_event_cancel_free(). Successfully initialized events should be sent out
+ * using drm_send_event() or drm_send_event_locked() to signal completion of the
+ * asynchronous event to userspace.
  *
  * If callers embedded @p into a larger structure it must be allocated with
  * kmalloc and @p must be the first member element.
@@ -739,3 +741,41 @@ void drm_event_cancel_free(struct drm_device *dev,
 	p->destroy(p);
 }
 EXPORT_SYMBOL(drm_event_cancel_free);
+
+/**
+ * drm_send_event_locked - send DRM event to file descriptor
+ * @dev: DRM device
+ * @e: DRM event to deliver
+ *
+ * This function sends the event @e, initialized with drm_event_reserve_init(),
+ * to its associated userspace DRM file. Callers must already hold
+ * dev->event_lock, see drm_send_event() for the unlocked version.
+ */
+void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
+{
+	assert_spin_locked(&dev->event_lock);
+
+	list_add_tail(&e->link,
+		      &e->file_priv->event_list);
+	wake_up_interruptible(&e->file_priv->event_wait);
+}
+EXPORT_SYMBOL(drm_send_event_locked);
+
+/**
+ * drm_send_event - send DRM event to file descriptor
+ * @dev: DRM device
+ * @e: DRM event to deliver
+ *
+ * This function sends the event @e, initialized with drm_event_reserve_init(),
+ * to its associated userspace DRM file. This function acquires dev->event_lock,
+ * see drm_send_event_locked() for callers which already hold this lock.
+ */
+void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev->event_lock, irqflags);
+	drm_send_event_locked(dev, e);
+	spin_unlock_irqrestore(&dev->event_lock, irqflags);
+}
+EXPORT_SYMBOL(drm_send_event);
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a70b29909974..3fe8dbff6058 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -983,15 +983,12 @@ static void send_vblank_event(struct drm_device *dev,
 		struct drm_pending_vblank_event *e,
 		unsigned long seq, struct timeval *now)
 {
-	assert_spin_locked(&dev->event_lock);
-
 	e->event.sequence = seq;
 	e->event.tv_sec = now->tv_sec;
 	e->event.tv_usec = now->tv_usec;
 
-	list_add_tail(&e->base.link,
-		      &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
+	drm_send_event_locked(dev, &e->base);
+
 	trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
 					 e->event.sequence);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 82e7f95dfed9..db56c8259f18 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -893,10 +893,7 @@ static void g2d_finish_event(struct g2d_data *g2d, u32 cmdlist_no)
 	e->event.tv_usec = now.tv_usec;
 	e->event.cmdlist_no = cmdlist_no;
 
-	spin_lock_irqsave(&drm_dev->event_lock, flags);
-	list_move_tail(&e->base.link, &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
-	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+	drm_send_event(dev, &e->base);
 }
 
 static irqreturn_t g2d_irq_handler(int irq, void *dev_id)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c8819c05e2dd..1f6a6c1881d6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1520,10 +1520,7 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv,
 	for_each_ipp_ops(i)
 		e->event.buf_id[i] = tbuf_id[i];
 
-	spin_lock_irqsave(&drm_dev->event_lock, flags);
-	list_move_tail(&e->base.link, &e->base.file_priv->event_list);
-	wake_up_interruptible(&e->base.file_priv->event_wait);
-	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+	drm_send_event(dev, &e->base);
 	mutex_unlock(&c_node->event_lock);
 
 	DRM_DEBUG_KMS("done cmd[%d]prop_id[%d]buf_id[%d]\n",
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index eda93bf52a6e..e0edf149d9d5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -880,9 +880,8 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
 	}
 
 	list_del_init(&eaction->fpriv_head);
-	list_add_tail(&eaction->event->link, &file_priv->event_list);
 	eaction->event = NULL;
-	wake_up_all(&file_priv->event_wait);
+	drm_send_event_locked(dev, eaction->event);
 	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
 }
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ad4d0a31294d..ae73abf5c2cf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -932,6 +932,8 @@ int drm_event_reserve_init(struct drm_device *dev,
 			   struct drm_event *e);
 void drm_event_cancel_free(struct drm_device *dev,
 			   struct drm_pending_event *p);
+void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e);
+void drm_send_event(struct drm_device *dev, struct drm_pending_event *e);
 
 /* Misc. IOCTL support (drm_ioctl.c) */
 int drm_noop(struct drm_device *dev, void *data,
-- 
2.6.4

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

[PATCH] drm: Clean up pending events in the core

[Daniel Vetter]

There's really no reason to not do so, instead of replicating this
for every use-case and every driver. Now we can't just nuke the events,
since that would still mean that all drm_event users would need to know
when that has happened, since calling e.g. drm_send_event isn't allowed
any more. Instead just unlink them from the file, and detect this case
and handle it appropriately in all functions.

v2: Adjust existing kerneldoc too.

Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fops.c | 35 ++++++++++++++++++++++++++---------
 include/drm/drmP.h         |  2 ++
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index d85af1b2a238..d32b24c74e08 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -264,6 +264,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 	INIT_LIST_HEAD(&priv->fbs);
 	mutex_init(&priv->fbs_lock);
 	INIT_LIST_HEAD(&priv->blobs);
+	INIT_LIST_HEAD(&priv->pending_event_list);
 	INIT_LIST_HEAD(&priv->event_list);
 	init_waitqueue_head(&priv->event_wait);
 	priv->event_space = 4096; /* set aside 4k for event buffer */
@@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file *file_priv)
 {
 	struct drm_device *dev = file_priv->minor->dev;
 	struct drm_pending_event *e, *et;
-	struct drm_pending_vblank_event *v, *vt;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	/* Remove pending flips */
-	list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
-		if (v->base.file_priv == file_priv) {
-			list_del(&v->base.link);
-			drm_vblank_put(dev, v->pipe);
-			v->base.destroy(&v->base);
-		}
+	/* Unlink pending events */
+	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
+				 pending_link) {
+		list_del(&e->pending_link);
+		e->file_priv = NULL;
+	}
 
 	/* Remove unconsumed events */
 	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
@@ -712,6 +711,7 @@ int drm_event_reserve_init(struct drm_device *dev,
 	file_priv->event_space -= e->length;
 
 	p->event = e;
+	list_add(&p->pending_link, &file_priv->pending_event_list);
 	p->file_priv = file_priv;
 
 	/* we *could* pass this in as arg, but everyone uses kfree: */
@@ -736,7 +736,10 @@ void drm_event_cancel_free(struct drm_device *dev,
 {
 	unsigned long flags;
 	spin_lock_irqsave(&dev->event_lock, flags);
-	p->file_priv->event_space += p->event->length;
+	if (p->file_priv) {
+		p->file_priv->event_space += p->event->length;
+		list_del(&p->pending_link);
+	}
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 	p->destroy(p);
 }
@@ -750,11 +753,21 @@ EXPORT_SYMBOL(drm_event_cancel_free);
  * This function sends the event @e, initialized with drm_event_reserve_init(),
  * to its associated userspace DRM file. Callers must already hold
  * dev->event_lock, see drm_send_event() for the unlocked version.
+ *
+ * Note that the core will take care of unlinking and disarming events when the
+ * corresponding DRM file is closed. Drivers need to worry about that and can
+ * call this function upon completion of the asynchrnous work unconditionally.
  */
 void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
 {
 	assert_spin_locked(&dev->event_lock);
 
+	if (!e->file_priv) {
+		e->destroy(e);
+		return;
+	}
+
+	list_del(&e->pending_link);
 	list_add_tail(&e->link,
 		      &e->file_priv->event_list);
 	wake_up_interruptible(&e->file_priv->event_wait);
@@ -769,6 +782,10 @@ EXPORT_SYMBOL(drm_send_event_locked);
  * This function sends the event @e, initialized with drm_event_reserve_init(),
  * to its associated userspace DRM file. This function acquires dev->event_lock,
  * see drm_send_event_locked() for callers which already hold this lock.
+ *
+ * Note that the core will take care of unlinking and disarming events when the
+ * corresponding DRM file is closed. Drivers need to worry about that and can
+ * call this function upon completion of the asynchrnous work unconditionally.
  */
 void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
 {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ae73abf5c2cf..3d78a7406d54 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -283,6 +283,7 @@ struct drm_ioctl_desc {
 struct drm_pending_event {
 	struct drm_event *event;
 	struct list_head link;
+	struct list_head pending_link;
 	struct drm_file *file_priv;
 	pid_t pid; /* pid of requester, no guarantee it's valid by the time
 		      we deliver the event, for tracing only */
@@ -346,6 +347,7 @@ struct drm_file {
 	struct list_head blobs;
 
 	wait_queue_head_t event_wait;
+	struct list_head pending_event_list;
 	struct list_head event_list;
 	int event_space;
 
-- 
2.6.4

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

Re: [PATCH 00/21] drm_event cleanup

[Daniel Vetter]

On Fri, Jan 08, 2016 at 09:36:32PM +0100, Daniel Vetter wrote:
> Hi all,
> 
> This patch series is inspired by a WIP patch from Rob Clark to consolidate the
> drm_event handling a bit. I've went a bit further and also moved the pending
> event handling and unlinking into the core, which allows us to nuke a bunch of
> code from drivers who all copypasted this themselves. Plus fix up all the others
> who failed to handle this correctly.
> 
> Net -500 lines of code, plus kerneldoc for drm_fops.c and all the new functions
> as bonus.
> 
> Comments and review highly welcome as usual.

What I've forgotten to mention: nouveau does something uncommon with
events around the usif notification. It looks fishy, since it neither
seems to handle file_priv disappearing nor does it properly reserve space
upfront for the event. But that's just my guess, I didn't really follow
what's going on there. Therefore I left fixing up nouveau (or just moving
it over to these new functions here) to someone else.
-Daniel

> 
> Cheers, Daniel
> 
> Daniel Vetter (21):
>   drm: kerneldoc for drm_fops.c
>   drm: Add functions to setup/tear down drm_events.
>   drm/exynos: Use the new event init/free functions
>   drm/vmwgfx: Use the new event init/free functions
>   drm: Create drm_send_event helpers
>   drm/fsl: Remove preclose hook
>   drm/armada: Remove NULL open/pre/postclose hooks
>   drm/gma500: Remove empty preclose hook
>   drm: Clean up pending events in the core
>   drm/i915: Nuke intel_modeset_preclose
>   drm/atmel: Nuke preclose
>   drm/exynos: Remove event cancelling from postclose
>   drm/imx: Unconfuse preclose logic
>   drm/msm: Nuke preclose hooks
>   drm/omap: Nuke close hooks
>   drm/rcar: Nuke preclose hook
>   drm/shmob: Nuke preclose hook
>   drm/tegra: Stop cancelling page flip events
>   drm/tilcdc: Nuke preclose hook
>   drm/vc4: Nuke preclose hook
>   drm/vmwgfx: Nuke preclose hook
> 
>  Documentation/DocBook/gpu.tmpl                 |  48 +----
>  drivers/gpu/drm/armada/armada_drv.c            |   3 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  18 --
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  10 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |   3 -
>  drivers/gpu/drm/drm_atomic.c                   |  44 ++---
>  drivers/gpu/drm/drm_crtc.c                     |  36 +---
>  drivers/gpu/drm/drm_fops.c                     | 259 ++++++++++++++++++++++---
>  drivers/gpu/drm/drm_irq.c                      |   7 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c        |  14 --
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c        |  36 +---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c        |  28 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c      |   5 -
>  drivers/gpu/drm/gma500/psb_drv.c               |   9 -
>  drivers/gpu/drm/i915/i915_dma.c                |   2 -
>  drivers/gpu/drm/i915/intel_display.c           |  21 --
>  drivers/gpu/drm/i915/intel_drv.h               |   1 -
>  drivers/gpu/drm/imx/imx-drm-core.c             |  13 --
>  drivers/gpu/drm/imx/ipuv3-crtc.c               |   4 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c       |   7 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c        |  11 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h        |   1 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c       |   6 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c        |  11 --
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h        |   1 -
>  drivers/gpu/drm/omapdrm/omap_drv.c             |  29 ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c         |  20 --
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h         |   2 -
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c          |  10 -
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c      |  20 --
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.h      |   2 -
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c       |   8 -
>  drivers/gpu/drm/tegra/dc.c                     |  17 --
>  drivers/gpu/drm/tegra/drm.c                    |   3 -
>  drivers/gpu/drm/tegra/drm.h                    |   1 -
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c           |  20 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c            |   8 -
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h            |   1 -
>  drivers/gpu/drm/vc4/vc4_crtc.c                 |  20 --
>  drivers/gpu/drm/vc4/vc4_drv.c                  |  10 -
>  drivers/gpu/drm/vc4/vc4_drv.h                  |   1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c            |  10 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c          |  73 +------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h          |   2 -
>  include/drm/drmP.h                             |  26 ++-
>  45 files changed, 299 insertions(+), 582 deletions(-)
> 
> -- 
> 2.6.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 21/21] drm/vmwgfx: Nuke preclose hook

[Thomas Hellstrom]

On 01/09/2016 11:43 AM, Daniel Vetter wrote:
> On Fri, Jan 8, 2016 at 9:53 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> On 01/08/2016 09:36 PM, Daniel Vetter wrote:
>>> Again since the drm core takes care of event unlinking/disarming this
>>> is now just needless code.
>>>
>>> Cc: Thomas Hellström <thellstrom@vmware.com>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> Hmm,
>>
>> IIRC this is actually a list of events that core drm is not aware of
>> yet. They sit on this list waiting for a fence to pass and are then
>> transferred to core drm....
> Yes I know. Earlier patches in the series extract new core functions
> to setup/tear down such events and send them out, which is what's
> needed to make this trick possible. Exynos similarly uses events
> similarly, and is also converted. Same for nouveau it seems, but there
> the code doesn't use the reserve/send split, so I'm unclear
> how/whether at all it correctly handles this race.
> -Daniel

Ah. Hmm I should've looked more closely at the rest of the series.

In any case, this particular patch leaves, from what I can tell, the
eaction fpriv list intact when it is later freed in postclose, which is
bad. Also each eaction is left with a dangling pointer to a freed
pending event which is also very bad since that pointer will be
dereferenced as soon as the fence's seqno has passed. So as far as i can
tell, this function needs to remain except for the event destruction.

Thinking of it, this must be a problem that is more general problem than
for vmwgfx only, I mean, unless the driver traverses all core pending
event list to find relevant pending events to process, something in the
driver must actually point to the pending event (be it a pointer in the
fence object or, as in the vmwgfx case, a pointer in the fence action
object) and that pointer must somehow be invalidated when the pending
event is freed...

Which also brings up a question, where are the pending events actually
destroyed? I can see they are unlinked in drm_fops.c.

/Thomas





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

Re: [PATCH 21/21] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

On Sun, Jan 10, 2016 at 9:52 PM, Thomas Hellstrom <thomas@shipmail.org> wrote:
> On 01/09/2016 11:43 AM, Daniel Vetter wrote:
>> On Fri, Jan 8, 2016 at 9:53 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>>> On 01/08/2016 09:36 PM, Daniel Vetter wrote:
>>>> Again since the drm core takes care of event unlinking/disarming this
>>>> is now just needless code.
>>>>
>>>> Cc: Thomas Hellström <thellstrom@vmware.com>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Hmm,
>>>
>>> IIRC this is actually a list of events that core drm is not aware of
>>> yet. They sit on this list waiting for a fence to pass and are then
>>> transferred to core drm....
>> Yes I know. Earlier patches in the series extract new core functions
>> to setup/tear down such events and send them out, which is what's
>> needed to make this trick possible. Exynos similarly uses events
>> similarly, and is also converted. Same for nouveau it seems, but there
>> the code doesn't use the reserve/send split, so I'm unclear
>> how/whether at all it correctly handles this race.
>> -Daniel
>
> Ah. Hmm I should've looked more closely at the rest of the series.
>
> In any case, this particular patch leaves, from what I can tell, the
> eaction fpriv list intact when it is later freed in postclose, which is
> bad. Also each eaction is left with a dangling pointer to a freed
> pending event which is also very bad since that pointer will be
> dereferenced as soon as the fence's seqno has passed. So as far as i can
> tell, this function needs to remain except for the event destruction.

Oops, I missed that part, it needs to go too. All other drivers look
for events to clean up from some other objects (for the crtc page flip
stuff) or are just outright confused, I didn't check vmwgfx code
carefully enough.

> Thinking of it, this must be a problem that is more general problem than
> for vmwgfx only, I mean, unless the driver traverses all core pending
> event list to find relevant pending events to process, something in the
> driver must actually point to the pending event (be it a pointer in the
> fence object or, as in the vmwgfx case, a pointer in the fence action
> object) and that pointer must somehow be invalidated when the pending
> event is freed...

This is exactly what this patch series attempts to do, by unlinking
events from the fpriv if that disappears. Driver code can still call
drm_even_send&friends (which are all rolled out in this series), they
will simply only free up the event and not try to send it out.

> Which also brings up a question, where are the pending events actually
> destroyed? I can see they are unlinked in drm_fops.c.

When the driver eventually calls drm_send_event. That way fpriv
disappearing is completely transparent to the driver.

I'll resend the vmwgfx patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

v2: I've completely missed eaction->fpriv_head and all the related
code. We need to nuke that too to avoid accidentally deferencing the
freed-up vmwgfx-private fpriv.

Cc: Thomas Hellström <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h         |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 10 --------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 -----------------------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
 4 files changed, 61 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 080865ec2bae..4c734d087d7f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
 extern struct platform_driver vc4_crtc_driver;
 int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
 void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
-void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
 
 /* vc4_debugfs.c */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c49812b80dd0..c04c4d1e2f3e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
 	return 0;
 }
 
-static void vmw_preclose(struct drm_device *dev,
-			 struct drm_file *file_priv)
-{
-	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
-	struct vmw_private *dev_priv = vmw_priv(dev);
-
-	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
-}
-
 static void vmw_postclose(struct drm_device *dev,
 			 struct drm_file *file_priv)
 {
@@ -1500,7 +1491,6 @@ static struct drm_driver driver = {
 	.master_set = vmw_master_set,
 	.master_drop = vmw_master_drop,
 	.open = vmw_driver_open,
-	.preclose = vmw_preclose,
 	.postclose = vmw_postclose,
 	.set_busid = drm_pci_set_busid,
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index e0edf149d9d5..bb95bd20d415 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -71,7 +71,6 @@ struct vmw_user_fence {
  */
 struct vmw_event_fence_action {
 	struct vmw_fence_action action;
-	struct list_head fpriv_head;
 
 	struct drm_pending_event *event;
 	struct vmw_fence_obj *fence;
@@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 }
 
 /**
- * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
- *
- * @fman: Pointer to a struct vmw_fence_manager
- * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
- * with pointers to a struct drm_file object about to be closed.
- *
- * This function removes all pending fence events with references to a
- * specific struct drm_file object about to be closed. The caller is required
- * to pass a list of all struct vmw_event_fence_action objects with such
- * events attached. This function is typically called before the
- * struct drm_file object's event management is taken down.
- */
-void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				struct list_head *event_list)
-{
-	struct vmw_event_fence_action *eaction;
-	struct drm_pending_event *event;
-	unsigned long irq_flags;
-
-	while (1) {
-		spin_lock_irqsave(&fman->lock, irq_flags);
-		if (list_empty(event_list))
-			goto out_unlock;
-		eaction = list_first_entry(event_list,
-					   struct vmw_event_fence_action,
-					   fpriv_head);
-		list_del_init(&eaction->fpriv_head);
-		event = eaction->event;
-		eaction->event = NULL;
-		spin_unlock_irqrestore(&fman->lock, irq_flags);
-		event->destroy(event);
-	}
-out_unlock:
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-}
-
-
-/**
  * vmw_event_fence_action_seq_passed
  *
  * @action: The struct vmw_fence_action embedded in a struct
@@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
 		*eaction->tv_usec = tv.tv_usec;
 	}
 
-	list_del_init(&eaction->fpriv_head);
 	eaction->event = NULL;
 	drm_send_event_locked(dev, eaction->event);
 	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
@@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action)
 	struct vmw_fence_manager *fman = fman_from_fence(eaction->fence);
 	unsigned long irq_flags;
 
-	spin_lock_irqsave(&fman->lock, irq_flags);
-	list_del(&eaction->fpriv_head);
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-
 	vmw_fence_obj_unreference(&eaction->fence);
 	kfree(eaction);
 }
@@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
 	eaction->tv_sec = tv_sec;
 	eaction->tv_usec = tv_usec;
 
-	spin_lock_irqsave(&fman->lock, irq_flags);
-	list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events);
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-
 	vmw_fence_obj_add_action(fence, &eaction->action);
 
 	return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
index 8be6c29f5eb5..83ae301ee141 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
@@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 				     struct drm_file *file_priv);
 extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv);
-extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				       struct list_head *event_list);
 extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
 					struct vmw_fence_obj *fence,
 					struct drm_pending_event *event,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/vmwgfx: Nuke preclose hook

[Thomas Hellstrom]

In general, looks good. Two things though.

1) vc4_drv.h looks like it ended up in the wrong patch.
2) Should be able to nuke also struct vmw_fpriv::fence_events and struct
vmw_event_fence_action::fpriv_head?

/Thomas

On 01/10/2016 11:02 PM, Daniel Vetter wrote:
> Again since the drm core takes care of event unlinking/disarming this
> is now just needless code.
>
> v2: I've completely missed eaction->fpriv_head and all the related
> code. We need to nuke that too to avoid accidentally deferencing the
> freed-up vmwgfx-private fpriv.
>
> Cc: Thomas Hellström <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/vc4/vc4_drv.h         |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 10 --------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 -----------------------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
>  4 files changed, 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 080865ec2bae..4c734d087d7f 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
>  extern struct platform_driver vc4_crtc_driver;
>  int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
>  void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
> -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
>  int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
>  
>  /* vc4_debugfs.c */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index c49812b80dd0..c04c4d1e2f3e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void vmw_preclose(struct drm_device *dev,
> -			 struct drm_file *file_priv)
> -{
> -	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
> -	struct vmw_private *dev_priv = vmw_priv(dev);
> -
> -	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
> -}
> -
>  static void vmw_postclose(struct drm_device *dev,
>  			 struct drm_file *file_priv)
>  {
> @@ -1500,7 +1491,6 @@ static struct drm_driver driver = {
>  	.master_set = vmw_master_set,
>  	.master_drop = vmw_master_drop,
>  	.open = vmw_driver_open,
> -	.preclose = vmw_preclose,
>  	.postclose = vmw_postclose,
>  	.set_busid = drm_pci_set_busid,
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index e0edf149d9d5..bb95bd20d415 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -71,7 +71,6 @@ struct vmw_user_fence {
>   */
>  struct vmw_event_fence_action {
>  	struct vmw_fence_action action;
> -	struct list_head fpriv_head;
>  
>  	struct drm_pending_event *event;
>  	struct vmw_fence_obj *fence;
> @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  }
>  
>  /**
> - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
> - *
> - * @fman: Pointer to a struct vmw_fence_manager
> - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
> - * with pointers to a struct drm_file object about to be closed.
> - *
> - * This function removes all pending fence events with references to a
> - * specific struct drm_file object about to be closed. The caller is required
> - * to pass a list of all struct vmw_event_fence_action objects with such
> - * events attached. This function is typically called before the
> - * struct drm_file object's event management is taken down.
> - */
> -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				struct list_head *event_list)
> -{
> -	struct vmw_event_fence_action *eaction;
> -	struct drm_pending_event *event;
> -	unsigned long irq_flags;
> -
> -	while (1) {
> -		spin_lock_irqsave(&fman->lock, irq_flags);
> -		if (list_empty(event_list))
> -			goto out_unlock;
> -		eaction = list_first_entry(event_list,
> -					   struct vmw_event_fence_action,
> -					   fpriv_head);
> -		list_del_init(&eaction->fpriv_head);
> -		event = eaction->event;
> -		eaction->event = NULL;
> -		spin_unlock_irqrestore(&fman->lock, irq_flags);
> -		event->destroy(event);
> -	}
> -out_unlock:
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -}
> -
> -
> -/**
>   * vmw_event_fence_action_seq_passed
>   *
>   * @action: The struct vmw_fence_action embedded in a struct
> @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
>  		*eaction->tv_usec = tv.tv_usec;
>  	}
>  
> -	list_del_init(&eaction->fpriv_head);
>  	eaction->event = NULL;
>  	drm_send_event_locked(dev, eaction->event);
>  	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
> @@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action)
>  	struct vmw_fence_manager *fman = fman_from_fence(eaction->fence);
>  	unsigned long irq_flags;
>  
> -	spin_lock_irqsave(&fman->lock, irq_flags);
> -	list_del(&eaction->fpriv_head);
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -
>  	vmw_fence_obj_unreference(&eaction->fence);
>  	kfree(eaction);
>  }
> @@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
>  	eaction->tv_sec = tv_sec;
>  	eaction->tv_usec = tv_usec;
>  
> -	spin_lock_irqsave(&fman->lock, irq_flags);
> -	list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events);
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -
>  	vmw_fence_obj_add_action(fence, &eaction->action);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> index 8be6c29f5eb5..83ae301ee141 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  				     struct drm_file *file_priv);
>  extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
>  				 struct drm_file *file_priv);
> -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				       struct list_head *event_list);
>  extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
>  					struct vmw_fence_obj *fence,
>  					struct drm_pending_event *event,

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

Re: [PATCH] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

On Sun, Jan 10, 2016 at 11:17 PM, Thomas Hellstrom
<thellstrom@vmware.com> wrote:
> In general, looks good. Two things though.
>
> 1) vc4_drv.h looks like it ended up in the wrong patch.
Oops, will fix.

> 2) Should be able to nuke also struct vmw_fpriv::fence_events and struct
> vmw_event_fence_action::fpriv_head?

Yes. I already remove fpriv_head, but fence_events should go to. Plus
a few other now unused local variable.
-Daniel

> /Thomas
>
> On 01/10/2016 11:02 PM, Daniel Vetter wrote:
>> Again since the drm core takes care of event unlinking/disarming this
>> is now just needless code.
>>
>> v2: I've completely missed eaction->fpriv_head and all the related
>> code. We need to nuke that too to avoid accidentally deferencing the
>> freed-up vmwgfx-private fpriv.
>>
>> Cc: Thomas Hellström <thellstrom@vmware.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/vc4/vc4_drv.h         |  1 -
>>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 10 --------
>>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 -----------------------------------
>>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
>>  4 files changed, 61 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
>> index 080865ec2bae..4c734d087d7f 100644
>> --- a/drivers/gpu/drm/vc4/vc4_drv.h
>> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
>> @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
>>  extern struct platform_driver vc4_crtc_driver;
>>  int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
>>  void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
>> -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
>>  int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
>>
>>  /* vc4_debugfs.c */
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> index c49812b80dd0..c04c4d1e2f3e 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
>>       return 0;
>>  }
>>
>> -static void vmw_preclose(struct drm_device *dev,
>> -                      struct drm_file *file_priv)
>> -{
>> -     struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
>> -     struct vmw_private *dev_priv = vmw_priv(dev);
>> -
>> -     vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
>> -}
>> -
>>  static void vmw_postclose(struct drm_device *dev,
>>                        struct drm_file *file_priv)
>>  {
>> @@ -1500,7 +1491,6 @@ static struct drm_driver driver = {
>>       .master_set = vmw_master_set,
>>       .master_drop = vmw_master_drop,
>>       .open = vmw_driver_open,
>> -     .preclose = vmw_preclose,
>>       .postclose = vmw_postclose,
>>       .set_busid = drm_pci_set_busid,
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
>> index e0edf149d9d5..bb95bd20d415 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
>> @@ -71,7 +71,6 @@ struct vmw_user_fence {
>>   */
>>  struct vmw_event_fence_action {
>>       struct vmw_fence_action action;
>> -     struct list_head fpriv_head;
>>
>>       struct drm_pending_event *event;
>>       struct vmw_fence_obj *fence;
>> @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>>  }
>>
>>  /**
>> - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
>> - *
>> - * @fman: Pointer to a struct vmw_fence_manager
>> - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
>> - * with pointers to a struct drm_file object about to be closed.
>> - *
>> - * This function removes all pending fence events with references to a
>> - * specific struct drm_file object about to be closed. The caller is required
>> - * to pass a list of all struct vmw_event_fence_action objects with such
>> - * events attached. This function is typically called before the
>> - * struct drm_file object's event management is taken down.
>> - */
>> -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
>> -                             struct list_head *event_list)
>> -{
>> -     struct vmw_event_fence_action *eaction;
>> -     struct drm_pending_event *event;
>> -     unsigned long irq_flags;
>> -
>> -     while (1) {
>> -             spin_lock_irqsave(&fman->lock, irq_flags);
>> -             if (list_empty(event_list))
>> -                     goto out_unlock;
>> -             eaction = list_first_entry(event_list,
>> -                                        struct vmw_event_fence_action,
>> -                                        fpriv_head);
>> -             list_del_init(&eaction->fpriv_head);
>> -             event = eaction->event;
>> -             eaction->event = NULL;
>> -             spin_unlock_irqrestore(&fman->lock, irq_flags);
>> -             event->destroy(event);
>> -     }
>> -out_unlock:
>> -     spin_unlock_irqrestore(&fman->lock, irq_flags);
>> -}
>> -
>> -
>> -/**
>>   * vmw_event_fence_action_seq_passed
>>   *
>>   * @action: The struct vmw_fence_action embedded in a struct
>> @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
>>               *eaction->tv_usec = tv.tv_usec;
>>       }
>>
>> -     list_del_init(&eaction->fpriv_head);
>>       eaction->event = NULL;
>>       drm_send_event_locked(dev, eaction->event);
>>       spin_unlock_irqrestore(&dev->event_lock, irq_flags);
>> @@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action)
>>       struct vmw_fence_manager *fman = fman_from_fence(eaction->fence);
>>       unsigned long irq_flags;
>>
>> -     spin_lock_irqsave(&fman->lock, irq_flags);
>> -     list_del(&eaction->fpriv_head);
>> -     spin_unlock_irqrestore(&fman->lock, irq_flags);
>> -
>>       vmw_fence_obj_unreference(&eaction->fence);
>>       kfree(eaction);
>>  }
>> @@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
>>       eaction->tv_sec = tv_sec;
>>       eaction->tv_usec = tv_usec;
>>
>> -     spin_lock_irqsave(&fman->lock, irq_flags);
>> -     list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events);
>> -     spin_unlock_irqrestore(&fman->lock, irq_flags);
>> -
>>       vmw_fence_obj_add_action(fence, &eaction->action);
>>
>>       return 0;
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
>> index 8be6c29f5eb5..83ae301ee141 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
>> @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>>                                    struct drm_file *file_priv);
>>  extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
>>                                struct drm_file *file_priv);
>> -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
>> -                                    struct list_head *event_list);
>>  extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
>>                                       struct vmw_fence_obj *fence,
>>                                       struct drm_pending_event *event,
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] drm/shmob: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

v2: Fixup misplaced hunk.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 20 --------------------
 drivers/gpu/drm/shmobile/shmob_drm_crtc.h |  2 --
 drivers/gpu/drm/shmobile/shmob_drm_drv.c  |  8 --------
 3 files changed, 30 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index b80802f55143..de7959a60774 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -438,26 +438,6 @@ static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
 	.mode_set_base = shmob_drm_crtc_mode_set_base,
 };
 
-void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc *scrtc,
-				     struct drm_file *file)
-{
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = scrtc->crtc.dev;
-	unsigned long flags;
-
-	/* Destroy the pending vertical blanking event associated with the
-	 * pending page flip, if any, and disable vertical blanking interrupts.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = scrtc->event;
-	if (event && event->base.file_priv == file) {
-		scrtc->event = NULL;
-		event->base.destroy(&event->base);
-		drm_vblank_put(dev, 0);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)
 {
 	struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
index eddad6dcc88a..38ed4ff8aaf2 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
@@ -47,8 +47,6 @@ struct shmob_drm_connector {
 
 int shmob_drm_crtc_create(struct shmob_drm_device *sdev);
 void shmob_drm_crtc_enable_vblank(struct shmob_drm_device *sdev, bool enable);
-void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc *scrtc,
-				     struct drm_file *file);
 void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc);
 void shmob_drm_crtc_suspend(struct shmob_drm_crtc *scrtc);
 void shmob_drm_crtc_resume(struct shmob_drm_crtc *scrtc);
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 04e66e3751b4..7700ff172079 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -200,13 +200,6 @@ done:
 	return ret;
 }
 
-static void shmob_drm_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct shmob_drm_device *sdev = dev->dev_private;
-
-	shmob_drm_crtc_cancel_page_flip(&sdev->crtc, file);
-}
-
 static irqreturn_t shmob_drm_irq(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -266,7 +259,6 @@ static struct drm_driver shmob_drm_driver = {
 				| DRIVER_PRIME,
 	.load			= shmob_drm_load,
 	.unload			= shmob_drm_unload,
-	.preclose		= shmob_drm_preclose,
 	.set_busid		= drm_platform_set_busid,
 	.irq_handler		= shmob_drm_irq,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/tegra: Stop cancelling page flip events

[Daniel Vetter]

The core takes care of that now.

v2: Fixup misplaced hunk.

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Terje Bergström <tbergstrom@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/tegra/dc.c  | 17 -----------------
 drivers/gpu/drm/tegra/drm.c |  3 ---
 drivers/gpu/drm/tegra/drm.h |  1 -
 3 files changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index dde6f208c347..fb2b4b0271a2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -988,23 +988,6 @@ static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 	spin_unlock_irqrestore(&drm->event_lock, flags);
 }
 
-void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct tegra_dc *dc = to_tegra_dc(crtc);
-	struct drm_device *drm = crtc->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&drm->event_lock, flags);
-
-	if (dc->event && dc->event->base.file_priv == file) {
-		dc->event->base.destroy(&dc->event->base);
-		drm_crtc_vblank_put(crtc);
-		dc->event = NULL;
-	}
-
-	spin_unlock_irqrestore(&drm->event_lock, flags);
-}
-
 static void tegra_dc_destroy(struct drm_crtc *crtc)
 {
 	drm_crtc_cleanup(crtc);
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index c5c856a0879d..021d0e1398fb 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -860,9 +860,6 @@ static void tegra_drm_preclose(struct drm_device *drm, struct drm_file *file)
 	struct tegra_drm_context *context, *tmp;
 	struct drm_crtc *crtc;
 
-	list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
-		tegra_dc_cancel_page_flip(crtc, file);
-
 	list_for_each_entry_safe(context, tmp, &fpriv->contexts, list)
 		tegra_drm_context_free(context);
 
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index c088f2f67eda..8a10f5b7d9dc 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -195,7 +195,6 @@ struct tegra_dc_window {
 u32 tegra_dc_get_vblank_counter(struct tegra_dc *dc);
 void tegra_dc_enable_vblank(struct tegra_dc *dc);
 void tegra_dc_disable_vblank(struct tegra_dc *dc);
-void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 void tegra_dc_commit(struct tegra_dc *dc);
 int tegra_dc_state_setup_clock(struct tegra_dc *dc,
 			       struct drm_crtc_state *crtc_state,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] drm/tilcdc: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

v2: Fixup misplaced hunks.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 20 --------------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  8 --------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 -
 3 files changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 7d07733bdc86..4802da8e6d6f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -662,26 +662,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 	return IRQ_HANDLED;
 }
 
-void tilcdc_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct drm_pending_vblank_event *event;
-	struct drm_device *dev = crtc->dev;
-	unsigned long flags;
-
-	/* Destroy the pending vertical blanking event associated with the
-	 * pending page flip, if any, and disable vertical blanking interrupts.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	event = tilcdc_crtc->event;
-	if (event && event->base.file_priv == file) {
-		tilcdc_crtc->event = NULL;
-		event->base.destroy(&event->base);
-		drm_vblank_put(dev, 0);
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 {
 	struct tilcdc_crtc *tilcdc_crtc;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4ddb21e7f52f..b39ba213e303 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -349,13 +349,6 @@ fail_free_priv:
 	return ret;
 }
 
-static void tilcdc_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct tilcdc_drm_private *priv = dev->dev_private;
-
-	tilcdc_crtc_cancel_page_flip(priv->crtc, file);
-}
-
 static void tilcdc_lastclose(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
@@ -556,7 +549,6 @@ static struct drm_driver tilcdc_driver = {
 	.driver_features    = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET,
 	.load               = tilcdc_load,
 	.unload             = tilcdc_unload,
-	.preclose           = tilcdc_preclose,
 	.lastclose          = tilcdc_lastclose,
 	.set_busid          = drm_platform_set_busid,
 	.irq_handler        = tilcdc_irq,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index e863ad0d26fe..66105d8dc620 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -163,7 +163,6 @@ struct tilcdc_panel_info {
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
 struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev);
-void tilcdc_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc);
 void tilcdc_crtc_update_clk(struct drm_crtc *crtc);
 void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/5] drm/vc4: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

v2: Fixup misplaced hunk.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 20 --------------------
 drivers/gpu/drm/vc4/vc4_drv.c  | 10 ----------
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 -
 3 files changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 018145e0b87d..937409792b97 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -593,26 +593,6 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {
 	.atomic_flush = vc4_crtc_atomic_flush,
 };
 
-/* Frees the page flip event when the DRM device is closed with the
- * event still outstanding.
- */
-void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
-{
-	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	if (vc4_crtc->event && vc4_crtc->event->base.file_priv == file) {
-		vc4_crtc->event->base.destroy(&vc4_crtc->event->base);
-		drm_crtc_vblank_put(crtc);
-		vc4_crtc->event = NULL;
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 static const struct vc4_crtc_data pv0_data = {
 	.hvs_channel = 0,
 	.encoder0_type = VC4_ENCODER_TYPE_DSI0,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index f1655fff8425..b7d2ff0e6e1f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -43,14 +43,6 @@ void __iomem *vc4_ioremap_regs(struct platform_device *dev, int index)
 	return map;
 }
 
-static void vc4_drm_preclose(struct drm_device *dev, struct drm_file *file)
-{
-	struct drm_crtc *crtc;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		vc4_cancel_page_flip(crtc, file);
-}
-
 static void vc4_lastclose(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -91,8 +83,6 @@ static struct drm_driver vc4_drm_driver = {
 			    DRIVER_HAVE_IRQ |
 			    DRIVER_PRIME),
 	.lastclose = vc4_lastclose,
-	.preclose = vc4_drm_preclose,
-
 	.irq_handler = vc4_irq,
 	.irq_preinstall = vc4_irq_preinstall,
 	.irq_postinstall = vc4_irq_postinstall,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 080865ec2bae..4c734d087d7f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
 extern struct platform_driver vc4_crtc_driver;
 int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id);
 void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id);
-void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
 
 /* vc4_debugfs.c */
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/5] drm/vmwgfx: Nuke preclose hook

[Daniel Vetter]

Again since the drm core takes care of event unlinking/disarming this
is now just needless code.

v2: I've completely missed eaction->fpriv_head and all the related
code. We need to nuke that too to avoid accidentally deferencing the
freed-up vmwgfx-private fpriv.

v3: Also remove vmw_fpriv->fence_events and unused variables I missed.

Cc: Thomas Hellström <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 11 --------
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 52 -----------------------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
 4 files changed, 66 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c49812b80dd0..c96a2d2d5107 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
 	return 0;
 }
 
-static void vmw_preclose(struct drm_device *dev,
-			 struct drm_file *file_priv)
-{
-	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
-	struct vmw_private *dev_priv = vmw_priv(dev);
-
-	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
-}
-
 static void vmw_postclose(struct drm_device *dev,
 			 struct drm_file *file_priv)
 {
@@ -1010,7 +1001,6 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
 	if (unlikely(vmw_fp == NULL))
 		return ret;
 
-	INIT_LIST_HEAD(&vmw_fp->fence_events);
 	vmw_fp->tfile = ttm_object_file_init(dev_priv->tdev, 10);
 	if (unlikely(vmw_fp->tfile == NULL))
 		goto out_no_tfile;
@@ -1500,7 +1490,6 @@ static struct drm_driver driver = {
 	.master_set = vmw_master_set,
 	.master_drop = vmw_master_drop,
 	.open = vmw_driver_open,
-	.preclose = vmw_preclose,
 	.postclose = vmw_postclose,
 	.set_busid = drm_pci_set_busid,
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 469cdd520615..5cb1b1687cd4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -80,7 +80,6 @@
 struct vmw_fpriv {
 	struct drm_master *locked_master;
 	struct ttm_object_file *tfile;
-	struct list_head fence_events;
 	bool gb_aware;
 };
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index e0edf149d9d5..ac477863fc07 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -71,7 +71,6 @@ struct vmw_user_fence {
  */
 struct vmw_event_fence_action {
 	struct vmw_fence_action action;
-	struct list_head fpriv_head;
 
 	struct drm_pending_event *event;
 	struct vmw_fence_obj *fence;
@@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 }
 
 /**
- * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
- *
- * @fman: Pointer to a struct vmw_fence_manager
- * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
- * with pointers to a struct drm_file object about to be closed.
- *
- * This function removes all pending fence events with references to a
- * specific struct drm_file object about to be closed. The caller is required
- * to pass a list of all struct vmw_event_fence_action objects with such
- * events attached. This function is typically called before the
- * struct drm_file object's event management is taken down.
- */
-void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				struct list_head *event_list)
-{
-	struct vmw_event_fence_action *eaction;
-	struct drm_pending_event *event;
-	unsigned long irq_flags;
-
-	while (1) {
-		spin_lock_irqsave(&fman->lock, irq_flags);
-		if (list_empty(event_list))
-			goto out_unlock;
-		eaction = list_first_entry(event_list,
-					   struct vmw_event_fence_action,
-					   fpriv_head);
-		list_del_init(&eaction->fpriv_head);
-		event = eaction->event;
-		eaction->event = NULL;
-		spin_unlock_irqrestore(&fman->lock, irq_flags);
-		event->destroy(event);
-	}
-out_unlock:
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-}
-
-
-/**
  * vmw_event_fence_action_seq_passed
  *
  * @action: The struct vmw_fence_action embedded in a struct
@@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
 		*eaction->tv_usec = tv.tv_usec;
 	}
 
-	list_del_init(&eaction->fpriv_head);
 	eaction->event = NULL;
 	drm_send_event_locked(dev, eaction->event);
 	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
@@ -898,12 +858,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action)
 {
 	struct vmw_event_fence_action *eaction =
 		container_of(action, struct vmw_event_fence_action, action);
-	struct vmw_fence_manager *fman = fman_from_fence(eaction->fence);
-	unsigned long irq_flags;
-
-	spin_lock_irqsave(&fman->lock, irq_flags);
-	list_del(&eaction->fpriv_head);
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
 
 	vmw_fence_obj_unreference(&eaction->fence);
 	kfree(eaction);
@@ -983,8 +937,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
 {
 	struct vmw_event_fence_action *eaction;
 	struct vmw_fence_manager *fman = fman_from_fence(fence);
-	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
-	unsigned long irq_flags;
 
 	eaction = kzalloc(sizeof(*eaction), GFP_KERNEL);
 	if (unlikely(eaction == NULL))
@@ -1001,10 +953,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
 	eaction->tv_sec = tv_sec;
 	eaction->tv_usec = tv_usec;
 
-	spin_lock_irqsave(&fman->lock, irq_flags);
-	list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events);
-	spin_unlock_irqrestore(&fman->lock, irq_flags);
-
 	vmw_fence_obj_add_action(fence, &eaction->action);
 
 	return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
index 8be6c29f5eb5..83ae301ee141 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
@@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
 				     struct drm_file *file_priv);
 extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv);
-extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
-				       struct list_head *event_list);
 extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
 					struct vmw_fence_obj *fence,
 					struct drm_pending_event *event,
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Clean up pending events in the core

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Saturday 09 January 2016 14:28:46 Daniel Vetter wrote:
> There's really no reason to not do so, instead of replicating this
> for every use-case and every driver. Now we can't just nuke the events,
> since that would still mean that all drm_event users would need to know
> when that has happened, since calling e.g. drm_send_event isn't allowed
> any more. Instead just unlink them from the file, and detect this case
> and handle it appropriately in all functions.
> 
> v2: Adjust existing kerneldoc too.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_fops.c | 35 ++++++++++++++++++++++++++---------
>  include/drm/drmP.h         |  2 ++
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index d85af1b2a238..d32b24c74e08 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -264,6 +264,7 @@ static int drm_open_helper(struct file *filp, struct
> drm_minor *minor) INIT_LIST_HEAD(&priv->fbs);
>  	mutex_init(&priv->fbs_lock);
>  	INIT_LIST_HEAD(&priv->blobs);
> +	INIT_LIST_HEAD(&priv->pending_event_list);
>  	INIT_LIST_HEAD(&priv->event_list);
>  	init_waitqueue_head(&priv->event_wait);
>  	priv->event_space = 4096; /* set aside 4k for event buffer */
> @@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file
> *file_priv) {
>  	struct drm_device *dev = file_priv->minor->dev;
>  	struct drm_pending_event *e, *et;
> -	struct drm_pending_vblank_event *v, *vt;
>  	unsigned long flags;
> 
>  	spin_lock_irqsave(&dev->event_lock, flags);
> 
> -	/* Remove pending flips */
> -	list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
> -		if (v->base.file_priv == file_priv) {
> -			list_del(&v->base.link);
> -			drm_vblank_put(dev, v->pipe);
> -			v->base.destroy(&v->base);
> -		}

Where does this code go ?

> +	/* Unlink pending events */
> +	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
> +				 pending_link) {
> +		list_del(&e->pending_link);
> +		e->file_priv = NULL;
> +	}
> 
>  	/* Remove unconsumed events */
>  	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
> @@ -712,6 +711,7 @@ int drm_event_reserve_init(struct drm_device *dev,
>  	file_priv->event_space -= e->length;
> 
>  	p->event = e;
> +	list_add(&p->pending_link, &file_priv->pending_event_list);
>  	p->file_priv = file_priv;
> 
>  	/* we *could* pass this in as arg, but everyone uses kfree: */
> @@ -736,7 +736,10 @@ void drm_event_cancel_free(struct drm_device *dev,
>  {
>  	unsigned long flags;
>  	spin_lock_irqsave(&dev->event_lock, flags);
> -	p->file_priv->event_space += p->event->length;
> +	if (p->file_priv) {
> +		p->file_priv->event_space += p->event->length;
> +		list_del(&p->pending_link);
> +	}
>  	spin_unlock_irqrestore(&dev->event_lock, flags);
>  	p->destroy(p);
>  }
> @@ -750,11 +753,21 @@ EXPORT_SYMBOL(drm_event_cancel_free);
>   * This function sends the event @e, initialized with
> drm_event_reserve_init(),
>   * to its associated userspace DRM file. Callers must already hold
>   * dev->event_lock, see drm_send_event() for the unlocked version.
> + *
> + * Note that the core will take care of unlinking and disarming events when
> the
> + * corresponding DRM file is closed. Drivers need to worry about that and

s/need to/need not/ ?

> can
> + * call this function upon completion of the asynchrnous work

s/asynchrnous/asynchronous/

> unconditionally. */
>  void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event
> *e) {
>  	assert_spin_locked(&dev->event_lock);
> 
> +	if (!e->file_priv) {

I don't think this could happen before this patch as e->file_priv is 
dereferenced below, and I don't see anything in this patch that makes the 
condition possible.

> +		e->destroy(e);
> +		return;
> +	}
> +
> +	list_del(&e->pending_link);
>  	list_add_tail(&e->link,
>  		      &e->file_priv->event_list);
>  	wake_up_interruptible(&e->file_priv->event_wait);
> @@ -769,6 +782,10 @@ EXPORT_SYMBOL(drm_send_event_locked);
>   * This function sends the event @e, initialized with
> drm_event_reserve_init(),
>   * to its associated userspace DRM file. This function acquires
> dev->event_lock,
>   * see drm_send_event_locked() for callers which already hold this lock.
> + *
> + * Note that the core will take care of unlinking and disarming events when
> the
> + * corresponding DRM file is closed. Drivers need to worry about that and

s/need to/need not/ ?

> can
> + * call this function upon completion of the asynchrnous work

s/asynchrnous/asynchronous/

> unconditionally. */
>  void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
>  {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index ae73abf5c2cf..3d78a7406d54 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -283,6 +283,7 @@ struct drm_ioctl_desc {
>  struct drm_pending_event {
>  	struct drm_event *event;
>  	struct list_head link;
> +	struct list_head pending_link;
>  	struct drm_file *file_priv;
>  	pid_t pid; /* pid of requester, no guarantee it's valid by the time
>  		      we deliver the event, for tracing only */
> @@ -346,6 +347,7 @@ struct drm_file {
>  	struct list_head blobs;
> 
>  	wait_queue_head_t event_wait;
> +	struct list_head pending_event_list;
>  	struct list_head event_list;
>  	int event_space;

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/shmob: Nuke preclose hook

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Sunday 10 January 2016 23:26:06 Daniel Vetter wrote:
> Again since the drm core takes care of event unlinking/disarming this
> is now just needless code.
> 
> v2: Fixup misplaced hunk.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

This looks good to me assuming that the mechanism works in the core, which 
looks like magic to me at the moment :-) After closing the "[PATCH] drm: Clean 
up pending events in the core" discussion (and assuming the conclusion is that 
is works),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 20 --------------------
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.h |  2 --
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c  |  8 --------
>  3 files changed, 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index
> b80802f55143..de7959a60774 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> @@ -438,26 +438,6 @@ static const struct drm_crtc_helper_funcs
> crtc_helper_funcs = { .mode_set_base = shmob_drm_crtc_mode_set_base,
>  };
> 
> -void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc *scrtc,
> -				     struct drm_file *file)
> -{
> -	struct drm_pending_vblank_event *event;
> -	struct drm_device *dev = scrtc->crtc.dev;
> -	unsigned long flags;
> -
> -	/* Destroy the pending vertical blanking event associated with the
> -	 * pending page flip, if any, and disable vertical blanking interrupts.
> -	 */
> -	spin_lock_irqsave(&dev->event_lock, flags);
> -	event = scrtc->event;
> -	if (event && event->base.file_priv == file) {
> -		scrtc->event = NULL;
> -		event->base.destroy(&event->base);
> -		drm_vblank_put(dev, 0);
> -	}
> -	spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
>  void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)
>  {
>  	struct drm_pending_vblank_event *event;
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
> b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h index
> eddad6dcc88a..38ed4ff8aaf2 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.h
> @@ -47,8 +47,6 @@ struct shmob_drm_connector {
> 
>  int shmob_drm_crtc_create(struct shmob_drm_device *sdev);
>  void shmob_drm_crtc_enable_vblank(struct shmob_drm_device *sdev, bool
> enable); -void shmob_drm_crtc_cancel_page_flip(struct shmob_drm_crtc
> *scrtc, -				     struct drm_file *file);
>  void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc);
>  void shmob_drm_crtc_suspend(struct shmob_drm_crtc *scrtc);
>  void shmob_drm_crtc_resume(struct shmob_drm_crtc *scrtc);
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> b/drivers/gpu/drm/shmobile/shmob_drm_drv.c index 04e66e3751b4..7700ff172079
> 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -200,13 +200,6 @@ done:
>  	return ret;
>  }
> 
> -static void shmob_drm_preclose(struct drm_device *dev, struct drm_file
> *file) -{
> -	struct shmob_drm_device *sdev = dev->dev_private;
> -
> -	shmob_drm_crtc_cancel_page_flip(&sdev->crtc, file);
> -}
> -
>  static irqreturn_t shmob_drm_irq(int irq, void *arg)
>  {
>  	struct drm_device *dev = arg;
> @@ -266,7 +259,6 @@ static struct drm_driver shmob_drm_driver = {
> 
>  				| DRIVER_PRIME,
> 
>  	.load			= shmob_drm_load,
>  	.unload			= shmob_drm_unload,
> -	.preclose		= shmob_drm_preclose,
>  	.set_busid		= drm_platform_set_busid,
>  	.irq_handler		= shmob_drm_irq,
>  	.get_vblank_counter	= drm_vblank_no_hw_counter,

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 16/21] drm/rcar: Nuke preclose hook

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Friday 08 January 2016 21:36:48 Daniel Vetter wrote:
> Again since the drm core takes care of event unlinking/disarming this
> is now just needless code.

This looks good to me assuming that the mechanism works in the core, which 
looks like magic to me at the moment :-) After closing the "[PATCH] drm: Clean 
up pending events in the core" discussion (and assuming the conclusion is that 
is works),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 20 --------------------
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  2 --
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 10 ----------
>  3 files changed, 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 88a4b706be16..4ec80ae1fa99
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -282,26 +282,6 @@ static void rcar_du_crtc_update_planes(struct
> rcar_du_crtc *rcrtc) * Page Flip
>   */
> 
> -void rcar_du_crtc_cancel_page_flip(struct rcar_du_crtc *rcrtc,
> -				   struct drm_file *file)
> -{
> -	struct drm_pending_vblank_event *event;
> -	struct drm_device *dev = rcrtc->crtc.dev;
> -	unsigned long flags;
> -
> -	/* Destroy the pending vertical blanking event associated with the
> -	 * pending page flip, if any, and disable vertical blanking interrupts.
> -	 */
> -	spin_lock_irqsave(&dev->event_lock, flags);
> -	event = rcrtc->event;
> -	if (event && event->base.file_priv == file) {
> -		rcrtc->event = NULL;
> -		event->base.destroy(&event->base);
> -		drm_crtc_vblank_put(&rcrtc->crtc);
> -	}
> -	spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
>  static void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc)
>  {
>  	struct drm_pending_vblank_event *event;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 4b95d9d08c49..2bbe3f5aab65
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -67,8 +67,6 @@ enum rcar_du_output {
> 
>  int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index);
>  void rcar_du_crtc_enable_vblank(struct rcar_du_crtc *rcrtc, bool enable);
> -void rcar_du_crtc_cancel_page_flip(struct rcar_du_crtc *rcrtc,
> -				   struct drm_file *file);
>  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
>  void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 40422f6b645e..0bb2b31555bf
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -220,15 +220,6 @@ done:
>  	return ret;
>  }
> 
> -static void rcar_du_preclose(struct drm_device *dev, struct drm_file *file)
> -{
> -	struct rcar_du_device *rcdu = dev->dev_private;
> -	unsigned int i;
> -
> -	for (i = 0; i < rcdu->num_crtcs; ++i)
> -		rcar_du_crtc_cancel_page_flip(&rcdu->crtcs[i], file);
> -}
> -
>  static void rcar_du_lastclose(struct drm_device *dev)
>  {
>  	struct rcar_du_device *rcdu = dev->dev_private;
> @@ -271,7 +262,6 @@ static struct drm_driver rcar_du_driver = {
> 
>  				| DRIVER_ATOMIC,
> 
>  	.load			= rcar_du_load,
>  	.unload			= rcar_du_unload,
> -	.preclose		= rcar_du_preclose,
>  	.lastclose		= rcar_du_lastclose,
>  	.set_busid		= drm_platform_set_busid,
>  	.get_vblank_counter	= drm_vblank_no_hw_counter,

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 15/21] drm/omap: Nuke close hooks

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Friday 08 January 2016 21:36:47 Daniel Vetter wrote:
> Again since the core takes care of this we can remove them. While at
> it also remove the postclose hook, it's empty.
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 29 -----------------------------
>  1 file changed, 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index dfafdb602ad2..e8b5080f6e2d
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -787,33 +787,6 @@ static void dev_lastclose(struct drm_device *dev)
>  	}
>  }
> 
> -static void dev_preclose(struct drm_device *dev, struct drm_file *file)
> -{
> -	struct omap_drm_private *priv = dev->dev_private;
> -	struct drm_pending_event *event;
> -	unsigned long flags;
> -
> -	DBG("preclose: dev=%p", dev);
> -
> -	/*
> -	 * Unlink all pending CRTC events to make sure they won't be queued up
> -	 * by a pending asynchronous commit.
> -	 */
> -	spin_lock_irqsave(&dev->event_lock, flags);
> -	list_for_each_entry(event, &priv->commit.events, link) {
> -		if (event->file_priv == file) {
> -			file->event_space += event->event->length;
> -			event->file_priv = NULL;
> -		}
> -	}
> -	spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
> -static void dev_postclose(struct drm_device *dev, struct drm_file *file)
> -{
> -	DBG("postclose: dev=%p, file=%p", dev, file);
> -}
> -

You can also nuke the following code from omap_atomic_commit():

        /* Keep track of all CRTC events to unlink them in preclose(). */
        spin_lock_irqsave(&dev->event_lock, flags);
        for (i = 0; i < dev->mode_config.num_crtc; ++i) {
                struct drm_crtc_state *cstate = state->crtc_states[i];

                if (cstate && cstate->event)
                        list_add_tail(&cstate->event->base.link,
                                      &priv->commit.events);
        }
        spin_unlock_irqrestore(&dev->event_lock, flags);

and the priv->commit.events field in omap_drv.h (don't forget the structure 
kerneldoc).

>  static const struct vm_operations_struct omap_gem_vm_ops = {
>  	.fault = omap_gem_fault,
>  	.open = drm_gem_vm_open,
> @@ -838,8 +811,6 @@ static struct drm_driver omap_drm_driver = {
>  	.unload = dev_unload,
>  	.open = dev_open,
>  	.lastclose = dev_lastclose,
> -	.preclose = dev_preclose,
> -	.postclose = dev_postclose,
>  	.set_busid = drm_platform_set_busid,
>  	.get_vblank_counter = drm_vblank_no_hw_counter,
>  	.enable_vblank = omap_irq_enable_vblank,

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 15/21] drm/omap: Nuke close hooks

[Daniel Vetter]

On Mon, Jan 11, 2016 at 02:03:34AM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Friday 08 January 2016 21:36:47 Daniel Vetter wrote:
> > Again since the core takes care of this we can remove them. While at
> > it also remove the postclose hook, it's empty.
> >
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/omapdrm/omap_drv.c | 29 -----------------------------
> >  1 file changed, 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> > b/drivers/gpu/drm/omapdrm/omap_drv.c index dfafdb602ad2..e8b5080f6e2d
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -787,33 +787,6 @@ static void dev_lastclose(struct drm_device *dev)
> >  	}
> >  }
> > 
> > -static void dev_preclose(struct drm_device *dev, struct drm_file *file)
> > -{
> > -	struct omap_drm_private *priv = dev->dev_private;
> > -	struct drm_pending_event *event;
> > -	unsigned long flags;
> > -
> > -	DBG("preclose: dev=%p", dev);
> > -
> > -	/*
> > -	 * Unlink all pending CRTC events to make sure they won't be queued up
> > -	 * by a pending asynchronous commit.
> > -	 */
> > -	spin_lock_irqsave(&dev->event_lock, flags);
> > -	list_for_each_entry(event, &priv->commit.events, link) {
> > -		if (event->file_priv == file) {
> > -			file->event_space += event->event->length;
> > -			event->file_priv = NULL;
> > -		}
> > -	}
> > -	spin_unlock_irqrestore(&dev->event_lock, flags);
> > -}
> > -
> > -static void dev_postclose(struct drm_device *dev, struct drm_file *file)
> > -{
> > -	DBG("postclose: dev=%p, file=%p", dev, file);
> > -}
> > -
> 
> You can also nuke the following code from omap_atomic_commit():
> 
>         /* Keep track of all CRTC events to unlink them in preclose(). */
>         spin_lock_irqsave(&dev->event_lock, flags);
>         for (i = 0; i < dev->mode_config.num_crtc; ++i) {
>                 struct drm_crtc_state *cstate = state->crtc_states[i];
> 
>                 if (cstate && cstate->event)
>                         list_add_tail(&cstate->event->base.link,
>                                       &priv->commit.events);
>         }
>         spin_unlock_irqrestore(&dev->event_lock, flags);
> 
> and the priv->commit.events field in omap_drv.h (don't forget the structure 
> kerneldoc).

Oh this is really good, since hunting for related code I noticed that omap
doesn't send out the vblank event when the fpriv disappeared. So found
even more code to remove. I didn't find any kerneldoc though.
-Daniel

> 
> >  static const struct vm_operations_struct omap_gem_vm_ops = {
> >  	.fault = omap_gem_fault,
> >  	.open = drm_gem_vm_open,
> > @@ -838,8 +811,6 @@ static struct drm_driver omap_drm_driver = {
> >  	.unload = dev_unload,
> >  	.open = dev_open,
> >  	.lastclose = dev_lastclose,
> > -	.preclose = dev_preclose,
> > -	.postclose = dev_postclose,
> >  	.set_busid = drm_platform_set_busid,
> >  	.get_vblank_counter = drm_vblank_no_hw_counter,
> >  	.enable_vblank = omap_irq_enable_vblank,
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/vmwgfx: Nuke preclose hook

[Thomas Hellstrom]

LGTM.

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>


On 01/10/2016 11:26 PM, Daniel Vetter wrote:
> Again since the drm core takes care of event unlinking/disarming this
> is now just needless code.
>
> v2: I've completely missed eaction->fpriv_head and all the related
> code. We need to nuke that too to avoid accidentally deferencing the
> freed-up vmwgfx-private fpriv.
>
> v3: Also remove vmw_fpriv->fence_events and unused variables I missed.
>
> Cc: Thomas Hellström <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   | 11 --------
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h   |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 52 -----------------------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.h |  2 --
>  4 files changed, 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index c49812b80dd0..c96a2d2d5107 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void vmw_preclose(struct drm_device *dev,
> -			 struct drm_file *file_priv)
> -{
> -	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
> -	struct vmw_private *dev_priv = vmw_priv(dev);
> -
> -	vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events);
> -}
> -
>  static void vmw_postclose(struct drm_device *dev,
>  			 struct drm_file *file_priv)
>  {
> @@ -1010,7 +1001,6 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
>  	if (unlikely(vmw_fp == NULL))
>  		return ret;
>  
> -	INIT_LIST_HEAD(&vmw_fp->fence_events);
>  	vmw_fp->tfile = ttm_object_file_init(dev_priv->tdev, 10);
>  	if (unlikely(vmw_fp->tfile == NULL))
>  		goto out_no_tfile;
> @@ -1500,7 +1490,6 @@ static struct drm_driver driver = {
>  	.master_set = vmw_master_set,
>  	.master_drop = vmw_master_drop,
>  	.open = vmw_driver_open,
> -	.preclose = vmw_preclose,
>  	.postclose = vmw_postclose,
>  	.set_busid = drm_pci_set_busid,
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 469cdd520615..5cb1b1687cd4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -80,7 +80,6 @@
>  struct vmw_fpriv {
>  	struct drm_master *locked_master;
>  	struct ttm_object_file *tfile;
> -	struct list_head fence_events;
>  	bool gb_aware;
>  };
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index e0edf149d9d5..ac477863fc07 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -71,7 +71,6 @@ struct vmw_user_fence {
>   */
>  struct vmw_event_fence_action {
>  	struct vmw_fence_action action;
> -	struct list_head fpriv_head;
>  
>  	struct drm_pending_event *event;
>  	struct vmw_fence_obj *fence;
> @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  }
>  
>  /**
> - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects
> - *
> - * @fman: Pointer to a struct vmw_fence_manager
> - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects
> - * with pointers to a struct drm_file object about to be closed.
> - *
> - * This function removes all pending fence events with references to a
> - * specific struct drm_file object about to be closed. The caller is required
> - * to pass a list of all struct vmw_event_fence_action objects with such
> - * events attached. This function is typically called before the
> - * struct drm_file object's event management is taken down.
> - */
> -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				struct list_head *event_list)
> -{
> -	struct vmw_event_fence_action *eaction;
> -	struct drm_pending_event *event;
> -	unsigned long irq_flags;
> -
> -	while (1) {
> -		spin_lock_irqsave(&fman->lock, irq_flags);
> -		if (list_empty(event_list))
> -			goto out_unlock;
> -		eaction = list_first_entry(event_list,
> -					   struct vmw_event_fence_action,
> -					   fpriv_head);
> -		list_del_init(&eaction->fpriv_head);
> -		event = eaction->event;
> -		eaction->event = NULL;
> -		spin_unlock_irqrestore(&fman->lock, irq_flags);
> -		event->destroy(event);
> -	}
> -out_unlock:
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -}
> -
> -
> -/**
>   * vmw_event_fence_action_seq_passed
>   *
>   * @action: The struct vmw_fence_action embedded in a struct
> @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action)
>  		*eaction->tv_usec = tv.tv_usec;
>  	}
>  
> -	list_del_init(&eaction->fpriv_head);
>  	eaction->event = NULL;
>  	drm_send_event_locked(dev, eaction->event);
>  	spin_unlock_irqrestore(&dev->event_lock, irq_flags);
> @@ -898,12 +858,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action)
>  {
>  	struct vmw_event_fence_action *eaction =
>  		container_of(action, struct vmw_event_fence_action, action);
> -	struct vmw_fence_manager *fman = fman_from_fence(eaction->fence);
> -	unsigned long irq_flags;
> -
> -	spin_lock_irqsave(&fman->lock, irq_flags);
> -	list_del(&eaction->fpriv_head);
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
>  
>  	vmw_fence_obj_unreference(&eaction->fence);
>  	kfree(eaction);
> @@ -983,8 +937,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
>  {
>  	struct vmw_event_fence_action *eaction;
>  	struct vmw_fence_manager *fman = fman_from_fence(fence);
> -	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
> -	unsigned long irq_flags;
>  
>  	eaction = kzalloc(sizeof(*eaction), GFP_KERNEL);
>  	if (unlikely(eaction == NULL))
> @@ -1001,10 +953,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv,
>  	eaction->tv_sec = tv_sec;
>  	eaction->tv_usec = tv_usec;
>  
> -	spin_lock_irqsave(&fman->lock, irq_flags);
> -	list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events);
> -	spin_unlock_irqrestore(&fman->lock, irq_flags);
> -
>  	vmw_fence_obj_add_action(fence, &eaction->action);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> index 8be6c29f5eb5..83ae301ee141 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
> @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data,
>  				     struct drm_file *file_priv);
>  extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
>  				 struct drm_file *file_priv);
> -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman,
> -				       struct list_head *event_list);
>  extern int vmw_event_fence_action_queue(struct drm_file *filee_priv,
>  					struct vmw_fence_obj *fence,
>  					struct drm_pending_event *event,

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

✗ failure: Fi.CI.BAT

[Patchwork]

== Summary ==

HEAD is now at ff88655 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest
Applying: drm: kerneldoc for drm_fops.c
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 drm: kerneldoc for drm_fops.c

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Clean up pending events in the core

[Daniel Stone]

Hi,

On 10 January 2016 at 23:48, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Saturday 09 January 2016 14:28:46 Daniel Vetter wrote:
>> @@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file
>> *file_priv) {
>>       struct drm_device *dev = file_priv->minor->dev;
>>       struct drm_pending_event *e, *et;
>> -     struct drm_pending_vblank_event *v, *vt;
>>       unsigned long flags;
>>
>>       spin_lock_irqsave(&dev->event_lock, flags);
>>
>> -     /* Remove pending flips */
>> -     list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
>> -             if (v->base.file_priv == file_priv) {
>> -                     list_del(&v->base.link);
>> -                     drm_vblank_put(dev, v->pipe);
>> -                     v->base.destroy(&v->base);
>> -             }
>
> Where does this code go ?

It doesn't: instead of deleting the events, the helpers to either
cancel or send the event just notice that file_priv is NULL and bail
out early.

>> +     /* Unlink pending events */
>> +     list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
>> +                              pending_link) {
>> +             list_del(&e->pending_link);
>> +             e->file_priv = NULL;
>> +     }

file_priv gets reset here ...

>> @@ -736,7 +736,10 @@ void drm_event_cancel_free(struct drm_device *dev,
>>  {
>>       unsigned long flags;
>>       spin_lock_irqsave(&dev->event_lock, flags);
>> -     p->file_priv->event_space += p->event->length;
>> +     if (p->file_priv) {
>> +             p->file_priv->event_space += p->event->length;
>> +             list_del(&p->pending_link);
>> +     }
>>       spin_unlock_irqrestore(&dev->event_lock, flags);
>>       p->destroy(p);


Allowing us to DTRT here ...

>>  void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event
>> *e) {
>>       assert_spin_locked(&dev->event_lock);
>>
>> +     if (!e->file_priv) {
>
> I don't think this could happen before this patch as e->file_priv is
> dereferenced below, and I don't see anything in this patch that makes the
> condition possible.
>
>> +             e->destroy(e);
>> +             return;
>> +     }

... and now here.

This looks good to me, and a good sight better than doing it in every
driver. Still drowning in stuff after three weeks off though, so the
best I can offer for the series right now is:
Acked-by: Daniel Stone <daniels@collabora.com>

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

Re: ✗ failure: Fi.CI.BAT

[Daniel Vetter]

On Mon, Jan 11, 2016 at 11:20:21AM -0000, Patchwork wrote:
> == Summary ==
> 
> HEAD is now at ff88655 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest
> Applying: drm: kerneldoc for drm_fops.c
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 drm: kerneldoc for drm_fops.c

Can we please patchwork CI about patch -p1 or maybe even wiggle?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Clean up pending events in the core

[Laurent Pinchart]

Hi Daniel,

On Monday 11 January 2016 14:51:46 Daniel Stone wrote:
> On 10 January 2016 at 23:48, Laurent Pinchart wrote:
> > On Saturday 09 January 2016 14:28:46 Daniel Vetter wrote:
> >> @@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file
> >> *file_priv) {
> >> 
> >>       struct drm_device *dev = file_priv->minor->dev;
> >>       struct drm_pending_event *e, *et;
> >> 
> >> -     struct drm_pending_vblank_event *v, *vt;
> >> 
> >>       unsigned long flags;
> >>       
> >>       spin_lock_irqsave(&dev->event_lock, flags);
> >> 
> >> -     /* Remove pending flips */
> >> -     list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
> >> -             if (v->base.file_priv == file_priv) {
> >> -                     list_del(&v->base.link);
> >> -                     drm_vblank_put(dev, v->pipe);
> >> -                     v->base.destroy(&v->base);
> >> -             }
> > 
> > Where does this code go ?
> 
> It doesn't: instead of deleting the events, the helpers to either
> cancel or send the event just notice that file_priv is NULL and bail
> out early.

Thank you for the explanation. What bothered me was that the drm_vblank_put() 
call got dropped. With v2 of this patch series that splits that change to a 
separate patch it's easier to understand.

> >> +     /* Unlink pending events */
> >> +     list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
> >> +                              pending_link) {
> >> +             list_del(&e->pending_link);
> >> +             e->file_priv = NULL;
> >> +     }
> 
> file_priv gets reset here ...
> 
> >> @@ -736,7 +736,10 @@ void drm_event_cancel_free(struct drm_device *dev,
> >> 
> >>  {
> >>  
> >>       unsigned long flags;
> >>       spin_lock_irqsave(&dev->event_lock, flags);
> >> 
> >> -     p->file_priv->event_space += p->event->length;
> >> +     if (p->file_priv) {
> >> +             p->file_priv->event_space += p->event->length;
> >> +             list_del(&p->pending_link);
> >> +     }
> >> 
> >>       spin_unlock_irqrestore(&dev->event_lock, flags);
> >>       p->destroy(p);
> 
> Allowing us to DTRT here ...
> 
> >>  void drm_send_event_locked(struct drm_device *dev, struct
> >>  drm_pending_event
> >> 
> >> *e) {
> >> 
> >>       assert_spin_locked(&dev->event_lock);
> >> 
> >> +     if (!e->file_priv) {
> > 
> > I don't think this could happen before this patch as e->file_priv is
> > dereferenced below, and I don't see anything in this patch that makes the
> > condition possible.
> > 
> >> +             e->destroy(e);
> >> +             return;
> >> +     }
> 
> ... and now here.
> 
> This looks good to me, and a good sight better than doing it in every
> driver. Still drowning in stuff after three weeks off though, so the
> best I can offer for the series right now is:
> Acked-by: Daniel Stone <daniels@collabora.com>

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx