Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/44795 )
Change subject: gma baytrail: Add PLL initialisation code ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... File common/valleyview/hw-gfx-gma-plls.adb:
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 31: type Count_Range is new Natural range 0 .. 2; : : type PLL_State is record : Use_Count : Count_Range; : Used_For_DP : Boolean; : Link_Rate : DP_Bandwidth; : Mode : Mode_Type; : end record; : : type PLL_State_Array is array (DPLLs) of PLL_State; : : PLLs : PLL_State_Array; This should only be necessary if we want to keep track of the current state for potential DPLL sharing. However, each pipe has its own PLL. So I guess we'll never need it?
The situation seems similar to BXT, only that there the PLLs are tied to the ports, not the pipe.
(I know there's a comment below that sharing is possible, but if it's never necessary, we can reduce complexity instead.)
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 129: if P = 0 then Doesn't GNATProve complain here? How can it be 0?
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 336: end Unlock_DPLL_Regs; IIRC, we always have all the registers unlocked (by HW.GFX.GMA.Panel). Please check.
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 363: Nit, drop empty line.
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 548: | Heh, didn't know this works :)
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 584: return (if PLL = DPLL_B then 1 else 0); Is this actually used? I couldn't find a register where it's written to...
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... File common/valleyview/hw-gfx-gma-plls.ads:
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 20: -- XXX: Types should be private (but that triggers a bug in SPARK GPL 2016) Could try if it works now with newer GNATProve.