Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows us to configure a default screen rotation in 90 degree steps. The framebuffer contents will then be displayed by the same amount in the other direction.
The 90 and 270 degree settings are only supported by newer display engines, from Skylake / Apollo Lake on.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 47 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38922/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index a25bb91..6859c24 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -478,6 +478,27 @@ Set the maximum height of the framebuffer. This may help with default fonts too tiny for high-resolution displays.
+choice DEFAULT_SCREEN_ROTATION + prompt "Default screen rotation" + depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT + default DEFAULT_SCREEN_ROTATION_NONE + +config DEFAULT_SCREEN_ROTATION_NONE + bool "None" + +config DEFAULT_SCREEN_ROTATION_90 + bool "90 degrees CCW" + depends on GFX_GMA_GENERATION = "Broxton" || GFX_GMA_GENERATION = "Skylake" + +config DEFAULT_SCREEN_ROTATION_180 + bool "180 degrees" + +config DEFAULT_SCREEN_ROTATION_270 + bool "90 degrees CW" + depends on GFX_GMA_GENERATION = "Broxton" || GFX_GMA_GENERATION = "Skylake" + +endchoice + endmenu # "Display"
config PCI diff --git a/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb b/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb index 1393784..014e92b 100644 --- a/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb +++ b/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb @@ -54,9 +54,20 @@
----------------------------------------------------------------------------
+ procedure Screen_Rotation (rotation : out Rotation_Type) + is + begin + rotation := + (if Config.DEFAULT_SCREEN_ROTATION_90 then Rotated_90 + elsif Config.DEFAULT_SCREEN_ROTATION_180 then Rotated_180 + elsif Config.DEFAULT_SCREEN_ROTATION_270 then Rotated_270 + else No_Rotation); + end Screen_Rotation; + procedure gfxinit (lightup_ok : out Interfaces.C.int) is use type pos32; + use type word32; use type word64;
ports : Port_List; @@ -84,10 +95,21 @@ end loop;
fb := configs (Primary).Framebuffer; - fb.Width := Width_Type (min_h); - fb.Height := Height_Type (min_v); - fb.Stride := Div_Round_Up (fb.Width, 16) * 16; - fb.V_Stride := fb.Height; + Screen_Rotation (fb.Rotation); + + if fb.Rotation = Rotated_90 or fb.Rotation = Rotated_270 then + fb.Width := Width_Type (min_v); + fb.Height := Height_Type (min_h); + fb.Stride := Div_Round_Up (fb.Width, 32) * 32; + fb.V_Stride := Div_Round_Up (fb.Height, 32) * 32; + fb.Tiling := Y_Tiled; + fb.Offset := word32 (GTT_Rotation_Offset) * GTT_Page_Size; + else + fb.Width := Width_Type (min_h); + fb.Height := Height_Type (min_v); + fb.Stride := Div_Round_Up (fb.Width, 16) * 16; + fb.V_Stride := fb.Height; + end if;
for i in Pipe_Index loop exit when configs (i).Port = Disabled;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG@10 PS1, Line 10: The framebuffer contents will then be displayed by the same : amount in the other direction. I'm not sure if I follow
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG@10 PS1, Line 10: The framebuffer contents will then be displayed by the same : amount in the other direction.
I'm not sure if I follow
If you turn your screen to the left, the contents need to be rotated to the right to compensate?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG@14 PS1, Line 14: engines, from Skylake / Apollo Lake on. Needs some more care. I just realized that older hardware supports 180 degree rotation but (beside that it doesn't seem very useful) it's not implemented.
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38922
to look at the new patch set (#2).
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows to configure a default screen rotation in 90-degree steps. The framebuffer contents will then be displayed rotated, by the same amount in the other direction; i.e. if you turn the screen to the left, the picture has to be rotated to the right to accommodate.
This is only supported by libgfxinit from Skylake / Apollo Lake on (earlier GPUs didn't support the 90-degree steps anyway) and it only works with the linear-framebuffer option.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 52 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38922/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG@10 PS1, Line 10: The framebuffer contents will then be displayed by the same : amount in the other direction.
If you turn your screen to the left, the contents need to be rotated […]
My Philips 221B monitor and arandr agree with you
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38922
to look at the new patch set (#6).
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows to configure a default screen rotation in 90-degree steps. The framebuffer contents will then be displayed rotated, by the same amount in the other direction; i.e. if you turn the screen to the left, the picture has to be rotated to the right to accommodate.
This is only supported by libgfxinit from Skylake / Apollo Lake on (earlier GPUs didn't support the 90-degree steps anyway) and it only works with the linear-framebuffer option.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 60 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38922/6
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38922/1//COMMIT_MSG@14 PS1, Line 14: engines, from Skylake / Apollo Lake on.
Needs some more care. I just realized that older hardware supports […]
Done
Attention is currently required from: Nico Huber. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 6: Code-Review+1
Attention is currently required from: Nico Huber. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
Patchset:
PS6: works as expected / documented on google/fizz, though not sure that specifying the screen rotation and adjusting the framebuffer the opposite direction is the most straightforward way to handle
Attention is currently required from: Matt DeVillier. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
works as expected / documented on google/fizz, though not sure that specifying the screen rotation a […]
I have to admit I'm biased by the OEM perspective. If we build a panel into a machine rotated to the left, I want to set 90°. But that's generally what we do, isn't it? describe the hardware, not the software mechanism that adapts to it.
Of course, if the integrator failed to configure the correct setting, the user will want to rotate the framebuffer and that's just the opposite direction... not sure how to make both happy :-/
Attention is currently required from: Nico Huber, Matt DeVillier. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
I have to admit I'm biased by the OEM perspective. If we build a panel into […]
Specify whether the rotation is clockwise or counterclockwise? For example, physical panel rotation is measured clockwise, and framebuffer rotation is measured counterclockwise.
Attention is currently required from: Nico Huber, Matt DeVillier. Hello build bot (Jenkins), Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38922
to look at the new patch set (#7).
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows to configure a default screen rotation in 90-degree steps. The framebuffer contents will then be displayed rotated, by the same amount in the other direction; i.e. if you turn the screen to the left, the picture has to be rotated to the right to accommodate.
This is only supported by libgfxinit from Skylake / Apollo Lake on (earlier GPUs didn't support the 90-degree steps anyway) and it only works with the linear-framebuffer option.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 60 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38922/7
Attention is currently required from: Matt DeVillier, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS6:
Specify whether the rotation is clockwise or counterclockwise? For example, physical panel rotation […]
I don't see how that could provide anything but more confusion. However it gave me an idea: list both screen and framebuffer rotation for the 90 degree ones. Let me know what you think.
I still think the screen rotation should come first. We specify a property of the hardware. Only if that went wrong, somebody should feel the urge to (counter) rotate the framebuffer.
Attention is currently required from: Nico Huber, Matt DeVillier. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7: Code-Review+1
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/8ad46245_818fb233 PS7, Line 500: how the physical screen is mounted in : 90 degree steps are the steps clockwise or counter-clockwise?
https://review.coreboot.org/c/coreboot/+/38922/comment/dac918be_661607de PS7, Line 507: Straight nit: How about "No rotation"?
Attention is currently required from: Matt DeVillier, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7:
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/ea8cb72a_52da8224 PS7, Line 500: how the physical screen is mounted in : 90 degree steps
are the steps clockwise or counter-clockwise?
Positive degrees are counter-clockwise. Is it necessary to teach people that in Kconfig help texts? ;)
https://review.coreboot.org/c/coreboot/+/38922/comment/f76b1963_15c3c161 PS7, Line 507: Straight
nit: How about "No rotation"?
How about "Upright"? I strugle a bit to find the most common, least stigmatized vocable, but I would prefer to have an adjective here.
Attention is currently required from: Nico Huber, Matt DeVillier. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7:
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/16146c36_50a2cf13 PS7, Line 500: how the physical screen is mounted in : 90 degree steps
Positive degrees are counter-clockwise. Is it necessary to […]
I guess not
https://review.coreboot.org/c/coreboot/+/38922/comment/e7f76666_bdeb93f0 PS7, Line 507: Straight
How about "Upright"? I strugle a bit to find the most common, least […]
Adjectives, hmmm... There's `unrotated` and `nonrotated`
Attention is currently required from: Nico Huber, Angel Pons. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7:
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/3b64ad46_ece1276b PS7, Line 500: how the physical screen is mounted in : 90 degree steps
Positive degrees are counter-clockwise.
In what context? I would absolutely assume the opposite
https://review.coreboot.org/c/coreboot/+/38922/comment/18a5af33_f3ebbd52 PS7, Line 507: Straight
Adjectives, hmmm... There's `unrotated` and `nonrotated`
I'd vote for 'non-rotated.' 'Upright' could be read to imply portrait mode, which would mean a 90* rotation for most monitors
Attention is currently required from: Matt DeVillier, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922 )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 7:
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/7b61eeb7_309ef7ea PS7, Line 500: how the physical screen is mounted in : 90 degree steps
In what context?
This planet? At least I thought we agreed on it. But maybe that's what went wrong on the ISS lately xD
I would absolutely assume the opposite
I guess, I'll state it explicitly.
https://review.coreboot.org/c/coreboot/+/38922/comment/e9973971_ffb01279 PS7, Line 507: Straight
Adjectives, hmmm... There's `unrotated` and `nonrotated` […]
Thanks.
Attention is currently required from: Matt DeVillier, Angel Pons. Hello build bot (Jenkins), Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38922
to look at the new patch set (#8).
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows to configure a default screen rotation in 90-degree steps. The framebuffer contents will then be displayed rotated, by the same amount in the other direction; i.e. if you turn the screen to the left, the picture has to be rotated to the right to accommodate.
This is only supported by libgfxinit from Skylake / Apollo Lake on (earlier GPUs didn't support the 90-degree steps anyway) and it only works with the linear-framebuffer option.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 60 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38922/8
Attention is currently required from: Angel Pons, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38922?usp=email )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 9:
(3 comments)
Patchset:
PS9: Found this old patch... didn't test the rebase.
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/38922/comment/52d19223_903e3273 : PS7, Line 500: how the physical screen is mounted in : 90 degree steps
In what context? […]
Done
https://review.coreboot.org/c/coreboot/+/38922/comment/43d80fd9_5db897cc : PS7, Line 507: Straight
Thanks.
Done
Attention is currently required from: Matt DeVillier, Nico Huber.
Angel Pons has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/38922?usp=email )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
Patch Set 11: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38922?usp=email )
Change subject: libgfxinit: Allow to configure screen rotation ......................................................................
libgfxinit: Allow to configure screen rotation
This allows to configure a default screen rotation in 90-degree steps. The framebuffer contents will then be displayed rotated, by the same amount in the other direction; i.e. if you turn the screen to the left, the picture has to be rotated to the right to accommodate.
This is only supported by libgfxinit from Skylake / Apollo Lake on (earlier GPUs didn't support the 90-degree steps anyway) and it only works with the linear-framebuffer option.
Change-Id: Iac75cefbd34f28c55ec20ee152fe67351cc48653 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38922 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/device/Kconfig M src/drivers/intel/gma/hires_fb/gma-gfx_init.adb 2 files changed, 59 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/device/Kconfig b/src/device/Kconfig index 243e23e..bcff6fd 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -519,6 +519,39 @@ Set the maximum height of the framebuffer. This may help with default fonts too tiny for high-resolution displays.
+choice DEFAULT_SCREEN_ROTATION + prompt "Default screen orientation" + depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT + depends on GFX_GMA_GENERATION = "Broxton" || GFX_GMA_GENERATION = "Skylake" + default DEFAULT_SCREEN_ROTATION_NONE + help + This allows to configure how the physical screen is mounted in + 90 degree steps (counter-clockwise). The framebuffer contents + will then be displayed rotated by the same amount in the other + direction; i.e. if you turn the screen to the left, the picture + has to be rotated to the right to accommodate. + +config DEFAULT_SCREEN_ROTATION_NONE + bool "Non-rotated" + +config DEFAULT_SCREEN_ROTATION_90 + bool "Rotated 90 degrees (rotate framebuffer to the right)" + +config DEFAULT_SCREEN_ROTATION_180 + bool "Rotated 180 degrees" + +config DEFAULT_SCREEN_ROTATION_270 + bool "Rotated 270 degrees (rotate framebuffer to the left)" + +endchoice + +config DEFAULT_SCREEN_ROTATION_INT + int + default 90 if DEFAULT_SCREEN_ROTATION_90 + default 180 if DEFAULT_SCREEN_ROTATION_180 + default 270 if DEFAULT_SCREEN_ROTATION_270 + default 0 + endmenu # "Display"
config PCI diff --git a/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb b/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb index 49d0ca4..66269b2 100644 --- a/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb +++ b/src/drivers/intel/gma/hires_fb/gma-gfx_init.adb @@ -21,6 +21,17 @@ configs : Pipe_Configs; ----------------------------------------------------------------------------
+ procedure Screen_Rotation (rotation : out Rotation_Type) + is + begin + rotation := + (case Config.DEFAULT_SCREEN_ROTATION_INT is + when 90 => Rotated_90, + when 180 => Rotated_180, + when 270 => Rotated_270, + when others => No_Rotation); + end Screen_Rotation; + procedure gfxinit (lightup_ok : out Interfaces.C.int) is use type pos32; @@ -60,10 +71,21 @@ end loop;
fb := configs (Primary).Framebuffer; - fb.Width := Width_Type (min_h); - fb.Height := Height_Type (min_v); - fb.Stride := Div_Round_Up (fb.Width, 16) * 16; - fb.V_Stride := fb.Height; + Screen_Rotation (fb.Rotation); + + if fb.Rotation = Rotated_90 or fb.Rotation = Rotated_270 then + fb.Width := Width_Type (min_v); + fb.Height := Height_Type (min_h); + fb.Stride := Div_Round_Up (fb.Width, 32) * 32; + fb.V_Stride := Div_Round_Up (fb.Height, 32) * 32; + fb.Tiling := Y_Tiled; + fb.Offset := word32 (GTT_Rotation_Offset) * GTT_Page_Size; + else + fb.Width := Width_Type (min_h); + fb.Height := Height_Type (min_v); + fb.Stride := Div_Round_Up (fb.Width, 16) * 16; + fb.V_Stride := fb.Height; + end if;
for i in Pipe_Index loop exit when configs (i).Port = Disabled;
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)