On Fri, Feb 21, 2014 at 09:03:33PM +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Allow the driver to specify whether all new vblank requests after drm_vblank_off() should be rejected. And add a counterpart called drm_vblank_on() which will again allow vblank requests to come in.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Not really happy about this - drm_irq.c is already a giant mess, adding more driver-specific hacks doesn't help. I think we need a few bits of polish on top of your code:
- Add stern warnings to pre/post_modeset that they're inherently racy.
- Add calls to drm_vblank_off to every driver lacking them. Put it at the beginning of their crtc disable functions expect when there's a call to pre_modeset. Then it should be right after that.
- Sprinkle calls to drm_vblank_on over all drivers. Put them at the end of their crtc enable functions except when there's a call to post_modeset. Then put it right before that.
- Rip out the reject argument again and make it the default. If we have drm_vblank_off everywhere then all old vblank waits should complete and there's no userspace yet out there which races a modeset with vblank ioctl calls. Then only issue would be userspace which does vblank waits on disabled ioctls which a) is buggy b) we can easily fix with a driver quirk flag if _really_ needed.
This way the drm_irq.c mess will at least converge a bit and so should help generic display servers (like Wayland) a lot.
I can volunteer for this if you want to punt on it yourself. -Daniel
drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/drm_irq.c | 29 ++++++++++++++++++++++++++++- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/gma500/gma_display.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 6 +++--- drivers/gpu/drm/tegra/dc.c | 2 +- include/drm/drmP.h | 4 +++- 7 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index d8e3982..74317b2 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -257,7 +257,7 @@ static void armada_drm_vblank_off(struct armada_crtc *dcrtc) * Tell the DRM core that vblank IRQs aren't going to happen for * a while. This cleans up any pending vblank events for us. */
- drm_vblank_off(dev, dcrtc->num);
drm_vblank_off(dev, dcrtc->num, false);
/* Handle any pending flip event. */ spin_lock_irq(&dev->event_lock);
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3211158..6e5d820 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -890,6 +890,12 @@ int drm_vblank_get(struct drm_device *dev, int crtc) int ret = 0;
spin_lock_irqsave(&dev->vbl_lock, irqflags);
- if (dev->vblank[crtc].reject) {
ret = -EINVAL;goto out;- }
- /* Going from 0->1 means we have to enable interrupts again */ if (atomic_add_return(1, &dev->vblank[crtc].refcount) == 1) { spin_lock(&dev->vblank_time_lock);
@@ -917,6 +923,8 @@ int drm_vblank_get(struct drm_device *dev, int crtc) ret = -EINVAL; } }
out: spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
return ret;
@@ -947,8 +955,9 @@ EXPORT_SYMBOL(drm_vblank_put);
- drm_vblank_off - disable vblank events on a CRTC
- @dev: DRM device
- @crtc: CRTC in question
*/
- @reject: reject drm_vblank_get() until drm_vblank_on() has been called?
-void drm_vblank_off(struct drm_device *dev, int crtc) +void drm_vblank_off(struct drm_device *dev, int crtc, bool reject) { struct drm_pending_vblank_event *e, *t; struct timeval now; @@ -956,6 +965,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc) unsigned int seq;
spin_lock_irqsave(&dev->vbl_lock, irqflags);
- dev->vblank[crtc].reject = reject; vblank_disable_and_save(dev, crtc); wake_up(&dev->vblank[crtc].queue);
@@ -979,6 +989,22 @@ void drm_vblank_off(struct drm_device *dev, int crtc) } EXPORT_SYMBOL(drm_vblank_off);
+/**
- drm_vblank_on - enable vblank events on a CRTC
- @dev: DRM device
- @crtc: CRTC in question
- */
+void drm_vblank_on(struct drm_device *dev, int crtc) +{
- unsigned long irqflags;
- spin_lock_irqsave(&dev->vbl_lock, irqflags);
- dev->vblank[crtc].reject = false;
- spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+} +EXPORT_SYMBOL(drm_vblank_on);
/**
- drm_vblank_pre_modeset - account for vblanks across mode sets
- @dev: DRM device
@@ -1224,6 +1250,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data, DRM_WAIT_ON(ret, dev->vblank[crtc].queue, 3 * HZ, (((drm_vblank_count(dev, crtc) - vblwait->request.sequence) <= (1 << 23)) ||
dev->vblank[crtc].reject || !dev->irq_enabled));if (ret != -EINTR) {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index ebc0150..e2d6b9d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -68,7 +68,7 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) /* wait for the completion of page flip. */ wait_event(exynos_crtc->pending_flip_queue, atomic_read(&exynos_crtc->pending_flip) == 0);
drm_vblank_off(crtc->dev, exynos_crtc->pipe);
drm_vblank_off(crtc->dev, exynos_crtc->pipe, false);}
exynos_drm_fn_encoder(crtc, &mode, exynos_drm_encoder_crtc_dpms);
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index 386de2c..ff18220 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -281,7 +281,7 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode) REG_WRITE(VGACNTRL, VGA_DISP_DISABLE);
/* Turn off vblank interrupts */
drm_vblank_off(dev, pipe);
drm_vblank_off(dev, pipe, false);/* Wait for vblank for the disable to take effect */ gma_wait_for_vblank(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f19e6ea..bab0d08 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3643,7 +3643,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc) int plane = intel_crtc->plane;
intel_crtc_wait_for_pending_flips(crtc);
- drm_vblank_off(dev, pipe);
drm_vblank_off(dev, pipe, false);
/* FBC must be disabled before disabling the plane on HSW. */ if (dev_priv->fbc.plane == plane)
@@ -3774,7 +3774,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) encoder->disable(encoder);
intel_crtc_wait_for_pending_flips(crtc);
- drm_vblank_off(dev, pipe);
drm_vblank_off(dev, pipe, false);
if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev);
@@ -4239,7 +4239,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
/* Give the overlay scaler a chance to disable if it's on this pipe */ intel_crtc_wait_for_pending_flips(crtc);
- drm_vblank_off(dev, pipe);
drm_vblank_off(dev, pipe, false);
if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev);
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 9336006..480bfec 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -324,7 +324,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc) } }
- drm_vblank_off(drm, dc->pipe);
- drm_vblank_off(drm, dc->pipe, false);
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index f974da9..ee40483 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1090,6 +1090,7 @@ struct drm_vblank_crtc { int crtc; /* crtc index */ bool enabled; /* so we don't call enable more than once per disable */
- bool reject; /* reject drm_vblank_get()? */
};
/** @@ -1400,7 +1401,8 @@ extern void drm_send_vblank_event(struct drm_device *dev, int crtc, extern bool drm_handle_vblank(struct drm_device *dev, int crtc); extern int drm_vblank_get(struct drm_device *dev, int crtc); extern void drm_vblank_put(struct drm_device *dev, int crtc); -extern void drm_vblank_off(struct drm_device *dev, int crtc); +extern void drm_vblank_off(struct drm_device *dev, int crtc, bool reject); +extern void drm_vblank_on(struct drm_device *dev, int crtc); extern void drm_vblank_cleanup(struct drm_device *dev); extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, struct timeval *tvblank, unsigned flags); -- 1.8.3.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel