Nico Huber submitted this change.

View Change

Approvals: Nico Huber: Verified Angel Pons: Looks good to me, approved
gma pcode: Move and revise mailbox handling

Unify the PCODE mailbox implementations (Linux' i915 uses the same
implementation since Sandy Bridge, too) and add `Wait` and `Success`
parameters so we can act correctly if something failed. This adds
a lot of boilerplate. But we keep it contained in the new package
`PCode` and the code outside it looks cleaner and handles errors
more gracefully.

In GNATprove, we track state of the mailbox' readiness in a ghost
variable `Mailbox_Ready`. This allows us to skip the initial wait
loop if we know that we already waited at the end of a previous
call. The first call to a mailbox procedure has to be made with
`Wait_Ready => True`.

Also, start to experiment with a `use` clause for the `Register`
package. It allows us to write a little more condensed code, with-
out sacrificing much (in this program we can expect that `Read`/
`Write` means register access?) So far it looks good?

Change-Id: I5daa3effb7ab774e4a35bd8794b0f67f57e4caa4
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/35710
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
---
M common/Makefile.inc
M common/broxton/hw-gfx-gma-power_and_clocks.adb
M common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.adb
M common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.ads
A common/hw-gfx-gma-pcode.adb
A common/hw-gfx-gma-pcode.ads
M common/hw-gfx-gma.adb
M common/skylake/hw-gfx-gma-power_and_clocks_skylake.adb
8 files changed, 234 insertions(+), 63 deletions(-)

diff --git a/common/Makefile.inc b/common/Makefile.inc
index 7c5403b..67b6c17 100644
--- a/common/Makefile.inc
+++ b/common/Makefile.inc
@@ -31,6 +31,8 @@
gfxinit-y += hw-gfx-gma-pch-vga.adb
gfxinit-y += hw-gfx-gma-pch-vga.ads
gfxinit-y += hw-gfx-gma-pch.ads
+gfxinit-y += hw-gfx-gma-pcode.adb
+gfxinit-y += hw-gfx-gma-pcode.ads
gfxinit-y += hw-gfx-gma-pipe_setup.adb
gfxinit-y += hw-gfx-gma-pipe_setup.ads
gfxinit-y += hw-gfx-gma-port_detect.ads
diff --git a/common/broxton/hw-gfx-gma-power_and_clocks.adb b/common/broxton/hw-gfx-gma-power_and_clocks.adb
index f6576d9..77f6b35 100644
--- a/common/broxton/hw-gfx-gma-power_and_clocks.adb
+++ b/common/broxton/hw-gfx-gma-power_and_clocks.adb
@@ -17,6 +17,7 @@
with HW.Debug;
with HW.GFX.GMA.Config;
with HW.GFX.GMA.Registers;
+with HW.GFX.GMA.PCode;
with HW.GFX.GMA.Power_And_Clocks_Haswell;
with HW.GFX.GMA.DDI_Phy;

@@ -242,10 +243,19 @@
when others => CDCLK_CD2X_DIV_SEL_1); -- for CDClk = CDClk_Ref
CDCLK_CD2X_SSA_Precharge : constant Word32 :=
(if Freq >= 500_000_000 then CDCLK_CD2X_SSA_PRECHARGE_ENABLE else 0);
+
+ Success : Boolean;
begin
- Power_And_Clocks_Haswell.GT_Mailbox_Write
+ PCode.Mailbox_Write
(MBox => BXT_PCODE_CDCLK_CONTROL,
- Value => BXT_CDCLK_PREPARE_FOR_CHANGE);
+ Command => BXT_CDCLK_PREPARE_FOR_CHANGE,
+ Wait_Ready => True,
+ Success => Success);
+ if not Success then
+ pragma Debug (Debug.Put_Line
+ ("ERROR: PCODE didn't acknowledge frequency change."));
+ return;
+ end if;

Write
(Register => BXT_DE_PLL_ENABLE,
@@ -274,9 +284,9 @@
CDCLK_CD2X_SSA_Precharge or
CDCLK_CTL_CD_FREQ_DECIMAL (Freq));

- Power_And_Clocks_Haswell.GT_Mailbox_Write
- (MBox => BXT_PCODE_CDCLK_CONTROL,
- Value => Word32 ((Freq + (25_000_000 - 1)) / 25_000_000));
+ PCode.Mailbox_Write
+ (MBox => BXT_PCODE_CDCLK_CONTROL,
+ Command => Word64 ((Freq + (25_000_000 - 1)) / 25_000_000));
end Set_CDClk;

----------------------------------------------------------------------------
diff --git a/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.adb b/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.adb
index 1c23254..332c3df 100644
--- a/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.adb
+++ b/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.adb
@@ -17,6 +17,7 @@
with HW.Time;
with HW.Debug;
with HW.GFX.GMA.Config;
+with HW.GFX.GMA.PCode;
with HW.GFX.GMA.Registers;

package body HW.GFX.GMA.Power_And_Clocks_Haswell is
@@ -55,8 +56,6 @@
IPS_CTL_ENABLE : constant := 1 * 2 ** 31;
DISPLAY_IPS_CONTROL : constant := 16#19#;

- GT_MAILBOX_READY : constant := 1 * 2 ** 31;
-
----------------------------------------------------------------------------

procedure PSR_Off
@@ -88,18 +87,6 @@

----------------------------------------------------------------------------

- procedure GT_Mailbox_Write (MBox : Word32; Value : Word32) is
- begin
- pragma Debug (Debug.Put_Line (GNAT.Source_Info.Enclosing_Entity));
-
- Registers.Wait_Unset_Mask (Registers.GT_MAILBOX, GT_MAILBOX_READY);
- Registers.Write (Registers.GT_MAILBOX_DATA, Value);
- Registers.Write (Registers.GT_MAILBOX, GT_MAILBOX_READY or MBox);
-
- Registers.Wait_Unset_Mask (Registers.GT_MAILBOX, GT_MAILBOX_READY);
- Registers.Write (Registers.GT_MAILBOX_DATA, 0);
- end GT_Mailbox_Write;
-
procedure IPS_Off
is
Enabled : Boolean;
@@ -110,7 +97,7 @@
Registers.Is_Set_Mask (Registers.IPS_CTL, IPS_CTL_ENABLE, Enabled);
if Enabled then
if Config.Has_IPS_CTL_Mailbox then
- GT_Mailbox_Write (DISPLAY_IPS_CONTROL, 0);
+ PCode.Mailbox_Write (DISPLAY_IPS_CONTROL, 0, Wait_Ready => True);
Registers.Wait_Unset_Mask
(Register => Registers.IPS_CTL,
Mask => IPS_CTL_ENABLE,
diff --git a/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.ads b/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.ads
index 87f5d07..7c5a647 100644
--- a/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.ads
+++ b/common/haswell_shared/hw-gfx-gma-power_and_clocks_haswell.ads
@@ -15,7 +15,6 @@
private package HW.GFX.GMA.Power_And_Clocks_Haswell is

procedure PSR_Off;
- procedure GT_Mailbox_Write (MBox : Word32; Value : Word32);

procedure Pre_All_Off;
procedure Post_All_Off is null;
diff --git a/common/hw-gfx-gma-pcode.adb b/common/hw-gfx-gma-pcode.adb
new file mode 100644
index 0000000..dd0fc6e
--- /dev/null
+++ b/common/hw-gfx-gma-pcode.adb
@@ -0,0 +1,138 @@
+--
+-- Copyright (C) 2014-2016, 2019 secunet Security Networks AG
+--
+-- This program is free software; you can redistribute it and/or modify
+-- it under the terms of the GNU General Public License as published by
+-- the Free Software Foundation; either version 2 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+-- GNU General Public License for more details.
+--
+
+with GNAT.Source_Info;
+
+with HW.Debug;
+with HW.GFX.GMA.Registers;
+
+use HW.GFX.GMA.Registers;
+
+package body HW.GFX.GMA.PCode is
+
+ GT_MAILBOX_READY : constant := 1 * 2 ** 31;
+
+ -- Send a command and optionally wait for and return the reply.
+ procedure Mailbox_Write_Read
+ (MBox : in Word32;
+ Command : in Word64;
+ Reply : out Word64;
+ Wait_Ready : in Boolean := False;
+ Wait_Ack : in Boolean := True;
+ Success : out Boolean)
+ with
+ Pre => Mailbox_Ready or Wait_Ready,
+ Post => (if Wait_Ack and Success then Mailbox_Ready)
+ is
+ use type HW.Word64;
+
+ Data : Word32;
+ begin
+ pragma Debug (Debug.Put_Line (GNAT.Source_Info.Enclosing_Entity));
+
+ Reply := 0;
+ Success := True;
+
+ if Wait_Ready then
+ Wait_Unset_Mask (GT_MAILBOX, GT_MAILBOX_READY, Success => Success);
+ if not Success then
+ return;
+ end if;
+ end if;
+
+ Write (GT_MAILBOX_DATA, Word32 (Command and 16#ffff_ffff#));
+ Write (GT_MAILBOX_DATA_1, Word32 (Shift_Right (Command, 32)));
+ Write (GT_MAILBOX, GT_MAILBOX_READY or MBox);
+ Mailbox_Ready := False;
+
+ if Wait_Ack then
+ Wait_Unset_Mask (GT_MAILBOX, GT_MAILBOX_READY, Success => Success);
+ Mailbox_Ready := Success;
+
+ Read (GT_MAILBOX_DATA, Data);
+ Reply := Word64 (Data);
+ Read (GT_MAILBOX_DATA_1, Data);
+ Reply := Shift_Left (Word64 (Data), 32) or Reply;
+
+ Write (GT_MAILBOX_DATA, 0);
+ Write (GT_MAILBOX_DATA_1, 0);
+ end if;
+ end Mailbox_Write_Read;
+
+ procedure Mailbox_Write
+ (MBox : in Word32;
+ Command : in Word64;
+ Wait_Ready : in Boolean := False;
+ Wait_Ack : in Boolean := True;
+ Success : out Boolean)
+ is
+ pragma Warnings (GNATprove, Off, "unused assignment to ""Ignored_R""");
+ Ignored_R : Word64;
+ begin
+ Mailbox_Write_Read
+ (MBox, Command, Ignored_R, Wait_Ready, Wait_Ack, Success);
+ end Mailbox_Write;
+
+ procedure Mailbox_Request
+ (MBox : in Word32;
+ Command : in Word64;
+ Reply_Mask : in Word64;
+ Reply : in Word64 := 16#ffff_ffff_ffff_ffff#;
+ TOut_MS : in Natural := Registers.Default_Timeout_MS;
+ Wait_Ready : in Boolean := False;
+ Success : out Boolean)
+ is
+ use type HW.Word64;
+
+ Timeout : constant Time.T := Time.MS_From_Now (TOut_MS);
+ Timed_Out : Boolean := False;
+
+ Received_Reply : Word64;
+ begin
+ Success := False;
+ loop
+ pragma Loop_Invariant ((not Success and Wait_Ready) or Mailbox_Ready);
+ Mailbox_Write_Read
+ (MBox => MBox,
+ Command => Command,
+ Reply => Received_Reply,
+ Wait_Ready => not Success and Wait_Ready,
+ Success => Success);
+ exit when not Success;
+
+ if (Received_Reply and Reply_Mask) = (Reply and Reply_Mask) then
+ -- Ignore timeout if we succeeded anyway.
+ Timed_Out := False;
+ exit;
+ end if;
+ exit when Timed_Out;
+
+ Timed_Out := Time.Timed_Out (Timeout);
+ end loop;
+
+ Success := Success and then not Timed_Out;
+ end Mailbox_Request;
+
+ procedure Mailbox_Write
+ (MBox : Word32;
+ Command : Word64;
+ Wait_Ready : Boolean := False)
+ is
+ pragma Warnings (GNATprove, Off, "unused assignment to ""Ignored_S""");
+ Ignored_S : Boolean;
+ begin
+ Mailbox_Write (MBox, Command, Wait_Ready, False, Ignored_S);
+ end Mailbox_Write;
+
+end HW.GFX.GMA.PCode;
diff --git a/common/hw-gfx-gma-pcode.ads b/common/hw-gfx-gma-pcode.ads
new file mode 100644
index 0000000..ef2564a
--- /dev/null
+++ b/common/hw-gfx-gma-pcode.ads
@@ -0,0 +1,60 @@
+--
+-- Copyright (C) 2016, 2019 secunet Security Networks AG
+--
+-- This program is free software; you can redistribute it and/or modify
+-- it under the terms of the GNU General Public License as published by
+-- the Free Software Foundation; either version 2 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+-- GNU General Public License for more details.
+--
+
+with HW.GFX.GMA.Registers;
+
+private package HW.GFX.GMA.PCode is
+
+ -- We have to ensure that previous usage of the mailbox finished
+ -- (Wait_Ready) or know that we already did so (Mailbox_Ready).
+ --
+ -- If we wait for the other side to acknowledge (Wait_Ack), we
+ -- know that it's ready (=> Mailbox_Ready).
+
+ -- XXX: Supposed to be a `Ghost` variable, but GNAT seems too broken?
+ Mailbox_Ready : Boolean with Part_Of => HW.GFX.GMA.State;
+
+ -- Just send a command, discard the reply.
+ procedure Mailbox_Write
+ (MBox : in Word32;
+ Command : in Word64;
+ Wait_Ready : in Boolean := False;
+ Wait_Ack : in Boolean := True;
+ Success : out Boolean)
+ with
+ Pre => Mailbox_Ready or Wait_Ready,
+ Post => (if Wait_Ack and Success then Mailbox_Ready);
+
+ -- Repeatedly send a request command the expected reply is received.
+ procedure Mailbox_Request
+ (MBox : in Word32;
+ Command : in Word64;
+ Reply_Mask : in Word64;
+ Reply : in Word64 := 16#ffff_ffff_ffff_ffff#;
+ TOut_MS : in Natural := Registers.Default_Timeout_MS;
+ Wait_Ready : in Boolean := False;
+ Success : out Boolean)
+ with
+ Pre => Mailbox_Ready or Wait_Ready,
+ Post => (if Success then Mailbox_Ready);
+
+ -- For final mailbox commands that don't have to wait.
+ procedure Mailbox_Write
+ (MBox : Word32;
+ Command : Word64;
+ Wait_Ready : Boolean := False)
+ with
+ Pre => Mailbox_Ready or Wait_Ready;
+
+end HW.GFX.GMA.PCode;
diff --git a/common/hw-gfx-gma.adb b/common/hw-gfx-gma.adb
index 15d234a..dbdee5c 100644
--- a/common/hw-gfx-gma.adb
+++ b/common/hw-gfx-gma.adb
@@ -21,6 +21,7 @@
with HW.GFX.GMA.Config;
with HW.GFX.GMA.Config_Helpers;
with HW.GFX.GMA.Registers;
+with HW.GFX.GMA.PCode;
with HW.GFX.GMA.Power_And_Clocks;
with HW.GFX.GMA.Panel;
with HW.GFX.GMA.PLLs;
@@ -39,6 +40,7 @@
(State =>
(Dev.Address_State,
Registers.Address_State,
+ PCode.Mailbox_Ready,
PLLs.State, Panel.Panel_State,
Cur_Configs, Allocated_PLLs,
HPD_Delay, Wait_For_HPD,
@@ -376,6 +378,7 @@
(Config.Variable,
Dev.Address_State,
Registers.Address_State,
+ PCode.Mailbox_Ready,
PLLs.State, Panel.Panel_State,
Cur_Configs, Allocated_PLLs,
HPD_Delay, Wait_For_HPD,
@@ -435,6 +438,7 @@
pragma Debug (Debug.Set_Register_Write_Delay (Write_Delay));

Linear_FB_Base := 0;
+ PCode.Mailbox_Ready := False;
Wait_For_HPD := HPD_Type'(others => False);
HPD_Delay := HPD_Delay_Type'(others => Now);
Allocated_PLLs := (others => PLLs.Invalid);
diff --git a/common/skylake/hw-gfx-gma-power_and_clocks_skylake.adb b/common/skylake/hw-gfx-gma-power_and_clocks_skylake.adb
index c6eb2cf..2ab7f21 100644
--- a/common/skylake/hw-gfx-gma-power_and_clocks_skylake.adb
+++ b/common/skylake/hw-gfx-gma-power_and_clocks_skylake.adb
@@ -18,6 +18,7 @@
with HW.Debug;
with HW.GFX.GMA.Config;
with HW.GFX.GMA.Registers;
+with HW.GFX.GMA.PCode;
with HW.GFX.GMA.Power_And_Clocks_Haswell;

use type HW.Word64;
@@ -86,8 +87,6 @@
SKL_CDCLK_PREPARE_FOR_CHANGE : constant := 3;
SKL_CDCLK_READY_FOR_CHANGE : constant := 1;

- GT_MAILBOX_READY : constant := 1 * 2 ** 31;
-
function CDCLK_CTL_CD_FREQ_DECIMAL
(Freq : Pos16;
Plus_Half : Boolean)
@@ -98,22 +97,6 @@

----------------------------------------------------------------------------

- procedure GT_Mailbox_Write (MBox : Word32; Value : Word64) is
- begin
- pragma Debug (Debug.Put_Line (GNAT.Source_Info.Enclosing_Entity));
-
- Registers.Wait_Unset_Mask (Registers.GT_MAILBOX, GT_MAILBOX_READY);
- Registers.Write
- (Registers.GT_MAILBOX_DATA, Word32 (Value and 16#ffff_ffff#));
- Registers.Write
- (Registers.GT_MAILBOX_DATA_1, Word32 (Shift_Right (Value, 32)));
- Registers.Write (Registers.GT_MAILBOX, GT_MAILBOX_READY or MBox);
-
- Registers.Wait_Unset_Mask (Registers.GT_MAILBOX, GT_MAILBOX_READY);
- end GT_Mailbox_Write;
-
- ----------------------------------------------------------------------------
-
procedure PD_Off (PD : Power_Domain)
is
Ctl1, Ctl2, Ctl3, Ctl4 : Word32;
@@ -251,10 +234,7 @@

procedure Initialize
is
- CDClk_Change_Timeout : Time.T;
- Timed_Out : Boolean;
-
- MBox_Data0 : Word32;
+ Success : Boolean;
begin
Registers.Set_Mask
(Register => Registers.NDE_RSTWRN_OPT,
@@ -282,29 +262,20 @@
(Register => Registers.LCPLL1_CTL,
Mask => LCPLL1_CTL_PLL_LOCK);

- CDClk_Change_Timeout := Time.MS_From_Now (3);
- Timed_Out := False;
- loop
- GT_Mailbox_Write
- (MBox => SKL_PCODE_CDCLK_CONTROL,
- Value => SKL_CDCLK_PREPARE_FOR_CHANGE);
- Registers.Read (Registers.GT_MAILBOX_DATA, MBox_Data0);
- if (MBox_Data0 and SKL_CDCLK_READY_FOR_CHANGE) /= 0 then
- -- Ignore timeout if we succeeded anyway.
- Timed_Out := False;
- exit;
- end if;
- exit when Timed_Out;
+ PCode.Mailbox_Request
+ (MBox => SKL_PCODE_CDCLK_CONTROL,
+ Command => SKL_CDCLK_PREPARE_FOR_CHANGE,
+ Reply_Mask => SKL_CDCLK_READY_FOR_CHANGE,
+ Wait_Ready => True,
+ Success => Success);

- Timed_Out := Time.Timed_Out (CDClk_Change_Timeout);
- end loop;
- pragma Debug (Timed_Out, Debug.Put_Line
- ("ERROR: PCODE not ready for frequency change after 3ms."));
+ pragma Debug (not Success, Debug.Put_Line
+ ("ERROR: PCODE not ready for frequency change."));

- if not Timed_Out then
- GT_Mailbox_Write
+ if Success then
+ PCode.Mailbox_Write
(MBox => SKL_PCODE_CDCLK_CONTROL,
- Value => 16#0000_0000#); -- 0 - 337.5MHz
+ Command => 16#0000_0000#); -- 0 - 337.5MHz
-- 1 - 450.0MHz
-- 2 - 540.0MHz
-- 3 - 675.0MHz

To view, visit change 35710. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: I5daa3effb7ab774e4a35bd8794b0f67f57e4caa4
Gerrit-Change-Number: 35710
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged