AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Add custom brightness curve support
@ 2025-02-21 17:09 Mario Limonciello
  2025-02-21 17:09 ` [PATCH 1/5] drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps() Mario Limonciello
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:09 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

Some OEMs support custom brightness curves where the ATIF method includes
a collection of data points where the input signal is mapped out to
percentage of luminance. This series shuffles around some code to add in
the ability to do that mapping in amdgpu_dm when brightness is set.

Mario Limonciello (5):
  drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps()
  drm/amd: Pass luminance data to amdgpu_dm_backlight_caps
  drm/amd/display: Avoid operating on copies of backlight caps
  drm/amd/display: Add support for custom brightness curve
  drm/amd/display: Add a new dcdebugmask to allow turning off brightness
    curve

 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c      | 10 +--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 ++++++++++++-------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 20 +++++
 drivers/gpu/drm/amd/include/amd_acpi.h        |  9 +--
 drivers/gpu/drm/amd/include/amd_shared.h      |  4 +
 5 files changed, 81 insertions(+), 42 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps()
  2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
@ 2025-02-21 17:09 ` Mario Limonciello
  2025-02-21 17:09 ` [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps Mario Limonciello
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:09 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

As new members are introduced to the structure copying the entire
structure will help avoid missing them.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index b8d4e07d2043..515c6f32448d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1277,11 +1277,7 @@ void amdgpu_acpi_get_backlight_caps(struct amdgpu_dm_backlight_caps *caps)
 {
 	struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
 
-	caps->caps_valid = atif->backlight_caps.caps_valid;
-	caps->min_input_signal = atif->backlight_caps.min_input_signal;
-	caps->max_input_signal = atif->backlight_caps.max_input_signal;
-	caps->ac_level = atif->backlight_caps.ac_level;
-	caps->dc_level = atif->backlight_caps.dc_level;
+	memcpy(caps, &atif->backlight_caps, sizeof(*caps));
 }
 
 /**
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps
  2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
  2025-02-21 17:09 ` [PATCH 1/5] drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps() Mario Limonciello
@ 2025-02-21 17:09 ` Mario Limonciello
  2025-02-24 21:26   ` Deucher, Alexander
  2025-02-21 17:10 ` [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps Mario Limonciello
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:09 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

The ATIF method on some systems will provide a backlight curve. Pass
this curve into amdgpu_dm add it to the structures.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c      |  4 ++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 20 +++++++++++++++++++
 drivers/gpu/drm/amd/include/amd_acpi.h        |  9 +--------
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 515c6f32448d..b7f8f2ff143d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -394,6 +394,10 @@ static int amdgpu_atif_query_backlight_caps(struct amdgpu_atif *atif)
 			characteristics.max_input_signal;
 	atif->backlight_caps.ac_level = characteristics.ac_level;
 	atif->backlight_caps.dc_level = characteristics.dc_level;
+	atif->backlight_caps.data_points = characteristics.number_of_points;
+	memcpy(atif->backlight_caps.luminance_data,
+	       characteristics.data_points,
+	       sizeof(atif->backlight_caps.luminance_data));
 out:
 	kfree(info);
 	return err;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index f3bc00e587ad..85b64c457ed6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -151,6 +151,18 @@ struct idle_workqueue {
 	bool running;
 };
 
+#define MAX_LUMINANCE_DATA_POINTS 99
+
+/**
+ * struct amdgpu_dm_luminance_data - Custom luminance data
+ * @luminance: Luminance in percent
+ * @input_signal: Input signal in range 0-255
+ */
+struct amdgpu_dm_luminance_data {
+	u8 luminance;
+	u8 input_signal;
+} __packed;
+
 /**
  * struct amdgpu_dm_backlight_caps - Information about backlight
  *
@@ -195,6 +207,14 @@ struct amdgpu_dm_backlight_caps {
 	 * @dc_level: the default brightness if booted on DC
 	 */
 	u8 dc_level;
+	/**
+	 * @data_points: the number of custom luminance data points
+	 */
+	u8 data_points;
+	/**
+	 * @luminance_data: custom luminance data
+	 */
+	struct amdgpu_dm_luminance_data luminance_data[MAX_LUMINANCE_DATA_POINTS];
 };
 
 /**
diff --git a/drivers/gpu/drm/amd/include/amd_acpi.h b/drivers/gpu/drm/amd/include/amd_acpi.h
index 2d089d30518f..63713fdca428 100644
--- a/drivers/gpu/drm/amd/include/amd_acpi.h
+++ b/drivers/gpu/drm/amd/include/amd_acpi.h
@@ -57,13 +57,6 @@ struct atif_qbtc_arguments {
 	u8 requested_display;	/* which display is requested */
 } __packed;
 
-#define ATIF_QBTC_MAX_DATA_POINTS 99
-
-struct atif_qbtc_data_point {
-	u8 luminance;		/* luminance in percent */
-	u8 ipnut_signal;	/* input signal in range 0-255 */
-} __packed;
-
 struct atif_qbtc_output {
 	u16 size;		/* structure size in bytes (includes size field) */
 	u16 flags;		/* all zeroes */
@@ -73,7 +66,7 @@ struct atif_qbtc_output {
 	u8 min_input_signal;	/* max input signal in range 0-255 */
 	u8 max_input_signal;	/* min input signal in range 0-255 */
 	u8 number_of_points;	/* number of data points */
-	struct atif_qbtc_data_point data_points[ATIF_QBTC_MAX_DATA_POINTS];
+	struct amdgpu_dm_luminance_data data_points[MAX_LUMINANCE_DATA_POINTS];
 } __packed;
 
 #define ATIF_NOTIFY_MASK	0x3
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps
  2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
  2025-02-21 17:09 ` [PATCH 1/5] drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps() Mario Limonciello
  2025-02-21 17:09 ` [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps Mario Limonciello
@ 2025-02-21 17:10 ` Mario Limonciello
  2025-02-28 18:23   ` Alex Hung
  2025-02-21 17:10 ` [PATCH 4/5] drm/amd/display: Add support for custom brightness curve Mario Limonciello
  2025-02-21 17:10 ` [PATCH 5/5] drm/amd/display: Add a new dcdebugmask to allow turning off " Mario Limonciello
  4 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:10 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

Making a copy of the backlight caps structure between uses is unnecessary.
Refer to pointers to the same structure when using it.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 ++++++++-----------
 1 file changed, 21 insertions(+), 29 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 0d21448ea700..70c8d800e173 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4646,47 +4646,39 @@ static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm,
 					    int bl_idx)
 {
 #if defined(CONFIG_ACPI)
-	struct amdgpu_dm_backlight_caps caps;
-
-	memset(&caps, 0, sizeof(caps));
+	struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[bl_idx];
 
-	if (dm->backlight_caps[bl_idx].caps_valid)
+	if (caps->caps_valid)
 		return;
 
-	amdgpu_acpi_get_backlight_caps(&caps);
+	amdgpu_acpi_get_backlight_caps(caps);
 
 	/* validate the firmware value is sane */
-	if (caps.caps_valid) {
-		int spread = caps.max_input_signal - caps.min_input_signal;
+	if (caps->caps_valid) {
+		int spread = caps->max_input_signal - caps->min_input_signal;
 
-		if (caps.max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
-		    caps.min_input_signal < 0 ||
+		if (caps->max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
+		    caps->min_input_signal < 0 ||
 		    spread > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
 		    spread < AMDGPU_DM_MIN_SPREAD) {
 			DRM_DEBUG_KMS("DM: Invalid backlight caps: min=%d, max=%d\n",
-				      caps.min_input_signal, caps.max_input_signal);
-			caps.caps_valid = false;
+				      caps->min_input_signal, caps->max_input_signal);
+			caps->caps_valid = false;
 		}
 	}
 
-	if (caps.caps_valid) {
-		dm->backlight_caps[bl_idx].caps_valid = true;
-		if (caps.aux_support)
-			return;
-		dm->backlight_caps[bl_idx].min_input_signal = caps.min_input_signal;
-		dm->backlight_caps[bl_idx].max_input_signal = caps.max_input_signal;
-	} else {
-		dm->backlight_caps[bl_idx].min_input_signal =
-				AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-		dm->backlight_caps[bl_idx].max_input_signal =
-				AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+	if (!caps->caps_valid) {
+		caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
+		caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+		caps->caps_valid = true;
 	}
 #else
-	if (dm->backlight_caps[bl_idx].aux_support)
+	if (caps->aux_support)
 		return;
 
-	dm->backlight_caps[bl_idx].min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-	dm->backlight_caps[bl_idx].max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+	caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
+	caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+	caps->caps_valid = true;
 #endif
 }
 
@@ -4740,19 +4732,19 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
 					 int bl_idx,
 					 u32 user_brightness)
 {
-	struct amdgpu_dm_backlight_caps caps;
+	struct amdgpu_dm_backlight_caps *caps;
 	struct dc_link *link;
 	u32 brightness;
 	bool rc, reallow_idle = false;
 
 	amdgpu_dm_update_backlight_caps(dm, bl_idx);
-	caps = dm->backlight_caps[bl_idx];
+	caps = &dm->backlight_caps[bl_idx];
 
 	dm->brightness[bl_idx] = user_brightness;
 	/* update scratch register */
 	if (bl_idx == 0)
 		amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, dm->brightness[bl_idx]);
-	brightness = convert_brightness_from_user(&caps, dm->brightness[bl_idx]);
+	brightness = convert_brightness_from_user(caps, dm->brightness[bl_idx]);
 	link = (struct dc_link *)dm->backlight_link[bl_idx];
 
 	/* Change brightness based on AUX property */
@@ -4762,7 +4754,7 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
 		reallow_idle = true;
 	}
 
-	if (caps.aux_support) {
+	if (caps->aux_support) {
 		rc = dc_link_set_backlight_level_nits(link, true, brightness,
 						      AUX_BL_DEFAULT_TRANSITION_TIME_MS);
 		if (!rc)
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] drm/amd/display: Add support for custom brightness curve
  2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
                   ` (2 preceding siblings ...)
  2025-02-21 17:10 ` [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps Mario Limonciello
@ 2025-02-21 17:10 ` Mario Limonciello
  2025-02-21 17:10 ` [PATCH 5/5] drm/amd/display: Add a new dcdebugmask to allow turning off " Mario Limonciello
  4 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:10 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

Some systems specify in the firmware a brightness curve that better
reflects the characteristics of the panel used. This is done in the
form of data points and matching luminance percentage.

When converting a userspace requested brightness value use that curve
to convert to a firmware intended brightness value.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

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 70c8d800e173..136abfcdb76d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4704,10 +4704,35 @@ static u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *c
 					uint32_t brightness)
 {
 	unsigned int min, max;
+	u8 prev_signal = 0, prev_lum = 0;
 
 	if (!get_brightness_range(caps, &min, &max))
 		return brightness;
 
+	for (int i = 0; i < caps->data_points; i++) {
+		u8 signal, lum;
+
+		signal = caps->luminance_data[i].input_signal;
+		lum = caps->luminance_data[i].luminance;
+
+		/*
+		 * brightness == signal: luminance is percent numerator
+		 * brightness < signal: interpolate between previous and current luminance numerator
+		 * brightness > signal: find next data point
+		 */
+		if (brightness < signal)
+			lum = prev_lum + DIV_ROUND_CLOSEST((lum - prev_lum) *
+							   (brightness - prev_signal),
+							   signal - prev_signal);
+		else if (brightness > signal) {
+			prev_signal = signal;
+			prev_lum = lum;
+			continue;
+		}
+		brightness = DIV_ROUND_CLOSEST(lum * brightness, 101);
+		break;
+	}
+
 	// Rescale 0..255 to min..max
 	return min + DIV_ROUND_CLOSEST((max - min) * brightness,
 				       AMDGPU_MAX_BL_LEVEL);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] drm/amd/display: Add a new dcdebugmask to allow turning off brightness curve
  2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
                   ` (3 preceding siblings ...)
  2025-02-21 17:10 ` [PATCH 4/5] drm/amd/display: Add support for custom brightness curve Mario Limonciello
@ 2025-02-21 17:10 ` Mario Limonciello
  4 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2025-02-21 17:10 UTC (permalink / raw)
  To: amd-gfx @ lists . freedesktop . org, Alex Hung
  Cc: Harry Wentland, Mario Limonciello

Upgrading the kernel may cause some systems that were previously not using
a firmware specified brightness curve to use one.

In the event of problems with this curve (for example an interpolation
error) add a new dcdebugmask value that can be used to turn it off.  Also
add an info message to show that custom brightness curves are currently in
use.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++++
 drivers/gpu/drm/amd/include/amd_shared.h          | 4 ++++
 2 files changed, 9 insertions(+)

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 136abfcdb76d..904a2d2a9664 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4712,6 +4712,9 @@ static u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *c
 	for (int i = 0; i < caps->data_points; i++) {
 		u8 signal, lum;
 
+		if (amdgpu_dc_debug_mask & DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE)
+			break;
+
 		signal = caps->luminance_data[i].input_signal;
 		lum = caps->luminance_data[i].luminance;
 
@@ -4896,6 +4899,8 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
 	} else
 		props.brightness = AMDGPU_MAX_BL_LEVEL;
 
+	if (caps.data_points && !(amdgpu_dc_debug_mask & DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE))
+		drm_info(drm, "Using custom brightness curve\n");
 	props.max_brightness = AMDGPU_MAX_BL_LEVEL;
 	props.type = BACKLIGHT_RAW;
 
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index c0538763ec1a..485b713cfad0 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -354,6 +354,10 @@ enum DC_DEBUG_MASK {
 	 * @DC_DISABLE_SUBVP: If set, disable DCN Sub-Viewport feature in amdgpu driver.
 	 */
 	DC_DISABLE_SUBVP = 0x20000,
+	/**
+	 * @DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE: If set, disable support for custom brightness curves
+	 */
+	DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE = 0x40000,
 };
 
 enum amd_dpm_forced_level;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps
  2025-02-21 17:09 ` [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps Mario Limonciello
@ 2025-02-24 21:26   ` Deucher, Alexander
  0 siblings, 0 replies; 8+ messages in thread
From: Deucher, Alexander @ 2025-02-24 21:26 UTC (permalink / raw)
  To: Limonciello, Mario, amd-gfx @ lists . freedesktop . org,
	Hung, Alex
  Cc: Wentland, Harry, Limonciello, Mario

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mario
> Limonciello
> Sent: Friday, February 21, 2025 12:10 PM
> To: amd-gfx @ lists . freedesktop . org <amd-gfx@lists.freedesktop.org>; Hung,
> Alex <Alex.Hung@amd.com>
> Cc: Wentland, Harry <Harry.Wentland@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: [PATCH 2/5] drm/amd: Pass luminance data to
> amdgpu_dm_backlight_caps
>
> The ATIF method on some systems will provide a backlight curve. Pass this curve
> into amdgpu_dm add it to the structures.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c      |  4 ++++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 20 +++++++++++++++++++
>  drivers/gpu/drm/amd/include/amd_acpi.h        |  9 +--------
>  3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 515c6f32448d..b7f8f2ff143d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -394,6 +394,10 @@ static int amdgpu_atif_query_backlight_caps(struct
> amdgpu_atif *atif)
>                       characteristics.max_input_signal;
>       atif->backlight_caps.ac_level = characteristics.ac_level;
>       atif->backlight_caps.dc_level = characteristics.dc_level;
> +     atif->backlight_caps.data_points = characteristics.number_of_points;
> +     memcpy(atif->backlight_caps.luminance_data,
> +            characteristics.data_points,
> +            sizeof(atif->backlight_caps.luminance_data));
>  out:
>       kfree(info);
>       return err;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index f3bc00e587ad..85b64c457ed6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -151,6 +151,18 @@ struct idle_workqueue {
>       bool running;
>  };
>
> +#define MAX_LUMINANCE_DATA_POINTS 99
> +
> +/**
> + * struct amdgpu_dm_luminance_data - Custom luminance data
> + * @luminance: Luminance in percent
> + * @input_signal: Input signal in range 0-255  */ struct
> +amdgpu_dm_luminance_data {
> +     u8 luminance;
> +     u8 input_signal;
> +} __packed;
> +
>  /**
>   * struct amdgpu_dm_backlight_caps - Information about backlight
>   *
> @@ -195,6 +207,14 @@ struct amdgpu_dm_backlight_caps {
>        * @dc_level: the default brightness if booted on DC
>        */
>       u8 dc_level;
> +     /**
> +      * @data_points: the number of custom luminance data points
> +      */
> +     u8 data_points;
> +     /**
> +      * @luminance_data: custom luminance data
> +      */
> +     struct amdgpu_dm_luminance_data
> +luminance_data[MAX_LUMINANCE_DATA_POINTS];
>  };
>
>  /**
> diff --git a/drivers/gpu/drm/amd/include/amd_acpi.h
> b/drivers/gpu/drm/amd/include/amd_acpi.h
> index 2d089d30518f..63713fdca428 100644
> --- a/drivers/gpu/drm/amd/include/amd_acpi.h
> +++ b/drivers/gpu/drm/amd/include/amd_acpi.h
> @@ -57,13 +57,6 @@ struct atif_qbtc_arguments {
>       u8 requested_display;   /* which display is requested */
>  } __packed;
>
> -#define ATIF_QBTC_MAX_DATA_POINTS 99
> -
> -struct atif_qbtc_data_point {
> -     u8 luminance;           /* luminance in percent */
> -     u8 ipnut_signal;        /* input signal in range 0-255 */
> -} __packed;


I'd be careful here lest someone changes the definition of struct amdgpu_dm_luminance_data not realizing that it used here as well.  The ACPI definition should be separate IMHO.

Alex


> -
>  struct atif_qbtc_output {
>       u16 size;               /* structure size in bytes (includes size field) */
>       u16 flags;              /* all zeroes */
> @@ -73,7 +66,7 @@ struct atif_qbtc_output {
>       u8 min_input_signal;    /* max input signal in range 0-255 */
>       u8 max_input_signal;    /* min input signal in range 0-255 */
>       u8 number_of_points;    /* number of data points */
> -     struct atif_qbtc_data_point data_points[ATIF_QBTC_MAX_DATA_POINTS];
> +     struct amdgpu_dm_luminance_data
> +data_points[MAX_LUMINANCE_DATA_POINTS];
>  } __packed;
>
>  #define ATIF_NOTIFY_MASK     0x3
> --
> 2.48.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps
  2025-02-21 17:10 ` [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps Mario Limonciello
@ 2025-02-28 18:23   ` Alex Hung
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Hung @ 2025-02-28 18:23 UTC (permalink / raw)
  To: Mario Limonciello, amd-gfx @ lists . freedesktop . org
  Cc: Harry Wentland, Wayne Lin



On 2/21/25 10:10, Mario Limonciello wrote:
> Making a copy of the backlight caps structure between uses is unnecessary.
> Refer to pointers to the same structure when using it.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 ++++++++-----------
>   1 file changed, 21 insertions(+), 29 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 0d21448ea700..70c8d800e173 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4646,47 +4646,39 @@ static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm,
>   					    int bl_idx)
>   {
>   #if defined(CONFIG_ACPI)
> -	struct amdgpu_dm_backlight_caps caps;
> -
> -	memset(&caps, 0, sizeof(caps));
> +	struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[bl_idx];
>   
> -	if (dm->backlight_caps[bl_idx].caps_valid)
> +	if (caps->caps_valid)
>   		return;
>   
> -	amdgpu_acpi_get_backlight_caps(&caps);
> +	amdgpu_acpi_get_backlight_caps(caps);
>   
>   	/* validate the firmware value is sane */
> -	if (caps.caps_valid) {
> -		int spread = caps.max_input_signal - caps.min_input_signal;
> +	if (caps->caps_valid) {
> +		int spread = caps->max_input_signal - caps->min_input_signal;
>   
> -		if (caps.max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
> -		    caps.min_input_signal < 0 ||
> +		if (caps->max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
> +		    caps->min_input_signal < 0 ||
>   		    spread > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
>   		    spread < AMDGPU_DM_MIN_SPREAD) {
>   			DRM_DEBUG_KMS("DM: Invalid backlight caps: min=%d, max=%d\n",
> -				      caps.min_input_signal, caps.max_input_signal);
> -			caps.caps_valid = false;
> +				      caps->min_input_signal, caps->max_input_signal);
> +			caps->caps_valid = false;
>   		}
>   	}
>   
> -	if (caps.caps_valid) {
> -		dm->backlight_caps[bl_idx].caps_valid = true;
> -		if (caps.aux_support)
> -			return;
> -		dm->backlight_caps[bl_idx].min_input_signal = caps.min_input_signal;
> -		dm->backlight_caps[bl_idx].max_input_signal = caps.max_input_signal;
> -	} else {
> -		dm->backlight_caps[bl_idx].min_input_signal =
> -				AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> -		dm->backlight_caps[bl_idx].max_input_signal =
> -				AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	if (!caps->caps_valid) {
> +		caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> +		caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +		caps->caps_valid = true;
>   	}
>   #else
> -	if (dm->backlight_caps[bl_idx].aux_support)
> +	if (caps->aux_support)
>   		return;
>   
> -	dm->backlight_caps[bl_idx].min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> -	dm->backlight_caps[bl_idx].max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> +	caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	caps->caps_valid = true;


caps is not defined in "#else" so this fails when CONFIG_ACPI is not 
defined.

Below are errors messages for your references

[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4714:6: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         if (caps->aux_support)
[2025-02-27T05:12:05.659Z]             ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4717:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->min_input_signal = 
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4718:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->max_input_signal = 
AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4719:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->caps_valid = true;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.760Z] 4 errors generated.

>   #endif
>   }
>   
> @@ -4740,19 +4732,19 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
>   					 int bl_idx,
>   					 u32 user_brightness)
>   {
> -	struct amdgpu_dm_backlight_caps caps;
> +	struct amdgpu_dm_backlight_caps *caps;
>   	struct dc_link *link;
>   	u32 brightness;
>   	bool rc, reallow_idle = false;
>   
>   	amdgpu_dm_update_backlight_caps(dm, bl_idx);
> -	caps = dm->backlight_caps[bl_idx];
> +	caps = &dm->backlight_caps[bl_idx];
>   
>   	dm->brightness[bl_idx] = user_brightness;
>   	/* update scratch register */
>   	if (bl_idx == 0)
>   		amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, dm->brightness[bl_idx]);
> -	brightness = convert_brightness_from_user(&caps, dm->brightness[bl_idx]);
> +	brightness = convert_brightness_from_user(caps, dm->brightness[bl_idx]);
>   	link = (struct dc_link *)dm->backlight_link[bl_idx];
>   
>   	/* Change brightness based on AUX property */
> @@ -4762,7 +4754,7 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
>   		reallow_idle = true;
>   	}
>   
> -	if (caps.aux_support) {
> +	if (caps->aux_support) {
>   		rc = dc_link_set_backlight_level_nits(link, true, brightness,
>   						      AUX_BL_DEFAULT_TRANSITION_TIME_MS);
>   		if (!rc)


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-02-28 18:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-21 17:09 [PATCH 0/5] Add custom brightness curve support Mario Limonciello
2025-02-21 17:09 ` [PATCH 1/5] drm/amd: Copy entire structure in amdgpu_acpi_get_backlight_caps() Mario Limonciello
2025-02-21 17:09 ` [PATCH 2/5] drm/amd: Pass luminance data to amdgpu_dm_backlight_caps Mario Limonciello
2025-02-24 21:26   ` Deucher, Alexander
2025-02-21 17:10 ` [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps Mario Limonciello
2025-02-28 18:23   ` Alex Hung
2025-02-21 17:10 ` [PATCH 4/5] drm/amd/display: Add support for custom brightness curve Mario Limonciello
2025-02-21 17:10 ` [PATCH 5/5] drm/amd/display: Add a new dcdebugmask to allow turning off " Mario Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox