On Wed, May 21, 2014 at 01:32:35PM +0200, Thierry Reding wrote:
On Wed, May 14, 2014 at 08:51:06PM +0200, Daniel Vetter wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
drm_vblank_off() will turn off vblank interrupts, but as long as the refcount is elevated drm_vblank_get() will not re-enable them. This is a problem is someone is holding a vblank reference while a modeset is happening, and the driver requires vblank interrupt to work during that time.
Add drm_vblank_on() as a counterpart to drm_vblank_off() which will re-enabled vblank interrupts if the refcount is already elevated. This will allow drivers to choose the specific places in the modeset sequence at which vblank interrupts get disabled and enabled.
Testcase: igt/kms_flip/*-vs-suspend Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com [danvet: Add Testcase tag for the igt I've written.] Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_irq.c | 72 ++++++++++++++++++++++++++---------- drivers/gpu/drm/i915/intel_display.c | 8 ++++ include/drm/drmP.h | 1 + 3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 13d671ed3421..dd786d84daab 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -840,6 +840,41 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) }
/**
- drm_vblank_enable - enable the vblank interrupt on a CRTC
- @dev: DRM device
- @crtc: CRTC in question
- */
Perhaps the kernel-doc here should contain some of what's described in the commit message? Also a "Return:" section would be useful here to specify what's an error and what isn't.
+static int drm_vblank_enable(struct drm_device *dev, int crtc)
On second thought, since this is a local function, my comments above apply to drm_vblank_on() below rather than drm_vblank_enable().
Yeah, we don't pull this into the kerneldoc, only functions exported to drivers. Follow-up patch should (hopefully) sufficiently pimp the kerneldoc for drm_vblank_on. If not please complain.
+{
- int ret = 0;
- assert_spin_locked(&dev->vbl_lock);
- spin_lock(&dev->vblank_time_lock);
- if (!dev->vblank[crtc].enabled) {
/* Enable vblank irqs under vblank_time_lock protection.* All vblank count & timestamp updates are held off* until we are done reinitializing master counter and* timestamps. Filtercode in drm_handle_vblank() will* prevent double-accounting of same vblank interval.*/Coding style:
/* * Enable... */
?
Yeah, will polish.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
Perhaps split off the i915 changes into a separate patch?
Yeah, should have. But I've got fed up with the conflicts this topic branch caused so pulled it in. I'll apply your review feedback as follow-up patches. -Daniel