Re: [Intel-gfx] [PATCH 05/12] drm/i915: Remove drm_vblank_pre/post_modeset calls

2014-05-21 Thread Thierry Reding
On Wed, May 14, 2014 at 08:51:07PM +0200, Daniel Vetter wrote:
 Originally these functions have been for user modesetting drivers to
 ensure vblank processing doesn't fall over completely around modeset
 changes. This has been carried over ever since then.
 
 Now that Ville cleaned our vblank handling with an explicit
 drm_vblank_off/on braket when disabling/enabling crtcs. So this seems

s/braket/bracket/

 to be unnecessary now.

Should we document that drivers should start converting to this new set
of functions? Maybe deprecate the drm_vblank_pre/post_modeset()?

 The most important side effect was that due to
 the delayed vblank disabling we have been pretty much guaranteed to
 receive a vblank interrupt soonish after a crtc was enabled.

I don't understand what this sentence means and whether it relates to
code prior to or after this patch.

 Note that our vblank handling across modeset is still fairly decent
 fubar - we don't actually handle vblank counter all to well.
 drm_update_vblank_count will make sure that the frame counter always
 rolls forward, but userspace isn't really all to ready to cope with
 the big jumps this causes.
 
 This isn't a big mostly because the hardware retains the frame

Not a big what?

 counter. But with runtime pm and also across suspend/resume we fall
 over.
 
 Fixing this is a lot more involved and also needs som i-g-ts. So
 material for another patch series.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 5 -
  1 file changed, 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 858c393b051f..d0eff53a8ad1 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7207,15 +7207,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
   struct intel_encoder *encoder;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   struct drm_display_mode *mode = intel_crtc-config.requested_mode;
 - int pipe = intel_crtc-pipe;
   int ret;
  
 - drm_vblank_pre_modeset(dev, pipe);
 -
   ret = dev_priv-display.crtc_mode_set(crtc, x, y, fb);
  
 - drm_vblank_post_modeset(dev, pipe);
 -
   if (ret != 0)

Nit: There's now a blank line between ret = ... and if (...).

Thierry


pgpowDAKeoTqC.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 05/12] drm/i915: Remove drm_vblank_pre/post_modeset calls

2014-05-14 Thread Daniel Vetter
Originally these functions have been for user modesetting drivers to
ensure vblank processing doesn't fall over completely around modeset
changes. This has been carried over ever since then.

Now that Ville cleaned our vblank handling with an explicit
drm_vblank_off/on braket when disabling/enabling crtcs. So this seems
to be unnecessary now. The most important side effect was that due to
the delayed vblank disabling we have been pretty much guaranteed to
receive a vblank interrupt soonish after a crtc was enabled.

Note that our vblank handling across modeset is still fairly decent
fubar - we don't actually handle vblank counter all to well.
drm_update_vblank_count will make sure that the frame counter always
rolls forward, but userspace isn't really all to ready to cope with
the big jumps this causes.

This isn't a big mostly because the hardware retains the frame
counter. But with runtime pm and also across suspend/resume we fall
over.

Fixing this is a lot more involved and also needs som i-g-ts. So
material for another patch series.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_display.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 858c393b051f..d0eff53a8ad1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7207,15 +7207,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
struct intel_encoder *encoder;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_display_mode *mode = intel_crtc-config.requested_mode;
-   int pipe = intel_crtc-pipe;
int ret;
 
-   drm_vblank_pre_modeset(dev, pipe);
-
ret = dev_priv-display.crtc_mode_set(crtc, x, y, fb);
 
-   drm_vblank_post_modeset(dev, pipe);
-
if (ret != 0)
return ret;
 
-- 
1.8.3.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx