On Mon, 2010-04-12 at 17:09 +0800, Zhenyu Wang wrote: > Update the self-refresh watermark for display plane/cursor and enable > the memory self-refresh on Ironlake. The watermark is also updated for > the active display plane. > > More than 1W idle power is saved on one Ironlake laptop after enabling > memory self-refresh.
Looks good at a quick read, though I haven't tested it yet. > + /* Calculate and update the watermark for plane A */ > + if (planea_clock) { > + entries_required = ((planea_clock / 1000) * pixel_size * > + ILK_LP0_PLANE_LATENCY) / 1000; > + entries_required = DIV_ROUND_UP(entries_required, > + ironlake_display_wm_info.cacheline_size); > + planea_wm = entries_required + > + ironlake_display_wm_info.guard_size; > + > + if (planea_wm > (int)ironlake_display_wm_info.max_wm) > + planea_wm = ironlake_display_wm_info.max_wm; > + > + cursora_wm = 16; > + reg_value = I915_READ(WM0_PIPEA_ILK); > + reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK); > + reg_value |= (planea_wm << WM0_PIPE_PLANE_SHIFT) | > + (cursora_wm & WM0_PIPE_CURSOR_MASK); > + I915_WRITE(WM0_PIPEA_ILK, reg_value); > + DRM_DEBUG_KMS("FIFO watermarks For pipe A - plane %d, " > + "cursor: %d\n", planea_wm, cursora_wm); > + } This doesn't adjust the video sprite plane watermark. I suppose it doesn't really matter, since we're not enabling it yet, but for clarity I'd also mask off WM0_PIPE_SPRITE_MASK. Also, the cursor watermark is a magic number. I assume it's derived from the hardware cursor size. In which case, it'd be nice to show the derivation; Ironlake can do hardware cursors up to 256x256, it'd be nice to have that work automatically when and if we enable that in the driver. > + /* configure watermark and enable self-refresh */ > + reg_value = I915_READ(WM1_LP_ILK); > + reg_value &= ~(WM1_LP_LATENCY_MASK | WM1_LP_SR_MASK | > + WM1_LP_CURSOR_MASK); > + reg_value |= WM1_LP_SR_EN | > + (ilk_sr_latency << WM1_LP_LATENCY_SHIFT) | > + (sr_wm << WM1_LP_SR_SHIFT) | cursor_wm; > + > + I915_WRITE(WM1_LP_ILK, reg_value); > + DRM_DEBUG_KMS("self-refresh watermark: display plane %d " > + "cursor %d\n", sr_wm, cursor_wm); What are the conditions for entering LP2 or LP3 state, and is it worth trying to set them up too? > + /* > + * According to the spec the following bits should be set in > + * order to enable memory self-refresh > + * The bit 22/21 of 0x42004 > + * The bit 5 of 0x42020 > + * The bit 15 of 0x45000 > + */ > + if (IS_IRONLAKE(dev)) { > + I915_WRITE(ILK_DISPLAY_CHICKEN2, > + (I915_READ(ILK_DISPLAY_CHICKEN2) | > + ILK_DPARB_GATE | ILK_VSDPFD_FULL)); I can only hope this is named "chicken" in allusion to: http://en.wikipedia.org/wiki/Chicken_(game) I certainly can't find any reference to it in the documentation. - ajax
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx