[Intel-gfx] [PATCH 09/76] drm/i915: add crtc->enable/disable vfuncs insted of dpms

[Jesse Barnes]

On Thu, 26 Jul 2012 20:48:34 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:

> Because that's what we're essentially calling. This is the first step
> in untangling the crtc_helper induced dpms handling mess we have - at
> the crtc level we only have 2 states and the magic is just in
> selecting which one (and atm there isn't even much magic, but on
> recent platforms where not even the crt output has more than 2 states
> we could do better).
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    3 +-
>  drivers/gpu/drm/i915/intel_display.c |   62
> ++++++++++------------------------ 2 files changed, 20 insertions(+),
> 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 1258a74..c81c6e8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -240,7 +240,6 @@ struct drm_i915_error_state {
>  };
>  
>  struct drm_i915_display_funcs {
> -	void (*dpms)(struct drm_crtc *crtc, int mode);
>  	bool (*fbc_enabled)(struct drm_device *dev);
>  	void (*enable_fbc)(struct drm_crtc *crtc, unsigned long
> interval); void (*disable_fbc)(struct drm_device *dev);
> @@ -256,6 +255,8 @@ struct drm_i915_display_funcs {
>  			     struct drm_display_mode *adjusted_mode,
>  			     int x, int y,
>  			     struct drm_framebuffer *old_fb);
> +	void (*crtc_enable)(struct drm_crtc *crtc);
> +	void (*crtc_disable)(struct drm_crtc *crtc);
>  	void (*off)(struct drm_crtc *crtc);
>  	void (*write_eld)(struct drm_connector *connector,
>  			  struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 1d5a811..17eb87b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3341,30 +3341,6 @@ static void ironlake_crtc_disable(struct
> drm_crtc *crtc) mutex_unlock(&dev->struct_mutex);
>  }
>  
> -static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> -	int plane = intel_crtc->plane;
> -
> -	/* XXX: When our outputs are all unaware of DPMS modes other
> than off
> -	 * and on, we should map those modes to DRM_MODE_DPMS_OFF in
> the CRTC.
> -	 */
> -	switch (mode) {
> -	case DRM_MODE_DPMS_ON:
> -	case DRM_MODE_DPMS_STANDBY:
> -	case DRM_MODE_DPMS_SUSPEND:
> -		DRM_DEBUG_KMS("crtc %d/%d dpms on\n", pipe, plane);
> -		ironlake_crtc_enable(crtc);
> -		break;
> -
> -	case DRM_MODE_DPMS_OFF:
> -		DRM_DEBUG_KMS("crtc %d/%d dpms off\n", pipe, plane);
> -		ironlake_crtc_disable(crtc);
> -		break;
> -	}
> -}
> -
>  static void ironlake_crtc_off(struct drm_crtc *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -3444,23 +3420,6 @@ static void i9xx_crtc_disable(struct drm_crtc
> *crtc) intel_update_watermarks(dev);
>  }
>  
> -static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode)
> -{
> -	/* XXX: When our outputs are all unaware of DPMS modes other
> than off
> -	 * and on, we should map those modes to DRM_MODE_DPMS_OFF in
> the CRTC.
> -	 */
> -	switch (mode) {
> -	case DRM_MODE_DPMS_ON:
> -	case DRM_MODE_DPMS_STANDBY:
> -	case DRM_MODE_DPMS_SUSPEND:
> -		i9xx_crtc_enable(crtc);
> -		break;
> -	case DRM_MODE_DPMS_OFF:
> -		i9xx_crtc_disable(crtc);
> -		break;
> -	}
> -}
> -
>  static void i9xx_crtc_off(struct drm_crtc *crtc)
>  {
>  }
> @@ -3482,7 +3441,20 @@ static void intel_crtc_dpms(struct drm_crtc
> *crtc, int mode) 
>  	intel_crtc->dpms_mode = mode;
>  
> -	dev_priv->display.dpms(crtc, mode);
> +	/* XXX: When our outputs are all unaware of DPMS modes other
> than off
> +	 * and on, we should map those modes to DRM_MODE_DPMS_OFF in
> the CRTC.
> +	 */
> +	switch (mode) {
> +	case DRM_MODE_DPMS_ON:
> +	case DRM_MODE_DPMS_STANDBY:
> +	case DRM_MODE_DPMS_SUSPEND:
> +		dev_priv->display.crtc_enable(crtc);
> +		break;
> +
> +	case DRM_MODE_DPMS_OFF:
> +		dev_priv->display.crtc_disable(crtc);
> +		break;
> +	}
>  
>  	if (!dev->primary->master)
>  		return;
> @@ -6898,13 +6870,15 @@ static void intel_init_display(struct
> drm_device *dev) 
>  	/* We always want a DPMS function */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		dev_priv->display.dpms = ironlake_crtc_dpms;
>  		dev_priv->display.crtc_mode_set =
> ironlake_crtc_mode_set;
> +		dev_priv->display.crtc_enable = ironlake_crtc_enable;
> +		dev_priv->display.crtc_disable =
> ironlake_crtc_disable; dev_priv->display.off = ironlake_crtc_off;
>  		dev_priv->display.update_plane =
> ironlake_update_plane; } else {
> -		dev_priv->display.dpms = i9xx_crtc_dpms;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> +		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  		dev_priv->display.off = i9xx_crtc_off;
>  		dev_priv->display.update_plane = i9xx_update_plane;
>  	}


I like it.

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>