Re: [PATCH v7 08/12] drm/amd/display: use drm backlight

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Data race and unconditional kernel WARN_ON due to dynamic DRM property creation after connector registration.
- [Medium] Luminance property updates are silently dropped when an atomic commit includes a modeset.

Pre-existing issues:
- [High] Atomic check and commit paths dereference a NULL CRTC state if a connector property is updated without the associated CRTC being pulled into the atomic state.
--

commit 3fc3b85f7e3897f97fa8465e7158825ddd3e0dd7
Author: Mario Limonciello <mario.limonciello@amd.com>

drm/amd/display: use drm backlight

Convert the AMD display driver to the DRM backlight infrastructure so
brightness can be controlled through the connector LUMINANCE property.
Link the registered backlight_device to the eDP connector and unlink it
on teardown; the DRM core owns the embedded backlight state and its
property, so no explicit allocation or failure handling is needed.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 284aac4d96bce..b5254c775a778 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5048,6 +5048,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
>  		if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state))

[Severity: High]
This is a pre-existing issue, but does this code dereference a NULL pointer
if a connector-only property is updated?

When a connector-only property is updated, the DRM core stages the property
but does not add the CRTC to the atomic state. The drm_atomic_get_new_crtc_state
call returns NULL when the CRTC is not in the state.

If userspace submits an atomic commit that updates only the newly exposed
LUMINANCE property without modifying the CRTC, new_crtc_state will be NULL
and immediately dereferenced inside drm_atomic_crtc_needs_modeset().

>  			continue;
>  
> +		drm_connector_update_privacy_screen(new_con_state);
> +		drm_atomic_helper_connector_apply_luminance(new_con_state);

[Severity: Medium]
Will this cause luminance property updates to be silently dropped when an
atomic commit includes a modeset?

The continue statement immediately above skips the newly added
drm_atomic_helper_connector_apply_luminance() call if the CRTC needs a
modeset (e.g., when changing display resolution).

Since amdgpu_dm does not use standard DRM modeset helpers, the skipped
luminance update would never be applied to the hardware, and the backlight
would remain at its previous value.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
> index ed1f85650e280..0174339f0ee79 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
> @@ -496,6 +497,9 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
>  			dm->actual_brightness[aconnector->bl_idx] = real_brightness;
>  			dm->brightness[aconnector->bl_idx] = real_brightness;
>  		}
> +		/* Link the registered backlight device to the DRM connector. */
> +		drm_backlight_link(&aconnector->base, dm->backlight_dev[aconnector->bl_idx]);

[Severity: High]
Does this trigger a kernel WARN_ON and data race by dynamically creating the
LUMINANCE property after connector registration?

Because amdgpu_dm_register_backlight_device() is executed during the late
registration phase (amdgpu_dm_connector_late_register), the connector has
already been exposed to userspace.

The drm_backlight_link() call creates the LUMINANCE property dynamically,
but adding properties after registration is prohibited by the DRM core.

Could this also cause a data race if userspace iterates the connector's
properties array (e.g., reacting to a sysfs KOBJ_ADD uevent by issuing
DRM_IOCTL_MODE_GETCONNECTOR) locklessly while it is being reallocated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831210531.2334221-1-mario.limonciello@amd.com?part=8