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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2418 bytes --]

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.vetter@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

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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