[Intel-gfx] 945 with SR doesn't need manual enable/disable
Li Peng
peng.li at linux.intel.com
Thu Aug 26 10:40:46 CEST 2010
Hello, Alexander:
I have met system hang issue when directly enable SR on 945, so I
enable/disable SR depends on h/w idle status.
If you don't see hang anymore. Then it should be fixed in other commits
(probably as you said, 944001201ca0196bcdb088129e5866a9f379d08c)
Please fix the patch format in your email client. I will give it a test
on my side.
Thanks
Peng
On Mon, 2010-08-23 at 17:34 -0400, Alexander Lam wrote:
> Hi all,
>
> Using 2.6.35.2, I changed 945's self refresh to work without the need
> for the driver to enable/disable self refresh manually based on the
> idle state of the gpu.
>
> Something must have been fixed in the driver between the initial
> implementation of 945 self refresh and now.
> (maybe 944001201ca0196bcdb088129e5866a9f379d08c: drm/i915: enable low
> power render writes on GEN3 hardware?)
>
> patch (which probably doesn't cover all cases concerning if SR should
> be enabled/disabled, not to mention doing things in the wrong order or
> in the wrong place):
>
> diff -uNrp a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> --- a/drivers/gpu/drm/i915/intel_display.c 2010-08-01
> 18:11:14.000000000 -0400
> +++ b/drivers/gpu/drm/i915/intel_display.c 2010-08-22
> 16:52:24.168999994 -0400
> @@ -3016,7 +3016,7 @@ static void i9xx_update_wm(struct drm_de
> int planea_wm, planeb_wm;
> struct intel_watermark_params planea_params, planeb_params;
> unsigned long line_time_us;
> - int sr_clock, sr_entries = 0;
> + int sr_clock, sr_entries = 0, sr_enabled = 0;
>
> /* Create copies of the base settings for each pipe */
> if (IS_I965GM(dev) || IS_I945GM(dev))
> @@ -3063,8 +3063,11 @@ static void i9xx_update_wm(struct drm_de
> if (srwm < 0)
> srwm = 1;
>
> - if (IS_I945G(dev) || IS_I945GM(dev))
> + if (IS_I945G(dev) || IS_I945GM(dev)){
> I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_FIFO_MASK |
> (srwm & 0xff));
> + DRM_DEBUG_DRIVER("enable memory self refresh on 945\n");
> + sr_enabled = 1;
> + }
> else if (IS_I915GM(dev)) {
> /* 915M has a smaller SRWM field */
> I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
> @@ -3073,6 +3076,8 @@ static void i9xx_update_wm(struct drm_de
> } else {
> /* Turn off self refresh if both pipes are enabled */
> if (IS_I945G(dev) || IS_I945GM(dev)) {
> + DRM_DEBUG_DRIVER("disable memory self refresh
> on 945\n");
> + sr_enabled = 0;
> I915_WRITE(FW_BLC_SELF, I915_READ(FW_BLC_SELF)
> & ~FW_BLC_SELF_EN);
> } else if (IS_I915GM(dev)) {
> @@ -3092,6 +3097,8 @@ static void i9xx_update_wm(struct drm_de
>
> I915_WRITE(FW_BLC, fwater_lo);
> I915_WRITE(FW_BLC2, fwater_hi);
> + if (sr_enabled)
> + I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN);
> }
>
> static void i830_update_wm(struct drm_device *dev, int planea_clock,
> int unused,
> @@ -4506,7 +4513,6 @@ static void intel_idle_update(struct wor
> struct drm_device *dev = dev_priv->dev;
> struct drm_crtc *crtc;
> struct intel_crtc *intel_crtc;
> - int enabled = 0;
>
> if (!i915_powersave)
> return;
> @@ -4520,16 +4526,11 @@ static void intel_idle_update(struct wor
> if (!crtc->fb)
> continue;
>
> - enabled++;
> intel_crtc = to_intel_crtc(crtc);
> if (!intel_crtc->busy)
> intel_decrease_pllclock(crtc);
> }
>
> - if ((enabled == 1) && (IS_I945G(dev) || IS_I945GM(dev))) {
> - DRM_DEBUG_DRIVER("enable memory self refresh on 945\n");
> - I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN);
> - }
>
> mutex_unlock(&dev->struct_mutex);
> }
> @@ -4554,17 +4555,9 @@ void intel_mark_busy(struct drm_device *
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return;
>
> - if (!dev_priv->busy) {
> - if (IS_I945G(dev) || IS_I945GM(dev)) {
> - u32 fw_blc_self;
> -
> - DRM_DEBUG_DRIVER("disable memory self refresh
> on 945\n");
> - fw_blc_self = I915_READ(FW_BLC_SELF);
> - fw_blc_self &= ~FW_BLC_SELF_EN;
> - I915_WRITE(FW_BLC_SELF, fw_blc_self |
> FW_BLC_SELF_EN_MASK);
> - }
> + if (!dev_priv->busy)
> dev_priv->busy = true;
> - } else
> + else
> mod_timer(&dev_priv->idle_timer, jiffies +
> msecs_to_jiffies(GPU_IDLE_TIMEOUT));
>
> @@ -4576,14 +4569,6 @@ void intel_mark_busy(struct drm_device *
> intel_fb = to_intel_framebuffer(crtc->fb);
> if (intel_fb->obj == obj) {
> if (!intel_crtc->busy) {
> - if (IS_I945G(dev) || IS_I945GM(dev)) {
> - u32 fw_blc_self;
> -
> - DRM_DEBUG_DRIVER("disable
> memory self refresh on 945\n");
> - fw_blc_self = I915_READ(FW_BLC_SELF);
> - fw_blc_self &= ~FW_BLC_SELF_EN;
> - I915_WRITE(FW_BLC_SELF,
> fw_blc_self | FW_BLC_SELF_EN_MASK);
> - }
> /* Non-busy -> busy, upclock */
> intel_increase_pllclock(crtc, true);
> intel_crtc->busy = true;
>
>
> --
> Alexander Lam
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20100826/21137da9/attachment.html>
More information about the Intel-gfx
mailing list