[Intel-gfx] [PATCH 03/58] drm/i915: add direct encoder disable/enable infrastructure

[Jesse Barnes]

On Sun, 19 Aug 2012 21:12:20 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:

> Just prep work, not yet put to some use.
> 
> Note that because we're still using the crtc helper to switch modes
> (and their complicated way to do partial modesets), we need to call
> the encoder's disable function unconditionally.
> 
> But once this is cleaned up we shouldn't call the encoder's disable
> function unconditionally any more, because then we know that we'll
> only call it if the encoder is actually enabled. Also note that we
> then need to be careful about which crtc we're filtering the encoder
> list on: We want to filter on the crtc of the _current_ mode, not the
> one we're about to set up.
> 
> For the enabling side we need to do the same trick. And again, we
> should be able to simplify this quite a bit when things have settled
> into place.
> 
> Also note that this simply does not take cloning into account, so dpms
> needs to be handled specially for the few outputs where we even bother
> with it.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 38
> ++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h     |  2 ++ 2 files changed, 38
> insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 04bec4b..82aaded 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3209,13 +3209,16 @@ static void ironlake_crtc_enable(struct
> drm_crtc *crtc) struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  	u32 temp;
>  	bool is_pch_port;
>  
> +	/* XXX: For compatability with the crtc helper code, call
> the encoder's
> +	 * enable function unconditionally for now. */
>  	if (intel_crtc->active)
> -		return;
> +		goto encoders;
>  
>  	intel_crtc->active = true;
>  	intel_update_watermarks(dev);
> @@ -3262,6 +3265,12 @@ static void ironlake_crtc_enable(struct
> drm_crtc *crtc) mutex_unlock(&dev->struct_mutex);
>  
>  	intel_crtc_update_cursor(crtc, true);
> +
> +encoders:
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		if (encoder->enable)
> +			encoder->enable(encoder);
> +	}
>  }
>  
>  static void ironlake_crtc_disable(struct drm_crtc *crtc)
> @@ -3269,10 +3278,18 @@ static void ironlake_crtc_disable(struct
> drm_crtc *crtc) struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  	u32 reg, temp;
>  
> +	/* XXX: For compatability with the crtc helper code, call
> the encoder's
> +	 * disable function unconditionally for now. */
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		if (encoder->disable)
> +			encoder->disable(encoder);
> +	}
> +
>  	if (!intel_crtc->active)
>  		return;
>  
> @@ -3371,11 +3388,14 @@ static void i9xx_crtc_enable(struct drm_crtc
> *crtc) struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  
> +	/* XXX: For compatability with the crtc helper code, call
> the encoder's
> +	 * enable function unconditionally for now. */
>  	if (intel_crtc->active)
> -		return;
> +		goto encoders;
>  
>  	intel_crtc->active = true;
>  	intel_update_watermarks(dev);
> @@ -3390,6 +3410,12 @@ static void i9xx_crtc_enable(struct drm_crtc
> *crtc) /* Give the overlay scaler a chance to enable if it's on this
> pipe */ intel_crtc_dpms_overlay(intel_crtc, true);
>  	intel_crtc_update_cursor(crtc, true);
> +
> +encoders:
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		if (encoder->enable)
> +			encoder->enable(encoder);
> +	}
>  }
>  
>  static void i9xx_crtc_disable(struct drm_crtc *crtc)
> @@ -3397,9 +3423,17 @@ static void i9xx_crtc_disable(struct drm_crtc
> *crtc) struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  
> +	/* XXX: For compatability with the crtc helper code, call
> the encoder's
> +	 * disable function unconditionally for now. */
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		if (encoder->disable)
> +			encoder->disable(encoder);
> +	}
> +
>  	if (!intel_crtc->active)
>  		return;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index 95f635b..9a5adcc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -141,6 +141,8 @@ struct intel_encoder {
>  	 */
>  	bool cloneable;
>  	void (*hot_plug)(struct intel_encoder *);
> +	void (*enable)(struct intel_encoder *);
> +	void (*disable)(struct intel_encoder *);
>  	int crtc_mask;
>  };
>  


Yep, looks good.

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