Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/41343 )
Change subject: haswell: Make VGA on FDI work ......................................................................
Patch Set 9:
(8 comments)
https://review.coreboot.org/c/libgfxinit/+/41343/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/libgfxinit/+/41343/8//COMMIT_MSG@26 PS8, Line 26: However, this was not : the case, because one of the link ends programmed the link width for DP : unconditionally, so one end of FDI would always end up using x1 width. :
The link is controlled by DDI_BUF_CTL which should have been correct […]
Sounds good.
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... File common/haswell/hw-gfx-gma-pch-lynxpoint.adb:
PS8:
Nit, it doesn't need to be in `haswell/`.
It's haswell-specific, though.
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 2: -- Copyright (C) 2015-2016 secunet Security Networks AG
Why?
Because I copied the file from elsewhere and didn't change the copyright notice. I'm used to getting comments whenever I touch a copyright line that I ignore them.
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 38: 100
PRM says 100us how about: […]
Oh, it's milliseconds. Sounds good.
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 57: Mask_Set => 16#12# * 2 ** 24);
At your convenience, I wonder if this would be easier to read as a table, e.g. […]
Hrm, maybe it's a good idea.
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 158: end Unbend_Clkout_Dp;
As I understand it, this should only be done with the CLKOUT_DP disabled. […]
Will add a disable procedure
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... File common/haswell/hw-gfx-gma-pch-lynxpoint.ads:
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 2: -- Copyright (C) 2015-2016 secunet Security Networks AG
???
Same as body
https://review.coreboot.org/c/libgfxinit/+/41343/8/common/haswell/hw-gfx-gma... PS8, Line 18: procedure Enable_Clkout_Dp;
This is more like `Enable_Clkout_DP_For_FDI_And_Configure_mPHY`. Name […]
The thing is that FDI mPHY config happens in between. I can call it `Enable_Clkout_DP_For_FDI` though