On Monday, June 11, 2007 8:59:14 Dave Jones wrote: > On Tue, Jun 12, 2007 at 11:34:25AM +0800, Wang Zhenyu wrote: > > I understand. Before James reported his problem on i915, I have > > thought the basic restore on that chip should already be enough, > > but he proved I was wrong and I'm not sure if this also happens on > > other i915 board with different BIOS. > > > > And with my patch it has already removed the restore cases for > > 440BX like type, coz it's gmch_chip_id == 0 and > > intel_private.pcidev is NULL, so it won't save extra space on > > those chips. > > The 440BX was one example, for all we know there are similar ordering > issues with other chipsets. We hit this problem with the code that > restores the first 64 bytes first of all. Then we found out we had > to restore them in reverse order to be safe. We were able to do > this generically, because those bytes are standardised across > devices. > > The upper config space isn't standardised, so we have to obey the > per-device rules as to what order we read/write things. > Writing back an "enable" bit somewhere before we've written back > addresses in later registers for example may result in really > bizarre things happening. These are the kind of bugs that aren't > obvious, and turn out to be "that weird reboot that happens > every 3rd tuesday" six months after we've merged the changes > and everyones forgotten all about the potential problems. > > The AGP code has had more than its fair share of really nasty > bugs like this to track down, so I'm strongly opposed to introducing > hacks that may trip us up later. > > Whilst I'm not a huge fan of the 815 patch in -mm as it stands, > I think it's a better direction to go in to have per-chipset > save/restore routines.
I agree. I think saving the registers explicitly rather than as a block is the best long term direction. That way we can carefully evaluate which ones actually need to be saved/restored, and it what order. Doing it as a block hides that and makes it seem chipset independent when it really might not be... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
