<p>Nico Huber has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/22864">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gma: Reverse meaning of GTT_Rotation_Offset<br><br>We used to add the second mapping for a 90 degree rotated frame-<br>buffer at `FB.Offset + GTT_Rotation_Offset` and patched the offset<br>when configuring the pipe. Instead, we expect the final offset in<br>`FB.Offset` (which should include `GTT_Rotation_Offset` when using<br>the default framebuffer setup.<br><br>Change-Id: I321a3db3af4b21eefbfc1bf0c2a7005feaf83c7a<br>Signed-off-by: Nico Huber <nico.h@gmx.de><br>---<br>M common/hw-gfx-gma-pipe_setup.adb<br>M common/hw-gfx-gma.adb<br>M common/hw-gfx-gma.ads<br>M gfxtest/hw-gfx-gma-gfx_test.adb<br>4 files changed, 33 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/22864/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/common/hw-gfx-gma-pipe_setup.adb b/common/hw-gfx-gma-pipe_setup.adb<br>index e1ff835..4dc6e2b 100644<br>--- a/common/hw-gfx-gma-pipe_setup.adb<br>+++ b/common/hw-gfx-gma-pipe_setup.adb<br>@@ -167,19 +167,16 @@<br> <br> if Config.Has_Plane_Control then<br> declare<br>- Stride, Offset, GTT_Addr : Word32;<br>+ Stride, Offset : Word32;<br> Width : constant Pos16 := Rotated_Width (FB);<br> Height : constant Pos16 := Rotated_Height (FB);<br> begin<br> if Rotation_90 (FB) then<br> Stride := Word32 (FB_Pitch (FB.V_Stride, FB));<br> Offset := Word32 (FB.V_Stride - FB.Height);<br>- GTT_Addr :=<br>- FB.Offset + Word32 (GTT_Rotation_Offset) * GTT_Page_Size;<br> else<br> Stride := Word32 (FB_Pitch (FB.Stride, FB));<br> Offset := 0;<br>- GTT_Addr := FB.Offset;<br> end if;<br> Registers.Write<br> (Register => Controller.PLANE_CTL,<br>@@ -192,7 +189,7 @@<br> Registers.Write (Controller.PLANE_SIZE, Encode (Width, Height));<br> Registers.Write (Controller.PLANE_STRIDE, Stride);<br> Registers.Write (Controller.PLANE_POS, 16#0000_0000#);<br>- Registers.Write (Controller.PLANE_SURF, GTT_Addr and 16#ffff_f000#);<br>+ Registers.Write (Controller.PLANE_SURF, FB.Offset and 16#ffff_f000#);<br> end;<br> else<br> if Config.Disable_Trickle_Feed then<br>diff --git a/common/hw-gfx-gma.adb b/common/hw-gfx-gma.adb<br>index d8438ab..b325000 100644<br>--- a/common/hw-gfx-gma.adb<br>+++ b/common/hw-gfx-gma.adb<br>@@ -471,22 +471,29 @@<br> ----------------------------------------------------------------------------<br> <br> function FB_First_Page (FB : Framebuffer_Type) return Natural is<br>- (Natural (FB.Offset / GTT_Page_Size));<br>+ (Natural (Phys_Offset (FB) / GTT_Page_Size));<br> function FB_Pages (FB : Framebuffer_Type) return Natural is<br> (Natural (Div_Round_Up (FB_Size (FB), GTT_Page_Size)));<br> function FB_Last_Page (FB : Framebuffer_Type) return Natural is<br> (FB_First_Page (FB) + FB_Pages (FB) - 1);<br> <br>- -- Check basics and that it fits in GTT<br>+ -- Check basics and that it fits in GTT. For 90 degree rotations,<br>+ -- the Offset should be above GTT_Rotation_Offset. The latter will<br>+ -- be subtracted for the aperture mapping.<br> function Valid_FB (FB : Framebuffer_Type) return Boolean is<br>- (Valid_Stride (FB) and FB_Last_Page (FB) < GTT_Rotation_Offset);<br>+ (Valid_Stride (FB) and<br>+ FB_First_Page (FB) in GTT_Range and<br>+ FB_Last_Page (FB) in GTT_Range and<br>+ (not Rotation_90 (FB) or<br>+ (FB_Last_Page (FB) + GTT_Rotation_Offset in GTT_Range and<br>+ FB.Offset >= Word32 (GTT_Rotation_Offset) * GTT_Page_Size)));<br> <br> -- Also check that we don't overflow the GTT's 39-bit space<br> -- (always true with a 32-bit base)<br> function Valid_Phys_FB (FB : Framebuffer_Type; Phys_Base : Word32)<br> return Boolean is<br> (Valid_FB (FB) and<br>- Int64 (Phys_Base) + Int64 (FB.Offset) + Int64 (FB_Size (FB)) <=<br>+ Int64 (Phys_Base) + Int64 (Phys_Offset (FB)) + Int64 (FB_Size (FB)) <=<br> Int64 (GTT_Address_Type'Last))<br> with<br> Ghost;<br>@@ -505,7 +512,7 @@<br> Pre => Is_Initialized and Valid_Phys_FB (FB, Phys_Base)<br> is<br> Phys_Addr : GTT_Address_Type :=<br>- GTT_Address_Type (Phys_Base) + GTT_Address_Type (FB.Offset);<br>+ GTT_Address_Type (Phys_Base) + GTT_Address_Type (Phys_Offset (FB));<br> begin<br> for Idx in FB_First_Page (FB) .. FB_Last_Page (FB) loop<br> Registers.Write_GTT<br>@@ -522,7 +529,7 @@<br> GTT_Address_Type (Pixel_To_Bytes (32 * FB.Stride, FB));<br> begin<br> Phys_Addr := GTT_Address_Type (Phys_Base) +<br>- GTT_Address_Type (FB.Offset) +<br>+ GTT_Address_Type (Phys_Offset (FB)) +<br> GTT_Address_Type (FB_Size (FB));<br> for Page in FB_First_Page (FB) .. FB_Last_Page (FB) loop<br> Phys_Addr := Phys_Addr - Bytes_Per_Row;<br>@@ -627,7 +634,7 @@<br> FB_Last_Page (FB) < GTT_Size / Config.GTT_PTE_Size and<br> FB_Last_Page (FB) < Natural (Stolen_Size / GTT_Page_Size) and<br> FB_Last_Page (FB) < Aperture_Size / GTT_Page_Size;<br>- pragma Debug (not Valid, Debug.Put<br>+ pragma Debug (not Valid, Debug.Put_Line<br> ("Stolen memory too small to hold framebuffer."));<br> end if;<br> end Validate_FB;<br>@@ -699,7 +706,7 @@<br> if Linear_FB_Base /= 0 then<br> Validate_FB (FB, Valid);<br> if Valid then<br>- Linear_FB := Linear_FB_Base + Word64 (FB.Offset);<br>+ Linear_FB := Linear_FB_Base + Word64 (Phys_Offset (FB));<br> end if;<br> end if;<br> end Map_Linear_FB;<br>diff --git a/common/hw-gfx-gma.ads b/common/hw-gfx-gma.ads<br>index 50a76a0..c59b392 100644<br>--- a/common/hw-gfx-gma.ads<br>+++ b/common/hw-gfx-gma.ads<br>@@ -102,6 +102,15 @@<br> Device_Address : GTT_Address_Type;<br> Valid : Boolean);<br> <br>+ -- For the default framebuffer setup (see below) with 90 degree rotations,<br>+ -- we expect the offset which is used for the final scanout to be above<br>+ -- `GTT_Rotation_Offset`. So we can use `Offset - GTT_Rotation_Offset` for<br>+ -- the physical memory location and aperture mapping.<br>+ function Phys_Offset (FB : Framebuffer_Type) return Word32 is<br>+ (if Rotation_90 (FB)<br>+ then FB.Offset - Word32 (GTT_Rotation_Offset) * GTT_Page_Size<br>+ else FB.Offset);<br>+<br> procedure Setup_Default_FB<br> (FB : in Framebuffer_Type;<br> Clear : in Boolean := True;<br>diff --git a/gfxtest/hw-gfx-gma-gfx_test.adb b/gfxtest/hw-gfx-gma-gfx_test.adb<br>index 514e60f..cb09c63 100644<br>--- a/gfxtest/hw-gfx-gma-gfx_test.adb<br>+++ b/gfxtest/hw-gfx-gma-gfx_test.adb<br>@@ -78,13 +78,16 @@<br> 0 .. 3 * (Max_W * Max_H + FB_Align / 4) - 1;<br> type Screen_Type is array (Screen_Index) of Word32;<br> <br>+ function Screen_Offset (FB : Framebuffer_Type) return Natural is<br>+ (Natural (Phys_Offset (FB) / 4));<br>+<br> package Screen is new MMIO_Range (0, Word32, Screen_Index, Screen_Type);<br> <br> Screen_Backup : Screen_Type;<br> <br> procedure Backup_Screen (FB : Framebuffer_Type)<br> is<br>- First : constant Screen_Index := Natural (FB.Offset) / 4;<br>+ First : constant Screen_Index := Screen_Offset (FB);<br> Last : constant Screen_Index := First + Natural (FB_Size (FB)) / 4 - 1;<br> begin<br> for Idx in Screen_Index range First .. Last loop<br>@@ -94,7 +97,7 @@<br> <br> procedure Restore_Screen (FB : Framebuffer_Type)<br> is<br>- First : constant Screen_Index := Natural (FB.Offset) / 4;<br>+ First : constant Screen_Index := Screen_Offset (FB);<br> Last : constant Screen_Index := First + Natural (FB_Size (FB)) / 4 - 1;<br> begin<br> for Idx in Screen_Index range First .. Last loop<br>@@ -158,7 +161,7 @@<br> is<br> P : Pixel_Type;<br> -- We have pixel offset wheras the framebuffer has a byte offset<br>- Offset_Y : Natural := Natural (Framebuffer.Offset / 4);<br>+ Offset_Y : Natural := Screen_Offset (Framebuffer);<br> Offset : Natural;<br> <br> function Top_Test (X, Y : Natural) return Boolean<br>@@ -214,7 +217,7 @@<br> V_Stride => Div_Round_Up (Pos32 (Mode.H_Visible), 32) * 32,<br> Tiling => Y_Tiled,<br> Rotation => Rotation,<br>- Offset => Offset);<br>+ Offset => Offset + Word32 (GTT_Rotation_Offset) * GTT_Page_Size);<br> else<br> FB :=<br> (Width => Width_Type (Mode.H_Visible),<br></pre><p>To view, visit <a href="https://review.coreboot.org/22864">change 22864</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/22864"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libgfxinit </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I321a3db3af4b21eefbfc1bf0c2a7005feaf83c7a </div>
<div style="display:none"> Gerrit-Change-Number: 22864 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Nico Huber <nico.h@gmx.de> </div>