[Intel-gfx] [PATCH] drm: Return current vblank value for drmWaitVBlank queries

[Michel Dänzer]

On 18.03.2015 11:53, Michel Dänzer wrote:
> On 18.03.2015 00:44, Chris Wilson wrote:
>> When userspace queries the current vblank for the CRTC, we can reply
>> with the cached value (using atomic reads to serialise with the vblank
>> interrupt as necessary) without having to touch registers. In the
>> instant disable case, this saves us from enabling/disabling the vblank
>> around every query, greatly reducing the number of registers read and
>> written.
>>
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Cc: Dave Airlie <airlied at redhat.com>
>> ---
>>  drivers/gpu/drm/drm_irq.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index c8a34476570a..6c4570082b65 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -1585,7 +1585,18 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
>>  	if (crtc >= dev->num_crtcs)
>>  		return -EINVAL;
>>  
>> -	vblank = &dev->vblank[crtc];
>> +	/* Fast-path the query for the current value (without an event)
>> +	 * to avoid having to enable/disable the vblank interrupts.
>> +	 */
>> +	if ((vblwait->request.type & (_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK)) == _DRM_VBLANK_RELATIVE &&
>> +	    vblwait->request.sequence == 0) {
>> +		struct timeval now;
>> +
>> +		vblwait->reply.sequence = drm_vblank_count_and_time(dev, crtc, &now);
>> +		vblwait->reply.tval_sec = now.tv_sec;
>> +		vblwait->reply.tval_usec = now.tv_usec;
>> +		return 0;
>> +	}
> 
> drm_vblank_count_and_time() doesn't return the correct sequence number
> while the vblank interrupt is disabled, does it? It returns the sequence
> number from the last time vblank_disable_and_save() was called (when the
> vblank interrupt was disabled). That's why drm_vblank_get() is needed here.

It might be possible to avoid drm_vblank_get() and use
drm_update_vblank_count() before (or in) drm_vblank_count_and_time()
instead when the vblank interrupt is disabled, but then you'd first need
to make it safe to call drm_update_vblank_count() several times while
the vblank interrupt is disabled, by making it update vblank->last at least.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer