[RFC] drm/amd/display: Pass proper parent for DM backlight device registration

[Alex Deucher]

On Wed, Feb 15, 2023 at 6:38 AM Hans de Goede <hdegoede at redhat.com> 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 at redhat.com>

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
>