On Tue, Oct 9, 2012 at 10:33 PM, Mario Kleiner
<mario.klei...@tuebingen.mpg.de> wrote:
> On 08.10.12 21:50, Rob Clark wrote:
>>
>> From: Rob Clark <r...@ti.com>
>>
>> Signed-off-by: Rob Clark <r...@ti.com>
>> ---
>> drivers/staging/omapdrm/omap_crtc.c | 31
>> ++++++-------------------------
>> 1 file changed, 6 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/staging/omapdrm/omap_crtc.c
>> b/drivers/staging/omapdrm/omap_crtc.c
>> index 732f2ad..74e019a 100644
>> --- a/drivers/staging/omapdrm/omap_crtc.c
>> +++ b/drivers/staging/omapdrm/omap_crtc.c
>> @@ -114,40 +114,21 @@ static void omap_crtc_load_lut(struct drm_crtc
>> *crtc)
>>
>> static void vblank_cb(void *arg)
>> {
>> - static uint32_t sequence = 0;
>> struct drm_crtc *crtc = arg;
>> struct drm_device *dev = crtc->dev;
>> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>> - struct drm_pending_vblank_event *event = omap_crtc->event;
>> unsigned long flags;
>> - struct timeval now;
>>
>> WARN_ON(!event);
>> + spin_lock_irqsave(&dev->event_lock, flags);
>> +
>> + /* wakeup userspace */
>> + if (omap_crtc->event)
>> + drm_send_vblank_event(dev, -1, omap_crtc->event);
>>
>> omap_crtc->event = NULL;
>>
>> - /* wakeup userspace */
>> - if (event) {
>> - do_gettimeofday(&now);
>> -
>> - spin_lock_irqsave(&dev->event_lock, flags);
>> - /* TODO: we can't yet use the vblank time accounting,
>> - * because omapdss lower layer is the one that knows
>> - * the irq # and registers the handler, which more or
>> - * less defeats how drm_irq works.. for now just fake
>> - * the sequence number and use gettimeofday..
>> - *
>> - event->event.sequence = drm_vblank_count_and_time(
>> - dev, omap_crtc->id, &now);
>> - */
>
>
> I think this TOO comment should be retained as a reminder that there is work
> to do.
>
>
>> - event->event.sequence = sequence++;
>
>
> This is problematic. You lose the pseudo vblank counter implemented here,
> which is a violation of the spec, and from my own experience it causes extra
> pain and the need for awful workarounds in userspace clients. Nouveau-kms
> has the same problem for no good reason.
>
> But then, on second thought, the way it is implemented here in the original
> is even more wrong, returning zero might be better.
I was actually debating whether or not to bother sending the omap
patches, because I'm in middle of a re-write of the kms code in
omapdrm that will (among many other things) give us proper vblank
accounting..
There are a surprising # of other drivers that are just using
do_gettimeofday() + seqn=0.. I guess, at least to be consistent,
using seqn=0 is better than the completely bogus seqn. Esp. when you
consider having multiple CRTCs, they would end up not even having
successive sequence numbers with the existing scheme. But like I
said, it's about to be replaced with something sane anyways :-P
BR,
-R
> -mario
>
>
>
>> - event->event.tv_sec = now.tv_sec;
>> - event->event.tv_usec = now.tv_usec;
>> - list_add_tail(&event->base.link,
>> - &event->base.file_priv->event_list);
>> - wake_up_interruptible(&event->base.file_priv->event_wait);
>> - spin_unlock_irqrestore(&dev->event_lock, flags);
>> - }
>> + spin_unlock_irqrestore(&dev->event_lock, flags);
>> }
>>
>> static void page_flip_cb(void *arg)
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
