Message-ID: <20251221200434.2735218-2-samuel@dionne-riel.com>
Date: Sun, 21 Dec 2025 15:04:34 -0500
From: Samuel Dionne-Riel <samuel@...nne-riel.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>,
	Simona Vetter <simona@...ll.ch>
Cc: Samuel Dionne-Riel <samuel@...nne-riel.com>,
	dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org
Subject: [RFC PATCH] drm: panel-backlight-quirks: Add kernel parameters for quirks

The goal is to enable end-users to try the different values quickly, so
that they can document, for example, the better minimum backlight values
for their systems.

All quirks should be implemented, since this is a change meant to
empower end-users in providing quick and complete quirks data for later
addition to the quirks data.

This is attempting to keep the largely unspecified behaviour as close as
possible to the previous behaviour.

This is currently an amdgpu-specific driver, but it shouldn't stay
specific to that driver. The current usage always copies the values to
its own runtime data (caps, in amdgpu).

Without knowing the contract between this module and other modules, it's
impossible to know if the driver could instead keep the pointer to the
returned quirk.

This quirk also currently has no multi-panel uses, but theoretically
could be used on a system with dual screens, and with multiple different
EDID matched panels.

As such, the change has been authored to harden against this situation,
and ensure that when no parameter has been given, that this would
continue returning the same values (i.e. pointing at the quirk data
structure elements directly).

It may be preferrable to instead document that the values returned by
this function are only good until then next time the function is called.

Or it may be that filling a caller-owned structure may be preferred, so
that there never is any concurrency concerns.

Signed-off-by: Samuel Dionne-Riel <samuel@...nne-riel.com>

---

This is sent as an RFC as I have basically no idea if this is correct,
or desirable in its form.

This was authored on top of the backlight curve changes, so this exact
implementation depends on "Add quirk to disable custom brightness
curve", but decoupling it is trivial, if for some reason that change
gets held back for longer than this one.

As stated in the patch, the rationale is that end-users should be able
to quickly try values without having to mess with either a full kernel
rebuild (easiest) or somehow messing with their systems so an updated
quirks module is loaded before amdgpu gets loaded.

I believe that the current semantics are *sufficient*, and that
supporting more than one display on the kernel cmdline might not be
worth the pain, when this would be a rare situation, and mainly as a
means to gather the correct values for sending the appropriate changes.

The implementation, though, I don't know how good or bad it is, and how
palatable specifically for its maintainers. So please provide
suggestions on how this should be done.

Thanks!

Link: https://lore.kernel.org/all/20251221192422.2726883-1-samuel@dionne-riel.com/
Signed-off-by: Samuel Dionne-Riel <samuel@...nne-riel.com>
---
 drivers/gpu/drm/drm_panel_backlight_quirks.c | 79 +++++++++++++++++---
 1 file changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
index 537dc6dd05343..1682409613d22 100644
--- a/drivers/gpu/drm/drm_panel_backlight_quirks.c
+++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
@@ -8,6 +8,21 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_utils.h>
 
+// The parameters use the type-specific max value as a flag for being unset.
+// The semantics of these quirk values are drivers and device-specific.
+
+u16 param_min_brightness = U16_MAX;
+MODULE_PARM_DESC(min_brightness, "minimum brightness override for all panel backlights. Value usage is driver-specific.");
+module_param_named(min_brightness, param_min_brightness, ushort, 0444);
+
+u32 param_brightness_mask = U32_MAX;
+MODULE_PARM_DESC(brightness_mask, "integer mask to bitwise OR with set brightness values for panel-specific fixes. Value usage is driver-specific.");
+module_param_named(brightness_mask, param_brightness_mask, uint, 0444);
+
+bool param_disable_custom_brightness_curve = false;
+MODULE_PARM_DESC(disable_custom_brightness_curve, "when true, custom brightness curve support is disabled in the driver. Value usage is driver-specific.");
+module_param_named(disable_custom_brightness_curve, param_disable_custom_brightness_curve, bool, 0444);
+
 struct drm_panel_match {
 	enum dmi_field field;
 	const char * const value;
@@ -98,6 +113,8 @@ static const struct drm_get_panel_backlight_quirk drm_panel_min_backlight_quirks
 	},
 };
 
+static struct drm_panel_backlight_quirk quirk_from_param = { };
+
 static bool drm_panel_min_backlight_quirk_matches(
 	const struct drm_get_panel_backlight_quirk *quirk,
 	const struct drm_edid *edid)
@@ -132,23 +149,65 @@ static bool drm_panel_min_backlight_quirk_matches(
 const struct drm_panel_backlight_quirk *
 drm_get_panel_backlight_quirk(const struct drm_edid *edid)
 {
-	const struct drm_get_panel_backlight_quirk *quirk;
+	const struct drm_get_panel_backlight_quirk *quirk = NULL;
 	size_t i;
+	bool parameters_given = false;
 
-	if (!IS_ENABLED(CONFIG_DMI))
-		return ERR_PTR(-ENODATA);
+	if (param_min_brightness < U16_MAX)
+		parameters_given = true;
 
-	if (!edid)
-		return ERR_PTR(-EINVAL);
+	if (param_brightness_mask < U32_MAX)
+		parameters_given = true;
 
-	for (i = 0; i < ARRAY_SIZE(drm_panel_min_backlight_quirks); i++) {
-		quirk = &drm_panel_min_backlight_quirks[i];
+	if (param_disable_custom_brightness_curve)
+		parameters_given = true;
 
-		if (drm_panel_min_backlight_quirk_matches(quirk, edid))
-			return &quirk->quirk;
+	if (IS_ENABLED(CONFIG_DMI) && edid) {
+		for (i = 0; i < ARRAY_SIZE(drm_panel_min_backlight_quirks); i++) {
+			quirk = &drm_panel_min_backlight_quirks[i];
+
+			if (drm_panel_min_backlight_quirk_matches(quirk, edid))
+				break;
+		}
 	}
 
-	return ERR_PTR(-ENODATA);
+	if (i == ARRAY_SIZE(drm_panel_min_backlight_quirks)) {
+		// Found no quirk.
+		quirk = NULL;
+	}
+
+	if (!parameters_given && !quirk) {
+		if (!edid)
+			return ERR_PTR(-EINVAL);
+
+		return ERR_PTR(-ENODATA);
+	}
+
+	// We are returning from quirk_from_param only when parameters are given.
+	// This assumes that the return value's data can be kept by the driver,
+	// and that the driver supports more than one match at a time.
+	// (Think multiple-display devices.)
+	// When using a module parameter, only one set of values is supported.
+	if (parameters_given) {
+		// First copy the matched data, when found.
+		if (quirk)
+			memcpy(&quirk_from_param, &quirk->quirk, sizeof(quirk_from_param));
+
+		// Apply module parameters to the found quirk.
+		if (param_min_brightness < U16_MAX)
+			quirk_from_param.min_brightness = param_min_brightness;
+		if (param_brightness_mask < U32_MAX)
+			quirk_from_param.brightness_mask = param_brightness_mask;
+
+		// We force the disable_custom_brightness_curve value when any parameter is given.
+		// This ensures users can override the quirk-defined value.
+		// This comes at the cost of requiring any other value to be specified to work.
+		quirk_from_param.disable_custom_brightness_curve = param_disable_custom_brightness_curve;
+
+		return &quirk_from_param;
+	}
+
+	return &quirk->quirk;
 }
 EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
 
-- 
2.51.0

Powered by blists - more mailing lists