On Sun, Feb 10, 2013 at 11:37:03PM +0100, Paul Menzel wrote: > Am Sonntag, den 10.02.2013, 19:38 +0000 schrieb Chris Wilson: > > Reading the cspec pays dividends once again, as I found the 'Base of > > Stolen Memory' config register so that we can skip the fragile > > computation from Top of Usable Draw and use the real value provided by > > the BIOS. > > Nice find. What is the expected functionality change as the code > beforehand was a no-op due to the `if 0`? It enables the driver to use stolen memory - memory that is hidden from the system and only available, in principal, to the gfx device. > If testing is needed, how can this be tested? Boot a 915g/945g. The driver will utilize stolen memory for its ringbuffers and fbcon. > > - /* Stolen is immediately below Top of Low Usable DRAM */ > > - pci_read_config_byte(pdev, 0x9c, &val); > > - base = val >> 3 << 27; > > - base -= dev_priv->mm.gtt->stolen_size; > > + /* Read D2:F0 Base of Stolen Memory directly */ > > + pci_read_config_dword(dev->pdev, 0x5c, &base); > > Should macros be defined for the register names? My argument is basically where else are you planning to use this value. Each chipset calls it something different and uses a different register, so DRY is not violated and instead of hiding the value in a header, we can define it locally and clearly. Makes adapting it for the next generation much easier as you only have the single location to worry about. (The normal argument for the macro, but in reverse.) -Chris -- Chris Wilson, Intel Open Source Technology Centre