[Intel-gfx] [PATCH 1/3] drm/i915: implement IPS feature

[Chris Wilson]

On Mon, May 13, 2013 at 04:00:08PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> Intermediate Pixel Storage is a feature that should reduce the number
> of times the display engine wakes up memory to read pixels, so it
> should allow deeper PC states. IPS can only be enabled on ULT port A
> with 8:8:8 pipe pixel formats.
> 
> With eDP 1920x1080 and correct watermarks but without FBC this moves
> my PC7 residency from 2.5% to around 38%.

You need to read initial IPS state and add that to pipe-config so that
takeover from the BIOS is correct and to make the code less fiddly.

[snip]

> +static bool hsw_crtc_supports_ips(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder;
> +	bool is_port_a_edp = false;
> +
> +	if (!IS_ULT(dev))
> +		return false;
> +
> +	for_each_encoder_on_crtc(dev, crtc, encoder)
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
> +				is_port_a_edp = true;
> +	if (!is_port_a_edp)
> +		return false;
> +
> +	if (intel_crtc->config.pipe_bpp != 24)
> +		return false;
> +

Try:
 {
  if (!IS_ULT(dev)) return false;
  if (intel_crtc->config.pipe_bpp != 24) return false;
  for_each_encoder_on_crtc() if (encoder->type == INTEL_OUTPUT_EDP && enc_to_dig_port(encoder) == PORT_A) return true;
  return false;
 }

> +	return true;
> +}
> +
> +static void hsw_enable_ips(struct drm_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +
> +	if (!hsw_crtc_supports_ips(crtc))
> +		return;
> +
> +	DRM_DEBUG_KMS("Enabling IPS\n");
> +
> +	/* We can only enable IPS after we enable a plane and wait for a vblank.
> +	 * We guarantee that the plane is enabled by calling intel_enable_ips
> +	 * only after intel_enable_plane. And intel_enable_plane already waits
> +	 * for a vblank, so all we need to do here is to enable the IPS bit. */

Throw in an assert_pipe_enabled() for good measure.

> +	I915_WRITE(IPS_CTL, IPS_ENABLE);
> +}
> +
> +static void hsw_disable_ips(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);
> +
> +	if (!hsw_crtc_supports_ips(crtc))
> +		return;

For disabling, it would be better to check if the IPS feature was
enabled on this pipe. As written, one can change the module parameter to
trick the system into running with IPS always enabled - presumably that
is bad.

> +
> +	DRM_DEBUG_KMS("Disabling IPS\n");
> +
> +	I915_WRITE(IPS_CTL, 0);
> +
> +	/* We need to wait for a vblank before we can disable the plane. */
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +}
> +
>  static void haswell_crtc_enable(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -3387,6 +3443,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  			  intel_crtc->config.has_pch_encoder);
>  	intel_enable_plane(dev_priv, plane, pipe);
>  
> +	hsw_enable_ips(crtc);
> +
>  	if (intel_crtc->config.has_pch_encoder)
>  		lpt_pch_enable(crtc);
>  
> @@ -3516,6 +3574,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	if (dev_priv->cfb_plane == plane)
>  		intel_disable_fbc(dev);
>  
> +	hsw_disable_ips(crtc);
> +
>  	intel_disable_plane(dev_priv, plane, pipe);
>  
>  	if (intel_crtc->config.has_pch_encoder)
> @@ -6291,8 +6351,10 @@ void intel_crtc_load_lut(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);
> -	int palreg = PALETTE(intel_crtc->pipe);
> +	enum pipe pipe = intel_crtc->pipe;
> +	int palreg = PALETTE(pipe);
>  	int i;
> +	bool reenable_ips = false;
>  
>  	/* The clocks have to be on to load the palette. */
>  	if (!crtc->enabled || !intel_crtc->active)
> @@ -6300,7 +6362,18 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
>  
>  	/* use legacy palette for Ironlake */
>  	if (HAS_PCH_SPLIT(dev))
> -		palreg = LGC_PALETTE(intel_crtc->pipe);
> +		palreg = LGC_PALETTE(pipe);
> +
> +	/* Workaround : Do not read or write the pipe palette/gamma data while
> +	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
> +	 */
> +	if (hsw_crtc_supports_ips(crtc) &&

Bring on pipe_config IPS state - as this can provoked into a user
triggerable hang (or something)...

> +	    (I915_READ(IPS_CTL) & IPS_ENABLE) &&
> +	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
> +	     GAMMA_MODE_MODE_SPLIT)) {
> +		hsw_disable_ips(crtc);
> +		reenable_ips = true;
> +	}
>  
>  	for (i = 0; i < 256; i++) {
>  		I915_WRITE(palreg + 4 * i,
> @@ -6308,6 +6381,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
>  			   (intel_crtc->lut_g[i] << 8) |
>  			   intel_crtc->lut_b[i]);
>  	}
> +
> +	if (reenable_ips)
> +		hsw_enable_ips(crtc);

See above.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre