| Message ID | 1461052359-24154-5-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 19, 2016 at 09:52:24AM +0200, Maarten Lankhorst wrote: > This uses the newly created drm_accurate_vblank_count_and_time to accurately > get a vblank count when the hw counter is unavailable. > --- > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_sprite.c | 8 ++------ > 3 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ccbc2a448258..2086e8bd10da 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13530,6 +13530,16 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, > return ret; > } > > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) > +{ > + struct drm_device *dev = crtc->base.dev; > + > + if (!dev->max_vblank_count) > + return drm_accurate_vblank_count(&crtc->base); > + > + return dev->driver->get_vblank_counter(dev, crtc->pipe); > +} > + > static void intel_atomic_wait_for_vblanks(struct drm_device *dev, > struct drm_i915_private *dev_priv, > unsigned crtc_mask) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index fecc89600667..8efeb90eac07 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1146,6 +1146,9 @@ intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe) > if (crtc->active) > intel_wait_for_vblank(dev, pipe); > } > + > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > + > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > struct intel_digital_port *dport, > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 0f3e2303e0e9..e2de6b0df5a8 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -80,9 +80,7 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, > */ > void intel_pipe_update_start(struct intel_crtc *crtc) > { > - struct drm_device *dev = crtc->base.dev; > const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; > - enum pipe pipe = crtc->pipe; > long timeout = msecs_to_jiffies_timeout(1); > int scanline, min, max, vblank_start; > wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base); > @@ -139,8 +137,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > > crtc->debug.scanline_start = scanline; > crtc->debug.start_vbl_time = ktime_get(); > - crtc->debug.start_vbl_count = > - dev->driver->get_vblank_counter(dev, pipe); > + crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc); > > trace_i915_pipe_update_vblank_evaded(crtc); > } > @@ -156,10 +153,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > */ > void intel_pipe_update_end(struct intel_crtc *crtc) > { > - struct drm_device *dev = crtc->base.dev; > enum pipe pipe = crtc->pipe; > int scanline_end = intel_get_crtc_scanline(crtc); > - u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe); > + u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc); > ktime_t end_vbl_time = ktime_get(); > > trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end); Do we need to use intel_crtc_get_vblank_counter() in display_pipe_crc_irq_handler() as well? > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Apr 27, 2016 at 04:06:16PM +0200, Patrik Jakobsson wrote: > On Tue, Apr 19, 2016 at 09:52:24AM +0200, Maarten Lankhorst wrote: > > This uses the newly created drm_accurate_vblank_count_and_time to accurately > > get a vblank count when the hw counter is unavailable. > > --- > > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > > drivers/gpu/drm/i915/intel_sprite.c | 8 ++------ > > 3 files changed, 15 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index ccbc2a448258..2086e8bd10da 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13530,6 +13530,16 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, > > return ret; > > } > > > > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) > > +{ > > + struct drm_device *dev = crtc->base.dev; > > + > > + if (!dev->max_vblank_count) > > + return drm_accurate_vblank_count(&crtc->base); > > + > > + return dev->driver->get_vblank_counter(dev, crtc->pipe); > > +} > > + > > static void intel_atomic_wait_for_vblanks(struct drm_device *dev, > > struct drm_i915_private *dev_priv, > > unsigned crtc_mask) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index fecc89600667..8efeb90eac07 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1146,6 +1146,9 @@ intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe) > > if (crtc->active) > > intel_wait_for_vblank(dev, pipe); > > } > > + > > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > > + > > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); > > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > > struct intel_digital_port *dport, > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 0f3e2303e0e9..e2de6b0df5a8 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -80,9 +80,7 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, > > */ > > void intel_pipe_update_start(struct intel_crtc *crtc) > > { > > - struct drm_device *dev = crtc->base.dev; > > const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; > > - enum pipe pipe = crtc->pipe; > > long timeout = msecs_to_jiffies_timeout(1); > > int scanline, min, max, vblank_start; > > wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base); > > @@ -139,8 +137,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > > > > crtc->debug.scanline_start = scanline; > > crtc->debug.start_vbl_time = ktime_get(); > > - crtc->debug.start_vbl_count = > > - dev->driver->get_vblank_counter(dev, pipe); > > + crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc); > > > > trace_i915_pipe_update_vblank_evaded(crtc); > > } > > @@ -156,10 +153,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > > */ > > void intel_pipe_update_end(struct intel_crtc *crtc) > > { > > - struct drm_device *dev = crtc->base.dev; > > enum pipe pipe = crtc->pipe; > > int scanline_end = intel_get_crtc_scanline(crtc); > > - u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe); > > + u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc); > > ktime_t end_vbl_time = ktime_get(); > > > > trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end); > > Do we need to use intel_crtc_get_vblank_counter() in > display_pipe_crc_irq_handler() as well? There was a bit of talk whether we should use hw or sw counter for the crc frame numbers, but I can't remember if we reached any real conclusion. In the meantime the crc frame counters are all still zero on gen2, meaning the tests don't work all that well. See [1]. And we still have the %8d bug highlited in that same patch series. Not sure we reached any conclusion about that on either. In any case using drm_accurate_vblank_count() from the irq handler would be somewhat silly since the irq handler should have just updated the sw counter to be uptodate, assuming we had vblank irqs enabled. [1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/083035.html
On Wed, Apr 27, 2016 at 05:23:06PM +0300, Ville Syrjälä wrote: > On Wed, Apr 27, 2016 at 04:06:16PM +0200, Patrik Jakobsson wrote: > > On Tue, Apr 19, 2016 at 09:52:24AM +0200, Maarten Lankhorst wrote: > > > This uses the newly created drm_accurate_vblank_count_and_time to accurately > > > get a vblank count when the hw counter is unavailable. > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > > > drivers/gpu/drm/i915/intel_sprite.c | 8 ++------ > > > 3 files changed, 15 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index ccbc2a448258..2086e8bd10da 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13530,6 +13530,16 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, > > > return ret; > > > } > > > > > > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) > > > +{ > > > + struct drm_device *dev = crtc->base.dev; > > > + > > > + if (!dev->max_vblank_count) > > > + return drm_accurate_vblank_count(&crtc->base); > > > + > > > + return dev->driver->get_vblank_counter(dev, crtc->pipe); > > > +} > > > + > > > static void intel_atomic_wait_for_vblanks(struct drm_device *dev, > > > struct drm_i915_private *dev_priv, > > > unsigned crtc_mask) > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index fecc89600667..8efeb90eac07 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1146,6 +1146,9 @@ intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe) > > > if (crtc->active) > > > intel_wait_for_vblank(dev, pipe); > > > } > > > + > > > +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); > > > + > > > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); > > > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > > > struct intel_digital_port *dport, > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index 0f3e2303e0e9..e2de6b0df5a8 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -80,9 +80,7 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, > > > */ > > > void intel_pipe_update_start(struct intel_crtc *crtc) > > > { > > > - struct drm_device *dev = crtc->base.dev; > > > const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; > > > - enum pipe pipe = crtc->pipe; > > > long timeout = msecs_to_jiffies_timeout(1); > > > int scanline, min, max, vblank_start; > > > wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base); > > > @@ -139,8 +137,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > > > > > > crtc->debug.scanline_start = scanline; > > > crtc->debug.start_vbl_time = ktime_get(); > > > - crtc->debug.start_vbl_count = > > > - dev->driver->get_vblank_counter(dev, pipe); > > > + crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc); > > > > > > trace_i915_pipe_update_vblank_evaded(crtc); > > > } > > > @@ -156,10 +153,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > > > */ > > > void intel_pipe_update_end(struct intel_crtc *crtc) > > > { > > > - struct drm_device *dev = crtc->base.dev; > > > enum pipe pipe = crtc->pipe; > > > int scanline_end = intel_get_crtc_scanline(crtc); > > > - u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe); > > > + u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc); > > > ktime_t end_vbl_time = ktime_get(); > > > > > > trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end); > > > > Do we need to use intel_crtc_get_vblank_counter() in > > display_pipe_crc_irq_handler() as well? > > There was a bit of talk whether we should use hw or sw counter for the > crc frame numbers, but I can't remember if we reached any real > conclusion. In the meantime the crc frame counters are all still zero > on gen2, meaning the tests don't work all that well. See [1]. > > And we still have the %8d bug highlited in that same patch series. Not > sure we reached any conclusion about that on either. > > In any case using drm_accurate_vblank_count() from the irq handler > would be somewhat silly since the irq handler should have just updated > the sw counter to be uptodate, assuming we had vblank irqs enabled. > > [1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/083035.html Ok, let's leave it as it is for now. Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ccbc2a448258..2086e8bd10da 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13530,6 +13530,16 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, return ret; } +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) +{ + struct drm_device *dev = crtc->base.dev; + + if (!dev->max_vblank_count) + return drm_accurate_vblank_count(&crtc->base); + + return dev->driver->get_vblank_counter(dev, crtc->pipe); +} + static void intel_atomic_wait_for_vblanks(struct drm_device *dev, struct drm_i915_private *dev_priv, unsigned crtc_mask) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index fecc89600667..8efeb90eac07 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1146,6 +1146,9 @@ intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe) if (crtc->active) intel_wait_for_vblank(dev, pipe); } + +u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc); + int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); void vlv_wait_port_ready(struct drm_i915_private *dev_priv, struct intel_digital_port *dport, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0f3e2303e0e9..e2de6b0df5a8 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -80,9 +80,7 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, */ void intel_pipe_update_start(struct intel_crtc *crtc) { - struct drm_device *dev = crtc->base.dev; const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; - enum pipe pipe = crtc->pipe; long timeout = msecs_to_jiffies_timeout(1); int scanline, min, max, vblank_start; wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base); @@ -139,8 +137,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc) crtc->debug.scanline_start = scanline; crtc->debug.start_vbl_time = ktime_get(); - crtc->debug.start_vbl_count = - dev->driver->get_vblank_counter(dev, pipe); + crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc); trace_i915_pipe_update_vblank_evaded(crtc); } @@ -156,10 +153,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) */ void intel_pipe_update_end(struct intel_crtc *crtc) { - struct drm_device *dev = crtc->base.dev; enum pipe pipe = crtc->pipe; int scanline_end = intel_get_crtc_scanline(crtc); - u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe); + u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc); ktime_t end_vbl_time = ktime_get(); trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);