On Fri, Feb 18, 2022 at 7:13 AM Simon Ser contact@emersion.fr wrote:
On Friday, February 18th, 2022 at 12:54, Hans de Goede hdegoede@redhat.com wrote:
On 2/18/22 12:39, Simon Ser wrote:
On Friday, February 18th, 2022 at 11:38, Hans de Goede hdegoede@redhat.com wrote:
What I'm reading in the above is that it is being considered to allow changing the panel-orientation value after the connector has been made available to userspace; and let userspace know about this through a uevent.
I believe that this is a bad idea, it is important to keep in mind here what userspace (e.g. plymouth) uses this prorty for. This property is used to rotate the image being rendered / shown on the framebuffer to adjust for the panel orientation.
So now lets assume we apply the correct upside-down orientation later on a device with an upside-down mounted LCD panel. Then on boot the following could happen:
- amdgpu exports a connector for the LCD panel to userspace without
setting panel-orient=upside-down 2. plymouth sees this and renders its splash normally, but since the panel is upside-down it will now actually show upside-down
At this point amdgpu hasn't probed the connector yet. So the connector will be marked as disconnected, and plymouth shouldn't render anything.
If before the initial probe of the connector there is a /dev/dri/card0 which plymouth can access, then plymouth may at this point decide to disable any seemingly unused crtcs, which will make the screen go black...
I'm not sure if plymouth will actually do this, but AFAICT this would not be invalid behavior for a userspace kms consumer to do and I believe it is likely that mutter will disable unused crtcs.
IMHO it is just a bad idea to register /dev/dri/card0 with userspace before the initial connector probe is done. Nothing good can come of that.
If all the exposed connectors initially are going to show up as disconnected anyways what is the value in registering /dev/dri/card0 with userspace early ?
OK. I'm still unsure how I feel about this, but I think I agree with you. That said, the amdgpu architecture is quite involved with multiple abstraction levels, so I don't think I'm equipped to write a patch to fix this...
cc Daniel Vetter: can you confirm probing all connectors is a good thing to do on driver module load?
I don't think it's a big deal to change, but at least my understanding, albeit this was back in the early KMS days, was that probing was driven by things outside of the driver. I.e., there is no need to probe displays if nothing is going to use them. If you want to use the displays, you'd call probe first before trying to use them so you know what is available.
Alex
I guess the initial modeline is inherited from the video-bios, but what about the physical size? Note that you cannot just change the physical size later either, that gets used to determine the hidpi scaling factor in the bootsplash, and changing that after the initial bootsplash dislay will also look ugly
b) Why you need the edid for the panel-orientation property at all, typically the edid prom is part of the panel and the panel does not know that it is mounted e.g. upside down at all, that is a property of the system as a whole not of the panel as a standalone unit so in my experience getting panel-orient info is something which comes from the firmware /video-bios not from edid ?
This is an internal DRM thing. The orientation quirks logic uses the mode size advertised by the EDID.
The DMI based quirking does, yes. But e.g. the quirk code directly reading this from the Intel VBT does not rely on the mode.
But if you are planning on using a DMI based quirk for the steamdeck then yes that needs the mode.
Thee mode check is there for 2 reasons:
- To avoid also applying the quirk to external displays, but
I think that that is also solved in most drivers by only checking for a quirk at all on the eDP connector
- Some laptop models ship with different panels in different badges
some of these are portrait (so need a panel-orient) setting and others are landscape.
That makes sense. So yeah the EDID mode based matching logic needs to stay to accomodate for these cases.
I agree that at least in the Steam Deck case it may not make a lot of sense to use any info from the EDID, but that's needed for the current status quo.
We could extend the DMI quirk mechanism to allow quirks which don't do the mode check, for use on devices where we can guarantee neither 1 nor 2 happens, then amdgpu could call the quirk code early simply passing 0x0 as resolution.
Yeah. But per the above amdgpu should maybe probe connectors on module load. If/when amdgpu is fixed to do this, then we don't need to disable the mode matching logic in panel-orientation quirks anymore.