Re: [PATCH] drm/amd: Query and use ACPI backlight caps

2018-11-21 Thread Wentland, Harry
On 2018-11-20 10:16 a.m., David Francis wrote:
> ACPI ATIF has a function called query
> backlight transfer characteristics.  Among the
> information returned by this function is
> the minimum and maximum input signals for the
> backlight
> 
> Call that function on ACPI init.  When DM
> backlight device is updated, copy over the
> backlight caps into DM, but only once.  Use
> the backlight caps in the backlight-to-dc
> calculation.
> 
> Signed-off-by: David Francis 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c  | 83 +++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 59 ++---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 13 +++
>  drivers/gpu/drm/amd/include/amd_acpi.h| 24 ++
>  5 files changed, 170 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 2c80453ca350..adbad0e2d4ea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1255,6 +1255,9 @@ bool 
> amdgpu_acpi_is_pcie_performance_request_supported(struct amdgpu_device *ade
>  int amdgpu_acpi_pcie_performance_request(struct amdgpu_device *adev,
>   u8 perf_req, bool advertise);
>  int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev);
> +
> +void amdgpu_acpi_get_backlight_caps(struct amdgpu_device *adev,
> + struct amdgpu_dm_backlight_caps *caps);
>  #else
>  static inline int amdgpu_acpi_init(struct amdgpu_device *adev) { return 0; }
>  static inline void amdgpu_acpi_fini(struct amdgpu_device *adev) { }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 471266901d1b..47db65926d71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -65,6 +65,7 @@ struct amdgpu_atif {
>   struct amdgpu_atif_functions functions;
>   struct amdgpu_atif_notification_cfg notification_cfg;
>   struct amdgpu_encoder *encoder_for_bl;
> + struct amdgpu_dm_backlight_caps backlight_caps;
>  };
>  
>  /* Call the ATIF method
> @@ -297,6 +298,65 @@ static int amdgpu_atif_get_notification_params(struct 
> amdgpu_atif *atif)
>   return err;
>  }
>  
> +/**
> + * amdgpu_atif_query_backlight_caps - get min and max backlight input signal
> + *
> + * @handle: acpi handle
> + *
> + * Execute the QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS ATIF function
> + * to determine the acceptable range of backlight values
> + *
> + * Backlight_caps.caps_valid will be set to true if the query is successful
> + *
> + * The input signals are in range 0-255
> + *
> + * This function assumes the display with backlight is the first LCD
> + *
> + * Returns 0 on success, error on failure.
> + */
> +static int amdgpu_atif_query_backlight_caps(struct amdgpu_atif *atif)
> +{
> + union acpi_object *info;
> + struct atif_qbtc_output characteristics;
> + struct atif_qbtc_arguments arguments;
> + struct acpi_buffer params;
> + size_t size;
> + int err = 0;
> +
> + arguments.size = sizeof(arguments);
> + arguments.requested_display = ATIF_QBTC_REQUEST_LCD1;
> +
> + params.length = sizeof(arguments);
> + params.pointer = (void *)&arguments;
> +
> + info = amdgpu_atif_call(atif,
> + ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS,
> + ¶ms);
> + if (!info) {
> + err = -EIO;
> + goto out;
> + }
> +
> + size = *(u16 *) info->buffer.pointer;
> + if (size < 10) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + memset(&characteristics, 0, sizeof(characteristics));
> + size = min(sizeof(characteristics), size);
> + memcpy(&characteristics, info->buffer.pointer, size);
> +
> + atif->backlight_caps.caps_valid = true;
> + atif->backlight_caps.min_input_signal =
> + characteristics.min_input_signal;
> + atif->backlight_caps.max_input_signal =
> + characteristics.max_input_signal;
> +out:
> + kfree(info);
> + return err;
> +}
> +
>  /**
>   * amdgpu_atif_get_sbios_requests - get requested sbios event
>   *
> @@ -786,6 +846,17 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
>   }
>   }
>  
> + if (atif->functions.query_backlight_transfer_characteristics) {
> + ret = amdgpu_atif_query_backlight_caps(atif);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Call to 
> QUERY_BACKLIGHT_TRANSFER_CHARACTERISTICS failed: %d\n",
> + ret);
> + atif->backlight_caps.caps_valid = false;
> + }
> + } else {
> + atif->backlight_caps.caps_valid = false;
> + }
> +
>  out:
>   adev->acpi_nb.notifier_call = amdgpu_acpi_

[PATCH] drm/amd: Query and use ACPI backlight caps

2018-11-20 Thread David Francis
ACPI ATIF has a function called query
backlight transfer characteristics.  Among the
information returned by this function is
the minimum and maximum input signals for the
backlight

Call that function on ACPI init.  When DM
backlight device is updated, copy over the
backlight caps into DM, but only once.  Use
the backlight caps in the backlight-to-dc
calculation.

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c  | 83 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 59 ++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 13 +++
 drivers/gpu/drm/amd/include/amd_acpi.h| 24 ++
 5 files changed, 170 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 2c80453ca350..adbad0e2d4ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1255,6 +1255,9 @@ bool 
amdgpu_acpi_is_pcie_performance_request_supported(struct amdgpu_device *ade
 int amdgpu_acpi_pcie_performance_request(struct amdgpu_device *adev,
u8 perf_req, bool advertise);
 int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev);
+
+void amdgpu_acpi_get_backlight_caps(struct amdgpu_device *adev,
+   struct amdgpu_dm_backlight_caps *caps);
 #else
 static inline int amdgpu_acpi_init(struct amdgpu_device *adev) { return 0; }
 static inline void amdgpu_acpi_fini(struct amdgpu_device *adev) { }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 471266901d1b..47db65926d71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -65,6 +65,7 @@ struct amdgpu_atif {
struct amdgpu_atif_functions functions;
struct amdgpu_atif_notification_cfg notification_cfg;
struct amdgpu_encoder *encoder_for_bl;
+   struct amdgpu_dm_backlight_caps backlight_caps;
 };
 
 /* Call the ATIF method
@@ -297,6 +298,65 @@ static int amdgpu_atif_get_notification_params(struct 
amdgpu_atif *atif)
return err;
 }
 
+/**
+ * amdgpu_atif_query_backlight_caps - get min and max backlight input signal
+ *
+ * @handle: acpi handle
+ *
+ * Execute the QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS ATIF function
+ * to determine the acceptable range of backlight values
+ *
+ * Backlight_caps.caps_valid will be set to true if the query is successful
+ *
+ * The input signals are in range 0-255
+ *
+ * This function assumes the display with backlight is the first LCD
+ *
+ * Returns 0 on success, error on failure.
+ */
+static int amdgpu_atif_query_backlight_caps(struct amdgpu_atif *atif)
+{
+   union acpi_object *info;
+   struct atif_qbtc_output characteristics;
+   struct atif_qbtc_arguments arguments;
+   struct acpi_buffer params;
+   size_t size;
+   int err = 0;
+
+   arguments.size = sizeof(arguments);
+   arguments.requested_display = ATIF_QBTC_REQUEST_LCD1;
+
+   params.length = sizeof(arguments);
+   params.pointer = (void *)&arguments;
+
+   info = amdgpu_atif_call(atif,
+   ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS,
+   ¶ms);
+   if (!info) {
+   err = -EIO;
+   goto out;
+   }
+
+   size = *(u16 *) info->buffer.pointer;
+   if (size < 10) {
+   err = -EINVAL;
+   goto out;
+   }
+
+   memset(&characteristics, 0, sizeof(characteristics));
+   size = min(sizeof(characteristics), size);
+   memcpy(&characteristics, info->buffer.pointer, size);
+
+   atif->backlight_caps.caps_valid = true;
+   atif->backlight_caps.min_input_signal =
+   characteristics.min_input_signal;
+   atif->backlight_caps.max_input_signal =
+   characteristics.max_input_signal;
+out:
+   kfree(info);
+   return err;
+}
+
 /**
  * amdgpu_atif_get_sbios_requests - get requested sbios event
  *
@@ -786,6 +846,17 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
}
}
 
+   if (atif->functions.query_backlight_transfer_characteristics) {
+   ret = amdgpu_atif_query_backlight_caps(atif);
+   if (ret) {
+   DRM_DEBUG_DRIVER("Call to 
QUERY_BACKLIGHT_TRANSFER_CHARACTERISTICS failed: %d\n",
+   ret);
+   atif->backlight_caps.caps_valid = false;
+   }
+   } else {
+   atif->backlight_caps.caps_valid = false;
+   }
+
 out:
adev->acpi_nb.notifier_call = amdgpu_acpi_event;
register_acpi_notifier(&adev->acpi_nb);
@@ -793,6 +864,18 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
return ret;
 }
 
+void amdgpu_acpi_get_backlight_caps(struct amdgpu_device *adev,
+   struct amdgpu_dm_bac