Re: [PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v4.

Re: [PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v4.

[Mario Kleiner]

Sorry for the late review, but see below...

On 04/19/2016 09:52 AM, Maarten Lankhorst wrote:
> This function is useful for gen2 intel devices which have no frame
> counter, but need a way to determine the current vblank count without
> racing with the vblank interrupt handler.
>
> intel_pipe_update_start checks if no vblank interrupt will occur
> during vblank evasion, but cannot check whether the vblank handler has
> run to completion. This function uses the timestamps to determine
> when the last vblank has happened, and interpolates from there.
>
> Changes since v1:
> - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> Changes since v2:
> - Don't return time of last vblank.
> Changes since v3:
> - Change pipe to unsigned int. (Ville)
> - Remove unused documentation for tv_ret. (kbuild)
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: David Airlie <airlied@linux.ie> #irc
> ---
>   drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++
>   include/drm/drmP.h        |  1 +
>   2 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f18e71c..f1bda13562da 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
>   	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
>   }
>
> +/**
> + * drm_accurate_vblank_count - retrieve the master vblank counter
> + * @crtc: which counter to retrieve
> + *
> + * This function is similar to @drm_crtc_vblank_count but this
> + * function interpolates to handle a race with vblank irq's.
> + */
> +
> +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	unsigned int pipe = drm_crtc_index(crtc);
> +	u32 vblank;
> +	unsigned long flags;
> +

This function is rather dangerous to use on any driver that doesn't have 
precise vblank timestamping, or doesn't have the guarantee that hw 
vblank counters (if there are any) and timestamps update exactly at 
leading edge of vblank, so i think we need some WARN() here and maybe 
much less encouraging docs to avoid this being called from incapable kms 
drivers or in general code.

- If the driver doesn't have precise scanoutpos based timestamping each 
call into drm_update_vblank_count from non-irq context will reset the 
vblank timestamps to zero, so clients will only receive invalid 
timestamps if this is frequently used. Also bogus vblank counts

Atm. only i915 Intel, AMD, NVidia desktop for >= NV-50, maybe nouveau 
driven Tegra parts, and some modern Adrenos (msm/mdp-5 - i assume from 
the code?) support this reliably.

- If the drivers scanoutpos timestamps and/or vblank counter don't 
increment at leading edge we will get funny off-by-one problems with 
vblank counters. That's why we normally only call 
drm_update_vblank_count() from vblank irq on such parts - the only safe 
place to avoid off-by-one problems, and limit vblank disable/enable to 
only at most once every 5 seconds to reduce the problems caused by 
off-by-one errors.

Which restricts the list to only the above parts, maybe minus Adreno 
where i don't know if it obeys the "leading edge" rule or not.

So on most SoC's one must not use this function.

WARN_ON(!dev->vblank_disable_immediate, "This function is unsafe on this 
driver.");

would probably prevent the worst abuse, unless drivers lie about 
vblank_disable_immediate. Not sure how much this was checked for msm / 
Adreno? At least drm_vblank_init() only allows vblank_disable_immediate 
if the driver at least implements proper timestamping.

Not sure how much general use this function will have outside Intel 
gen-2 with the restrictions on safe use?

> +	spin_lock_irqsave(&dev->vblank_time_lock, flags);
> +
> +	drm_update_vblank_count(dev, pipe, 0);
> +	vblank = dev->vblank[pipe].count;

Could do vblank = drm_vblank_count(dev, pipe); instead, given that we 
avoid open coding this in most places.

-mario

> +
> +	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> +
> +	return vblank;
> +}
> +EXPORT_SYMBOL(drm_accurate_vblank_count);
> +
>   /*
>    * Disable vblank irq's on crtc, make sure that last vblank count
>    * of hardware and corresponding consistent software vblank counter
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 005202ea5900..90527c41cd5a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>   extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
>   extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
>
>   extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v5.

[Maarten Lankhorst]

This function is useful for gen2 intel devices which have no frame
counter, but need a way to determine the current vblank count without
racing with the vblank interrupt handler.

intel_pipe_update_start checks if no vblank interrupt will occur
during vblank evasion, but cannot check whether the vblank handler has
run to completion. This function uses the timestamps to determine
when the last vblank has happened, and interpolates from there.

Changes since v1:
- Take vblank_time_lock and don't use drm_vblank_count_and_time.
Changes since v2:
- Don't return time of last vblank.
Changes since v3:
- Change pipe to unsigned int. (Ville)
- Remove unused documentation for tv_ret. (kbuild)
Changes since v4:
- Add warning to docs when the function is useful.
- Add a WARN_ON when get_vblank_timestamp is unavailable.
- Use drm_vblank_count.

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v4
Acked-by: David Airlie <airlied@linux.ie> #irc, v4
---

Unfortunately WARN_ON(!dev->disable_vblank_immediate) doesn't work on gen2,
which is the reason this function is created. So I used
WARN_ON(!get_vblank_timestamp) instead.

 drivers/gpu/drm/drm_irq.c | 31 +++++++++++++++++++++++++++++++
 include/drm/drmP.h        |  1 +
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3c1a6f18e71c..d3124b67f4a5 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -303,6 +303,37 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
 }
 
+/**
+ * drm_accurate_vblank_count - retrieve the master vblank counter
+ * @crtc: which counter to retrieve
+ *
+ * This function is similar to @drm_crtc_vblank_count but this
+ * function interpolates to handle a race with vblank irq's.
+ *
+ * This is mostly useful for hardware that can obtain the scanout
+ * position, but doesn't have a frame counter.
+ */
+u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	unsigned int pipe = drm_crtc_index(crtc);
+	u32 vblank;
+	unsigned long flags;
+
+	WARN(!dev->driver->get_vblank_timestamp,
+	     "This function requires support for accurate vblank timestamps.");
+
+	spin_lock_irqsave(&dev->vblank_time_lock, flags);
+
+	drm_update_vblank_count(dev, pipe, 0);
+	vblank = drm_vblank_count(dev, pipe);
+
+	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
+
+	return vblank;
+}
+EXPORT_SYMBOL(drm_accurate_vblank_count);
+
 /*
  * Disable vblank irq's on crtc, make sure that last vblank count
  * of hardware and corresponding consistent software vblank counter
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 005202ea5900..90527c41cd5a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
 extern void drm_vblank_cleanup(struct drm_device *dev);
+extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
 extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
 
 extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
-- 
2.1.0


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

Re: [PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v5.

[Mario Kleiner]

On 04/25/2016 08:32 AM, Maarten Lankhorst wrote:
> This function is useful for gen2 intel devices which have no frame
> counter, but need a way to determine the current vblank count without
> racing with the vblank interrupt handler.
>
> intel_pipe_update_start checks if no vblank interrupt will occur
> during vblank evasion, but cannot check whether the vblank handler has
> run to completion. This function uses the timestamps to determine
> when the last vblank has happened, and interpolates from there.
>
> Changes since v1:
> - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> Changes since v2:
> - Don't return time of last vblank.
> Changes since v3:
> - Change pipe to unsigned int. (Ville)
> - Remove unused documentation for tv_ret. (kbuild)
> Changes since v4:
> - Add warning to docs when the function is useful.
> - Add a WARN_ON when get_vblank_timestamp is unavailable.
> - Use drm_vblank_count.
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v4
> Acked-by: David Airlie <airlied@linux.ie> #irc, v4
> ---
>
> Unfortunately WARN_ON(!dev->disable_vblank_immediate) doesn't work on gen2,
> which is the reason this function is created. So I used
> WARN_ON(!get_vblank_timestamp) instead.

That's a weaker warning. I'd like to have the WARN_ON and the doc text 
to be more frightening/restrictive to discourage abuse.

But can't you simply remove that !IS_GEN2 check now and always set 
dev->disable_vblank_immediate = true? The reason for that exception was 
that GEN2 doesn't have a hw vblank counter. But it has scanout pos based 
vblank timestamping, which i'd assume is well behaved. With the new 
scanout based vblank counter emulation in drm_update_vblank_count() 
since around Linux 4.4 you therefore essentially have a proper emulated 
vblank counter, so this should be safe. Ville will probably know. 
Otherwise you couldn't trust drm_accurate_vblank_count() here, because 
it depends on the same logic, no?

-mario


>
>   drivers/gpu/drm/drm_irq.c | 31 +++++++++++++++++++++++++++++++
>   include/drm/drmP.h        |  1 +
>   2 files changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f18e71c..d3124b67f4a5 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -303,6 +303,37 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
>   	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
>   }
>
> +/**
> + * drm_accurate_vblank_count - retrieve the master vblank counter
> + * @crtc: which counter to retrieve
> + *
> + * This function is similar to @drm_crtc_vblank_count but this
> + * function interpolates to handle a race with vblank irq's.
> + *
> + * This is mostly useful for hardware that can obtain the scanout
> + * position, but doesn't have a frame counter.
> + */
> +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	unsigned int pipe = drm_crtc_index(crtc);
> +	u32 vblank;
> +	unsigned long flags;
> +
> +	WARN(!dev->driver->get_vblank_timestamp,
> +	     "This function requires support for accurate vblank timestamps.");
> +
> +	spin_lock_irqsave(&dev->vblank_time_lock, flags);
> +
> +	drm_update_vblank_count(dev, pipe, 0);
> +	vblank = drm_vblank_count(dev, pipe);
> +
> +	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> +
> +	return vblank;
> +}
> +EXPORT_SYMBOL(drm_accurate_vblank_count);
> +
>   /*
>    * Disable vblank irq's on crtc, make sure that last vblank count
>    * of hardware and corresponding consistent software vblank counter
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 005202ea5900..90527c41cd5a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>   extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
>   extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
>
>   extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v5.

[Mario Kleiner]

Anyway, although i would have liked the stricter check and warning docs, 
the v4 patch is ok with me:

Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario

On 04/25/2016 08:32 AM, Maarten Lankhorst wrote:
> This function is useful for gen2 intel devices which have no frame
> counter, but need a way to determine the current vblank count without
> racing with the vblank interrupt handler.
>
> intel_pipe_update_start checks if no vblank interrupt will occur
> during vblank evasion, but cannot check whether the vblank handler has
> run to completion. This function uses the timestamps to determine
> when the last vblank has happened, and interpolates from there.
>
> Changes since v1:
> - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> Changes since v2:
> - Don't return time of last vblank.
> Changes since v3:
> - Change pipe to unsigned int. (Ville)
> - Remove unused documentation for tv_ret. (kbuild)
> Changes since v4:
> - Add warning to docs when the function is useful.
> - Add a WARN_ON when get_vblank_timestamp is unavailable.
> - Use drm_vblank_count.
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v4
> Acked-by: David Airlie <airlied@linux.ie> #irc, v4
> ---
>
> Unfortunately WARN_ON(!dev->disable_vblank_immediate) doesn't work on gen2,
> which is the reason this function is created. So I used
> WARN_ON(!get_vblank_timestamp) instead.
>
>   drivers/gpu/drm/drm_irq.c | 31 +++++++++++++++++++++++++++++++
>   include/drm/drmP.h        |  1 +
>   2 files changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f18e71c..d3124b67f4a5 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -303,6 +303,37 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
>   	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
>   }
>
> +/**
> + * drm_accurate_vblank_count - retrieve the master vblank counter
> + * @crtc: which counter to retrieve
> + *
> + * This function is similar to @drm_crtc_vblank_count but this
> + * function interpolates to handle a race with vblank irq's.
> + *
> + * This is mostly useful for hardware that can obtain the scanout
> + * position, but doesn't have a frame counter.
> + */
> +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	unsigned int pipe = drm_crtc_index(crtc);
> +	u32 vblank;
> +	unsigned long flags;
> +
> +	WARN(!dev->driver->get_vblank_timestamp,
> +	     "This function requires support for accurate vblank timestamps.");
> +
> +	spin_lock_irqsave(&dev->vblank_time_lock, flags);
> +
> +	drm_update_vblank_count(dev, pipe, 0);
> +	vblank = drm_vblank_count(dev, pipe);
> +
> +	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> +
> +	return vblank;
> +}
> +EXPORT_SYMBOL(drm_accurate_vblank_count);
> +
>   /*
>    * Disable vblank irq's on crtc, make sure that last vblank count
>    * of hardware and corresponding consistent software vblank counter
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 005202ea5900..90527c41cd5a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>   extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>   extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
>   extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
>
>   extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/19] drm/core: Add drm_accurate_vblank_count, v4.

[Ville Syrjälä]

On Mon, Apr 25, 2016 at 06:35:48AM +0200, Mario Kleiner wrote:
> Sorry for the late review, but see below...
> 
> On 04/19/2016 09:52 AM, Maarten Lankhorst wrote:
> > This function is useful for gen2 intel devices which have no frame
> > counter, but need a way to determine the current vblank count without
> > racing with the vblank interrupt handler.
> >
> > intel_pipe_update_start checks if no vblank interrupt will occur
> > during vblank evasion, but cannot check whether the vblank handler has
> > run to completion. This function uses the timestamps to determine
> > when the last vblank has happened, and interpolates from there.
> >
> > Changes since v1:
> > - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> > Changes since v2:
> > - Don't return time of last vblank.
> > Changes since v3:
> > - Change pipe to unsigned int. (Ville)
> > - Remove unused documentation for tv_ret. (kbuild)
> >
> > Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Acked-by: David Airlie <airlied@linux.ie> #irc
> > ---
> >   drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++
> >   include/drm/drmP.h        |  1 +
> >   2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 3c1a6f18e71c..f1bda13562da 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
> >   	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
> >   }
> >
> > +/**
> > + * drm_accurate_vblank_count - retrieve the master vblank counter
> > + * @crtc: which counter to retrieve
> > + *
> > + * This function is similar to @drm_crtc_vblank_count but this
> > + * function interpolates to handle a race with vblank irq's.
> > + */
> > +
> > +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> > +{
> > +	struct drm_device *dev = crtc->dev;
> > +	unsigned int pipe = drm_crtc_index(crtc);
> > +	u32 vblank;
> > +	unsigned long flags;
> > +
> 
> This function is rather dangerous to use on any driver that doesn't have 
> precise vblank timestamping, or doesn't have the guarantee that hw 
> vblank counters (if there are any) and timestamps update exactly at 
> leading edge of vblank, so i think we need some WARN() here and maybe 
> much less encouraging docs to avoid this being called from incapable kms 
> drivers or in general code.
> 
> - If the driver doesn't have precise scanoutpos based timestamping each 
> call into drm_update_vblank_count from non-irq context will reset the 
> vblank timestamps to zero, so clients will only receive invalid 
> timestamps if this is frequently used. Also bogus vblank counts
> 
> Atm. only i915 Intel, AMD, NVidia desktop for >= NV-50, maybe nouveau 
> driven Tegra parts, and some modern Adrenos (msm/mdp-5 - i assume from 
> the code?) support this reliably.
> 
> - If the drivers scanoutpos timestamps and/or vblank counter don't 
> increment at leading edge we will get funny off-by-one problems with 
> vblank counters. That's why we normally only call 
> drm_update_vblank_count() from vblank irq on such parts - the only safe 
> place to avoid off-by-one problems, and limit vblank disable/enable to 
> only at most once every 5 seconds to reduce the problems caused by 
> off-by-one errors.
> 
> Which restricts the list to only the above parts, maybe minus Adreno 
> where i don't know if it obeys the "leading edge" rule or not.
> 
> So on most SoC's one must not use this function.
> 
> WARN_ON(!dev->vblank_disable_immediate, "This function is unsafe on this 
> driver.");
> 
> would probably prevent the worst abuse, unless drivers lie about 
> vblank_disable_immediate. Not sure how much this was checked for msm / 
> Adreno? At least drm_vblank_init() only allows vblank_disable_immediate 
> if the driver at least implements proper timestamping.
> 
> Not sure how much general use this function will have outside Intel 
> gen-2 with the restrictions on safe use?

This (or something like it) needs to be used also in generic vblank
wait functions. Currently those are racy.

> 
> > +	spin_lock_irqsave(&dev->vblank_time_lock, flags);
> > +
> > +	drm_update_vblank_count(dev, pipe, 0);
> > +	vblank = dev->vblank[pipe].count;
> 
> Could do vblank = drm_vblank_count(dev, pipe); instead, given that we 
> avoid open coding this in most places.
> 
> -mario
> 
> > +
> > +	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> > +
> > +	return vblank;
> > +}
> > +EXPORT_SYMBOL(drm_accurate_vblank_count);
> > +
> >   /*
> >    * Disable vblank irq's on crtc, make sure that last vblank count
> >    * of hardware and corresponding consistent software vblank counter
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 005202ea5900..90527c41cd5a 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> >   extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> >   extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> >   extern void drm_vblank_cleanup(struct drm_device *dev);
> > +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
> >   extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
> >
> >   extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx