[Intel-gfx] [PATCH 1/3] drm/i915: implement IPS feature
Daniel Vetter
daniel at ffwll.ch
Fri May 31 18:30:38 CEST 2013
On Thu, May 23, 2013 at 06:26:28PM -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 pipe 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%.
>
> v2: - It's tied to pipe A, not port A
> - Add pipe_config support (Chris)
> - Add some assertions (Chris)
> - Rebase against latest dinq
> v3: - Don't ever set ips_enabled to false (Daniel)
> - Only check for ips_enabled at hsw_disable_ips (Daniel)
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 11 ++++++
> drivers/gpu/drm/i915/intel_display.c | 73 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 3 files changed, 84 insertions(+), 2 deletions(-)
>
>
> The workaround we implement is called WANoName. What's the correct way to
> proceed for WANoName?
>
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 58230ea..5db20dc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -977,6 +977,8 @@
> /* Framebuffer compression for Ivybridge */
> #define IVB_FBC_RT_BASE 0x7020
>
> +#define IPS_CTL 0x43408
> +#define IPS_ENABLE (1 << 31)
>
> #define _HSW_PIPE_SLICE_CHICKEN_1_A 0x420B0
> #define _HSW_PIPE_SLICE_CHICKEN_1_B 0x420B4
> @@ -3620,6 +3622,15 @@
> #define _LGC_PALETTE_B 0x4a800
> #define LGC_PALETTE(pipe) _PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B)
>
> +#define _GAMMA_MODE_A 0x4a480
> +#define _GAMMA_MODE_B 0x4ac80
> +#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
> +#define GAMMA_MODE_MODE_MASK (3 << 0)
> +#define GAMMA_MODE_MODE_8bit (0 << 0)
> +#define GAMMA_MODE_MODE_10bit (1 << 0)
> +#define GAMMA_MODE_MODE_12bit (2 << 0)
> +#define GAMMA_MODE_MODE_SPLIT (3 << 0)
> +
> /* interrupts */
> #define DE_MASTER_IRQ_CONTROL (1 << 31)
> #define DE_SPRITEB_FLIP_DONE (1 << 29)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1c0d6d3..bc99183 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3340,6 +3340,51 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> intel_wait_for_vblank(dev, intel_crtc->pipe);
> }
>
> +/* IPS only exists on ULT machines and is tied to pipe A. */
> +static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
> +{
> + return (IS_ULT(crtc->base.dev) && crtc->pipe == PIPE_A);
> +}
> +
> +static void hsw_enable_ips(struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +
> + if (!hsw_crtc_supports_ips(crtc))
> + return;
> +
> + if (crtc->config.pipe_bpp != 24)
> + return;
> +
> + DRM_DEBUG_KMS("Enabling IPS\n");
> +
> + crtc->config.ips_enabled = true;
Oops, I've found another one.
The general rule (there are some violations due to in-progress conversion)
is that the pipe_config should _never_ be changed in the ->mode_set and
->enable hooks. Instead it should be completely pre-calculated in the
compute_config stage. So can you please move the above code into a
haswell_compute_ips function and call it with a IS_HASWELL check from
ironlake_fdi_compute_config (maybe at the end where we compute the fdi
config)?
You can also dump the debug output and move that into dump_pipe_config.
The reason for that strict separation is that in the end I want to use the
pipe_config to decide about the different ways we can do modesets like:
- full enable/disable sequence
- fastboot for special cases + some fixups
- just an fb update
If not everything is pre-computed we'll have to deal with even more
special cases for this.
Cheers, Daniel
> +
> + /* 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. */
> + assert_plane_enabled(dev_priv, crtc->plane);
> + I915_WRITE(IPS_CTL, IPS_ENABLE);
> +}
> +
> +static void hsw_disable_ips(struct intel_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (!crtc->config.ips_enabled)
> + return;
> +
> + DRM_DEBUG_KMS("Disabling IPS\n");
> +
> + assert_plane_enabled(dev_priv, crtc->plane);
> + I915_WRITE(IPS_CTL, 0);
> +
> + /* We need to wait for a vblank before we can disable the plane. */
> + intel_wait_for_vblank(dev, crtc->pipe);
> +}
> +
> static void haswell_crtc_enable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -3387,6 +3432,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(intel_crtc);
> +
> if (intel_crtc->config.has_pch_encoder)
> lpt_pch_enable(crtc);
>
> @@ -3529,6 +3576,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> if (dev_priv->cfb_plane == plane)
> intel_disable_fbc(dev);
>
> + hsw_disable_ips(intel_crtc);
> +
> intel_disable_plane(dev_priv, plane, pipe);
>
> if (intel_crtc->config.has_pch_encoder)
> @@ -6051,6 +6100,9 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> if (intel_display_power_enabled(dev, pfit_domain))
> ironlake_get_pfit_config(crtc, pipe_config);
>
> + pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> + (I915_READ(IPS_CTL) & IPS_ENABLE);
> +
> return true;
> }
>
> @@ -6355,8 +6407,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)
> @@ -6364,7 +6418,17 @@ 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 (intel_crtc->config.ips_enabled &&
> + ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
> + GAMMA_MODE_MODE_SPLIT)) {
> + hsw_disable_ips(intel_crtc);
> + reenable_ips = true;
> + }
>
> for (i = 0; i < 256; i++) {
> I915_WRITE(palreg + 4 * i,
> @@ -6372,6 +6436,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(intel_crtc);
> }
>
> static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
> @@ -8083,6 +8150,8 @@ intel_pipe_config_compare(struct drm_device *dev,
> PIPE_CONF_CHECK_I(pch_pfit.pos);
> PIPE_CONF_CHECK_I(pch_pfit.size);
>
> + PIPE_CONF_CHECK_I(ips_enabled);
> +
> #undef PIPE_CONF_CHECK_I
> #undef PIPE_CONF_CHECK_FLAGS
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 57de0c1..2045974 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -268,6 +268,8 @@ struct intel_crtc_config {
> /* FDI configuration, only valid if has_pch_encoder is set. */
> int fdi_lanes;
> struct intel_link_m_n fdi_m_n;
> +
> + bool ips_enabled;
> };
>
> struct intel_crtc {
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list