Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32044 )
Change subject: soc/intel/skylake: Update GFX devtree options ......................................................................
Patch Set 8:
Patch Set 7: Code-Review-1
IMO, this patch does too much at once to keep track of everything...
Open questions:
- Why change the PrimaryDisplay setting of h110m?
- Why would we ever set `PrimaryDisplay = Display_Auto` together with `SkipExtGfxScan`?
- If I didn't miss it, the `SkipExtGfxScan` UPD wasn't set before? So this change changes all boards without mentioning it?
- What does `SkipExtGfxScan` do anyway?
"Why would we ever set PrimaryDisplay = Display_Auto together with SkipExtGfxScan?" -
Sorry. I don`t pay attention to the value "1" for this parameter on intel boards. Matt DeVillier was right.
According to my experiments, SkipExtGfxScan option has higher priority than PrimaryDisplay. Therefore, if PrimaryDisplay = Display_Auto and SkipExtGfxScan = 1, then this is equivalent to the condition PrimaryDisplay = Display_iGFX. This is not a disaster, but it looks bad. I will remove PrimaryDisplay = Display_Auto from intel rvp boards (then the default condition will be used: PrimaryDisplay = Display_iGFX). Thanks you.