Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42410 )
Change subject: nb/intel/sandybridge/gma.c: Remove useless if condition ......................................................................
Patch Set 2:
(6 comments)
Some things that seem to be wrong
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/gma.c:
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 376: gtt_write(0xa004, 0x00000010); lockbit
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 382: gtt_write(0xa080, 0x00000004); lockbit
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 386: (1 << 31) lockbit
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 479: lockbits are set here:
gtt_write(0xa248, gtt_read(0xa248) | (1 << 31)); gtt_write(0xa004, gtt_read(0xa004) | (1 << 4)); gtt_write(0xa080, gtt_read(0xa080) | (1 << 2)); gtt_write(0xa180, gtt_read(0xa180) | (1 << 31));
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 510: gtt_write(0xa188, 0x1fffe); this should be ANDing the register with 0x0001ffff instead
bit0 is cleared in a separate write
https://review.coreboot.org/c/coreboot/+/42410/1/src/northbridge/intel/sandy... PS1, Line 515: /* 16: SW RC Control */ RC6 deepest