[coreboot-gerrit] Change in libgfxinit[master]: gma: Reverse meaning of GTT_Rotation_Offset

Nico Huber (Code Review) gerrit at coreboot.org
Thu Dec 14 11:19:23 CET 2017


Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22864


Change subject: gma: Reverse meaning of GTT_Rotation_Offset
......................................................................

gma: Reverse meaning of GTT_Rotation_Offset

We used to add the second mapping for a 90 degree rotated frame-
buffer at `FB.Offset + GTT_Rotation_Offset` and patched the offset
when configuring the pipe. Instead, we expect the final offset in
`FB.Offset` (which should include `GTT_Rotation_Offset` when using
the default framebuffer setup.

Change-Id: I321a3db3af4b21eefbfc1bf0c2a7005feaf83c7a
Signed-off-by: Nico Huber <nico.h at gmx.de>
---
M common/hw-gfx-gma-pipe_setup.adb
M common/hw-gfx-gma.adb
M common/hw-gfx-gma.ads
M gfxtest/hw-gfx-gma-gfx_test.adb
4 files changed, 33 insertions(+), 17 deletions(-)



  git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/22864/1

diff --git a/common/hw-gfx-gma-pipe_setup.adb b/common/hw-gfx-gma-pipe_setup.adb
index e1ff835..4dc6e2b 100644
--- a/common/hw-gfx-gma-pipe_setup.adb
+++ b/common/hw-gfx-gma-pipe_setup.adb
@@ -167,19 +167,16 @@
 
       if Config.Has_Plane_Control then
          declare
-            Stride, Offset, GTT_Addr : Word32;
+            Stride, Offset : Word32;
             Width : constant Pos16 := Rotated_Width (FB);
             Height : constant Pos16 := Rotated_Height (FB);
          begin
             if Rotation_90 (FB) then
                Stride            := Word32 (FB_Pitch (FB.V_Stride, FB));
                Offset            := Word32 (FB.V_Stride - FB.Height);
-               GTT_Addr          :=
-                  FB.Offset + Word32 (GTT_Rotation_Offset) * GTT_Page_Size;
             else
                Stride            := Word32 (FB_Pitch (FB.Stride, FB));
                Offset            := 0;
-               GTT_Addr          := FB.Offset;
             end if;
             Registers.Write
               (Register    => Controller.PLANE_CTL,
@@ -192,7 +189,7 @@
             Registers.Write (Controller.PLANE_SIZE, Encode (Width, Height));
             Registers.Write (Controller.PLANE_STRIDE, Stride);
             Registers.Write (Controller.PLANE_POS, 16#0000_0000#);
-            Registers.Write (Controller.PLANE_SURF, GTT_Addr and 16#ffff_f000#);
+            Registers.Write (Controller.PLANE_SURF, FB.Offset and 16#ffff_f000#);
          end;
       else
          if Config.Disable_Trickle_Feed then
diff --git a/common/hw-gfx-gma.adb b/common/hw-gfx-gma.adb
index d8438ab..b325000 100644
--- a/common/hw-gfx-gma.adb
+++ b/common/hw-gfx-gma.adb
@@ -471,22 +471,29 @@
    ----------------------------------------------------------------------------
 
    function FB_First_Page (FB : Framebuffer_Type) return Natural is
-     (Natural (FB.Offset / GTT_Page_Size));
+     (Natural (Phys_Offset (FB) / GTT_Page_Size));
    function FB_Pages (FB : Framebuffer_Type) return Natural is
      (Natural (Div_Round_Up (FB_Size (FB), GTT_Page_Size)));
    function FB_Last_Page (FB : Framebuffer_Type) return Natural is
      (FB_First_Page (FB) + FB_Pages (FB) - 1);
 
-   -- Check basics and that it fits in GTT
+   -- Check basics and that it fits in GTT. For 90 degree rotations,
+   -- the Offset should be above GTT_Rotation_Offset. The latter will
+   -- be subtracted for the aperture mapping.
    function Valid_FB (FB : Framebuffer_Type) return Boolean is
-     (Valid_Stride (FB) and FB_Last_Page (FB) < GTT_Rotation_Offset);
+     (Valid_Stride (FB) and
+      FB_First_Page (FB) in GTT_Range and
+      FB_Last_Page (FB) in GTT_Range and
+      (not Rotation_90 (FB) or
+       (FB_Last_Page (FB) + GTT_Rotation_Offset in GTT_Range and
+        FB.Offset >= Word32 (GTT_Rotation_Offset) * GTT_Page_Size)));
 
    -- Also check that we don't overflow the GTT's 39-bit space
    -- (always true with a 32-bit base)
    function Valid_Phys_FB (FB : Framebuffer_Type; Phys_Base : Word32)
       return Boolean is
      (Valid_FB (FB) and
-      Int64 (Phys_Base) + Int64 (FB.Offset) + Int64 (FB_Size (FB)) <=
+      Int64 (Phys_Base) + Int64 (Phys_Offset (FB)) + Int64 (FB_Size (FB)) <=
          Int64 (GTT_Address_Type'Last))
    with
       Ghost;
@@ -505,7 +512,7 @@
       Pre => Is_Initialized and Valid_Phys_FB (FB, Phys_Base)
    is
       Phys_Addr : GTT_Address_Type :=
-         GTT_Address_Type (Phys_Base) + GTT_Address_Type (FB.Offset);
+         GTT_Address_Type (Phys_Base) + GTT_Address_Type (Phys_Offset (FB));
    begin
       for Idx in FB_First_Page (FB) .. FB_Last_Page (FB) loop
          Registers.Write_GTT
@@ -522,7 +529,7 @@
                GTT_Address_Type (Pixel_To_Bytes (32 * FB.Stride, FB));
          begin
             Phys_Addr := GTT_Address_Type (Phys_Base) +
-                           GTT_Address_Type (FB.Offset) +
+                           GTT_Address_Type (Phys_Offset (FB)) +
                            GTT_Address_Type (FB_Size (FB));
             for Page in FB_First_Page (FB) .. FB_Last_Page (FB) loop
                Phys_Addr := Phys_Addr - Bytes_Per_Row;
@@ -627,7 +634,7 @@
             FB_Last_Page (FB) < GTT_Size / Config.GTT_PTE_Size and
             FB_Last_Page (FB) < Natural (Stolen_Size / GTT_Page_Size) and
             FB_Last_Page (FB) < Aperture_Size / GTT_Page_Size;
-         pragma Debug (not Valid, Debug.Put
+         pragma Debug (not Valid, Debug.Put_Line
            ("Stolen memory too small to hold framebuffer."));
       end if;
    end Validate_FB;
@@ -699,7 +706,7 @@
       if Linear_FB_Base /= 0 then
          Validate_FB (FB, Valid);
          if Valid then
-            Linear_FB := Linear_FB_Base + Word64 (FB.Offset);
+            Linear_FB := Linear_FB_Base + Word64 (Phys_Offset (FB));
          end if;
       end if;
    end Map_Linear_FB;
diff --git a/common/hw-gfx-gma.ads b/common/hw-gfx-gma.ads
index 50a76a0..c59b392 100644
--- a/common/hw-gfx-gma.ads
+++ b/common/hw-gfx-gma.ads
@@ -102,6 +102,15 @@
       Device_Address : GTT_Address_Type;
       Valid          : Boolean);
 
+   -- For the default framebuffer setup (see below) with 90 degree rotations,
+   -- we expect the offset which is used for the final scanout to be above
+   -- `GTT_Rotation_Offset`. So we can use `Offset - GTT_Rotation_Offset` for
+   -- the physical memory location and aperture mapping.
+   function Phys_Offset (FB : Framebuffer_Type) return Word32 is
+     (if Rotation_90 (FB)
+      then FB.Offset - Word32 (GTT_Rotation_Offset) * GTT_Page_Size
+      else FB.Offset);
+
    procedure Setup_Default_FB
      (FB       : in     Framebuffer_Type;
       Clear    : in     Boolean := True;
diff --git a/gfxtest/hw-gfx-gma-gfx_test.adb b/gfxtest/hw-gfx-gma-gfx_test.adb
index 514e60f..cb09c63 100644
--- a/gfxtest/hw-gfx-gma-gfx_test.adb
+++ b/gfxtest/hw-gfx-gma-gfx_test.adb
@@ -78,13 +78,16 @@
       0 .. 3 * (Max_W * Max_H + FB_Align / 4) - 1;
    type Screen_Type is array (Screen_Index) of Word32;
 
+   function Screen_Offset (FB : Framebuffer_Type) return Natural is
+     (Natural (Phys_Offset (FB) / 4));
+
    package Screen is new MMIO_Range (0, Word32, Screen_Index, Screen_Type);
 
    Screen_Backup : Screen_Type;
 
    procedure Backup_Screen (FB : Framebuffer_Type)
    is
-      First : constant Screen_Index := Natural (FB.Offset) / 4;
+      First : constant Screen_Index := Screen_Offset (FB);
       Last  : constant Screen_Index := First + Natural (FB_Size (FB)) / 4 - 1;
    begin
       for Idx in Screen_Index range First .. Last loop
@@ -94,7 +97,7 @@
 
    procedure Restore_Screen (FB : Framebuffer_Type)
    is
-      First : constant Screen_Index := Natural (FB.Offset) / 4;
+      First : constant Screen_Index := Screen_Offset (FB);
       Last  : constant Screen_Index := First + Natural (FB_Size (FB)) / 4 - 1;
    begin
       for Idx in Screen_Index range First .. Last loop
@@ -158,7 +161,7 @@
    is
       P        : Pixel_Type;
       -- We have pixel offset wheras the framebuffer has a byte offset
-      Offset_Y : Natural := Natural (Framebuffer.Offset / 4);
+      Offset_Y : Natural := Screen_Offset (Framebuffer);
       Offset   : Natural;
 
       function Top_Test (X, Y : Natural) return Boolean
@@ -214,7 +217,7 @@
             V_Stride => Div_Round_Up (Pos32 (Mode.H_Visible), 32) * 32,
             Tiling   => Y_Tiled,
             Rotation => Rotation,
-            Offset   => Offset);
+            Offset   => Offset + Word32 (GTT_Rotation_Offset) * GTT_Page_Size);
       else
          FB :=
            (Width    => Width_Type (Mode.H_Visible),

-- 
To view, visit https://review.coreboot.org/22864
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I321a3db3af4b21eefbfc1bf0c2a7005feaf83c7a
Gerrit-Change-Number: 22864
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h at gmx.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171214/f1df293f/attachment-0001.html>


More information about the coreboot-gerrit mailing list