Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
From: Mario Limonciello
Date: Mon Jun 10 2024 - 16:20:38 EST
On 6/10/2024 15:12, Thomas Weißschuh wrote:
On 2024-06-10 14:58:02+0000, Mario Limonciello wrote:
+Kieran
On 6/10/2024 14:26, Thomas Weißschuh wrote:
The value of "min_input_signal" returned from ATIF on a Framework AMD 13
is "12". This leads to a fairly bright minimum display backlight.
Introduce a quirk to override "min_input_signal" to "0" which leads to a
much lower minimum brightness, which is still readable even in daylight.
Tested on a Framework AMD 13 BIOS 3.05 and Framework AMD 16.
Link: https://community.frame.work/t/25711/9
Link: https://community.frame.work/t/47036
Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 35 ++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 7099ff9cf8c5..b481889f7491 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -25,6 +25,7 @@
#include <linux/pci.h>
#include <linux/acpi.h>
#include <linux/backlight.h>
+#include <linux/dmi.h>
#include <linux/slab.h>
#include <linux/xarray.h>
#include <linux/power_supply.h>
@@ -130,6 +131,35 @@ static struct amdgpu_acpi_priv {
struct amdgpu_atcs atcs;
} amdgpu_acpi_priv;
+struct amdgpu_acpi_quirks {
+ bool ignore_min_input_signal;
+};
+
+static const struct dmi_system_id amdgpu_acpi_quirk_table[] = {
+ {
+ /* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+ },
Two problems I see:
1) This really "should" be fixed in the BIOS. I added Kieran to the thread
for comments if that's viable.
Agreed!
2) IMO this is going to match too liberally across all potential Framework
models. If they introduce a refreshed motherboard for either product then
the quirk would apply to both products when we don't know that such a
deficiency would exist.
Also agreed.
In addition to be really specific this should also match by display type
(via EDID?).
So far this was only tested with the matte panel.
(I forgot to mention that, sorry)
Yeah; I would expect this also matters for the new high res panel that
they announced whether this value can work.
You can reference drivers/platform/x86/amd/pmc/pmc-quirks.c for what we used
for a quirk that was matching against a single product and single BIOS.
Will do for the next revision, but let's gather some feedback first.
👍
But FWIW if that issue isn't fixed in the next BIOS I think we'll end up
needing to tear out the BIOS string match and match just the platform.
I'm wondering what the longterm strategy will have to be.
Given that there are different kinds of displays, and new ones will be
released, each new display type will require an update to the firmware.
When there are no firmware updates for a device anymore, but new,
compatible displays are released, then the kernel will need the quirks
again.
Yeah I think all this points to the 'best' home for this is BIOS.
Framework can test whether the 0 value works on all the displays they
want to support and look for negative impacts for all OSes they support.
+ .driver_data = &(struct amdgpu_acpi_quirks) {
+ .ignore_min_input_signal = true,
+ },
+ },
+ {}
+};
+
+static const struct amdgpu_acpi_quirks *amdgpu_acpi_get_quirks(void)
+{
+ const struct dmi_system_id *dmi_id;
+
+ dmi_id = dmi_first_match(amdgpu_acpi_quirk_table);
+ if (!dmi_id)
+ return NULL;
+ return dmi_id->driver_data;
+}
+
/* Call the ATIF method
*/
/**
@@ -1388,6 +1418,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
*/
void amdgpu_acpi_detect(void)
{
+ const struct amdgpu_acpi_quirks *quirks = amdgpu_acpi_get_quirks();
struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
struct amdgpu_atcs *atcs = &amdgpu_acpi_priv.atcs;
struct pci_dev *pdev = NULL;
@@ -1429,6 +1460,10 @@ void amdgpu_acpi_detect(void)
ret);
atif->backlight_caps.caps_valid = false;
}
+ if (quirks && quirks->ignore_min_input_signal) {
+ DRM_INFO("amdgpu_acpi quirk: min_input_signal=0\n");
+ atif->backlight_caps.min_input_signal = 0;
+ }
} else {
atif->backlight_caps.caps_valid = false;
}
---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
Best regards,