Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/44795 )
Change subject: gma baytrail: Add PLL initialisation code ......................................................................
Patch Set 7:
(5 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 […]
Sharing is possible, but tricky to do (IIRC there's special bits in the DPLL registers to do it). I'll drop this and check if the DPLLs are active or not 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?
It can never be zero given the ranges. I'll drop this.
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.
I had this here because there are two sets of panel control registers (and a while ago only one panel was handled in HW.GFX.GMA.Panel).
https://review.coreboot.org/c/libgfxinit/+/44795/3/common/valleyview/hw-gfx-... PS3, Line 548: |
Heh, didn't know this works :)
Once one gets used to writing things in Ada, everything comes out smoothly.
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...
Probably not