On Thu, Oct 22, 2015 at 01:56:28PM +0200, Maarten Lankhorst wrote:
> By handling this after the atomic helper waits for vblanks there will
> be one less wait for vblank in the atomic path.
> 
> Also get rid of the double wait_for_vblank on broadwell, looks like
> it's a bug doing the vblank wait twice.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>

Problem is that the next plane update should be possible right after this
one happens, which is the potentially racing with the post_plane_update
code. What's your plan to prevent this?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 +++-------------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  2 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 051a1e2b1c55..b8e1a5471bed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4672,14 +4672,6 @@ intel_post_enable_primary(struct drm_crtc *crtc)
>       int pipe = intel_crtc->pipe;
>  
>       /*
> -      * BDW signals flip done immediately if the plane
> -      * is disabled, even if the plane enable is already
> -      * armed to occur at the next vblank :(
> -      */
> -     if (IS_BROADWELL(dev))
> -             intel_wait_for_vblank(dev, pipe);
> -
> -     /*
>        * FIXME IPS should be fine as long as one plane is
>        * enabled, but in practice it seems to have problems
>        * when going from primary only to sprite only and vice
> @@ -4760,9 +4752,6 @@ static void intel_post_plane_update(struct intel_crtc 
> *crtc)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       struct drm_plane *plane;
>  
> -     if (atomic->wait_vblank)
> -             intel_wait_for_vblank(dev, crtc->pipe);
> -
>       intel_frontbuffer_flip(dev, atomic->fb_bits);
>  
>       if (atomic->disable_cxsr)
> @@ -11661,15 +11650,12 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>               if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>                       intel_crtc->atomic.disable_cxsr = true;
>                       /* to potentially re-enable cxsr */
> -                     intel_crtc->atomic.wait_vblank = true;
>                       intel_crtc->atomic.update_wm_post = true;
>               }
>       } else if (turn_off) {
>               intel_crtc->atomic.update_wm_post = true;
>               /* must disable cxsr around plane enable/disable */
>               if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> -                     if (is_crtc_enabled)
> -                             intel_crtc->atomic.wait_vblank = true;
>                       intel_crtc->atomic.disable_cxsr = true;
>               }
>       } else if (intel_wm_need_update(plane, plane_state)) {
> @@ -11716,21 +11702,12 @@ int intel_plane_atomic_calc_changes(struct 
> drm_crtc_state *crtc_state,
>                   plane_state->rotation != BIT(DRM_ROTATE_0))
>                       intel_crtc->atomic.disable_fbc = true;
>  
> -             /*
> -              * BDW signals flip done immediately if the plane
> -              * is disabled, even if the plane enable is already
> -              * armed to occur at the next vblank :(
> -              */
> -             if (turn_on && IS_BROADWELL(dev))
> -                     intel_crtc->atomic.wait_vblank = true;
> -
>               intel_crtc->atomic.update_fbc |= visible || mode_changed;
>               break;
>       case DRM_PLANE_TYPE_CURSOR:
>               break;
>       case DRM_PLANE_TYPE_OVERLAY:
>               if (turn_off && !mode_changed) {
> -                     intel_crtc->atomic.wait_vblank = true;
>                       intel_crtc->atomic.update_sprite_watermarks |=
>                               1 << i;
>               }
> @@ -13271,14 +13248,15 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>  
>               if (put_domains)
>                       modeset_put_power_domains(dev_priv, put_domains);
> -
> -             intel_post_plane_update(intel_crtc);
>       }
>  
>       /* FIXME: add subpixel order */
>  
>       drm_atomic_helper_wait_for_vblanks(dev, state);
>  
> +     for_each_crtc_in_state(state, crtc, crtc_state, i)
> +             intel_post_plane_update(to_intel_crtc(crtc));
> +
>       mutex_lock(&dev->struct_mutex);
>       drm_atomic_helper_cleanup_planes(dev, state);
>       mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 54a2c0da2ece..4b6bb1adfddd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -522,7 +522,6 @@ struct intel_crtc_atomic_commit {
>  
>       /* Sleepable operations to perform after commit */
>       unsigned fb_bits;
> -     bool wait_vblank;
>       bool update_fbc;
>       bool post_enable_primary;
>       unsigned update_sprite_watermarks;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to