On Tue, Jul 29, 2014 at 08:31:55PM +0300, Ville Syrjälä wrote:
On Thu, Jun 19, 2014 at 05:41:24PM -0700, Matt Roper wrote:
On Mon, May 26, 2014 at 05:26:47PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a flag to drm_device which will cause the vblank code to bypass the disable timer and always disable the vblank interrupt immediately when the last reference is dropped.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_irq.c | 6 +++--- include/drm/drmP.h | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 20a4855..b008803 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -994,11 +994,11 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
/* Last user schedules interrupt disable */ if (atomic_dec_and_test(&vblank->refcount)) {
if (drm_vblank_offdelay > 0)
if (dev->vblank_disable_immediate || drm_vblank_offdelay == 0)vblank_disable_fn((unsigned long)vblank);else if (drm_vblank_offdelay > 0) mod_timer(&vblank->disable_timer, jiffies + ((drm_vblank_offdelay * HZ)/1000));
else if (drm_vblank_offdelay == 0)vblank_disable_fn((unsigned long)vblank);} EXPORT_SYMBOL(drm_vblank_put);
Would it be better if we just squashed this device flag logic back into patch 7, but kept the drm_vblank_offdelay == 0 meaning of "never disable?" I can see there being people who might already use this when debugging new and potentially flaky hardware platforms who would be surprised by the change in behavior. So basically something like:
if (drm_vblank_offdelay == 0) return else if (dev->vblank_disable_immediate) vblank_disable_fn((unsigned long)vblank); else mod_timer(...);I'm not sure I want drm_vblank_offdelay affecting drivers that have vblank_disable_immediate set since it's a global variable. If there are two devices on the system where one has vblank_disable_immediate==true and the other doesn't, the user might want to keep vblank interrupts enabled on the crappy device all time by frobbing drm_vblank_offdelay.
I agree that changing the meaning of drm_vblank_offdelay=0 is a bit questionable, but reversing the meaning so that '==0' meas 'never disable' and '<0' means 'disable immediately' seemed a bit weird for my taste. But I suppose I could make that change if people think it's better. Or maybe just forget about the 'disable immediately' behaviour when vblank_disable_immediate==false?
I'd also suggest adding a comment in drm_stub.c to indicate that drm_vblank_offdelay gets ignored for drivers that set vblank_disable_immediate.
Good idea.
Yeah, I think that's good enough. There really shouldn't be any need for drivers which support immediate vblank disabling to ever keep it on for a while. Presuming the immediate disable actually works ofc. -Daniel