Re: [PATCH 1/2] drm: Shortcircuit vblank queries

[Mario Kleiner <mario.kleiner.de@gmail.com>]

On 04/14/2015 08:36 PM, Chris Wilson wrote:
> On Tue, Apr 14, 2015 at 08:22:20PM +0200, Mario Kleiner wrote:
>> On 04/05/2015 05:40 PM, Chris Wilson wrote:
>>> Avoid adding to the waitqueue and reprobing the current vblank if the
>>> caller is only querying the current vblank sequence and timestamp and
>>> so we would return immediately.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Michel Dänzer <michel@daenzer.net>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Dave Airlie <airlied@redhat.com>,
>>> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
>>> ---
>>>   drivers/gpu/drm/drm_irq.c | 18 ++++++++++--------
>>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>>> index 6f5dc18779e2..ba80b51b4b00 100644
>>> --- a/drivers/gpu/drm/drm_irq.c
>>> +++ b/drivers/gpu/drm/drm_irq.c
>>> @@ -1617,14 +1617,16 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
>>>   		vblwait->request.sequence = seq + 1;
>>>   	}
>>>
>>> -	DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
>>> -		  vblwait->request.sequence, crtc);
>>> -	vblank->last_wait = vblwait->request.sequence;
>>> -	DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
>>> -		    (((drm_vblank_count(dev, crtc) -
>>> -		       vblwait->request.sequence) <= (1 << 23)) ||
>>> -		     !vblank->enabled ||
>>> -		     !dev->irq_enabled));
>>> +	if (vblwait->request.sequence != seq) {
>>> +		DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
>>> +			  vblwait->request.sequence, crtc);
>>> +		vblank->last_wait = vblwait->request.sequence;
>>> +		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
>>> +			    (((drm_vblank_count(dev, crtc) -
>>> +			       vblwait->request.sequence) <= (1 << 23)) ||
>>> +			     !vblank->enabled ||
>>> +			     !dev->irq_enabled));
>>> +	}
>>>
>>>   	if (ret != -EINTR) {
>>>   		struct timeval now;
>>>
>>
>> It would be good to have some DRM_DEBUG output for the skip-the-wait
>> case as well, so one can still follow from dmesg output when a
>> client does a drmWaitVblank call even if it is only a query.
>
> We still have DRM_DEBUG("returning %d to client"), as well as the
> drmIoctl:DRM_DEBUG(ioctl->name), is that not sufficient?
> -Chris
>

Oh right, that's good enough. Maybe add "on crtc %d" to that DRM_DEBUG, 
to make it unambiguous for which crtc some count is returned?

-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel