[Intel-gfx] [PATCH]DRM i915: IGD big FIFO support

[Eric Anholt]

On Mon, 2009-05-18 at 10:44 +0800, Shaohua Li wrote:
> Big FIFO is a feature to put memory into self-refresh mode when CPU
> enters C3+ state. Gfx has a FIFO to buffer memory access, when the
> watermark of the FIFO is under threshold, Gfx doesn't need access
> memory, so at that time memory can be put into self-refresh mode.
> 
> The watermark calculation is based on CPU C3 state exit latency. If
> watermark is wrong, when CPU enters C3+, display will be broken or
> flicker.
> 
> I had a power measurement about the feature:
> environment: 1920x1400 display, Atom CPU with C4 enabled, system FSB 
> is 667 and memory is DDR2 667. Launch X, and gives system several
> minutes to settle down, then test the power of the whole system in idle time:
> without big fifo, idle power is 19.8w
> with it, idle power is 18.6w
> 
> The patch doesn't enable HPLL off for CxSR. Last time I heard it's broken
> in current IGD chip, if it works, then I'll add it later.
> 
> Signed-off-by: Shaohua Li <shaohua.li at intel.com>

Nice, and thanks for including numbers in the commit message.  A few
comments:

> +
> +#define HOSTBAR_SIZE (1024 * 16)
> +static int igd_get_freq(struct drm_device *dev, unsigned long *fsb,
> +	unsigned long *mem)
> +{
> +	struct pci_dev *host;
> +	u64 mch_bar;
> +	u32 tmp;
> +	void *addr;
> +
> +	host = pci_get_device(PCI_VENDOR_ID_INTEL, 0xa000, NULL);
> +	if (!host)
> +		host = pci_get_device(PCI_VENDOR_ID_INTEL, 0xa010, NULL);
> +	if (!host)
> +		return -ENODEV;
> +	pci_read_config_dword(host, 0x48, &tmp);
> +	mch_bar = tmp;
> +	pci_read_config_dword(host, 0x4c, &tmp);
> +	mch_bar |= ((u64)tmp) << 32;
> +
> +	mch_bar &= ~((u64)(HOSTBAR_SIZE -1));
> +	pci_dev_put(host);
> +
> +	addr = ioremap(mch_bar, HOSTBAR_SIZE);
> +	if (!addr)
> +		return -ENOMEM;
> +	tmp = *(u32 *)(addr + 0xc00);
> +	iounmap(addr);

MCHBAR mapping is tricky.  I think we'll need to use the same code here
as i915_gem_tiling.c, and that's changing in 2.6.31 to successfully
enable it (safely) more often.

> +	switch (tmp & 0x7) {
> +	case 1:
> +		*fsb = 533; /* 133*4 */
> +		break;
> +	case 2:
> +		*fsb = 800; /* 200*4 */
> +		break;
> +	case 3:
> +		*fsb = 667; /* 167*4 */
> +		break;
> +	case 5:
> +		*fsb = 400; /* 100*4 */
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch ((tmp >> 4) & 0x7) {
> +	case 1:
> +		*mem = 533;
> +		break;
> +	case 2:
> +		*mem = 667;
> +		break;
> +	case 3:
> +		*mem = 800;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static void igd_setup_cxsr(struct drm_device *dev, unsigned long clock)
> +{
> +	u32 reg;
> +	unsigned long wm;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct cxsr_latency *latency;
> +	unsigned long fsb, mem;
> +	int i;
> +	int pipes_enabled = 0;
> +	struct drm_crtc *crtc;
> +
> +	if (!IS_IGD(dev))
> +		return;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		if (crtc->enabled)
> +			pipes_enabled++;
> +	}
> +	if (pipes_enabled != 1) {
> +		DRM_DEBUG("Two pipes are enabled, disable CxSR\n");
> +		goto disable_cxsr;
> +	}
> +
> +	if (igd_get_freq(dev, &fsb, &mem))
> +		goto disable_cxsr;
> +
> +	for (i = 0; i < ARRAY_SIZE(igd_cxsr_latency); i++) {
> +		latency = &igd_cxsr_latency[i];
> +		if ((!!IS_IGDG(dev)) == latency->is_desktop &&

IS_IGDG is already a boolean.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090522/e99a2530/attachment.sig>