On Wed, Feb 15, 2023 at 6:38 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > The parent for the backlight device should be the drm-connector object, > not the PCI device. > > Userspace relies on this to be able to detect which backlight class device > to use on hybrid gfx devices where there may be multiple native (raw) > backlight devices registered. > > Specifically gnome-settings-daemon expects the parent device to have > an "enabled" sysfs attribute (as drm_connector devices do) and tests > that this returns "enabled" when read. > > This aligns the parent of the backlight device with i915, nouveau, radeon. > Note that drivers/gpu/drm/amd/amdgpu/atombios_encoders.c also already > uses the drm_connector as parent, only amdgpu_dm.c used the PCI device > as parent before this change. > > Note this is marked as a RFC because I don't have hw to test, so this > has only been compile tested! If someone can test this on actual > hw which hits the changed code path that would be great. > > Link: https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/issues/730 > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> Patch looks fine to me. I'll apply it and we can run it through our CI system. Alex > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > 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 31bce529f685..33b0e1de2770 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4065,7 +4065,8 @@ static const struct backlight_ops amdgpu_dm_backlight_ops = { > }; > > static void > -amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) > +amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm, > + struct amdgpu_dm_connector *aconnector) > { > char bl_name[16]; > struct backlight_properties props = { 0 }; > @@ -4088,7 +4089,7 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) > adev_to_drm(dm->adev)->primary->index + dm->num_of_edps); > > dm->backlight_dev[dm->num_of_edps] = backlight_device_register(bl_name, > - adev_to_drm(dm->adev)->dev, > + aconnector->base.kdev, > dm, > &amdgpu_dm_backlight_ops, > &props); > @@ -4141,6 +4142,7 @@ static int initialize_plane(struct amdgpu_display_manager *dm, > > > static void register_backlight_device(struct amdgpu_display_manager *dm, > + struct amdgpu_dm_connector *aconnector, > struct dc_link *link) > { > if ((link->connector_signal & (SIGNAL_TYPE_EDP | SIGNAL_TYPE_LVDS)) && > @@ -4151,7 +4153,7 @@ static void register_backlight_device(struct amdgpu_display_manager *dm, > * is better then a black screen. > */ > if (!dm->backlight_dev[dm->num_of_edps]) > - amdgpu_dm_register_backlight_device(dm); > + amdgpu_dm_register_backlight_device(dm, aconnector); > > if (dm->backlight_dev[dm->num_of_edps]) { > dm->backlight_link[dm->num_of_edps] = link; > @@ -4337,7 +4339,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) > > if (ret) { > amdgpu_dm_update_connector_after_detect(aconnector); > - register_backlight_device(dm, link); > + register_backlight_device(dm, aconnector, link); > > if (dm->num_of_edps) > update_connector_ext_caps(aconnector); > -- > 2.39.1 >
