[PATCH 01/11] drm: add drm_send_vblank_event() helper (v5)

2012-10-16 Thread Rob Clark 
From: Rob Clark 

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

v1: original
v2: add back 'vblwait->reply.sequence = seq' which should not have
been deleted
v3: add WARN_ON() in case lock is not held and comments
v4: use WARN_ON_SMP() instead to fix issue with !SMP && !DEBUG_SPINLOCK
as pointed out by Marcin Slusarz
v5: update docbook

Signed-off-by: Rob Clark 
---
 Documentation/DocBook/drm.tmpl |   20 +++
 drivers/gpu/drm/drm_irq.c  |   74 
 include/drm/drmP.h |2 ++
 3 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b030052..c9cbb3f 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -1141,23 +1141,13 @@ int max_width, max_height;
 the page_flip operation will be called 
with a
 non-NULL event argument pointing to a
 drm_pending_vblank_event instance. Upon 
page
-flip completion the driver must fill the
-event::event
-sequence, 
tv_sec
-and tv_usec fields with the associated
-vertical blanking count and timestamp, add the event to the
-drm_file list of events to be signaled, and 
wake
-up any waiting process. This can be performed with
+flip completion the driver must call 
drm_send_vblank_event
+to fill in the event and send to wake up any waiting processes.
+This can be performed with
 
   
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..9bdcfd5 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,46 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);

+static void send_vblank_event(struct drm_device *dev,
+   struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   WARN_ON_SMP(!spin_is_locked(>event_lock));
+   e->event.sequence = seq;
+   e->event.tv_sec = now->tv_sec;
+   e->event.tv_usec = now->tv_usec;
+
+   list_add_tail(>base.link,
+ >base.file_priv->event_list);
+   wake_up_interruptible(>base.file_priv->event_wait);
+   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
+e->event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ * Caller must hold event lock.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc >= 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, );
+   } else {
+   seq = 0;
+   do_gettimeofday();
+   }
+   send_vblank_event(dev, e, seq, );
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -936,6 +976,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
 }
 EXPORT_SYMBOL(drm_vblank_put);

+/**
+ * drm_vblank_off - disable vblank events on a CRTC
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ *
+ * Caller must hold event lock.
+ */
 void drm_vblank_off(struct drm_device *dev, int crtc)
 {
struct drm_pending_vblank_event *e, *t;
@@ -955,15 +1002,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG("Sending premature vblank event on disable: \
  wanted %d, current %d\n",
  e->event.sequence, seq);
-
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(dev, e, seq, );
}

spin_unlock_irqrestore(>vbl_lock, irqflags);
@@ -1107,15 +1148,9 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-16 Thread Laurent Pinchart 
Hi Mario,

On Saturday 13 October 2012 02:28:03 Mario Kleiner wrote:
> On 11.10.12 16:19, Laurent Pinchart wrote:
> > On Monday 08 October 2012 14:50:39 Rob Clark wrote:
> >> From: Rob Clark 
> 
> ...
> 
> > Do you know why some drivers don't call drm_vblank_count_and_time() ? For
> > instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
> > looks like it could just call drm_vblank_count_and_time().
> 
> At least nouveau could use it. Lucas Stach and me wrote patches for
> nouveau-kms, and they went through many iterations and missed many
> kernel merge windows due to slow review until i think both of us got
> tired of resubmitting with tiny changes.

I totally understand the feeling, but please don't give up. You can CC me on 
the next iteration, I'll make sure to review the patches (even though I have 
no experience with the nouveau driver).

> The latest iteration is posted by Lucas on nouveau-devel from 26. April
> 2012. Not sure if they'd still apply after the nouveau-kms rewrite. I'll
> probably give them another try once that has landed when i have some spare
> time.
> 
> In principle it's very simple to use drm_vblank_count_and_time(). A
> driver needs to
> 
> 1. Call drm_handle_vblank() from its vblank irq handler.
> 
> 2. Make sure that in the vblank of pageflip completion
> drm_handle_vblank() is called before drm_vblank_count_and_time(), so the
> latter picks up updated counts and timestamps.
> 
> 3. Big bonus for high precision and robustness: Implement the
> driver->get_vblank_timestamp() hook to provide a precise vblank
> timestamp. One simple way to do that is like radeon-kms or intel-kms do
> it: Call back into drm_calc_vbltimestamp_from_scanoutpos() and provide
> the driver->get_scanout_position() function - a function that returns
> the current hardware scanline counter. This is precise down to ~ 10
> microseconds (at least confirmed by measurements on
> intel,radeon,nouveau) and robust against delayed vblank irq handling.
> 
> -mario
-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-16 Thread Laurent Pinchart 
Hi Mario,

On Saturday 13 October 2012 02:28:03 Mario Kleiner wrote:
 On 11.10.12 16:19, Laurent Pinchart wrote:
  On Monday 08 October 2012 14:50:39 Rob Clark wrote:
  From: Rob Clark r...@ti.com
 
 ...
 
  Do you know why some drivers don't call drm_vblank_count_and_time() ? For
  instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
  looks like it could just call drm_vblank_count_and_time().
 
 At least nouveau could use it. Lucas Stach and me wrote patches for
 nouveau-kms, and they went through many iterations and missed many
 kernel merge windows due to slow review until i think both of us got
 tired of resubmitting with tiny changes.

I totally understand the feeling, but please don't give up. You can CC me on 
the next iteration, I'll make sure to review the patches (even though I have 
no experience with the nouveau driver).

 The latest iteration is posted by Lucas on nouveau-devel from 26. April
 2012. Not sure if they'd still apply after the nouveau-kms rewrite. I'll
 probably give them another try once that has landed when i have some spare
 time.
 
 In principle it's very simple to use drm_vblank_count_and_time(). A
 driver needs to
 
 1. Call drm_handle_vblank() from its vblank irq handler.
 
 2. Make sure that in the vblank of pageflip completion
 drm_handle_vblank() is called before drm_vblank_count_and_time(), so the
 latter picks up updated counts and timestamps.
 
 3. Big bonus for high precision and robustness: Implement the
 driver-get_vblank_timestamp() hook to provide a precise vblank
 timestamp. One simple way to do that is like radeon-kms or intel-kms do
 it: Call back into drm_calc_vbltimestamp_from_scanoutpos() and provide
 the driver-get_scanout_position() function - a function that returns
 the current hardware scanline counter. This is precise down to ~ 10
 microseconds (at least confirmed by measurements on
 intel,radeon,nouveau) and robust against delayed vblank irq handling.
 
 -mario
-- 
Regards,

Laurent Pinchart

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

[PATCH 01/11] drm: add drm_send_vblank_event() helper (v5)

2012-10-16 Thread Rob Clark 
From: Rob Clark r...@ti.com

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

v1: original
v2: add back 'vblwait-reply.sequence = seq' which should not have
been deleted
v3: add WARN_ON() in case lock is not held and comments
v4: use WARN_ON_SMP() instead to fix issue with !SMP  !DEBUG_SPINLOCK
as pointed out by Marcin Slusarz
v5: update docbook

Signed-off-by: Rob Clark r...@ti.com
---
 Documentation/DocBook/drm.tmpl |   20 +++
 drivers/gpu/drm/drm_irq.c  |   74 
 include/drm/drmP.h |2 ++
 3 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b030052..c9cbb3f 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -1141,23 +1141,13 @@ int max_width, max_height;/synopsis
 the methodnamepage_flip/methodname operation will be called 
with a
 non-NULL parameterevent/parameter argument pointing to a
 structnamedrm_pending_vblank_event/structname instance. Upon 
page
-flip completion the driver must fill the
-parameterevent/parameter::structfieldevent/structfield
-structfieldsequence/structfield, 
structfieldtv_sec/structfield
-and structfieldtv_usec/structfield fields with the associated
-vertical blanking count and timestamp, add the event to the
-parameterdrm_file/parameter list of events to be signaled, and 
wake
-up any waiting process. This can be performed with
+flip completion the driver must call 
methodnamedrm_send_vblank_event/methodname
+to fill in the event and send to wake up any waiting processes.
+This can be performed with
 programlisting![CDATA[
-struct timeval now;
-
-event-event.sequence = drm_vblank_count_and_time(..., now);
-event-event.tv_sec = now.tv_sec;
-event-event.tv_usec = now.tv_usec;
-
 spin_lock_irqsave(dev-event_lock, flags);
-list_add_tail(event-base.link, 
event-base.file_priv-event_list);
-wake_up_interruptible(event-base.file_priv-event_wait);
+...
+drm_send_vblank_event(dev, pipe, event);
 spin_unlock_irqrestore(dev-event_lock, flags);
 ]]/programlisting
   /para
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..9bdcfd5 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,46 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);
 
+static void send_vblank_event(struct drm_device *dev,
+   struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   WARN_ON_SMP(!spin_is_locked(dev-event_lock));
+   e-event.sequence = seq;
+   e-event.tv_sec = now-tv_sec;
+   e-event.tv_usec = now-tv_usec;
+
+   list_add_tail(e-base.link,
+ e-base.file_priv-event_list);
+   wake_up_interruptible(e-base.file_priv-event_wait);
+   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
+e-event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ * Caller must hold event lock.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc = 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, now);
+   } else {
+   seq = 0;
+   do_gettimeofday(now);
+   }
+   send_vblank_event(dev, e, seq, now);
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -936,6 +976,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
 }
 EXPORT_SYMBOL(drm_vblank_put);
 
+/**
+ * drm_vblank_off - disable vblank events on a CRTC
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ *
+ * Caller must hold event lock.
+ */
 void drm_vblank_off(struct drm_device *dev, int crtc)
 {
struct drm_pending_vblank_event *e, *t;
@@ -955,15 +1002,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG(Sending premature vblank event on disable: \

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-13 Thread Mario Kleiner 
On 11.10.12 16:19, Laurent Pinchart wrote:
> Hi Rob,
>
> Thanks for the patch.
>
> On Monday 08 October 2012 14:50:39 Rob Clark wrote:
>> From: Rob Clark 
>>

...
>
> Do you know why some drivers don't call drm_vblank_count_and_time() ? For
> instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
> looks like it could just call drm_vblank_count_and_time().
>

At least nouveau could use it. Lucas Stach and me wrote patches for 
nouveau-kms, and they went through many iterations and missed many 
kernel merge windows due to slow review until i think both of us got 
tired of resubmitting with tiny changes. The latest iteration is posted 
by Lucas on nouveau-devel from 26. April 2012. Not sure if they'd still 
apply after the nouveau-kms rewrite. I'll probably give them another try 
once that has landed when i have some spare time.

In principle it's very simple to use drm_vblank_count_and_time(). A 
driver needs to

1. Call drm_handle_vblank() from its vblank irq handler.

2. Make sure that in the vblank of pageflip completion 
drm_handle_vblank() is called before drm_vblank_count_and_time(), so the 
latter picks up updated counts and timestamps.

3. Big bonus for high precision and robustness: Implement the 
driver->get_vblank_timestamp() hook to provide a precise vblank 
timestamp. One simple way to do that is like radeon-kms or intel-kms do 
it: Call back into drm_calc_vbltimestamp_from_scanoutpos() and provide 
the driver->get_scanout_position() function - a function that returns 
the current hardware scanline counter. This is precise down to ~ 10 
microseconds (at least confirmed by measurements on 
intel,radeon,nouveau) and robust against delayed vblank irq handling.

-mario

[PATCH 01/11] drm: add drm_send_vblank_event() helper (v3)

2012-10-12 Thread Marcin Slusarz 
On Thu, Oct 11, 2012 at 07:29:15PM -0500, Rob Clark wrote:
> From: Rob Clark 
> 
> A helper that drivers can use to send vblank event after a pageflip.
> If the driver doesn't support proper vblank irq based time/seqn then
> just pass -1 for the pipe # to get do_gettimestamp() behavior (since
> there are a lot of drivers that don't use drm_vblank_count_and_time())
> 
> Also an internal send_vblank_event() helper for the various other code
> paths within drm_irq that also need to send vblank events.
> 
> v1: original
> v2: add back 'vblwait->reply.sequence = seq' which should not have
> been deleted
> v3: add WARN_ON() in case lock is not held and comments
> 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/drm_irq.c |   74 
> +++--
>  include/drm/drmP.h|2 ++
>  2 files changed, 54 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 076c4a8..d623a06 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -802,6 +802,46 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, 
> int crtc,
>  }
>  EXPORT_SYMBOL(drm_vblank_count_and_time);
>  
> +static void send_vblank_event(struct drm_device *dev,
> + struct drm_pending_vblank_event *e,
> + unsigned long seq, struct timeval *now)
> +{
> + WARN_ON(!spin_is_locked(>event_lock));

This should be WARN_ON_SMP - on !SMP && !DEBUG_SPINLOCK spin_is_locked always 
returns 0.

> + e->event.sequence = seq;
> + e->event.tv_sec = now->tv_sec;
> + e->event.tv_usec = now->tv_usec;
> +
> + list_add_tail(>base.link,
> +   >base.file_priv->event_list);
> + wake_up_interruptible(>base.file_priv->event_wait);
> + trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> +  e->event.sequence);
> +}
> +
> +/**
> + * drm_send_vblank_event - helper to send vblank event after pageflip
> + * @dev: DRM device
> + * @crtc: CRTC in question
> + * @e: the event to send
> + *
> + * Updates sequence # and timestamp on event, and sends it to userspace.
> + * Caller must hold event lock.
> + */
> +void drm_send_vblank_event(struct drm_device *dev, int crtc,
> + struct drm_pending_vblank_event *e)
> +{
> + struct timeval now;
> + unsigned int seq;
> + if (crtc >= 0) {
> + seq = drm_vblank_count_and_time(dev, crtc, );
> + } else {
> + seq = 0;
> + do_gettimeofday();
> + }
> + send_vblank_event(dev, e, seq, );
> +}
> +EXPORT_SYMBOL(drm_send_vblank_event);
> +
>  /**
>   * drm_update_vblank_count - update the master vblank counter
>   * @dev: DRM device
> @@ -936,6 +976,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
>  }
>  EXPORT_SYMBOL(drm_vblank_put);
>  
> +/**
> + * drm_vblank_off - disable vblank events on a CRTC
> + * @dev: DRM device
> + * @crtc: CRTC in question
> + *
> + * Caller must hold event lock.
> + */
>  void drm_vblank_off(struct drm_device *dev, int crtc)
>  {
>   struct drm_pending_vblank_event *e, *t;
> @@ -955,15 +1002,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>   DRM_DEBUG("Sending premature vblank event on disable: \
> wanted %d, current %d\n",
> e->event.sequence, seq);
> -
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
> + list_del(>base.link);
>   drm_vblank_put(dev, e->pipe);
> - list_move_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> -  e->event.sequence);
> + send_vblank_event(dev, e, seq, );
>   }
>  
>   spin_unlock_irqrestore(>vbl_lock, irqflags);
> @@ -1107,15 +1148,9 @@ static int drm_queue_vblank_event(struct drm_device 
> *dev, int pipe,
>  
>   e->event.sequence = vblwait->request.sequence;
>   if ((seq - vblwait->request.sequence) <= (1 << 23)) {
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
>   drm_vblank_put(dev, pipe);
> - list_add_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> + send_vblank_event(dev, e, seq, );
>   vblwait->reply.sequence = seq;
> - trace_drm_vblank_event_delivered(current->pid, pipe,
> -  vblwait->request.sequence);
>   } else {
>   /* drm_handle_vblank_events will call drm_vblank_put */
>   list_add_tail(>base.link, >vblank_event_list);
> @@ -1256,14 +1291,9 @@ static void drm_handle_vblank_events(struct

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper (v3)

2012-10-12 Thread Marcin Slusarz 
On Thu, Oct 11, 2012 at 07:29:15PM -0500, Rob Clark wrote:
 From: Rob Clark r...@ti.com
 
 A helper that drivers can use to send vblank event after a pageflip.
 If the driver doesn't support proper vblank irq based time/seqn then
 just pass -1 for the pipe # to get do_gettimestamp() behavior (since
 there are a lot of drivers that don't use drm_vblank_count_and_time())
 
 Also an internal send_vblank_event() helper for the various other code
 paths within drm_irq that also need to send vblank events.
 
 v1: original
 v2: add back 'vblwait-reply.sequence = seq' which should not have
 been deleted
 v3: add WARN_ON() in case lock is not held and comments
 
 Signed-off-by: Rob Clark r...@ti.com
 ---
  drivers/gpu/drm/drm_irq.c |   74 
 +++--
  include/drm/drmP.h|2 ++
  2 files changed, 54 insertions(+), 22 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
 index 076c4a8..d623a06 100644
 --- a/drivers/gpu/drm/drm_irq.c
 +++ b/drivers/gpu/drm/drm_irq.c
 @@ -802,6 +802,46 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, 
 int crtc,
  }
  EXPORT_SYMBOL(drm_vblank_count_and_time);
  
 +static void send_vblank_event(struct drm_device *dev,
 + struct drm_pending_vblank_event *e,
 + unsigned long seq, struct timeval *now)
 +{
 + WARN_ON(!spin_is_locked(dev-event_lock));

This should be WARN_ON_SMP - on !SMP  !DEBUG_SPINLOCK spin_is_locked always 
returns 0.

 + e-event.sequence = seq;
 + e-event.tv_sec = now-tv_sec;
 + e-event.tv_usec = now-tv_usec;
 +
 + list_add_tail(e-base.link,
 +   e-base.file_priv-event_list);
 + wake_up_interruptible(e-base.file_priv-event_wait);
 + trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 +  e-event.sequence);
 +}
 +
 +/**
 + * drm_send_vblank_event - helper to send vblank event after pageflip
 + * @dev: DRM device
 + * @crtc: CRTC in question
 + * @e: the event to send
 + *
 + * Updates sequence # and timestamp on event, and sends it to userspace.
 + * Caller must hold event lock.
 + */
 +void drm_send_vblank_event(struct drm_device *dev, int crtc,
 + struct drm_pending_vblank_event *e)
 +{
 + struct timeval now;
 + unsigned int seq;
 + if (crtc = 0) {
 + seq = drm_vblank_count_and_time(dev, crtc, now);
 + } else {
 + seq = 0;
 + do_gettimeofday(now);
 + }
 + send_vblank_event(dev, e, seq, now);
 +}
 +EXPORT_SYMBOL(drm_send_vblank_event);
 +
  /**
   * drm_update_vblank_count - update the master vblank counter
   * @dev: DRM device
 @@ -936,6 +976,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
  }
  EXPORT_SYMBOL(drm_vblank_put);
  
 +/**
 + * drm_vblank_off - disable vblank events on a CRTC
 + * @dev: DRM device
 + * @crtc: CRTC in question
 + *
 + * Caller must hold event lock.
 + */
  void drm_vblank_off(struct drm_device *dev, int crtc)
  {
   struct drm_pending_vblank_event *e, *t;
 @@ -955,15 +1002,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
   DRM_DEBUG(Sending premature vblank event on disable: \
 wanted %d, current %d\n,
 e-event.sequence, seq);
 -
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
 + list_del(e-base.link);
   drm_vblank_put(dev, e-pipe);
 - list_move_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 -  e-event.sequence);
 + send_vblank_event(dev, e, seq, now);
   }
  
   spin_unlock_irqrestore(dev-vbl_lock, irqflags);
 @@ -1107,15 +1148,9 @@ static int drm_queue_vblank_event(struct drm_device 
 *dev, int pipe,
  
   e-event.sequence = vblwait-request.sequence;
   if ((seq - vblwait-request.sequence) = (1  23)) {
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
   drm_vblank_put(dev, pipe);
 - list_add_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 + send_vblank_event(dev, e, seq, now);
   vblwait-reply.sequence = seq;
 - trace_drm_vblank_event_delivered(current-pid, pipe,
 -  vblwait-request.sequence);
   } else {
   /* drm_handle_vblank_events will call drm_vblank_put */
   list_add_tail(e-base.link, dev-vblank_event_list);
 @@ -1256,14 +1291,9 @@ static void drm_handle_vblank_events(struct drm_device 
 *dev, int crtc)
   DRM_DEBUG(vblank event on %d, current %d\n,

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-12 Thread Mario Kleiner 

On 11.10.12 16:19, Laurent Pinchart wrote:

Hi Rob,

Thanks for the patch.

On Monday 08 October 2012 14:50:39 Rob Clark wrote:

From: Rob Clark r...@ti.com



...


Do you know why some drivers don't call drm_vblank_count_and_time() ? For
instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
looks like it could just call drm_vblank_count_and_time().



At least nouveau could use it. Lucas Stach and me wrote patches for 
nouveau-kms, and they went through many iterations and missed many 
kernel merge windows due to slow review until i think both of us got 
tired of resubmitting with tiny changes. The latest iteration is posted 
by Lucas on nouveau-devel from 26. April 2012. Not sure if they'd still 
apply after the nouveau-kms rewrite. I'll probably give them another try 
once that has landed when i have some spare time.


In principle it's very simple to use drm_vblank_count_and_time(). A 
driver needs to


1. Call drm_handle_vblank() from its vblank irq handler.

2. Make sure that in the vblank of pageflip completion 
drm_handle_vblank() is called before drm_vblank_count_and_time(), so the 
latter picks up updated counts and timestamps.


3. Big bonus for high precision and robustness: Implement the 
driver-get_vblank_timestamp() hook to provide a precise vblank 
timestamp. One simple way to do that is like radeon-kms or intel-kms do 
it: Call back into drm_calc_vbltimestamp_from_scanoutpos() and provide 
the driver-get_scanout_position() function - a function that returns 
the current hardware scanline counter. This is precise down to ~ 10 
microseconds (at least confirmed by measurements on 
intel,radeon,nouveau) and robust against delayed vblank irq handling.


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

[PATCH 01/11] drm: add drm_send_vblank_event() helper (v3)

2012-10-11 Thread Rob Clark 
From: Rob Clark 

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

v1: original
v2: add back 'vblwait->reply.sequence = seq' which should not have
been deleted
v3: add WARN_ON() in case lock is not held and comments

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_irq.c |   74 +++--
 include/drm/drmP.h|2 ++
 2 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..d623a06 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,46 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);

+static void send_vblank_event(struct drm_device *dev,
+   struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   WARN_ON(!spin_is_locked(>event_lock));
+   e->event.sequence = seq;
+   e->event.tv_sec = now->tv_sec;
+   e->event.tv_usec = now->tv_usec;
+
+   list_add_tail(>base.link,
+ >base.file_priv->event_list);
+   wake_up_interruptible(>base.file_priv->event_wait);
+   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
+e->event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ * Caller must hold event lock.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc >= 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, );
+   } else {
+   seq = 0;
+   do_gettimeofday();
+   }
+   send_vblank_event(dev, e, seq, );
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -936,6 +976,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
 }
 EXPORT_SYMBOL(drm_vblank_put);

+/**
+ * drm_vblank_off - disable vblank events on a CRTC
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ *
+ * Caller must hold event lock.
+ */
 void drm_vblank_off(struct drm_device *dev, int crtc)
 {
struct drm_pending_vblank_event *e, *t;
@@ -955,15 +1002,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG("Sending premature vblank event on disable: \
  wanted %d, current %d\n",
  e->event.sequence, seq);
-
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(dev, e, seq, );
}

spin_unlock_irqrestore(>vbl_lock, irqflags);
@@ -1107,15 +1148,9 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,

e->event.sequence = vblwait->request.sequence;
if ((seq - vblwait->request.sequence) <= (1 << 23)) {
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
drm_vblank_put(dev, pipe);
-   list_add_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
+   send_vblank_event(dev, e, seq, );
vblwait->reply.sequence = seq;
-   trace_drm_vblank_event_delivered(current->pid, pipe,
-vblwait->request.sequence);
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(>base.link, >vblank_event_list);
@@ -1256,14 +1291,9 @@ static void drm_handle_vblank_events(struct drm_device 
*dev, int crtc)
DRM_DEBUG("vblank event on %d, current %d\n",
  e->event.sequence, seq);

-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-11 Thread Laurent Pinchart 
Hi Rob,

Thanks for the patch.

On Monday 08 October 2012 14:50:39 Rob Clark wrote:
> From: Rob Clark 
> 
> A helper that drivers can use to send vblank event after a pageflip.
> If the driver doesn't support proper vblank irq based time/seqn then
> just pass -1 for the pipe # to get do_gettimestamp() behavior (since
> there are a lot of drivers that don't use drm_vblank_count_and_time())
> 
> Also an internal send_vblank_event() helper for the various other code
> paths within drm_irq that also need to send vblank events.
> 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/drm_irq.c |   65 +++---
>  include/drm/drmP.h|2 ++
>  2 files changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 076c4a8..7a00d94 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev,
> int crtc, }
>  EXPORT_SYMBOL(drm_vblank_count_and_time);
> 
> +static void send_vblank_event(struct drm_pending_vblank_event *e,
> + unsigned long seq, struct timeval *now)
> +{
> + e->event.sequence = seq;
> + e->event.tv_sec = now->tv_sec;
> + e->event.tv_usec = now->tv_usec;
> +
> + list_add_tail(>base.link,
> +   >base.file_priv->event_list);
> + wake_up_interruptible(>base.file_priv->event_wait);
> + trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> +  e->event.sequence);
> +}
> +
> +/**
> + * drm_send_vblank_event - helper to send vblank event after pageflip
> + * @dev: DRM device
> + * @crtc: CRTC in question
> + * @e: the event to send
> + *
> + * Updates sequence # and timestamp on event, and sends it to userspace.

Due to the list_add_tail above, drm_send_vblank_event() requires locking. As 
you don't take any lock I expect the caller to be supposed to handle locking. 
That should be documented, along with the lock that the caller should take. 
Looking at the existing drivers that should be dev->event_lock, but not all 
drivers take it when calling the vblank functions below (for instance the i915 
and gma500 drivers call drm_vblank_off() without holding the event_lock 
spinlock). We thus seem to have a locking issue that should be fixed, either 
as part of this patch set or as a preliminary patches series.

I have no strong opinion on who takes the spinlock (the caller or the 
send_vblank_event() function) at the moment, but taking it in 
send_vblank_event() would allow calling drm_vblank_count_and_time() and 
do_gettimeofday() below outside of the locked region.

> + */
> +void drm_send_vblank_event(struct drm_device *dev, int crtc,
> + struct drm_pending_vblank_event *e)
> +{
> + struct timeval now;
> + unsigned int seq;
> + if (crtc >= 0) {
> + seq = drm_vblank_count_and_time(dev, crtc, );
> + } else {
> + seq = 0;
> + do_gettimeofday();

Do you know why some drivers don't call drm_vblank_count_and_time() ? For 
instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it 
looks like it could just call drm_vblank_count_and_time().

> + }
> + send_vblank_event(e, seq, );
> +}
> +EXPORT_SYMBOL(drm_send_vblank_event);
> +
>  /**
>   * drm_update_vblank_count - update the master vblank counter
>   * @dev: DRM device
> @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>   DRM_DEBUG("Sending premature vblank event on disable: \
> wanted %d, current %d\n",
> e->event.sequence, seq);
> -
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
> + list_del(>base.link);
>   drm_vblank_put(dev, e->pipe);
> - list_move_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> -  e->event.sequence);
> + send_vblank_event(e, seq, );
>   }
> 
>   spin_unlock_irqrestore(>vbl_lock, irqflags);
> @@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device
> *dev, int pipe,
> 
>   e->event.sequence = vblwait->request.sequence;
>   if ((seq - vblwait->request.sequence) <= (1 << 23)) {
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
>   drm_vblank_put(dev, pipe);
> - list_add_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - vblwait->reply.sequence = seq;
> - trace_drm_vblank_event_delivered(current->pid, pipe,
> -

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-11 Thread Rob Clark 
On Thu, Oct 11, 2012 at 9:19 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> Thanks for the patch.
>
> On Monday 08 October 2012 14:50:39 Rob Clark wrote:
>> From: Rob Clark 
>>
>> A helper that drivers can use to send vblank event after a pageflip.
>> If the driver doesn't support proper vblank irq based time/seqn then
>> just pass -1 for the pipe # to get do_gettimestamp() behavior (since
>> there are a lot of drivers that don't use drm_vblank_count_and_time())
>>
>> Also an internal send_vblank_event() helper for the various other code
>> paths within drm_irq that also need to send vblank events.
>>
>> Signed-off-by: Rob Clark 
>> ---
>>  drivers/gpu/drm/drm_irq.c |   65 +++---
>>  include/drm/drmP.h|2 ++
>>  2 files changed, 44 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 076c4a8..7a00d94 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev,
>> int crtc, }
>>  EXPORT_SYMBOL(drm_vblank_count_and_time);
>>
>> +static void send_vblank_event(struct drm_pending_vblank_event *e,
>> + unsigned long seq, struct timeval *now)
>> +{
>> + e->event.sequence = seq;
>> + e->event.tv_sec = now->tv_sec;
>> + e->event.tv_usec = now->tv_usec;
>> +
>> + list_add_tail(>base.link,
>> +   >base.file_priv->event_list);
>> + wake_up_interruptible(>base.file_priv->event_wait);
>> + trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
>> +  e->event.sequence);
>> +}
>> +
>> +/**
>> + * drm_send_vblank_event - helper to send vblank event after pageflip
>> + * @dev: DRM device
>> + * @crtc: CRTC in question
>> + * @e: the event to send
>> + *
>> + * Updates sequence # and timestamp on event, and sends it to userspace.
>
> Due to the list_add_tail above, drm_send_vblank_event() requires locking. As
> you don't take any lock I expect the caller to be supposed to handle locking.
> That should be documented, along with the lock that the caller should take.
> Looking at the existing drivers that should be dev->event_lock, but not all
> drivers take it when calling the vblank functions below (for instance the i915
> and gma500 drivers call drm_vblank_off() without holding the event_lock
> spinlock). We thus seem to have a locking issue that should be fixed, either
> as part of this patch set or as a preliminary patches series.
>
> I have no strong opinion on who takes the spinlock (the caller or the
> send_vblank_event() function) at the moment, but taking it in
> send_vblank_event() would allow calling drm_vblank_count_and_time() and
> do_gettimeofday() below outside of the locked region.

I think probably I should add a comment that event_lock should be
held.  Most/all drivers are anyways having to hold the lock to update
their own state around the call to send_vblank_event().

Seems like there is some room for some cleanup around this..
drm_vblank_off() grabs a *different* lock.. I guess we need to
document that drm_vblank_off() caller also holds event_lock.

Might even be a good idea to throw in a WARN_ON(!spin_is_locked())?

>> + */
>> +void drm_send_vblank_event(struct drm_device *dev, int crtc,
>> + struct drm_pending_vblank_event *e)
>> +{
>> + struct timeval now;
>> + unsigned int seq;
>> + if (crtc >= 0) {
>> + seq = drm_vblank_count_and_time(dev, crtc, );
>> + } else {
>> + seq = 0;
>> + do_gettimeofday();
>
> Do you know why some drivers don't call drm_vblank_count_and_time() ? For
> instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
> looks like it could just call drm_vblank_count_and_time().

I know why the current omap driver doesn't, because it is not really
able to use the vblank stuff properly due to omapdss/omapdrm layering
issues.  (But that should be solved w/ the omapdss/omapdrm cleanups I
am working on and kms re-write)

Other drivers, I'm not really sure.  But there were enough drivers
that were using do_gettimeofday() that I thought it best not to ignore
them.

BR,
-R

>> + }
>> + send_vblank_event(e, seq, );
>> +}
>> +EXPORT_SYMBOL(drm_send_vblank_event);
>> +
>>  /**
>>   * drm_update_vblank_count - update the master vblank counter
>>   * @dev: DRM device
>> @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>>   DRM_DEBUG("Sending premature vblank event on disable: \
>> wanted %d, current %d\n",
>> e->event.sequence, seq);
>> -
>> - e->event.sequence = seq;
>> - e->event.tv_sec = now.tv_sec;
>> - e->event.tv_usec = now.tv_usec;
>> + list_del(>base.link);
>>   drm_vblank_put(dev, e->pipe);
>> - list_move_tail(>base.link, >base.file_priv->event_list);
>> -

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-11 Thread Laurent Pinchart 
Hi Rob,

Thanks for the patch.

On Monday 08 October 2012 14:50:39 Rob Clark wrote:
 From: Rob Clark r...@ti.com
 
 A helper that drivers can use to send vblank event after a pageflip.
 If the driver doesn't support proper vblank irq based time/seqn then
 just pass -1 for the pipe # to get do_gettimestamp() behavior (since
 there are a lot of drivers that don't use drm_vblank_count_and_time())
 
 Also an internal send_vblank_event() helper for the various other code
 paths within drm_irq that also need to send vblank events.
 
 Signed-off-by: Rob Clark r...@ti.com
 ---
  drivers/gpu/drm/drm_irq.c |   65 +++---
  include/drm/drmP.h|2 ++
  2 files changed, 44 insertions(+), 23 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
 index 076c4a8..7a00d94 100644
 --- a/drivers/gpu/drm/drm_irq.c
 +++ b/drivers/gpu/drm/drm_irq.c
 @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev,
 int crtc, }
  EXPORT_SYMBOL(drm_vblank_count_and_time);
 
 +static void send_vblank_event(struct drm_pending_vblank_event *e,
 + unsigned long seq, struct timeval *now)
 +{
 + e-event.sequence = seq;
 + e-event.tv_sec = now-tv_sec;
 + e-event.tv_usec = now-tv_usec;
 +
 + list_add_tail(e-base.link,
 +   e-base.file_priv-event_list);
 + wake_up_interruptible(e-base.file_priv-event_wait);
 + trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 +  e-event.sequence);
 +}
 +
 +/**
 + * drm_send_vblank_event - helper to send vblank event after pageflip
 + * @dev: DRM device
 + * @crtc: CRTC in question
 + * @e: the event to send
 + *
 + * Updates sequence # and timestamp on event, and sends it to userspace.

Due to the list_add_tail above, drm_send_vblank_event() requires locking. As 
you don't take any lock I expect the caller to be supposed to handle locking. 
That should be documented, along with the lock that the caller should take. 
Looking at the existing drivers that should be dev-event_lock, but not all 
drivers take it when calling the vblank functions below (for instance the i915 
and gma500 drivers call drm_vblank_off() without holding the event_lock 
spinlock). We thus seem to have a locking issue that should be fixed, either 
as part of this patch set or as a preliminary patches series.

I have no strong opinion on who takes the spinlock (the caller or the 
send_vblank_event() function) at the moment, but taking it in 
send_vblank_event() would allow calling drm_vblank_count_and_time() and 
do_gettimeofday() below outside of the locked region.

 + */
 +void drm_send_vblank_event(struct drm_device *dev, int crtc,
 + struct drm_pending_vblank_event *e)
 +{
 + struct timeval now;
 + unsigned int seq;
 + if (crtc = 0) {
 + seq = drm_vblank_count_and_time(dev, crtc, now);
 + } else {
 + seq = 0;
 + do_gettimeofday(now);

Do you know why some drivers don't call drm_vblank_count_and_time() ? For 
instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it 
looks like it could just call drm_vblank_count_and_time().

 + }
 + send_vblank_event(e, seq, now);
 +}
 +EXPORT_SYMBOL(drm_send_vblank_event);
 +
  /**
   * drm_update_vblank_count - update the master vblank counter
   * @dev: DRM device
 @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
   DRM_DEBUG(Sending premature vblank event on disable: \
 wanted %d, current %d\n,
 e-event.sequence, seq);
 -
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
 + list_del(e-base.link);
   drm_vblank_put(dev, e-pipe);
 - list_move_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 -  e-event.sequence);
 + send_vblank_event(e, seq, now);
   }
 
   spin_unlock_irqrestore(dev-vbl_lock, irqflags);
 @@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device
 *dev, int pipe,
 
   e-event.sequence = vblwait-request.sequence;
   if ((seq - vblwait-request.sequence) = (1  23)) {
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
   drm_vblank_put(dev, pipe);
 - list_add_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - vblwait-reply.sequence = seq;
 - trace_drm_vblank_event_delivered(current-pid, pipe,
 -  vblwait-request.sequence);
 + send_vblank_event(e, seq, now);
   }

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-11 Thread Rob Clark 
On Thu, Oct 11, 2012 at 9:19 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Rob,

 Thanks for the patch.

 On Monday 08 October 2012 14:50:39 Rob Clark wrote:
 From: Rob Clark r...@ti.com

 A helper that drivers can use to send vblank event after a pageflip.
 If the driver doesn't support proper vblank irq based time/seqn then
 just pass -1 for the pipe # to get do_gettimestamp() behavior (since
 there are a lot of drivers that don't use drm_vblank_count_and_time())

 Also an internal send_vblank_event() helper for the various other code
 paths within drm_irq that also need to send vblank events.

 Signed-off-by: Rob Clark r...@ti.com
 ---
  drivers/gpu/drm/drm_irq.c |   65 +++---
  include/drm/drmP.h|2 ++
  2 files changed, 44 insertions(+), 23 deletions(-)

 diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
 index 076c4a8..7a00d94 100644
 --- a/drivers/gpu/drm/drm_irq.c
 +++ b/drivers/gpu/drm/drm_irq.c
 @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev,
 int crtc, }
  EXPORT_SYMBOL(drm_vblank_count_and_time);

 +static void send_vblank_event(struct drm_pending_vblank_event *e,
 + unsigned long seq, struct timeval *now)
 +{
 + e-event.sequence = seq;
 + e-event.tv_sec = now-tv_sec;
 + e-event.tv_usec = now-tv_usec;
 +
 + list_add_tail(e-base.link,
 +   e-base.file_priv-event_list);
 + wake_up_interruptible(e-base.file_priv-event_wait);
 + trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 +  e-event.sequence);
 +}
 +
 +/**
 + * drm_send_vblank_event - helper to send vblank event after pageflip
 + * @dev: DRM device
 + * @crtc: CRTC in question
 + * @e: the event to send
 + *
 + * Updates sequence # and timestamp on event, and sends it to userspace.

 Due to the list_add_tail above, drm_send_vblank_event() requires locking. As
 you don't take any lock I expect the caller to be supposed to handle locking.
 That should be documented, along with the lock that the caller should take.
 Looking at the existing drivers that should be dev-event_lock, but not all
 drivers take it when calling the vblank functions below (for instance the i915
 and gma500 drivers call drm_vblank_off() without holding the event_lock
 spinlock). We thus seem to have a locking issue that should be fixed, either
 as part of this patch set or as a preliminary patches series.

 I have no strong opinion on who takes the spinlock (the caller or the
 send_vblank_event() function) at the moment, but taking it in
 send_vblank_event() would allow calling drm_vblank_count_and_time() and
 do_gettimeofday() below outside of the locked region.

I think probably I should add a comment that event_lock should be
held.  Most/all drivers are anyways having to hold the lock to update
their own state around the call to send_vblank_event().

Seems like there is some room for some cleanup around this..
drm_vblank_off() grabs a *different* lock.. I guess we need to
document that drm_vblank_off() caller also holds event_lock.

Might even be a good idea to throw in a WARN_ON(!spin_is_locked())?

 + */
 +void drm_send_vblank_event(struct drm_device *dev, int crtc,
 + struct drm_pending_vblank_event *e)
 +{
 + struct timeval now;
 + unsigned int seq;
 + if (crtc = 0) {
 + seq = drm_vblank_count_and_time(dev, crtc, now);
 + } else {
 + seq = 0;
 + do_gettimeofday(now);

 Do you know why some drivers don't call drm_vblank_count_and_time() ? For
 instance nouveau sets the sequence to 0 and uses do_gettimeofday(), but it
 looks like it could just call drm_vblank_count_and_time().

I know why the current omap driver doesn't, because it is not really
able to use the vblank stuff properly due to omapdss/omapdrm layering
issues.  (But that should be solved w/ the omapdss/omapdrm cleanups I
am working on and kms re-write)

Other drivers, I'm not really sure.  But there were enough drivers
that were using do_gettimeofday() that I thought it best not to ignore
them.

BR,
-R

 + }
 + send_vblank_event(e, seq, now);
 +}
 +EXPORT_SYMBOL(drm_send_vblank_event);
 +
  /**
   * drm_update_vblank_count - update the master vblank counter
   * @dev: DRM device
 @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
   DRM_DEBUG(Sending premature vblank event on disable: \
 wanted %d, current %d\n,
 e-event.sequence, seq);
 -
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
 + list_del(e-base.link);
   drm_vblank_put(dev, e-pipe);
 - list_move_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 -

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-09 Thread Marcin Slusarz 
On Mon, Oct 08, 2012 at 02:50:39PM -0500, Rob Clark wrote:
> From: Rob Clark 
> 
> A helper that drivers can use to send vblank event after a pageflip.
> If the driver doesn't support proper vblank irq based time/seqn then
> just pass -1 for the pipe # to get do_gettimestamp() behavior (since
> there are a lot of drivers that don't use drm_vblank_count_and_time())
> 
> Also an internal send_vblank_event() helper for the various other code
> paths within drm_irq that also need to send vblank events.
> 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/drm_irq.c |   65 
> +
>  include/drm/drmP.h|2 ++
>  2 files changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 076c4a8..7a00d94 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, 
> int crtc,
>  }
>  EXPORT_SYMBOL(drm_vblank_count_and_time);
>  
> +static void send_vblank_event(struct drm_pending_vblank_event *e,
> + unsigned long seq, struct timeval *now)
> +{
> + e->event.sequence = seq;
> + e->event.tv_sec = now->tv_sec;
> + e->event.tv_usec = now->tv_usec;
> +
> + list_add_tail(>base.link,
> +   >base.file_priv->event_list);
> + wake_up_interruptible(>base.file_priv->event_wait);
> + trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> +  e->event.sequence);
> +}
> +
> +/**
> + * drm_send_vblank_event - helper to send vblank event after pageflip
> + * @dev: DRM device
> + * @crtc: CRTC in question
> + * @e: the event to send
> + *
> + * Updates sequence # and timestamp on event, and sends it to userspace.
> + */
> +void drm_send_vblank_event(struct drm_device *dev, int crtc,
> + struct drm_pending_vblank_event *e)
> +{
> + struct timeval now;
> + unsigned int seq;
> + if (crtc >= 0) {
> + seq = drm_vblank_count_and_time(dev, crtc, );
> + } else {
> + seq = 0;
> + do_gettimeofday();
> + }
> + send_vblank_event(e, seq, );
> +}
> +EXPORT_SYMBOL(drm_send_vblank_event);
> +
>  /**
>   * drm_update_vblank_count - update the master vblank counter
>   * @dev: DRM device
> @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>   DRM_DEBUG("Sending premature vblank event on disable: \
> wanted %d, current %d\n",
> e->event.sequence, seq);
> -
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
> + list_del(>base.link);
>   drm_vblank_put(dev, e->pipe);
> - list_move_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> -  e->event.sequence);
> + send_vblank_event(e, seq, );
>   }
>  
>   spin_unlock_irqrestore(>vbl_lock, irqflags);
> @@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device 
> *dev, int pipe,
>  
>   e->event.sequence = vblwait->request.sequence;
>   if ((seq - vblwait->request.sequence) <= (1 << 23)) {
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
>   drm_vblank_put(dev, pipe);
> - list_add_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - vblwait->reply.sequence = seq;

D?j? vu

http://lists.freedesktop.org/archives/dri-devel/2011-April/010693.html

:)

> - trace_drm_vblank_event_delivered(current->pid, pipe,
> -  vblwait->request.sequence);
> + send_vblank_event(e, seq, );
>   } else {
>   /* drm_handle_vblank_events will call drm_vblank_put */
>   list_add_tail(>base.link, >vblank_event_list);
> @@ -1256,14 +1280,9 @@ static void drm_handle_vblank_events(struct drm_device 
> *dev, int crtc)
>   DRM_DEBUG("vblank event on %d, current %d\n",
> e->event.sequence, seq);
>  
> - e->event.sequence = seq;
> - e->event.tv_sec = now.tv_sec;
> - e->event.tv_usec = now.tv_usec;
> + list_del(>base.link);
>   drm_vblank_put(dev, e->pipe);
> - list_move_tail(>base.link, >base.file_priv->event_list);
> - wake_up_interruptible(>base.file_priv->event_wait);
> - trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> -  e->event.sequence);
> + send_vblank_event(e, seq, );
>   }
>  
>

[PATCH 01/11] drm: add drm_send_vblank_event() helper (v2)

2012-10-08 Thread Rob Clark 
From: Rob Clark 

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

v1: original
v2: add back 'vblwait->reply.sequence = seq' which should not have
been deleted

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_irq.c |   64 +
 include/drm/drmP.h|2 ++
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..0e6e804 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);

+static void send_vblank_event(struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   e->event.sequence = seq;
+   e->event.tv_sec = now->tv_sec;
+   e->event.tv_usec = now->tv_usec;
+
+   list_add_tail(>base.link,
+ >base.file_priv->event_list);
+   wake_up_interruptible(>base.file_priv->event_wait);
+   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
+e->event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc >= 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, );
+   } else {
+   seq = 0;
+   do_gettimeofday();
+   }
+   send_vblank_event(e, seq, );
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG("Sending premature vblank event on disable: \
  wanted %d, current %d\n",
  e->event.sequence, seq);
-
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(e, seq, );
}

spin_unlock_irqrestore(>vbl_lock, irqflags);
@@ -1107,15 +1138,9 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,

e->event.sequence = vblwait->request.sequence;
if ((seq - vblwait->request.sequence) <= (1 << 23)) {
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
drm_vblank_put(dev, pipe);
-   list_add_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
+   send_vblank_event(e, seq, );
vblwait->reply.sequence = seq;
-   trace_drm_vblank_event_delivered(current->pid, pipe,
-vblwait->request.sequence);
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(>base.link, >vblank_event_list);
@@ -1256,14 +1281,9 @@ static void drm_handle_vblank_events(struct drm_device 
*dev, int crtc)
DRM_DEBUG("vblank event on %d, current %d\n",
  e->event.sequence, seq);

-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(e, seq, );
}

spin_unlock_irqrestore(>event_lock, flags);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 05af0e7..ee8f927 100644
---

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-08 Thread Rob Clark 
From: Rob Clark 

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_irq.c |   65 +
 include/drm/drmP.h|2 ++
 2 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..7a00d94 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);

+static void send_vblank_event(struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   e->event.sequence = seq;
+   e->event.tv_sec = now->tv_sec;
+   e->event.tv_usec = now->tv_usec;
+
+   list_add_tail(>base.link,
+ >base.file_priv->event_list);
+   wake_up_interruptible(>base.file_priv->event_wait);
+   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
+e->event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc >= 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, );
+   } else {
+   seq = 0;
+   do_gettimeofday();
+   }
+   send_vblank_event(e, seq, );
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG("Sending premature vblank event on disable: \
  wanted %d, current %d\n",
  e->event.sequence, seq);
-
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(e, seq, );
}

spin_unlock_irqrestore(>vbl_lock, irqflags);
@@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,

e->event.sequence = vblwait->request.sequence;
if ((seq - vblwait->request.sequence) <= (1 << 23)) {
-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
drm_vblank_put(dev, pipe);
-   list_add_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   vblwait->reply.sequence = seq;
-   trace_drm_vblank_event_delivered(current->pid, pipe,
-vblwait->request.sequence);
+   send_vblank_event(e, seq, );
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(>base.link, >vblank_event_list);
@@ -1256,14 +1280,9 @@ static void drm_handle_vblank_events(struct drm_device 
*dev, int crtc)
DRM_DEBUG("vblank event on %d, current %d\n",
  e->event.sequence, seq);

-   e->event.sequence = seq;
-   e->event.tv_sec = now.tv_sec;
-   e->event.tv_usec = now.tv_usec;
+   list_del(>base.link);
drm_vblank_put(dev, e->pipe);
-   list_move_tail(>base.link, >base.file_priv->event_list);
-   wake_up_interruptible(>base.file_priv->event_wait);
-   trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
-e->event.sequence);
+   send_vblank_event(e, seq, );
}

spin_unlock_irqrestore(>event_lock, flags);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 05af0e7..ee8f927 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1437,6 +1437,8 @@ extern int drm_vblank_wait(struct drm_device

[PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-08 Thread Rob Clark 
From: Rob Clark r...@ti.com

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

Signed-off-by: Rob Clark r...@ti.com
---
 drivers/gpu/drm/drm_irq.c |   65 +
 include/drm/drmP.h|2 ++
 2 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..7a00d94 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);
 
+static void send_vblank_event(struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   e-event.sequence = seq;
+   e-event.tv_sec = now-tv_sec;
+   e-event.tv_usec = now-tv_usec;
+
+   list_add_tail(e-base.link,
+ e-base.file_priv-event_list);
+   wake_up_interruptible(e-base.file_priv-event_wait);
+   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
+e-event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc = 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, now);
+   } else {
+   seq = 0;
+   do_gettimeofday(now);
+   }
+   send_vblank_event(e, seq, now);
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG(Sending premature vblank event on disable: \
  wanted %d, current %d\n,
  e-event.sequence, seq);
-
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
+   list_del(e-base.link);
drm_vblank_put(dev, e-pipe);
-   list_move_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
-   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
-e-event.sequence);
+   send_vblank_event(e, seq, now);
}
 
spin_unlock_irqrestore(dev-vbl_lock, irqflags);
@@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,
 
e-event.sequence = vblwait-request.sequence;
if ((seq - vblwait-request.sequence) = (1  23)) {
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
drm_vblank_put(dev, pipe);
-   list_add_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
-   vblwait-reply.sequence = seq;
-   trace_drm_vblank_event_delivered(current-pid, pipe,
-vblwait-request.sequence);
+   send_vblank_event(e, seq, now);
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(e-base.link, dev-vblank_event_list);
@@ -1256,14 +1280,9 @@ static void drm_handle_vblank_events(struct drm_device 
*dev, int crtc)
DRM_DEBUG(vblank event on %d, current %d\n,
  e-event.sequence, seq);
 
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
+   list_del(e-base.link);
drm_vblank_put(dev, e-pipe);
-   list_move_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
-   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
-e-event.sequence);
+   send_vblank_event(e, seq, now);
}
 
spin_unlock_irqrestore(dev-event_lock, flags);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 05af0e7..ee8f927 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1437,6 +1437,8 @@ extern int drm_vblank_wait(struct

Re: [PATCH 01/11] drm: add drm_send_vblank_event() helper

2012-10-08 Thread Marcin Slusarz 
On Mon, Oct 08, 2012 at 02:50:39PM -0500, Rob Clark wrote:
 From: Rob Clark r...@ti.com
 
 A helper that drivers can use to send vblank event after a pageflip.
 If the driver doesn't support proper vblank irq based time/seqn then
 just pass -1 for the pipe # to get do_gettimestamp() behavior (since
 there are a lot of drivers that don't use drm_vblank_count_and_time())
 
 Also an internal send_vblank_event() helper for the various other code
 paths within drm_irq that also need to send vblank events.
 
 Signed-off-by: Rob Clark r...@ti.com
 ---
  drivers/gpu/drm/drm_irq.c |   65 
 +
  include/drm/drmP.h|2 ++
  2 files changed, 44 insertions(+), 23 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
 index 076c4a8..7a00d94 100644
 --- a/drivers/gpu/drm/drm_irq.c
 +++ b/drivers/gpu/drm/drm_irq.c
 @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, 
 int crtc,
  }
  EXPORT_SYMBOL(drm_vblank_count_and_time);
  
 +static void send_vblank_event(struct drm_pending_vblank_event *e,
 + unsigned long seq, struct timeval *now)
 +{
 + e-event.sequence = seq;
 + e-event.tv_sec = now-tv_sec;
 + e-event.tv_usec = now-tv_usec;
 +
 + list_add_tail(e-base.link,
 +   e-base.file_priv-event_list);
 + wake_up_interruptible(e-base.file_priv-event_wait);
 + trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 +  e-event.sequence);
 +}
 +
 +/**
 + * drm_send_vblank_event - helper to send vblank event after pageflip
 + * @dev: DRM device
 + * @crtc: CRTC in question
 + * @e: the event to send
 + *
 + * Updates sequence # and timestamp on event, and sends it to userspace.
 + */
 +void drm_send_vblank_event(struct drm_device *dev, int crtc,
 + struct drm_pending_vblank_event *e)
 +{
 + struct timeval now;
 + unsigned int seq;
 + if (crtc = 0) {
 + seq = drm_vblank_count_and_time(dev, crtc, now);
 + } else {
 + seq = 0;
 + do_gettimeofday(now);
 + }
 + send_vblank_event(e, seq, now);
 +}
 +EXPORT_SYMBOL(drm_send_vblank_event);
 +
  /**
   * drm_update_vblank_count - update the master vblank counter
   * @dev: DRM device
 @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
   DRM_DEBUG(Sending premature vblank event on disable: \
 wanted %d, current %d\n,
 e-event.sequence, seq);
 -
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
 + list_del(e-base.link);
   drm_vblank_put(dev, e-pipe);
 - list_move_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 -  e-event.sequence);
 + send_vblank_event(e, seq, now);
   }
  
   spin_unlock_irqrestore(dev-vbl_lock, irqflags);
 @@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device 
 *dev, int pipe,
  
   e-event.sequence = vblwait-request.sequence;
   if ((seq - vblwait-request.sequence) = (1  23)) {
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
   drm_vblank_put(dev, pipe);
 - list_add_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - vblwait-reply.sequence = seq;

Déjà vu

http://lists.freedesktop.org/archives/dri-devel/2011-April/010693.html

:)

 - trace_drm_vblank_event_delivered(current-pid, pipe,
 -  vblwait-request.sequence);
 + send_vblank_event(e, seq, now);
   } else {
   /* drm_handle_vblank_events will call drm_vblank_put */
   list_add_tail(e-base.link, dev-vblank_event_list);
 @@ -1256,14 +1280,9 @@ static void drm_handle_vblank_events(struct drm_device 
 *dev, int crtc)
   DRM_DEBUG(vblank event on %d, current %d\n,
 e-event.sequence, seq);
  
 - e-event.sequence = seq;
 - e-event.tv_sec = now.tv_sec;
 - e-event.tv_usec = now.tv_usec;
 + list_del(e-base.link);
   drm_vblank_put(dev, e-pipe);
 - list_move_tail(e-base.link, e-base.file_priv-event_list);
 - wake_up_interruptible(e-base.file_priv-event_wait);
 - trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
 -  e-event.sequence);
 + send_vblank_event(e, seq, now);
   }
  
   spin_unlock_irqrestore(dev-event_lock, flags);
 diff --git a/include/drm/drmP.h b/include/drm/drmP.h

[PATCH 01/11] drm: add drm_send_vblank_event() helper (v2)

2012-10-08 Thread Rob Clark 
From: Rob Clark r...@ti.com

A helper that drivers can use to send vblank event after a pageflip.
If the driver doesn't support proper vblank irq based time/seqn then
just pass -1 for the pipe # to get do_gettimestamp() behavior (since
there are a lot of drivers that don't use drm_vblank_count_and_time())

Also an internal send_vblank_event() helper for the various other code
paths within drm_irq that also need to send vblank events.

v1: original
v2: add back 'vblwait-reply.sequence = seq' which should not have
been deleted

Signed-off-by: Rob Clark r...@ti.com
---
 drivers/gpu/drm/drm_irq.c |   64 +
 include/drm/drmP.h|2 ++
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 076c4a8..0e6e804 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int 
crtc,
 }
 EXPORT_SYMBOL(drm_vblank_count_and_time);
 
+static void send_vblank_event(struct drm_pending_vblank_event *e,
+   unsigned long seq, struct timeval *now)
+{
+   e-event.sequence = seq;
+   e-event.tv_sec = now-tv_sec;
+   e-event.tv_usec = now-tv_usec;
+
+   list_add_tail(e-base.link,
+ e-base.file_priv-event_list);
+   wake_up_interruptible(e-base.file_priv-event_wait);
+   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
+e-event.sequence);
+}
+
+/**
+ * drm_send_vblank_event - helper to send vblank event after pageflip
+ * @dev: DRM device
+ * @crtc: CRTC in question
+ * @e: the event to send
+ *
+ * Updates sequence # and timestamp on event, and sends it to userspace.
+ */
+void drm_send_vblank_event(struct drm_device *dev, int crtc,
+   struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+   if (crtc = 0) {
+   seq = drm_vblank_count_and_time(dev, crtc, now);
+   } else {
+   seq = 0;
+   do_gettimeofday(now);
+   }
+   send_vblank_event(e, seq, now);
+}
+EXPORT_SYMBOL(drm_send_vblank_event);
+
 /**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
@@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
DRM_DEBUG(Sending premature vblank event on disable: \
  wanted %d, current %d\n,
  e-event.sequence, seq);
-
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
+   list_del(e-base.link);
drm_vblank_put(dev, e-pipe);
-   list_move_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
-   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
-e-event.sequence);
+   send_vblank_event(e, seq, now);
}
 
spin_unlock_irqrestore(dev-vbl_lock, irqflags);
@@ -1107,15 +1138,9 @@ static int drm_queue_vblank_event(struct drm_device 
*dev, int pipe,
 
e-event.sequence = vblwait-request.sequence;
if ((seq - vblwait-request.sequence) = (1  23)) {
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
drm_vblank_put(dev, pipe);
-   list_add_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
+   send_vblank_event(e, seq, now);
vblwait-reply.sequence = seq;
-   trace_drm_vblank_event_delivered(current-pid, pipe,
-vblwait-request.sequence);
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(e-base.link, dev-vblank_event_list);
@@ -1256,14 +1281,9 @@ static void drm_handle_vblank_events(struct drm_device 
*dev, int crtc)
DRM_DEBUG(vblank event on %d, current %d\n,
  e-event.sequence, seq);
 
-   e-event.sequence = seq;
-   e-event.tv_sec = now.tv_sec;
-   e-event.tv_usec = now.tv_usec;
+   list_del(e-base.link);
drm_vblank_put(dev, e-pipe);
-   list_move_tail(e-base.link, e-base.file_priv-event_list);
-   wake_up_interruptible(e-base.file_priv-event_wait);
-   trace_drm_vblank_event_delivered(e-base.pid, e-pipe,
-e-event.sequence);
+   send_vblank_event(e, seq, now);
}
 
spin_unlock_irqrestore(dev-event_lock, flags);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 05af0e7..ee8f927 100644
---