drm_event cleanup, round 2
Subsystems (20)
cover drm_event cleanup, round 2
Hi all,
Mostly just small changes from review feedback (plus a few misplaced hunks, silly me). Plus an attempt at better kerneldoc to explain how this works. Since that caused questions both from Thomas and Laurent let me explain things also here:
Currently anyone using drm_events (vblank code, atomic/legacy flips, drivers like exynos) need to handle event setup, sending to the drm file and handling when the drm file disappears themselves. This patch series provides functions to encapsulate all these steps. Furthermore the file closing is handled
differently: Events are no longer completely destroyed in preclose hooks like all current users do, but instead they're just unlinked from the file. The new drm_send_event can still be called, but instead of transmitting the event to the file it'll destroy it right away.
This means that when the drm file disappears it's entirely transparent to drivers, and we can remove all the special tracking and cleanup code around preclose functions.
The other consequence is that it's opt-in - drivers are still free to remove events explicitly when the file disappears, it's just needless code. Which means the patch series can be split up really nicely into core parts + driver patches. The only patch I didn't split up in this fashion is the one that creates drm_send_event - it's imo easier to review this way, and the driver changes are really simple.
Cheers, Daniel
Daniel Vetter (22):
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: Nuke vblank event file cleanup code 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 | 271 ++++++++++++++++++++++--- 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_crtc.c | 13 +- drivers/gpu/drm/omapdrm/omap_drv.c | 41 ---- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - 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 | 11 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 87 +------- drivers/gpu/drm/vmwgfx/vmwgfx_fence.h | 2 - include/drm/drmP.h | 26 ++- 48 files changed, 312 insertions(+), 623 deletions(-)
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/22 drm: kerneldoc for drm_fops.c
Just prep work before I throw more drm_event refactorings on top.
--- Documentation/DocBook/gpu.tmpl | 48 +-------------- drivers/gpu/drm/drm_fops.c | 129 ++++++++++++++++++++++++++++++++++------- include/drm/drmP.h | 17 +++--- 3 files changed, 117 insertions(+), 77 deletions(-)
Documentation/DocBook/gpu.tmpl +2 -46
@@ -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>
drivers/gpu/drm/drm_fops.c +107 -22
202 lines not on this page · raw message
include/drm/drmP.h +8 -9
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 2/22 drm: Add functions to setup/tear down drm_events.
In reply to Daniel VetterAn 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).
--- 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(-)
drivers/gpu/drm/drm_atomic.c +11 -33
@@ -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); } }
drivers/gpu/drm/drm_crtc.c +10 -26
@@ -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 {
drivers/gpu/drm/drm_fops.c +67 -0
70 lines not on this page · raw message
include/drm/drmP.h +6 -1
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/22] drm: Add functions to setup/tear down drm_events.
Hi Daniel,
Thank you for the patch.
Quoted 7 lines · On Monday 11 January 2016 22:40:56 Daniel Vetter wrote:
An attempt at not spreading out the
file_priv->event_spacestuff out quite so far and wide. And I think fixes something inipp_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.
Quoted 7 lines
v2: Spelling fixes (Alex).
Cc Alex Deucher <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Cc Rob Clark <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 235 lines
--- 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(-)drivers/gpu/drm/drm_atomic.c +11
-33@@ -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); } }drivers/gpu/drm/drm_crtc.c +10
-26@@ -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 ;drivers/gpu/drm/drm_crtc.c +0
-0}else {drivers/gpu/drm/drm_fops.c +9
-0@@ -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.
+ */drivers/gpu/drm/drm_fops.c +35
-0+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.
+ */drivers/gpu/drm/drm_fops.c +10
-0+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);include/drm/drmP.h +6
-1@@ -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,
Signature · 8 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 3/22 drm/exynos: Use the new event init/free functions
In reply to Daniel VetterAlso 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.
--- 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(-)
drivers/gpu/drm/exynos/exynos_drm_g2d.c +8 -23
@@ -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;
drivers/gpu/drm/exynos/exynos_drm_ipp.c +9 -14
@@ -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);
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 4/22 drm/vmwgfx: Use the new event init/free functions
In reply to Daniel Vetter--- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-)
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +8 -24
@@ -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; }
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 5/22 drm: Create drm_send_event helpers
In reply to Daniel VetterUse 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.
--- 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(-)
drivers/gpu/drm/drm_fops.c +41 -1
@@ -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);
drivers/gpu/drm/drm_irq.c +2 -5
@@ -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); }
drivers/gpu/drm/exynos/exynos_drm_g2d.c +1 -4
@@ -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)
drivers/gpu/drm/exynos/exynos_drm_ipp.c +1 -4
@@ -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",
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +1 -2
@@ -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); }
include/drm/drmP.h +2 -0
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/22] drm: Create drm_send_event helpers
Hi Daniel,
Thank you for the patch.
Quoted 6 lines · On Monday 11 January 2016 22:40:59 Daniel Vetter wrote:
Use them in the core vblank code and exynos/vmwgfx drivers.
Note that the difference between
wake_up_alland_interruptiblein vmwgfx doesn't matter since the only waiter is the core code indrm_fops.c. And that is interruptible.
Quoted 8 lines
v2: Adjust existing kerneldoc too.
Reviewed-by Alex Deucher <[email protected]> (v1)Acked-by Daniel Stone <[email protected]>Cc Alex Deucher <[email protected]>Cc Thomas Hellstrom <[email protected]>Cc Inki Dae <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 152 lines
--- 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(-)drivers/gpu/drm/drm_fops.c +0
-0@@ -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 + * usingdrm_send_event()ordrm_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.drivers/gpu/drm/drm_fops.c +7
-0@@ -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, seedrm_send_event()for the unlocked version. + */
+voiddrm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)+{drivers/gpu/drm/drm_fops.c +14
-0+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 acquiresdev->event_lock, + * seedrm_send_event_locked()for callers which already hold this lock. + */drivers/gpu/drm/drm_fops.c +9
-0+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);drivers/gpu/drm/drm_irq.c +2
-5@@ -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); }(quoted diff) +0
-0
b/drivers/gpu/drm/exynos/exynos_drm_g2d.cindex82e7f95dfed9..db56c8259f18100644drivers/gpu/drm/exynos/exynos_drm_g2d.c +1
-4@@ -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)(quoted diff) +0
-0
b/drivers/gpu/drm/exynos/exynos_drm_ipp.cindexc8819c05e2dd..1f6a6c1881d6100644drivers/gpu/drm/exynos/exynos_drm_ipp.c +1
-4@@ -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",(quoted diff) +0
-0
b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.cindexeda93bf52a6e..e0edf149d9d5100644drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +1
-2@@ -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); }include/drm/drmP.h +1
-0@@ -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,
Signature · 8 lines
Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 6/22 drm/fsl: Remove preclose hook
In reply to Daniel VetterDoesn't do anything, but annoys when auditing them all.
--- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 5 ----- 1 file changed, 5 deletions(-)
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +0 -5
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 7/22 drm/armada: Remove NULL open/pre/postclose hooks
In reply to Daniel VetterThe 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.
--- drivers/gpu/drm/armada/armada_drv.c | 3 --- 1 file changed, 3 deletions(-)
drivers/gpu/drm/armada/armada_drv.c +0 -3
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 07/22] drm/armada: Remove NULL open/pre/postclose hooks
Quoted 3 lines · 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.
I'll try to remember to queue this after -rc1, though a reminder after -rc1 would be useful.
Thanks.
Signature · 7 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 07/22] drm/armada: Remove NULL open/pre/postclose hooks
Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · 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.
I'll try to remember to queue this after -rc1, though a reminder after -rc1 would be useful.
I've planed to take the entire series in through drm-misc after -rc1, since at least conceptually it's all the same topic. Would that be ok with you too?
Thanks, Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 8/22 drm/gma500: Remove empty preclose hook
In reply to Daniel VetterI'm auditing them all, empty ones just confuse ...
--- drivers/gpu/drm/gma500/psb_drv.c | 9 --------- 1 file changed, 9 deletions(-)
drivers/gpu/drm/gma500/psb_drv.c +0 -9
@@ -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),
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/22] drm/gma500: Remove empty preclose hook
Quoted 6 lines · Daniel Vetter
I'm auditing them all, empty ones just confuse ...
Cc Patrik Jakobsson <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 32 lines
--- drivers/gpu/drm/gma500/psb_drv.c | 9 --------- 1 file changed, 9 deletions(-)drivers/gpu/drm/gma500/psb_drv.c +0
-10@@ -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), --
Quoted 2 lines
2.6.4
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 9/22 drm: Clean up pending events in the core
In reply to Daniel VetterThere'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.
v3: Improve wording of the kerneldoc and split out vblank cleanup (Laurent).
--- drivers/gpu/drm/drm_fops.c | 30 +++++++++++++++++++++++++++++- include/drm/drmP.h | 2 ++ 2 files changed, 31 insertions(+), 1 deletion(-)
drivers/gpu/drm/drm_fops.c +29 -1
@@ -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 */
@@ -366,6 +367,13 @@ static void drm_events_release(struct drm_file *file_priv) 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) {list_del (&e->link);
@@ -712,6 +720,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 +745,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 +762,22 @@ 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 not worry about whether the +* DRM file for this event still exists and can call this function upon +* completion of the asynchronous 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 +792,11 @@ 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 not worry about whether the +* DRM file for this event still exists and can call this function upon +* completion of the asynchronous work unconditionally. */ void drm_send_event (struct drm_device *dev,struct drm_pending_event *e) {
include/drm/drmP.h +2 -0
@@ -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;
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/22] drm: Clean up pending events in the core
Hi Daniel,
Thank you for the patch.
Quoted 7 lines · On Monday 11 January 2016 22:41:03 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_eventusers would need to know when that has happened, since calling e.g.drm_send_eventisn't allowed any more. Instead just unlink them from the file, and detect this case and handle it appropriately in all functions.
Quoted 6 lines
v2: Adjust existing kerneldoc too.
v3: Improve wording of the kerneldoc and split out vblank cleanup (Laurent).
Cc Alex Deucher <[email protected]>Cc Laurent Pinchart <[email protected]>
Quoted 108 lines
Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]> (v1)Signed-off-by Daniel Vetter <[email protected]>--- drivers/gpu/drm/drm_fops.c | 30 +++++++++++++++++++++++++++++- include/drm/drmP.h | 2 ++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_fops.c +13
-1@@ -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 */ @@ -366,6 +367,13 @@ static void drm_events_release(struct drm_file *file_priv) 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) {list_del (&e->link);@@ -712,6 +720,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 +745,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 +762,22 @@ 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, seedrm_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 not worry about whether the + * DRM file for this event still exists and can call this function upon + * completion of the asynchronous work unconditionally. */
void drm_send_event_locked(structdrm_device*dev, structdrm_pending_event
*e) {drivers/gpu/drm/drm_fops.c +6
-0assert_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 +792,11 @@ 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 acquiresdev->event_lock, * seedrm_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 not worry about whether the + * DRM file for this event still exists and can call this function upon + * completion of the asynchronous work unconditionally. */
voiddrm_send_event(struct drm_device *dev, struct drm_pending_event *e){include/drm/drmP.h +2
-0@@ -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;
Signature · 8 lines
Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH 9/22 drm: Clean up pending events in the core
In reply to Daniel VetterHey,
Op 11-01-16 om 22:41 schreef Daniel Vetter:
Quoted 9 lines
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_eventusers would need to know when that has happened, since calling e.g.drm_send_eventisn'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.
Quoted 7 lines
v3: Improve wording of the kerneldoc and split out vblank cleanup (Laurent).
Cc Alex Deucher <[email protected]>Cc Laurent Pinchart <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]> (v1)Signed-off-by Daniel Vetter <[email protected]>
This patch breaks kms_flip.basic-flip-vs-wf_vblank and probably other tests as well.
Quoted 98 lines
drivers/gpu/drm/drm_fops.c | 30 +++++++++++++++++++++++++++++- include/drm/drmP.h | 2 ++ 2 files changed, 31 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_fops.c +29
-1@@ -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 */ @@ -366,6 +367,13 @@ static void drm_events_release(struct drm_file *file_priv) 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) {list_del (&e->link);@@ -712,6 +720,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 +745,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 +762,22 @@ 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 not worry about whether the +* DRM file for this event still exists and can call this function upon +* completion of the asynchronous 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 +792,11 @@ 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 not worry about whether the +* DRM file for this event still exists and can call this function upon +* completion of the asynchronous work unconditionally. */ void drm_send_event (struct drm_device *dev,struct drm_pending_event *e) {include/drm/drmP.h +1
-0@@ -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 */
include/drm/drmP.h struct drm_file { · line 353
Quoted 7 lines
include/drm/drmP.h +1
-0@@ -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;
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 10/22 drm: Nuke vblank event file cleanup code
In reply to Daniel VetterThe core code now takes care of unlinking drm_events from the file in a generic way, so this code isn't needed any more.
For those wondering where the drm_vblank_put went to: With the new logic events only get unlinked, but still exist. Hence any resources (like vblank counters) don't need to be released since the event user will still process the event normally. In this case this is the callsites of send_vblank_event, which of course already have a drm_vblank_put.
--- drivers/gpu/drm/drm_fops.c | 9 --------- 1 file changed, 9 deletions(-)
drivers/gpu/drm/drm_fops.c +0 -9
@@ -354,19 +354,10 @@ 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) {
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/22] drm: Nuke vblank event file cleanup code
Hi Daniel,
Thank you for the patch.
Quoted 10 lines · On Monday 11 January 2016 22:41:04 Daniel Vetter wrote:
The core code now takes care of unlinking
drm_eventsfrom the file in a generic way, so this code isn't needed any more.For those wondering where the
drm_vblank_putwent to: With the new logic events only get unlinked, but still exist. Hence any resources (like vblank counters) don't need to be released since the event user will still process the event normally. In this case this is the callsites ofsend_vblank_event, which of course already have adrm_vblank_put.
Quoted 4 lines
Cc Laurent Pinchart <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 28 lines
--- drivers/gpu/drm/drm_fops.c | 9 --------- 1 file changed, 9 deletions(-)drivers/gpu/drm/drm_fops.c +0
-9@@ -354,19 +354,10 @@ 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) {
Signature · 8 lines
Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 11/22 drm/i915: Nuke intel_modeset_preclose
In reply to Daniel VetterNow that the drm core unlinks/disarms events there's no need to do so ourselves anymore. Nuke the code.
--- 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(-)
drivers/gpu/drm/i915/i915_dma.c +0 -2
@@ -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)
drivers/gpu/drm/i915/intel_display.c +0 -21
@@ -16357,24 +16357,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); - } -}
drivers/gpu/drm/i915/intel_drv.h +0 -1
@@ -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);
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 12/22 drm/atmel: Nuke preclose
In reply to Daniel VetterThe only thing this did was cancle pending flip events, and the core takes care of that now.
--- 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(-)
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +0 -18
@@ -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;
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +0 -10
@@ -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,
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +0 -3
@@ -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);
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 12/22] drm/atmel: Nuke preclose
Quoted 7 lines · On Mon, 11 Jan 2016 22:41:06 +0100 Daniel Vetter <[email protected]> wrote:
The only thing this did was cancle pending flip events, and the core takes care of that now.
Cc Boris Brezillon <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 77 lines
--- 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(-)drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +0
-18@@ -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;drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +0
-10@@ -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,drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +0
-3@@ -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);
Signature · 7 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 13/22 drm/exynos: Remove event cancelling from postclose
In reply to Daniel VetterThe core takes care of this now. And since kfree(NULL) is ok we can simplify the function even further now.
--- drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 -------------- 1 file changed, 14 deletions(-)
drivers/gpu/drm/exynos/exynos_drm_drv.c +0 -14
@@ -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 ; }
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 13/22] drm/exynos: Remove event cancelling from postclose
Hi Daniel,
It seems your patch is exactly same as below my one I posted before,
http://www.spinics.net/lists/dri-devel/msg97922.html
Anyway, it's ok if this patch can go to mainline.
2016년 01월 12일 06:41에 Daniel Vetter 이(가) 쓴 글:
Quoted 27 lines
The core takes care of this now. And since kfree(NULL) is ok we can simplify the function even further now.
Cc Inki Dae <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>--- drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 -------------- 1 file changed, 14 deletions(-)drivers/gpu/drm/exynos/exynos_drm_drv.c +0
-7@@ -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);
drivers/gpu/drm/exynos/exynos_drm_drv.c static void exynos_drm_preclose(struct drm_device *dev, · line 340
Quoted 10 lines
drivers/gpu/drm/exynos/exynos_drm_drv.c +0
-7-/* 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 ; }
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 14/22 drm/imx: Unconfuse preclose logic
In reply to Daniel VetterSo 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.
--- drivers/gpu/drm/imx/imx-drm-core.c | 13 ------------- drivers/gpu/drm/imx/ipuv3-crtc.c | 4 ---- 2 files changed, 17 deletions(-)
drivers/gpu/drm/imx/imx-drm-core.c +0 -13
@@ -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,
drivers/gpu/drm/imx/ipuv3-crtc.c +0 -4
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/22] drm/imx: Unconfuse preclose logic
Am Montag, den 11.01.2016, 22:41 +0100 schrieb Daniel Vetter:
Quoted 14 lines
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_vblankhook 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_lockfor this.
Quoted 5 lines
Cc Sascha Hauer <[email protected]>Cc Philipp Zabel <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
regards
Philipp
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 15/22 drm/msm: Nuke preclose hooks
In reply to Daniel VetterThey 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.
--- 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(-)
drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c +0 -7
@@ -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) {
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +0 -11
@@ -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,
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h +0 -1
@@ -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);
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +0 -6
@@ -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) {
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +0 -11
@@ -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,
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +0 -1
@@ -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);
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 16/22 drm/omap: Nuke close hooks
In reply to Daniel VetterAgain since the core takes care of this we can remove them. While at it also remove the postclose hook, it's empty.
v2: Laurent pointed me at even more code to delete.
--- drivers/gpu/drm/omapdrm/omap_crtc.c | 13 +----------- drivers/gpu/drm/omapdrm/omap_drv.c | 41 ------------------------------------- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - 3 files changed, 1 insertion(+), 54 deletions(-)
drivers/gpu/drm/omapdrm/omap_crtc.c +1 -12
@@ -269,18 +269,7 @@ static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)return ;spin_lock_irqsave (&dev->event_lock, flags); - -list_del (&event->base.link); - -/* -* Queue the event for delivery if it's still linked to a file -* handle, otherwise just destroy it. -*/ -if (event->base.file_priv) -drm_crtc_send_vblank_event (crtc, event); -else - event->base.destroy (&event->base); - +drm_crtc_send_vblank_event (crtc, event);spin_unlock_irqrestore (&dev->event_lock, flags); }
drivers/gpu/drm/omapdrm/omap_drv.c +0 -41
65 lines not on this page · raw message
drivers/gpu/drm/omapdrm/omap_drv.h +0 -1
@@ -106,7 +106,6 @@ struct omap_drm_private {/* atomic commit */ struct { -struct list_head events;wait_queue_head_t wait;u32 pending;spinlock_t lock;/* Protects commit.pending */
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 16/22] drm/omap: Nuke close hooks
Quoted 10 lines · 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.
v2: Laurent pointed me at even more code to delete.
Cc Laurent Pinchart <[email protected]>Cc Tomi Valkeinen <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 5 lines
--- drivers/gpu/drm/omapdrm/omap_crtc.c | 13 +----------- drivers/gpu/drm/omapdrm/omap_drv.c | 41 ------------------------------------- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - 3 files changed, 1 insertion(+), 54 deletions(-)
Tomi
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 16/22] drm/omap: Nuke close hooks
In reply to Daniel VetterHi Daniel,
Quoted 11 lines · Daniel Vetter
drivers/gpu/drm/omapdrm/omap_drv.c +0
-3@@ -175,17 +175,6 @@ static int omap_atomic_commit(struct drm_device *dev, priv->commit.pending |= commit->crtcs;spin_unlock (&priv->commit.lock); -/* 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) {
drivers/gpu/drm/omapdrm/omap_drv.c static int omap_atomic_commit(struct drm_device *dev, · line 179
Quoted 10 lines
drivers/gpu/drm/omapdrm/omap_drv.c +0
-8-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); -/* Swap the state, this is the point of no return. */ drm_atomic_helper_swap_state (dev, state);
'flags' local needs to be removed from omap_atomic_commit, otherwise:
drivers/gpu/drm/omapdrm/omap_drv.c:145:16: error: unused variable 'flags'
Tomi
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/1 drm/omap: Nuke close hooks
In reply to Daniel VetterAgain since the core takes care of this we can remove them. While at it also remove the postclose hook, it's empty.
v2: Laurent pointed me at even more code to delete.
v3: Remove unused flags (Tomi).
--- drivers/gpu/drm/omapdrm/omap_crtc.c | 13 +----------- drivers/gpu/drm/omapdrm/omap_drv.c | 42 ------------------------------------- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - 3 files changed, 1 insertion(+), 55 deletions(-)
drivers/gpu/drm/omapdrm/omap_crtc.c +1 -12
@@ -269,18 +269,7 @@ static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)return ;spin_lock_irqsave (&dev->event_lock, flags); - -list_del (&event->base.link); - -/* -* Queue the event for delivery if it's still linked to a file -* handle, otherwise just destroy it. -*/ -if (event->base.file_priv) -drm_crtc_send_vblank_event (crtc, event); -else - event->base.destroy (&event->base); - +drm_crtc_send_vblank_event (crtc, event);spin_unlock_irqrestore (&dev->event_lock, flags); }
drivers/gpu/drm/omapdrm/omap_drv.c +0 -42
72 lines not on this page · raw message
drivers/gpu/drm/omapdrm/omap_drv.h +0 -1
@@ -106,7 +106,6 @@ struct omap_drm_private {/* atomic commit */ struct { -struct list_head events;wait_queue_head_t wait;u32 pending;spinlock_t lock;/* Protects commit.pending */
Signature · 6 lines
2.7.0.rc3 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: Nuke close hooks
Hi Daniel,
Thank you for the patch.
Quoted 5 lines · On Wednesday 13 January 2016 12:05:14 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.
v2: Laurent pointed me at even more code to delete.
Quoted 7 lines
v3: Remove unused flags (Tomi).
Cc Laurent Pinchart <[email protected]>Cc Tomi Valkeinen <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 126 lines
--- drivers/gpu/drm/omapdrm/omap_crtc.c | 13 +----------- drivers/gpu/drm/omapdrm/omap_drv.c | 42 ---------------------------------- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - 3 files changed, 1 insertion(+), 55 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/omapdrm/omap_crtc.cindex2ed0754ed19e..d38fcbcc43a8100644drivers/gpu/drm/omapdrm/omap_crtc.c +1
-12@@ -269,18 +269,7 @@ static void omap_crtc_complete_page_flip(struct drm_crtc *crtc) return;spin_lock_irqsave (&dev->event_lock, flags); - -list_del (&event->base.link); - -/* -* Queue the event for delivery if it's still linked to a file -* handle, otherwise just destroy it. -*/ -if (event->base.file_priv) -drm_crtc_send_vblank_event (crtc, event); -else - event->base.destroy (&event->base); - +drm_crtc_send_vblank_event (crtc, event);spin_unlock_irqrestore (&dev->event_lock, flags); }(quoted diff) +0
-0
b/drivers/gpu/drm/omapdrm/omap_drv.cindexdfafdb602ad2..33370f42e4d7100644drivers/gpu/drm/omapdrm/omap_drv.c +0
-42@@ -142,7 +142,6 @@ static int omap_atomic_commit(struct drm_device *dev, {struct omap_drm_private *priv = dev->dev_private;struct omap_atomic_state_commit *commit; -unsigned long flags;unsigned int i;int ret;@@ -175,17 +174,6 @@ static int omap_atomic_commit(struct drm_device *dev, priv->commit.pending |= commit->crtcs;spin_unlock (&priv->commit.lock); -/* 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); -/* Swap the state, this is the point of no return. */ drm_atomic_helper_swap_state (dev, state);@@ -673,7 +661,6 @@ static int dev_load(struct drm_device *dev, unsigned long flags) priv->wq = alloc_ordered_workqueue("omapdrm", 0);init_waitqueue_head (&priv->commit.wait);spin_lock_init (&priv->commit.lock); -INIT_LIST_HEAD (&priv->commit.events);spin_lock_init (&priv->list_lock);INIT_LIST_HEAD (&priv->obj_list);@@ -787,33 +774,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 +798,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,(quoted diff) +0
-0
b/drivers/gpu/drm/omapdrm/omap_drv.hindex9e0030731c37..c23cbe6fe9e4100644drivers/gpu/drm/omapdrm/omap_drv.h +0
-1@@ -106,7 +106,6 @@ struct omap_drm_private {/* atomic commit */ struct { -struct list_head events;wait_queue_head_t wait;u32 pending;spinlock_t lock;/* Protects commit.pending */
Signature · 8 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 17/22 drm/rcar: Nuke preclose hook
In reply to Daniel VetterAgain since the drm core takes care of event unlinking/disarming this is now just needless code.
--- 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(-)
drivers/gpu/drm/rcar-du/rcar_du_crtc.c +0 -20
@@ -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;
drivers/gpu/drm/rcar-du/rcar_du_crtc.h +0 -2
@@ -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);
drivers/gpu/drm/rcar-du/rcar_du_drv.c +0 -10
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 18/22 drm/shmob: Nuke preclose hook
In reply to Daniel VetterAgain since the drm core takes care of event unlinking/disarming this is now just needless code.
v2: Fixup misplaced hunk.
--- 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(-)
drivers/gpu/drm/shmobile/shmob_drm_crtc.c +0 -20
@@ -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;
drivers/gpu/drm/shmobile/shmob_drm_crtc.h +0 -2
@@ -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);
drivers/gpu/drm/shmobile/shmob_drm_drv.c +0 -8
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 19/22 drm/tegra: Stop cancelling page flip events
In reply to Daniel VetterThe core takes care of that now.
v2: Fixup misplaced hunk.
--- 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(-)
drivers/gpu/drm/tegra/dc.c +0 -17
@@ -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);
drivers/gpu/drm/tegra/drm.c +0 -3
@@ -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);
drivers/gpu/drm/tegra/drm.h +0 -1
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 19/22] drm/tegra: Stop cancelling page flip events
Quoted 9 lines · Daniel Vetter
The core takes care of that now.
v2: Fixup misplaced hunk.
Cc Thierry Reding <[email protected]>Cc Terje Bergström <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]>Signed-off-by Daniel Vetter <[email protected]>
Quoted 5 lines
--- 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(-)
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 20/22 drm/tilcdc: Nuke preclose hook
In reply to Daniel VetterAgain since the drm core takes care of event unlinking/disarming this is now just needless code.
v2: Fixup misplaced hunks.
--- 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(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0 -20
@@ -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;
drivers/gpu/drm/tilcdc/tilcdc_drv.c +0 -8
@@ -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,
drivers/gpu/drm/tilcdc/tilcdc_drv.h +0 -1
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 20/22] drm/tilcdc: Nuke preclose hook
Quoted 33 lines · 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 <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]> (v1)Signed-off-by Daniel Vetter <[email protected]>--- 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(-)drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0
-10@@ -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. -*/
drivers/gpu/drm/tilcdc/tilcdc_crtc.c irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) · line 684
Quoted 10 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0
-10-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); -} -
Hmm, looks fine, but when I was comparing the omapdrm change and this one, I see tilcdc doing drm_vblank_put() in the removed code but omapdrm doesn't.
The other patches that nuke preclose hooks also contain vblank_put. Will there be a vblank_put call missing here, or will there be an extra vblank_put call happening somewhere on omapdrm?
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 20/22] drm/tilcdc: Nuke preclose hook
Quoted 50 lines · Tomi Valkeinen
Quoted 33 lines · 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 <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]> (v1)Signed-off-by Daniel Vetter <[email protected]>--- 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(-)drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0
-10@@ -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. -*/ drivers/gpu/drm/tilcdc/tilcdc_crtc.c irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) · line 684
Quoted 10 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0
-10-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); -} -Hmm, looks fine, but when I was comparing the omapdrm change and this one, I see tilcdc doing
drm_vblank_put()in the removed code but omapdrm doesn't.
Quoted 3 lines
The other patches that nuke preclose hooks also contain
vblank_put. Will there be avblank_putcall missing here, or will there be an extravblank_putcall happening somewhere on omapdrm?
Different approaches to the same problem:
- omap just unlinks the event from fpriv and still process it normally. But then before sending it out it checks whether the fpriv is still there or not and either sends it, or deletes the event directly. This way the vblank_put is always called from the worker/irq handler as part of the event processing.
This is the same approach I implemented in core with this series.
- tilcdc (and most other drivers) entirely destroy the event in the preclose hook, which means they must also release any other resources acquired as part of that event. Therefore they have the vblank_put here. But the vblank_put is obviously also in the normal event processing paths, so with the new approach of only unlinking it we can handle this without any special cases in the driver.
I hope this explains what's going on. Since you're about driver maintainer no. 3 with the same question: Can you pls review the kerneldoc and make sure it explains this well? I tried to improve it already a bit after Laurent's/Thomas' questions.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 20/22] drm/tilcdc: Nuke preclose hook
Quoted 17 lines · Daniel Vetter
Different approaches to the same problem:
- omap just unlinks the event from fpriv and still process it normally. But then before sending it out it checks whether the fpriv is still there or not and either sends it, or deletes the event directly. This way the
vblank_putis always called from the worker/irq handler as part of the event processing.This is the same approach I implemented in core with this series.
- tilcdc (and most other drivers) entirely destroy the event in the preclose hook, which means they must also release any other resources acquired as part of that event. Therefore they have the
vblank_puthere. But thevblank_putis obviously also in the normal event processing paths, so with the new approach of only unlinking it we can handle this without any special cases in the driver.
Quoted 4 lines
I hope this explains what's going on. Since you're about driver maintainer no. 3 with the same question: Can you pls review the kerneldoc and make sure it explains this well? I tried to improve it already a bit after Laurent's/Thomas' questions.
Ok, makes sense. I think the kernel doc is fine.
I wasn't able to test tilcdc, as it seems to crash on drm-next at the moment... Need to debug that first.
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 21/22 drm/vc4: Nuke preclose hook
In reply to Daniel VetterAgain since the drm core takes care of event unlinking/disarming this is now just needless code.
v2: Fixup misplaced hunk.
--- 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(-)
drivers/gpu/drm/vc4/vc4_crtc.c +0 -20
@@ -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 ,
drivers/gpu/drm/vc4/vc4_drv.c +0 -10
@@ -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,
drivers/gpu/drm/vc4/vc4_drv.h +0 -1
@@ -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 */
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 21/22] drm/vc4: Nuke preclose hook
Quoted 9 lines · 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 <[email protected]>Acked-by Daniel Stone <[email protected]>Reviewed-by Alex Deucher <[email protected]> (v1)Signed-off-by Daniel Vetter <[email protected]>
I love the cleanup from this series.
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 22/22 drm/vmwgfx: Nuke preclose hook
In reply to Daniel VetterAgain 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.
--- 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(-)
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +0 -11
@@ -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,
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +0 -1
@@ -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; };
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +0 -52
88 lines not on this page · raw message
drivers/gpu/drm/vmwgfx/vmwgfx_fence.h +0 -2
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/1 drm/vmwgfx: Nuke preclose hook
In reply to Daniel VetterAgain 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.
--- 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(-)
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +0 -11
@@ -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,
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +0 -1
@@ -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; };
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +0 -52
88 lines not on this page · raw message
drivers/gpu/drm/vmwgfx/vmwgfx_fence.h +0 -2
@@ -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,
Signature · 6 lines
2.6.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ failure: Fi.CI.BAT
In reply to Daniel Vetter== Summary ==
== Summary ==
HEAD is now at a907968 drm-intel-nightly: 2016y-01m-11d-17h-22m-54s 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 20 lines · Daniel Vetter
Hi all,
Mostly just small changes from review feedback (plus a few misplaced hunks, silly me). Plus an attempt at better kerneldoc to explain how this works. Since that caused questions both from Thomas and Laurent let me explain things also here:
Currently anyone using
drm_events(vblank code, atomic/legacy flips, drivers like exynos) need to handle event setup, sending to the drm file and handling when the drm file disappears themselves. This patch series provides functions to encapsulate all these steps. Furthermore the file closing is handled
differently: Events are no longer completely destroyed in preclose hooks like all current users do, but instead they're just unlinked from the file. The newdrm_send_eventcan still be called, but instead of transmitting the event to the file it'll destroy it right away.This means that when the drm file disappears it's entirely transparent to drivers, and we can remove all the special tracking and cleanup code around preclose functions.
Quoted 6 lines
The other consequence is that it's opt-in - drivers are still free to remove events explicitly when the file disappears, it's just needless code. Which means the patch series can be split up really nicely into core parts + driver patches. The only patch I didn't split up in this fashion is the one that creates
drm_send_event- it's imo easier to review this way, and the driver changes are really simple.
-rc1 is out, so I pulled this all into drm-misc. Thanks a lot for all the reviews&comments.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx