Re: [PATCH 02/81] drm/i915: rip out crtc prepare/commit indirection

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 11, 2012 at 04:27:45PM +0200, Daniel Vetter wrote:
> Just impendance matching with the the crtc helper stuff.
> 
> ... and somehow the design of this all ended up in this commit here,
> too ;-)
> 
> The big plan is that this new set of crtc display_funcs take full
> responsibility of modeset operations for the entire display output
> pipeline (by calling down into object-specific callbacks and
> functions). The platform-specific callbacks simply know best what the
> proper order is.
> 
> This has the drawback that we can't do minimal change-overs any more
> if a modeset just disables one encoder in a cloned configuration
> (because we will only expose a disable/enable action that takes
> down/sets up the entire crtc including all encoders). Imo that's the
> only sane way to do it though:
> - The use-case for this is pretty minimal, even when presenting (at
>   least sane people) should use a dual-screen output so that you can
>   see your notes on your panel. Clone mode is imo BS.
> - With all the clone mode constrains, shared resources, and special
>   ordering requirements (which differ even on the same platform
>   sometimes for different outputs) there's no way we'd get this right
>   for all cases. Especially since this is a under-used feature.
> - And to top it off: On haswell even dp link re-training requires us
>   to take down the entire display pipe - otherwise the chip dies.
> 
> So the only sane way is to do a full modeset on every crtc where the
> output config changes in any way.
> 
> To support global modeset (i.e. set the configuration for all crtcs at
> once) we'd then add one more function to allocate global and shared
> objects in the best ways (e.g. fdi links, pch plls, ...). The crtc
> functions would then simply use the pre-allocated stuff (and shouldn't
> be able to fail, ever). We could even do all the object pinning in
> there (and maybe try to defragment the global gtt if we fail)!
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   37 +--------------------------------
>  1 files changed, 2 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 135eed0..b1caa1b81 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3488,34 +3488,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
>  	}
>  }
>  
> -/* Prepare for a mode set.
> - *
> - * Note we could be a lot smarter here.  We need to figure out which outputs
> - * will be enabled, which disabled (in short, how the config will changes)
> - * and perform the minimum necessary steps to accomplish that, e.g. updating
> - * watermarks, FBC configuration, making sure PLLs are programmed correctly,
> - * panel fitting is in the proper state, etc.
> - */
> -static void i9xx_crtc_prepare(struct drm_crtc *crtc)
> -{
> -	i9xx_crtc_disable(crtc);
> -}
> -
> -static void i9xx_crtc_commit(struct drm_crtc *crtc)
> -{
> -	i9xx_crtc_enable(crtc);
> -}
> -
> -static void ironlake_crtc_prepare(struct drm_crtc *crtc)
> -{
> -	ironlake_crtc_disable(crtc);
> -}
> -
> -static void ironlake_crtc_commit(struct drm_crtc *crtc)
> -{
> -	ironlake_crtc_enable(crtc);
> -}
> -
>  void intel_encoder_prepare(struct drm_encoder *encoder)
>  {
>  	struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;
> @@ -6636,13 +6608,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	intel_crtc->active = true; /* force the pipe off on setup_init_config */
>  	intel_crtc->bpp = 24; /* default for pre-Ironlake */
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> -		intel_helper_funcs.prepare = ironlake_crtc_prepare;
> -		intel_helper_funcs.commit = ironlake_crtc_commit;
> -	} else {
> -		intel_helper_funcs.prepare = i9xx_crtc_prepare;
> -		intel_helper_funcs.commit = i9xx_crtc_commit;
> -	}
> +	intel_helper_funcs.prepare = dev_priv->display.crtc_disable;
> +	intel_helper_funcs.commit = dev_priv->display.crtc_enable;
>  
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  
> -- 
> 1.7.7.6
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48