Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting

[Emil Velikov]

Greetings everyone,

Padron for joining in so late o/

On Tue, 8 Feb 2022 at 08:42, Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> drm_dev_register() sets connector->registration_state to
> DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> drm_connector_set_panel_orientation() is first called after
> drm_dev_register(), it will fail several checks and results in following
> warning.
>
> Add a function to create panel orientation property and set default value
> to UNKNOWN, so drivers can call this function to init the property earlier
> , and let the panel set the real value later.
>

The warning illustrates a genuine race condition, where userspace will
read the old/invalid property value/state. So this patch masks away
the WARNING without addressing the actual issue.
Instead can we fix the respective drivers, so that no properties are
created after drm_dev_register()?

Longer version:
As we look into drm_dev_register() it's in charge of creating the
dev/sysfs nodes (et al). Note that connectors cannot disappear at
runtime.
For panel orientation, we are creating an immutable connector
properly, meaning that as soon as drm_dev_register() is called we must
ensure that the property is available (if applicable) and set to the
correct value.

For illustration, consider the following scenario:
 - DRM modules are loaded late - are not built-in and not part of
initrd (or there's no initrd)
 - kernel boots
 - plymouth/similar user-space component kicks in before the
driver/module is loaded
 - module gets loaded, drm_dev_register() kicks in populating /dev/dri/card0
 - plymouth opens the dev node and reads DRM_MODE_PANEL_ORIENTATION_UNKNOWN
 - module updates the orientation property

Thanks
Emil