<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN"> <HTML> <HEAD> <META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8"> <META NAME="GENERATOR" CONTENT="GtkHTML/3.30.0"> </HEAD> <BODY> 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: <BLOCKQUOTE TYPE=CITE> <PRE> 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 <A HREF="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</A> <A HREF="http://lists.freedesktop.org/mailman/listinfo/intel-gfx">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</A> </PRE> </BLOCKQUOTE> </BODY> </HTML>