Elyes HAOUAS has posted comments on this change. ( https://review.coreboot.org/28935 )
Change subject: nb/i945: Check if IGD is enabled before R/W to dev(0, 2) ......................................................................
Patch Set 1:
I don't see a way IGD could be disabled at this stage. But what
about
SKUs that don't have it at all? are there any (on supported
boards)?
1 - In case we use an external GPU, the IGD is disabled at arly_init.c (line #686).
Which is run after raminit.
(BTW: line +949 should be commented)
It doesn't work without that line. You probably mean 950 should be implemented for the desktop version. Please, go ahead.
The board I have works without that line. seems that nvidia external GPU didn't work properly (I don't have it for test), But ATI works just fine without that line. More, it works better without that line. please see https://review.coreboot.org/#/c/coreboot/+/27985/ (Arthur's comment on Patch Set 3 )
2 - there some desktop's version that do not have IGD at all.
That's why I was asking. Are the DEVEN bits hard-wired to 0 for them?
I don't think so, but as you know, intel's datasheet are not helpful :( maybe some one from intel can help ...
3 - at function "sdram_power_management", we set
integrated_graphics
= 1 and use it for test in raminit.c line #2305 .... this do not make sense.
It documents for the human reader that it should only be executed when integrated graphics are present (or maybe only when enabled; that's not clear). This can be useful, for instance, when somebody wants to implement it for system without IGD.
here I mean why we define "integrated_graphics" = 1 ? the test do not make sense : if (1) { .... }else { ..... <<=== this will never happen
}
so let give a sense to "integrated_graphics" and set it to 1 if IGD is enabled.