Nico Huber has submitted this change. ( https://review.coreboot.org/c/libgfxinit/+/35528 )
Change subject: gma: Refactor Update_Outputs() ......................................................................
gma: Refactor Update_Outputs()
In Update_Outputs(), we used to have two steps, iterating over all available pipes: We disabled all pipes whose mode changed or that would be disabled otherwise. Then, we enabled all pipes with new modes.
As checks for validity of the configurations were only done on demand in the second step, that left us with no knowledge about what configs would actually be tried. To make this information available for future work, we add a validation step before the existing two. That gives us:
1. Validate all new configs 2. Disable pipes - whose mode changed - that were disconnected - whose config didn't validate 3. Enable/update pipes with new configs
This way, we can make global decisions ahead of 2. and 3. based on the remaining, _valid_ configurations.
We had to turn Pipe_Setup.Scaler_Available() into a Reserve_Scaler() so we can keep track which new configuration gets to use a scaler. G45 is still the only case where we have less scalers than pipes available. To keep that transparent to Update_Outputs() we add the opaque type `Scaler_Reservation`.
The `Loop_Invariant` of 1. allows us to keep the validation in mind, to satisfy pre-conditions of the various steps ahead. To not lose the validation, Fill_Port_Config() needs a post condition and a little restructuring to prove it.
Change-Id: I079ae8f85c821a272b5d095c1ef437ee804aa9ac Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/35528 Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M common/hw-gfx-gma-config_helpers.adb M common/hw-gfx-gma-config_helpers.ads M common/hw-gfx-gma-pipe_setup.adb M common/hw-gfx-gma-pipe_setup.ads M common/hw-gfx-gma.adb 5 files changed, 110 insertions(+), 75 deletions(-)
Approvals: Nico Huber: Verified Arthur Heymans: Looks good to me, approved
diff --git a/common/hw-gfx-gma-config_helpers.adb b/common/hw-gfx-gma-config_helpers.adb index 162c9c6..ede140c 100644 --- a/common/hw-gfx-gma-config_helpers.adb +++ b/common/hw-gfx-gma-config_helpers.adb @@ -113,13 +113,20 @@ procedure Configure_FDI_Link (Port_Cfg : in out Port_Config; Success : out Boolean) - with Pre => True + with + Post => + Port_Cfg.Mode.H_Visible = Port_Cfg'Old.Mode.H_Visible and + Port_Cfg.Mode.V_Visible = Port_Cfg'Old.Mode.V_Visible is - procedure Limit_Lane_Count - is - FDI_TX_CTL_FDI_TX_ENABLE : constant := 1 * 2 ** 31; - Enabled : Boolean; - begin + FDI_TX_CTL_FDI_TX_ENABLE : constant := 1 * 2 ** 31; + Enabled : Boolean; + begin + Port_Cfg.FDI.Receiver_Caps.Max_Link_Rate := DP_Bandwidth_2_7; + Port_Cfg.FDI.Receiver_Caps.Max_Lane_Count := + Config.FDI_Lane_Count (Port_Cfg.Port); + Port_Cfg.FDI.Receiver_Caps.Enhanced_Framing := True; + + if Config.Has_FDI_C and then Port_Cfg.Port = DIGI_C then -- if DIGI_D enabled: (FDI names are off by one) Registers.Is_Set_Mask (Register => Registers.FDI_TX_CTL_C, @@ -128,15 +135,8 @@ if Enabled then Port_Cfg.FDI.Receiver_Caps.Max_Lane_Count := DP_Lane_Count_2; end if; - end Limit_Lane_Count; - begin - Port_Cfg.FDI.Receiver_Caps.Max_Link_Rate := DP_Bandwidth_2_7; - Port_Cfg.FDI.Receiver_Caps.Max_Lane_Count := - Config.FDI_Lane_Count (Port_Cfg.Port); - Port_Cfg.FDI.Receiver_Caps.Enhanced_Framing := True; - if Config.Has_FDI_C and then Port_Cfg.Port = DIGI_C then - Limit_Lane_Count; end if; + DP_Info.Preferred_Link_Setting (Port_Cfg.FDI, Port_Cfg.Mode, Success); end Configure_FDI_Link;
@@ -216,8 +216,7 @@ function Validate_Config (FB : Framebuffer_Type; Mode : Mode_Type; - Pipe : Pipe_Index; - Scaler_Available : Boolean) + Pipe : Pipe_Index) return Boolean is begin @@ -233,8 +232,7 @@ return ((Rotated_Width (FB) = Mode.H_Visible and Rotated_Height (FB) = Mode.V_Visible) or - (Scaler_Available and - Rotated_Width (FB) <= Config.Maximum_Scalable_Width (Pipe) and + (Rotated_Width (FB) <= Config.Maximum_Scalable_Width (Pipe) and Rotated_Width (FB) <= Mode.H_Visible and Rotated_Height (FB) <= Mode.V_Visible)) and (FB.Offset /= VGA_PLANE_FRAMEBUFFER_OFFSET or Pipe = Primary) and diff --git a/common/hw-gfx-gma-config_helpers.ads b/common/hw-gfx-gma-config_helpers.ads index f02b6e5..9d689c7 100644 --- a/common/hw-gfx-gma-config_helpers.ads +++ b/common/hw-gfx-gma-config_helpers.ads @@ -31,7 +31,12 @@ Pipe : in Pipe_Index; Port : in Port_Type; Mode : in Mode_Type; - Success : out Boolean); + Success : out Boolean) + with + Post => + (if Success then + Port_Cfg.Mode.H_Visible = Mode.H_Visible and + Port_Cfg.Mode.V_Visible = Mode.V_Visible);
----------------------------------------------------------------------------
@@ -54,8 +59,7 @@ function Validate_Config (FB : Framebuffer_Type; Mode : Mode_Type; - Pipe : Pipe_Index; - Scaler_Available : Boolean) + Pipe : Pipe_Index) return Boolean with Post => (if Validate_Config'Result then Valid_FB (FB, Mode)); diff --git a/common/hw-gfx-gma-pipe_setup.adb b/common/hw-gfx-gma-pipe_setup.adb index 000c536..82f5222 100644 --- a/common/hw-gfx-gma-pipe_setup.adb +++ b/common/hw-gfx-gma-pipe_setup.adb @@ -635,12 +635,20 @@ end if; end Setup_Scaling;
- procedure Scaler_Available (Available : out Boolean; Pipe : Pipe_Index) + procedure Reserve_Scaler + (Success : out Boolean; + Reservation : in out Scaler_Reservation; + Pipe : in Pipe_Index) is Pipe_Using_PF : Pipe_Index := Pipe_Index'First; PF_Enabled : Boolean; begin if Config.Has_GMCH_PFIT_CONTROL then + if Reservation.Reserved then + Success := Reservation.Pipe = Pipe; + return; + end if; + Registers.Is_Set_Mask (Register => Registers.GMCH_PFIT_CONTROL, Mask => PF_CTRL_ENABLE, @@ -649,11 +657,15 @@ Gmch_Panel_Fitter_Pipe (Pipe_Using_PF); end if;
- Available := not PF_Enabled or Pipe_Using_PF = Pipe; + Success := not PF_Enabled or Pipe_Using_PF = Pipe; + if Success then + Reservation.Reserved := True; + Reservation.Pipe := Pipe; + end if; else - Available := True; + Success := True; end if; - end Scaler_Available; + end Reserve_Scaler;
----------------------------------------------------------------------------
diff --git a/common/hw-gfx-gma-pipe_setup.ads b/common/hw-gfx-gma-pipe_setup.ads index 3ecb6dd..960e70f 100644 --- a/common/hw-gfx-gma-pipe_setup.ads +++ b/common/hw-gfx-gma-pipe_setup.ads @@ -51,10 +51,22 @@ FB : Framebuffer_Type; Cursor : Cursor_Type);
- procedure Scaler_Available (Available : out Boolean; Pipe : Pipe_Index); + type Scaler_Reservation is private; + Null_Scaler_Reservation : constant Scaler_Reservation; + procedure Reserve_Scaler + (Success : out Boolean; + Reservation : in out Scaler_Reservation; + Pipe : in Pipe_Index);
private
+ type Scaler_Reservation is record + Reserved : Boolean; + Pipe : Pipe_Index; + end record; + Null_Scaler_Reservation : constant Scaler_Reservation := + (Reserved => False, Pipe => Pipe_Index'First); + subtype WM_Levels is Natural range 0 .. 7; type PLANE_WM_Type is array (WM_Levels) of Registers.Registers_Index;
diff --git a/common/hw-gfx-gma.adb b/common/hw-gfx-gma.adb index dbdee5c..269feec 100644 --- a/common/hw-gfx-gma.adb +++ b/common/hw-gfx-gma.adb @@ -1,5 +1,5 @@ -- --- Copyright (C) 2014-2018 secunet Security Networks AG +-- Copyright (C) 2014-2019 secunet Security Networks AG -- Copyright (C) 2017 Nico Huber nico.h@gmx.de -- -- This program is free software; you can redistribute it and/or modify @@ -98,10 +98,11 @@ Pipe_Cfg : in Pipe_Config; Success : out Boolean) with - Pre => Pipe_Cfg.Port in Active_Port_Type + Pre => + Pipe_Cfg.Port in Active_Port_Type and + Config_Helpers.Valid_FB (Pipe_Cfg.Framebuffer, Pipe_Cfg.Mode) is Port_Cfg : Port_Config; - Scaler_Available : Boolean; begin pragma Debug (Debug.New_Line); pragma Debug (Debug.Put_Line @@ -111,12 +112,6 @@ (Port_Cfg, Pipe, Pipe_Cfg.Port, Pipe_Cfg.Mode, Success);
if Success then - Display_Controller.Scaler_Available (Scaler_Available, Pipe); - Success := Config_Helpers.Validate_Config - (Pipe_Cfg.Framebuffer, Port_Cfg.Mode, Pipe, Scaler_Available); - end if; - - if Success then Connector_Info.Preferred_Link_Setting (Port_Cfg, Success); end if;
@@ -223,19 +218,12 @@ end if; end Check_HPD;
- Power_Changed : Boolean := False; - Old_Configs : Pipe_Configs; + Scaler_Reservation : Display_Controller.Scaler_Reservation := + Display_Controller.Null_Scaler_Reservation;
- -- Only called when we actually tried to change something - -- so we don't congest the log with unnecessary messages. - procedure Update_Power - is - begin - if not Power_Changed then - Power_And_Clocks.Power_Up (Old_Configs, Configs); - Power_Changed := True; - end if; - end Update_Power; + Update_Power : Boolean := False; + Old_Configs, + New_Configs : Pipe_Configs;
function Full_Update (Cur_Config, New_Config : Pipe_Config) return Boolean is @@ -255,13 +243,48 @@ end Full_Update; begin Old_Configs := Cur_Configs; + New_Configs := Configs; + + -- validate new configs, filter invalid configs and those waiting for HPD + for Pipe in Pipe_Index loop + declare + Success : Boolean := True; + Cur_Config : Pipe_Config renames Cur_Configs (Pipe); + New_Config : Pipe_Config renames New_Configs (Pipe); + begin + if New_Config.Port /= Disabled then + if Wait_For_HPD (New_Config.Port) then + Check_HPD (New_Config.Port, Success); + Wait_For_HPD (New_Config.Port) := not Success; + end if; + + Success := Success and then + Config_Helpers.Validate_Config + (New_Config.Framebuffer, New_Config.Mode, Pipe); + + if Success and then Requires_Scaling (New_Config) then + Display_Controller.Reserve_Scaler + (Success, Scaler_Reservation, Pipe); + end if; + + if not Success then + New_Config.Port := Disabled; + end if; + end if; + end; + pragma Loop_Invariant + (for all P in Pipe_Index'First .. Pipe => + New_Configs (P).Port = Disabled or + Config_Helpers.Valid_FB + (New_Configs (P).Framebuffer, New_Configs (P).Mode)); + end loop;
-- disable all pipes that changed or had a hot-plug event for Pipe in Pipe_Index loop declare Unplug_Detected : Boolean; Cur_Config : Pipe_Config renames Cur_Configs (Pipe); - New_Config : Pipe_Config renames Configs (Pipe); + New_Config : Pipe_Config renames New_Configs (Pipe); begin if Cur_Config.Port /= Disabled then Check_HPD (Cur_Config.Port, Unplug_Detected); @@ -269,7 +292,7 @@ if Full_Update (Cur_Config, New_Config) or Unplug_Detected then Disable_Output (Pipe, Cur_Config); Cur_Config.Port := Disabled; - Update_Power; + Update_Power := True; end if; end if; end; @@ -279,25 +302,17 @@ for Pipe in Pipe_Index loop declare Success : Boolean; - Scaler_Available : Boolean; Cur_Config : Pipe_Config renames Cur_Configs (Pipe); - New_Config : Pipe_Config renames Configs (Pipe); + New_Config : Pipe_Config renames New_Configs (Pipe); begin + -- full update if New_Config.Port /= Disabled and Full_Update (Cur_Config, New_Config) then - if Wait_For_HPD (New_Config.Port) then - Check_HPD (New_Config.Port, Success); - Wait_For_HPD (New_Config.Port) := not Success; - else - Success := True; - end if; + Power_And_Clocks.Power_Up (Old_Configs, New_Configs); + Update_Power := True;
- if Success then - Update_Power; - Enable_Output (Pipe, New_Config, Success); - end if; - + Enable_Output (Pipe, New_Config, Success); if Success then Cur_Config := New_Config; end if; @@ -306,23 +321,17 @@ elsif New_Config.Port /= Disabled and Cur_Config.Framebuffer /= New_Config.Framebuffer then - Display_Controller.Scaler_Available (Scaler_Available, Pipe); - if Config_Helpers.Validate_Config - (New_Config.Framebuffer, New_Config.Mode, - Pipe, Scaler_Available) - then - Display_Controller.Setup_FB - (Pipe, New_Config.Mode, New_Config.Framebuffer); - Display_Controller.Update_Cursor - (Pipe, New_Config.Framebuffer, New_Config.Cursor); - Cur_Config := New_Config; - end if; + Display_Controller.Setup_FB + (Pipe, New_Config.Mode, New_Config.Framebuffer); + Display_Controller.Update_Cursor + (Pipe, New_Config.Framebuffer, New_Config.Cursor); + Cur_Config := New_Config; end if; end; end loop;
- if Power_Changed then - Power_And_Clocks.Power_Down (Old_Configs, Configs, Cur_Configs); + if Update_Power then + Power_And_Clocks.Power_Down (Old_Configs, New_Configs, Cur_Configs); end if; end Update_Outputs;