[Intel-gfx] [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Hans de Goede]

Hi,

On 08-05-17 12:44, Ville Syrjälä wrote:
> On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
>> On some (Bay Trail) devices the LCD panel is mounted upside-down.
>>
>> This commit uses the code to read back the initial rotation of the
>> primary plane in get_initial_plane_config from Ville Syrjala's
>> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
>> initial fb it stores that in intel_crtc.initial_rotation.
>>
>> It adds an intel_plane_get_rotation helper which combines this
>> initial_rotation with any rotation requested by userspace and
>> uses this in all places which look at a planes rotation, thus
>> transparently dealing with upside-down LCD panels without requiring
>> any user-space or fbcon changes.
>>
>> This fixes the kernel boot messages switching from being shown the right
>> way up in efifb to being shown upside down as soon as a native kms driver
>> loads, as well as any graphics displayed by userspace being upside-down.
>>
>> Note this only deals with upside-down LCD panels / 180 degrees
>> rotation as the hardware in question only supports 180 degrees
>> rotation in hardware. The one model known which has a panel mounted
>> with 90/270 degrees rotation will need to be fully dealt with in
>> userspace, even the firmware boot screen / menus are rotated 90 degrees
>> on this one and there simply is nothing the kernel can do about this.
> 
> This pretty much looks like a very limited attempt at full pipe
> rotation.

Correct, it is limited to 180 degree rotation because that is what
almost all hardware out there which needs rotation to correct
for LCD-panel mounting needs and all pipes on GEN4+ support
180 degree rotation and on GEN < 4 we will never set initial_rotation
so this patch is a nop.

> I have posted a more generic version of that in the past
> but it was pretty much shot down by everyone else.

I agree that trying to deal with 90/270 degree rotation in the
kernel is not a good idea, but 180 degree rotation is much
easier to deal with and all devices I know of where the kernel
can even detect it needs to do rotation use 180 degree rotation.

> So I'm not convinced this apporach is really the way to go.

This ways in at about the same amount of lines added as the previous
2 patches, but unlike the previous 2 patches it actually fully solves
the problem. As my testing has shown we really should not punt this
problem to userspace because then we will get a never ending job
of needing to fix the userspace kms consumer of the week. I'm aware
of at least 6 implementations which would need fixing without
even trying: intel-ddx modesetting-ddx gnome-wayland-compositor
kde-wayland-compositor, sway-wayland-compositor, weston.

> There are a few limitations even for the 180 degree rotation so
> trying to hide it inside the kernel isn't 100% possible.

Looking at supported_rotations for all planes I don't see any
limitations, or are the maybe stride limitations or some such ?

The only limitation I see is that on Cherry Trail Pipe B
supports reflect-x but not when combined with 180 degree
rotation, this will get caught in intel_plane_atomic_check_with_state
just as before only now reflect cannot be combined with
0 degree rotation instead of 180 degree rotation.

Regards,

Hans