RE: [PATCH] drm/amdgpu: simplify ATIF backlight handling

2020-05-21 Thread Quan, Evan
[AMD Official Use Only - Internal Distribution Only]

Acked-by: Evan Quan 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, May 6, 2020 3:48 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: simplify ATIF backlight handling

Just register the a pointer to the backlight device and use that. Unifies the 
DC and non-DC handling.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 73 ++--
 1 file changed, 30 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 956cbbda4793..913c8f0513bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -64,7 +64,9 @@ struct amdgpu_atif {
 struct amdgpu_atif_notifications notifications;
 struct amdgpu_atif_functions functions;
 struct amdgpu_atif_notification_cfg notification_cfg;
-struct amdgpu_encoder *encoder_for_bl;
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
+struct backlight_device *bd;
+#endif
 struct amdgpu_dm_backlight_caps backlight_caps;  };

@@ -444,45 +446,21 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,

 DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", count);

-if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
-!amdgpu_device_has_dc_support(adev)) {
-struct amdgpu_encoder *enc = atif->encoder_for_bl;
-
-if (enc) {
-struct amdgpu_encoder_atom_dig *dig = enc->enc_priv;
-
-DRM_DEBUG_DRIVER("Changing brightness to %d\n",
- req.backlight_level);
-
-amdgpu_display_backlight_set_level(adev, enc, req.backlight_level);
-
-#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
-backlight_force_update(dig->bl_dev,
-   BACKLIGHT_UPDATE_HOTKEY);
-#endif
-}
-}
-#if defined(CONFIG_DRM_AMD_DC)
+if (req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) {
 #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
-if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
-amdgpu_device_has_dc_support(adev)) {
-struct amdgpu_display_manager *dm = >dm;
-struct backlight_device *bd = dm->backlight_dev;
-
-if (bd) {
+if (atif->bd) {
 DRM_DEBUG_DRIVER("Changing brightness to %d\n",
  req.backlight_level);
-
 /*
  * XXX backlight_device_set_brightness() is
  * hardwired to post BACKLIGHT_UPDATE_SYSFS.
  * It probably should accept 'reason' parameter.
  */
-backlight_device_set_brightness(bd, req.backlight_level);
+backlight_device_set_brightness(atif->bd, req.backlight_level);
 }
-}
-#endif
 #endif
+}
+
 if (req.pending & ATIF_DGPU_DISPLAY_EVENT) {
 if (adev->flags & AMD_IS_PX) {
 pm_runtime_get_sync(adev->ddev->dev);
@@ -829,23 +807,32 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
 adev->atif = atif;

 if (atif->notifications.brightness_change) {
-struct drm_encoder *tmp;
-
-/* Find the encoder controlling the brightness */
-list_for_each_entry(tmp, >ddev->mode_config.encoder_list,
-head) {
-struct amdgpu_encoder *enc = to_amdgpu_encoder(tmp);
-
-if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) &&
-enc->enc_priv) {
-struct amdgpu_encoder_atom_dig *dig = enc->enc_priv;
-if (dig->bl_dev) {
-atif->encoder_for_bl = enc;
-break;
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
+if (amdgpu_device_has_dc_support(adev)) { #if
+defined(CONFIG_DRM_AMD_DC)
+struct amdgpu_display_manager *dm = >dm;
+atif->bd = dm->backlight_dev;
+#endif
+} else {
+struct drm_encoder *tmp;
+
+/* Find the encoder controlling the brightness */
+list_for_each_entry(tmp, >ddev->mode_config.encoder_list,
+head) {
+struct amdgpu_encoder *enc = to_amdgpu_encoder(tmp);
+
+if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) &&
+enc->enc_priv) {
+struct amdgpu_encoder_atom_dig *dig = enc->enc_priv;
+if (dig->bl_dev) {
+atif->bd = dig->bl_dev;
+break;
+}
 }
 }
 }
 }
+#endif

 if (atif->functions.sbios_requests && !atif->functions.system_params) {
 /* XXX check this workraround, if sbios request function is
--
2.25.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cevan.quan%40amd.com%7Cf8a40f42e8ec4ea3c9e008d7f12d4592%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243050173626661sdata=yj1OgR%2FXj4X0VvZ3%2F%2FOIGgA0RDN4JP%2FG7IOuBM9xEaA%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: simplify ATIF backlight handling

2020-05-21 Thread Alex Deucher
Ping?

On Tue, May 12, 2020 at 4:15 PM Alex Deucher  wrote:
>
> Ping?
>
> Alex
>
> On Thu, May 7, 2020 at 12:09 PM Alex Deucher  wrote:
> >
> > Ping?
> >
> > On Tue, May 5, 2020 at 3:48 PM Alex Deucher  wrote:
> > >
> > > Just register the a pointer to the backlight device and use
> > > that. Unifies the DC and non-DC handling.
> > >
> > > Signed-off-by: Alex Deucher 
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 73 ++--
> > >  1 file changed, 30 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > index 956cbbda4793..913c8f0513bd 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > @@ -64,7 +64,9 @@ struct amdgpu_atif {
> > > struct amdgpu_atif_notifications notifications;
> > > struct amdgpu_atif_functions functions;
> > > struct amdgpu_atif_notification_cfg notification_cfg;
> > > -   struct amdgpu_encoder *encoder_for_bl;
> > > +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > > +   struct backlight_device *bd;
> > > +#endif
> > > struct amdgpu_dm_backlight_caps backlight_caps;
> > >  };
> > >
> > > @@ -444,45 +446,21 @@ static int amdgpu_atif_handler(struct amdgpu_device 
> > > *adev,
> > >
> > > DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", 
> > > count);
> > >
> > > -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) 
> > > &&
> > > -   !amdgpu_device_has_dc_support(adev)) {
> > > -   struct amdgpu_encoder *enc = atif->encoder_for_bl;
> > > -
> > > -   if (enc) {
> > > -   struct amdgpu_encoder_atom_dig *dig = 
> > > enc->enc_priv;
> > > -
> > > -   DRM_DEBUG_DRIVER("Changing brightness to 
> > > %d\n",
> > > -req.backlight_level);
> > > -
> > > -   amdgpu_display_backlight_set_level(adev, 
> > > enc, req.backlight_level);
> > > -
> > > -#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > > -   backlight_force_update(dig->bl_dev,
> > > -  
> > > BACKLIGHT_UPDATE_HOTKEY);
> > > -#endif
> > > -   }
> > > -   }
> > > -#if defined(CONFIG_DRM_AMD_DC)
> > > +   if (req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) {
> > >  #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > > -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) 
> > > &&
> > > -   amdgpu_device_has_dc_support(adev)) {
> > > -   struct amdgpu_display_manager *dm = >dm;
> > > -   struct backlight_device *bd = dm->backlight_dev;
> > > -
> > > -   if (bd) {
> > > +   if (atif->bd) {
> > > DRM_DEBUG_DRIVER("Changing brightness to 
> > > %d\n",
> > >  req.backlight_level);
> > > -
> > > /*
> > >  * XXX backlight_device_set_brightness() 
> > > is
> > >  * hardwired to post 
> > > BACKLIGHT_UPDATE_SYSFS.
> > >  * It probably should accept 'reason' 
> > > parameter.
> > >  */
> > > -   backlight_device_set_brightness(bd, 
> > > req.backlight_level);
> > > +   backlight_device_set_brightness(atif->bd, 
> > > req.backlight_level);
> > > }
> > > -   }
> > > -#endif
> > >  #endif
> > > +   }
> > > +
> > > if (req.pending & ATIF_DGPU_DISPLAY_EVENT) {
> > > if (adev->flags & AMD_IS_PX) {
> > > pm_runtime_get_sync(adev->ddev->dev);
> > > @@ -829,23 +807,32 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> > > adev->atif = atif;
> > >
> > > if (atif->notifications.brightness_change) {
> > > -   struct drm_encoder *tmp;
> > > -
> > > -   /* Find the encoder controlling the brightness */
> > > -   list_for_each_entry(tmp, 
> > > >ddev->mode_config.encoder_list,
> > > -   head) {
> > > -   struct amdgpu_encoder *enc = 
> > > to_amdgpu_encoder(tmp);
> > > -
> > > -   if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) &&
> > > -   enc->enc_priv) {
> > > -   struct amdgpu_encoder_atom_dig *dig = 
> > > enc->enc_priv;
> > > -  

Re: [PATCH] drm/amdgpu: simplify ATIF backlight handling

2020-05-12 Thread Alex Deucher
Ping?

Alex

On Thu, May 7, 2020 at 12:09 PM Alex Deucher  wrote:
>
> Ping?
>
> On Tue, May 5, 2020 at 3:48 PM Alex Deucher  wrote:
> >
> > Just register the a pointer to the backlight device and use
> > that. Unifies the DC and non-DC handling.
> >
> > Signed-off-by: Alex Deucher 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 73 ++--
> >  1 file changed, 30 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > index 956cbbda4793..913c8f0513bd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -64,7 +64,9 @@ struct amdgpu_atif {
> > struct amdgpu_atif_notifications notifications;
> > struct amdgpu_atif_functions functions;
> > struct amdgpu_atif_notification_cfg notification_cfg;
> > -   struct amdgpu_encoder *encoder_for_bl;
> > +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > +   struct backlight_device *bd;
> > +#endif
> > struct amdgpu_dm_backlight_caps backlight_caps;
> >  };
> >
> > @@ -444,45 +446,21 @@ static int amdgpu_atif_handler(struct amdgpu_device 
> > *adev,
> >
> > DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", 
> > count);
> >
> > -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
> > -   !amdgpu_device_has_dc_support(adev)) {
> > -   struct amdgpu_encoder *enc = atif->encoder_for_bl;
> > -
> > -   if (enc) {
> > -   struct amdgpu_encoder_atom_dig *dig = 
> > enc->enc_priv;
> > -
> > -   DRM_DEBUG_DRIVER("Changing brightness to 
> > %d\n",
> > -req.backlight_level);
> > -
> > -   amdgpu_display_backlight_set_level(adev, 
> > enc, req.backlight_level);
> > -
> > -#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > -   backlight_force_update(dig->bl_dev,
> > -  
> > BACKLIGHT_UPDATE_HOTKEY);
> > -#endif
> > -   }
> > -   }
> > -#if defined(CONFIG_DRM_AMD_DC)
> > +   if (req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) {
> >  #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
> > -   amdgpu_device_has_dc_support(adev)) {
> > -   struct amdgpu_display_manager *dm = >dm;
> > -   struct backlight_device *bd = dm->backlight_dev;
> > -
> > -   if (bd) {
> > +   if (atif->bd) {
> > DRM_DEBUG_DRIVER("Changing brightness to 
> > %d\n",
> >  req.backlight_level);
> > -
> > /*
> >  * XXX backlight_device_set_brightness() is
> >  * hardwired to post BACKLIGHT_UPDATE_SYSFS.
> >  * It probably should accept 'reason' 
> > parameter.
> >  */
> > -   backlight_device_set_brightness(bd, 
> > req.backlight_level);
> > +   backlight_device_set_brightness(atif->bd, 
> > req.backlight_level);
> > }
> > -   }
> > -#endif
> >  #endif
> > +   }
> > +
> > if (req.pending & ATIF_DGPU_DISPLAY_EVENT) {
> > if (adev->flags & AMD_IS_PX) {
> > pm_runtime_get_sync(adev->ddev->dev);
> > @@ -829,23 +807,32 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> > adev->atif = atif;
> >
> > if (atif->notifications.brightness_change) {
> > -   struct drm_encoder *tmp;
> > -
> > -   /* Find the encoder controlling the brightness */
> > -   list_for_each_entry(tmp, 
> > >ddev->mode_config.encoder_list,
> > -   head) {
> > -   struct amdgpu_encoder *enc = to_amdgpu_encoder(tmp);
> > -
> > -   if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) &&
> > -   enc->enc_priv) {
> > -   struct amdgpu_encoder_atom_dig *dig = 
> > enc->enc_priv;
> > -   if (dig->bl_dev) {
> > -   atif->encoder_for_bl = enc;
> > -   break;
> > +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> > defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> > +   if 

Re: [PATCH] drm/amdgpu: simplify ATIF backlight handling

2020-05-07 Thread Alex Deucher
Ping?

On Tue, May 5, 2020 at 3:48 PM Alex Deucher  wrote:
>
> Just register the a pointer to the backlight device and use
> that. Unifies the DC and non-DC handling.
>
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 73 ++--
>  1 file changed, 30 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 956cbbda4793..913c8f0513bd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -64,7 +64,9 @@ struct amdgpu_atif {
> struct amdgpu_atif_notifications notifications;
> struct amdgpu_atif_functions functions;
> struct amdgpu_atif_notification_cfg notification_cfg;
> -   struct amdgpu_encoder *encoder_for_bl;
> +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> +   struct backlight_device *bd;
> +#endif
> struct amdgpu_dm_backlight_caps backlight_caps;
>  };
>
> @@ -444,45 +446,21 @@ static int amdgpu_atif_handler(struct amdgpu_device 
> *adev,
>
> DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", count);
>
> -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
> -   !amdgpu_device_has_dc_support(adev)) {
> -   struct amdgpu_encoder *enc = atif->encoder_for_bl;
> -
> -   if (enc) {
> -   struct amdgpu_encoder_atom_dig *dig = 
> enc->enc_priv;
> -
> -   DRM_DEBUG_DRIVER("Changing brightness to 
> %d\n",
> -req.backlight_level);
> -
> -   amdgpu_display_backlight_set_level(adev, enc, 
> req.backlight_level);
> -
> -#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> -   backlight_force_update(dig->bl_dev,
> -  
> BACKLIGHT_UPDATE_HOTKEY);
> -#endif
> -   }
> -   }
> -#if defined(CONFIG_DRM_AMD_DC)
> +   if (req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) {
>  #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> -   if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
> -   amdgpu_device_has_dc_support(adev)) {
> -   struct amdgpu_display_manager *dm = >dm;
> -   struct backlight_device *bd = dm->backlight_dev;
> -
> -   if (bd) {
> +   if (atif->bd) {
> DRM_DEBUG_DRIVER("Changing brightness to 
> %d\n",
>  req.backlight_level);
> -
> /*
>  * XXX backlight_device_set_brightness() is
>  * hardwired to post BACKLIGHT_UPDATE_SYSFS.
>  * It probably should accept 'reason' 
> parameter.
>  */
> -   backlight_device_set_brightness(bd, 
> req.backlight_level);
> +   backlight_device_set_brightness(atif->bd, 
> req.backlight_level);
> }
> -   }
> -#endif
>  #endif
> +   }
> +
> if (req.pending & ATIF_DGPU_DISPLAY_EVENT) {
> if (adev->flags & AMD_IS_PX) {
> pm_runtime_get_sync(adev->ddev->dev);
> @@ -829,23 +807,32 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> adev->atif = atif;
>
> if (atif->notifications.brightness_change) {
> -   struct drm_encoder *tmp;
> -
> -   /* Find the encoder controlling the brightness */
> -   list_for_each_entry(tmp, 
> >ddev->mode_config.encoder_list,
> -   head) {
> -   struct amdgpu_encoder *enc = to_amdgpu_encoder(tmp);
> -
> -   if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) &&
> -   enc->enc_priv) {
> -   struct amdgpu_encoder_atom_dig *dig = 
> enc->enc_priv;
> -   if (dig->bl_dev) {
> -   atif->encoder_for_bl = enc;
> -   break;
> +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || 
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
> +   if (amdgpu_device_has_dc_support(adev)) {
> +#if defined(CONFIG_DRM_AMD_DC)
> +   struct amdgpu_display_manager *dm = >dm;
> +   atif->bd = dm->backlight_dev;
> +#endif
> +   } else {
> +   struct drm_encoder *tmp;
> +
> +   /* Find the