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

Daniel Vetter
2012-07-12T14:44:41Z

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