Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
On 6/10/2024 15:12, Thomas Weißschuh wrote:
On 2024-06-10 14:58:02+, 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
---
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
#include
#include
+#include
#include
#include
#include
@@ -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 = _acpi_priv.atif;
struct amdgpu_atcs *atcs = _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,
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
On 6/10/2024 15:12, Thomas Weißschuh wrote:
On 2024-06-10 14:58:02+, 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
---
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
#include
#include
+#include
#include
#include
#include
@@ -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 = _acpi_priv.atif;
struct amdgpu_atcs *atcs = _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,
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
+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
---
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
#include
#include
+#include
#include
#include
#include
@@ -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.
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.
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.
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.
+ .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 = _acpi_priv.atif;
struct amdgpu_atcs *atcs = _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,
Re: [PATCH] drm/amd: force min_input_signal to 0 on Framework AMD 13/16
+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
---
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
#include
#include
+#include
#include
#include
#include
@@ -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.
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.
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.
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.
+ .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 = _acpi_priv.atif;
struct amdgpu_atcs *atcs = _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,
Re: [PATCH v2] drm/amd/display: Guard ACPI calls with CONFIG_ACPI
On 6/10/2024 10:58, sunpeng...@amd.com wrote:
From: Leo Li
To fix CONFIG_ACPI disabled build error.
v2: Instead of ifdef-ing inside function, define a no-op stub for
amdgpu_acpi_get_backlight_caps when CONFIG_ACPI=n
Fixes: ec6f30c776ad ("drm/amd/display: Set default brightness according to
ACPI")
Signed-off-by: Leo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1f71c7b98d77..083f353cff6e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1576,6 +1576,7 @@ static inline int amdgpu_acpi_power_shift_control(struct
amdgpu_device *adev,
u8 dev_state, bool drv_state)
{ return 0; }
static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev,
enum amdgpu_ss ss_state) {
return 0; }
+static inline void amdgpu_acpi_get_backlight_caps(struct
amdgpu_dm_backlight_caps *caps) { }
#endif
#if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
Reviewed-by: Mario Limonciello
Re: [PATCH] drm/amd/display: Guard ACPI calls with CONFIG_ACPI
On 6/10/2024 09:55, sunpeng...@amd.com wrote:
From: Leo Li
To fix CONFIG_ACPI disabled build error.
Fixes: ec6f30c776ad ("drm/amd/display: Set default brightness according to
ACPI")
Signed-off-by: Leo Li
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++
1 file changed, 6 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 a2c098f1b07c..6b3634db4c15 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4572,7 +4572,9 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_dm_connector *aconnector)
struct drm_device *drm = aconnector->base.dev;
struct amdgpu_display_manager *dm = _to_adev(drm)->dm;
struct backlight_properties props = { 0 };
+#if defined(CONFIG_ACPI)
struct amdgpu_dm_backlight_caps caps = { 0 };
+#endif
char bl_name[16];
if (aconnector->bl_idx == -1)
@@ -4585,6 +4587,7 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_dm_connector *aconnector)
return;
}
+#if defined(CONFIG_ACPI)
amdgpu_acpi_get_backlight_caps();
if (caps.caps_valid) {
if (power_supply_is_system_supplied() > 0)
@@ -4593,6 +4596,9 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_dm_connector *aconnector)
props.brightness = caps.dc_level;
} else
props.brightness = AMDGPU_MAX_BL_LEVEL;
+#else
+ props.brightness = AMDGPU_MAX_BL_LEVEL;
+#endif
Hey Leo,
Thanks for the patch!
As caps is initialized to {0} caps.caps_valid will be invalid. So I see
two other ways to solve this that are a little cleaner (IMO):
1) Just block the one call:
#if defined(CONFIG_ACPI)
amdgpu_acpi_get_backlight_caps();
#endif
2) Add a stub inline no-op function for amdgpu_acpi_get_backlight_caps()
to the header.
I personally think #2 is cleaner (less ifdef makes a lot more readable
code).
props.max_brightness = AMDGPU_MAX_BL_LEVEL;
props.type = BACKLIGHT_RAW;
[Bug 2068738] Re: Kernel update 5.15.0-112 might cause severe problems with specific AMD GPUs
AFAIK It's a bad backport to 5.15 stable. If it's what I think, here's the fix (IIRC). https://lore.kernel.org/amd-gfx/20240523173031.4212-1-w_ar...@gmx.de/ Try applying that to your 5.15 kernel. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2068738 Title: Kernel update 5.15.0-112 might cause severe problems with specific AMD GPUs To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2068738/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2068793] Re: Black screen on restart after update: fix "nomodeset"
AFAIK It's a bad backport to 5.15 stable. If it's what I think, here's the fix (IIRC). https://lore.kernel.org/amd-gfx/20240523173031.4212-1-w_ar...@gmx.de/ Try applying that to your 5.15 kernel. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2068793 Title: Black screen on restart after update: fix "nomodeset" To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2068793/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[PATCH] drm/amd/display: Set default brightness according to ACPI
Currently, amdgpu will always set up the brightness at 100% when it
loads. However this is jarring when the BIOS has it previously
programmed to a much lower value.
The ACPI ATIF method includes two members for "ac_level" and "dc_level".
These represent the default values that should be used if the system is
brought up in AC and DC respectively.
Use these values to set up the default brightness when the backlight
device is registered.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++-
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 7099ff9cf8c5..f85ace0384d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -383,6 +383,8 @@ static int amdgpu_atif_query_backlight_caps(struct
amdgpu_atif *atif)
characteristics.min_input_signal;
atif->backlight_caps.max_input_signal =
characteristics.max_input_signal;
+ atif->backlight_caps.ac_level = characteristics.ac_level;
+ atif->backlight_caps.dc_level = characteristics.dc_level;
out:
kfree(info);
return err;
@@ -1268,6 +1270,8 @@ void amdgpu_acpi_get_backlight_caps(struct
amdgpu_dm_backlight_caps *caps)
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;
}
/**
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 5fd7210b2479..71aa0c518951 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -77,6 +77,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -4321,6 +4322,7 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_dm_connector *aconnector)
struct drm_device *drm = aconnector->base.dev;
struct amdgpu_display_manager *dm = _to_adev(drm)->dm;
struct backlight_properties props = { 0 };
+ struct amdgpu_dm_backlight_caps caps = { 0 };
char bl_name[16];
if (aconnector->bl_idx == -1)
@@ -4333,8 +4335,16 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_dm_connector *aconnector)
return;
}
+ amdgpu_acpi_get_backlight_caps();
+ if (caps.caps_valid) {
+ if (power_supply_is_system_supplied() > 0)
+ props.brightness = caps.ac_level;
+ else
+ props.brightness = caps.dc_level;
+ } else
+ props.brightness = AMDGPU_MAX_BL_LEVEL;
+
props.max_brightness = AMDGPU_MAX_BL_LEVEL;
- props.brightness = AMDGPU_MAX_BL_LEVEL;
props.type = BACKLIGHT_RAW;
snprintf(bl_name, sizeof(bl_name), "amdgpu_bl%d",
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 b246e82f5b0d..df72cb71e95a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -173,6 +173,14 @@ struct amdgpu_dm_backlight_caps {
* @aux_support: Describes if the display supports AUX backlight.
*/
bool aux_support;
+ /**
+* @ac_level: the default brightness if booted on AC
+*/
+ u8 ac_level;
+ /**
+* @dc_level: the default brightness if booted on DC
+*/
+ u8 dc_level;
};
/**
--
2.43.0
Re: [PATCH v2] drm/amdgpu: Fix the BO release clear memory warning
On 6/6/2024 15:04, Arunpravin Paneer Selvam wrote: This happens when the amdgpu_bo_release_notify running before amdgpu_ttm_set_buffer_funcs_status set the buffer funcs to enabled. check the buffer funcs enablement before calling the fill buffer memory. v2:(Christian) - Apply it only for GEM buffers and since GEM buffers are only allocated/freed while the driver is loaded we never run into the issue to clear with buffer funcs disabled. Log snip: [6.036477] [drm:amdgpu_fill_buffer [amdgpu]] *ERROR* Trying to clear memory with ring turned off. [6.036667] [ cut here ] [6.036668] WARNING: CPU: 3 PID: 370 at drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:1355 amdgpu_bo_release_notify+0x201/0x220 [amdgpu] [6.036767] Modules linked in: hid_generic amdgpu(+) amdxcp drm_exec gpu_sched drm_buddy i2c_algo_bit usbhid drm_suballoc_helper drm_display_helper hid sd_mod cec rc_core drm_ttm_helper ahci ttm nvme libahci drm_kms_helper nvme_core r8169 xhci_pci libata t10_pi xhci_hcd realtek crc32_pclmul crc64_rocksoft mdio_devres crc64 drm crc32c_intel scsi_mod usbcore thunderbolt crc_t10dif i2c_piix4 libphy crct10dif_generic crct10dif_pclmul crct10dif_common scsi_common usb_common video wmi gpio_amdpt gpio_generic button [6.036793] CPU: 3 PID: 370 Comm: (udev-worker) Not tainted 6.8.7-dirty #1 [6.036795] Hardware name: ASRock X670E Taichi/X670E Taichi, BIOS 2.10 03/26/2024 [6.036796] RIP: 0010:amdgpu_bo_release_notify+0x201/0x220 [amdgpu] [6.036891] Code: 0b e9 af fe ff ff 48 ba ff ff ff ff ff ff ff 7f 31 f6 4c 89 e7 e8 7f 2f 7a d8 eb 98 e8 18 28 7a d8 eb b2 0f 0b e9 58 fe ff ff <0f> 0b eb a7 be 03 00 00 00 e8 e1 89 4e d8 eb 9b e8 aa 4d ad d8 66 [6.036892] RSP: 0018:bbe140d1f638 EFLAGS: 00010282 [6.036894] RAX: ffea RBX: 90cba9e4e858 RCX: 90dabde38c28 [6.036895] RDX: RSI: dfff RDI: 0001 [6.036896] RBP: 90cba980ef40 R08: R09: bbe140d1f3c0 [6.036896] R10: bbe140d1f3b8 R11: 0003 R12: 90cba9e4e800 [6.036897] R13: 90cba9e4e958 R14: 90cba980ef40 R15: 0258 [6.036898] FS: 7f2bd1679d00() GS:90da7e2c() knlGS: [6.036899] CS: 0010 DS: ES: CR0: 80050033 [6.036900] CR2: 55a9b0f7299d CR3: 00011bb6e000 CR4: 00750ef0 [6.036901] PKRU: 5554 [6.036901] Call Trace: [6.036903] [6.036904] ? amdgpu_bo_release_notify+0x201/0x220 [amdgpu] [6.036998] ? __warn+0x81/0x130 [6.037002] ? amdgpu_bo_release_notify+0x201/0x220 [amdgpu] [6.037095] ? report_bug+0x171/0x1a0 [6.037099] ? handle_bug+0x3c/0x80 [6.037101] ? exc_invalid_op+0x17/0x70 [6.037103] ? asm_exc_invalid_op+0x1a/0x20 [6.037107] ? amdgpu_bo_release_notify+0x201/0x220 [amdgpu] [6.037199] ? amdgpu_bo_release_notify+0x14a/0x220 [amdgpu] [6.037292] ttm_bo_release+0xff/0x2e0 [ttm] [6.037297] ? srso_alias_return_thunk+0x5/0xfbef5 [6.037299] ? srso_alias_return_thunk+0x5/0xfbef5 [6.037301] ? ttm_resource_move_to_lru_tail+0x140/0x1e0 [ttm] [6.037306] amdgpu_bo_free_kernel+0xcb/0x120 [amdgpu] [6.037399] dm_helpers_free_gpu_mem+0x41/0x80 [amdgpu] [6.037544] dcn315_clk_mgr_construct+0x198/0x7e0 [amdgpu] [6.037692] dc_clk_mgr_create+0x16e/0x5f0 [amdgpu] [6.037826] dc_create+0x28a/0x650 [amdgpu] [6.037958] amdgpu_dm_init.isra.0+0x2d5/0x1ec0 [amdgpu] [6.038085] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038087] ? prb_read_valid+0x1b/0x30 [6.038089] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038090] ? console_unlock+0x78/0x120 [6.038092] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038094] ? vprintk_emit+0x175/0x2c0 [6.038095] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038097] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038098] ? dev_printk_emit+0xa5/0xd0 [6.038104] dm_hw_init+0x12/0x30 [amdgpu] [6.038209] amdgpu_device_init+0x1e50/0x2500 [amdgpu] [6.038308] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038310] ? srso_alias_return_thunk+0x5/0xfbef5 [6.038313] amdgpu_driver_load_kms+0x19/0x190 [amdgpu] [6.038409] amdgpu_pci_probe+0x18b/0x510 [amdgpu] [6.038505] local_pci_probe+0x42/0xa0 [6.038508] pci_device_probe+0xc7/0x240 [6.038510] really_probe+0x19b/0x3e0 [6.038513] ? __pfx___driver_attach+0x10/0x10 [6.038514] __driver_probe_device+0x78/0x160 [6.038516] driver_probe_device+0x1f/0x90 [6.038517] __driver_attach+0xd2/0x1c0 [6.038519] bus_for_each_dev+0x85/0xd0 [6.038521] bus_add_driver+0x116/0x220 [6.038523] driver_register+0x59/0x100 [6.038525] ? __pfx_amdgpu_init+0x10/0x10 [amdgpu] [6.038618] do_one_initcall+0x58/0x320 [6.038621] do_init_module+0x60/0x230 [6.038624] init_module_from_file+0x89/0xe0 [6.038628] idempotent_init_module+0x120/0x2b0 [
[Bug 2067997] Re: Lenovo T14 Gen3 AMD laptop freezes shortly after suspend resume
*** This bug is a duplicate of bug 2064595 *** https://bugs.launchpad.net/bugs/2064595 ** This bug has been marked a duplicate of bug 2064595 S2idle regression -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2067997 Title: Lenovo T14 Gen3 AMD laptop freezes shortly after suspend resume To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2067997/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [syzbot] [mm?] general protection fault in dequeue_hugetlb_folio_nodemask
On 6/6/2024 09:39, syzbot wrote:
Hello,
syzbot found the following issue on:
HEAD commit:0e1980c40b6e Add linux-next specific files for 20240531
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=166086f298
kernel config: https://syzkaller.appspot.com/x/.config?x=d9c3ca4e54577b88
dashboard link: https://syzkaller.appspot.com/bug?extid=c019f68a83ef9b456444
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian)
2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12f4094a98
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15e1e43298
Downloadable assets:
disk image:
https://storage.googleapis.com/syzbot-assets/44fb1d8b5978/disk-0e1980c4.raw.xz
vmlinux:
https://storage.googleapis.com/syzbot-assets/a66ce5caf0b2/vmlinux-0e1980c4.xz
kernel image:
https://storage.googleapis.com/syzbot-assets/8992fc8fe046/bzImage-0e1980c4.xz
The issue was bisected to:
commit cd94d1b182d2986378550c9087571991bfee01d4
Author: Mario Limonciello
Date: Thu May 2 18:32:17 2024 +
dm/amd/pm: Fix problems with reboot/shutdown for some SMU 13.0.4/13.0.11
users
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=176121c298
console output: https://syzkaller.appspot.com/x/log.txt?x=10e121c298
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+c019f68a83ef9b456...@syzkaller.appspotmail.com
Fixes: cd94d1b182d2 ("dm/amd/pm: Fix problems with reboot/shutdown for some SMU
13.0.4/13.0.11 users")
Oops: general protection fault, probably for non-canonical address
0xdc000489: [#1] PREEMPT SMP KASAN PTI
KASAN: probably user-memory-access in range
[0x2448-0x244f]
CPU: 1 PID: 5089 Comm: syz-executor257 Not tainted
6.10.0-rc1-next-20240531-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
04/02/2024
RIP: 0010:zonelist_zone_idx include/linux/mmzone.h:1613 [inline]
RIP: 0010:next_zones_zonelist include/linux/mmzone.h:1644 [inline]
RIP: 0010:first_zones_zonelist include/linux/mmzone.h:1670 [inline]
RIP: 0010:dequeue_hugetlb_folio_nodemask+0x193/0xe40 mm/hugetlb.c:1362
Code: 13 9b a0 ff c7 44 24 14 00 00 00 00 83 7c 24 40 00 0f 85 97 0c 00 00 48 83 7c
24 20 00 0f 85 45 09 00 00 48 89 d8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 58
09 00 00 44 8b 33 44 89 f7 8b 5c 24
RSP: 0018:c900035ef720 EFLAGS: 00010002
RAX: 0489 RBX: 2448 RCX: 888026ef
RDX: RSI: RDI:
RBP: c900035ef858 R08: 81f5e070 R09: f520006bdee8
R10: dc00 R11: f520006bdee8 R12:
R13: dc00 R14: R15:
FS: 64010380() GS:8880b950() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 005fdeb8 CR3: 7bd96000 CR4: 003506f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
alloc_hugetlb_folio_nodemask+0xae/0x3f0 mm/hugetlb.c:2603
memfd_alloc_folio+0x15e/0x390 mm/memfd.c:75
memfd_pin_folios+0x1066/0x1720 mm/gup.c:3864
udmabuf_create+0x658/0x11c0 drivers/dma-buf/udmabuf.c:353
udmabuf_ioctl_create drivers/dma-buf/udmabuf.c:420 [inline]
udmabuf_ioctl+0x304/0x4f0 drivers/dma-buf/udmabuf.c:451
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:907 [inline]
__se_sys_ioctl+0xfc/0x170 fs/ioctl.c:893
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f5151a7a369
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6
48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48
c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7ffd962ee9e8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 7ffd962eebb8 RCX: 7f5151a7a369
RDX: 22c0 RSI: 40187542 RDI: 0003
RBP: 7f5151aed610 R08: 7ffd962eebb8 R09: 7ffd962eebb8
R10: 7ffd962eebb8 R11: 0246 R12: 0001
R13: 7ffd962eeba8 R14: 0001 R15: 0001
Modules linked in:
---[ end trace ]---
RIP: 0010:zonelist_zone_idx include/linux/mmzone.h:1613 [inline]
RIP: 0010:next_zones_zonelist include/linux/mmzone.h:1644 [inline]
RIP: 0010:first_zones_zonelist include/linux/mmzone.h:1670 [inline]
RIP: 0010:dequeue_hugetlb_folio_nodemask+0x193/0xe40 mm/hugetlb.c:1362
Code: 13 9b a0 ff c7 44 24 14 00 00 00 00 83 7c 24 40 00 0f 85 97 0c 00 00 48 83 7c
24 20 00 0f 85 45 09 00 00 48 89 d8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 58
09 00 00 44 8b 33 44 89 f7 8b 5c 24
RSP: 0018:c900035ef720 EFLAGS: 00010002
RAX: 04
Re: [syzbot] [mm?] general protection fault in dequeue_hugetlb_folio_nodemask
On 6/6/2024 09:39, syzbot wrote:
Hello,
syzbot found the following issue on:
HEAD commit:0e1980c40b6e Add linux-next specific files for 20240531
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=166086f298
kernel config: https://syzkaller.appspot.com/x/.config?x=d9c3ca4e54577b88
dashboard link: https://syzkaller.appspot.com/bug?extid=c019f68a83ef9b456444
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian)
2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12f4094a98
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15e1e43298
Downloadable assets:
disk image:
https://storage.googleapis.com/syzbot-assets/44fb1d8b5978/disk-0e1980c4.raw.xz
vmlinux:
https://storage.googleapis.com/syzbot-assets/a66ce5caf0b2/vmlinux-0e1980c4.xz
kernel image:
https://storage.googleapis.com/syzbot-assets/8992fc8fe046/bzImage-0e1980c4.xz
The issue was bisected to:
commit cd94d1b182d2986378550c9087571991bfee01d4
Author: Mario Limonciello
Date: Thu May 2 18:32:17 2024 +
dm/amd/pm: Fix problems with reboot/shutdown for some SMU 13.0.4/13.0.11
users
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=176121c298
console output: https://syzkaller.appspot.com/x/log.txt?x=10e121c298
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+c019f68a83ef9b456...@syzkaller.appspotmail.com
Fixes: cd94d1b182d2 ("dm/amd/pm: Fix problems with reboot/shutdown for some SMU
13.0.4/13.0.11 users")
Oops: general protection fault, probably for non-canonical address
0xdc000489: [#1] PREEMPT SMP KASAN PTI
KASAN: probably user-memory-access in range
[0x2448-0x244f]
CPU: 1 PID: 5089 Comm: syz-executor257 Not tainted
6.10.0-rc1-next-20240531-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
04/02/2024
RIP: 0010:zonelist_zone_idx include/linux/mmzone.h:1613 [inline]
RIP: 0010:next_zones_zonelist include/linux/mmzone.h:1644 [inline]
RIP: 0010:first_zones_zonelist include/linux/mmzone.h:1670 [inline]
RIP: 0010:dequeue_hugetlb_folio_nodemask+0x193/0xe40 mm/hugetlb.c:1362
Code: 13 9b a0 ff c7 44 24 14 00 00 00 00 83 7c 24 40 00 0f 85 97 0c 00 00 48 83 7c
24 20 00 0f 85 45 09 00 00 48 89 d8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 58
09 00 00 44 8b 33 44 89 f7 8b 5c 24
RSP: 0018:c900035ef720 EFLAGS: 00010002
RAX: 0489 RBX: 2448 RCX: 888026ef
RDX: RSI: RDI:
RBP: c900035ef858 R08: 81f5e070 R09: f520006bdee8
R10: dc00 R11: f520006bdee8 R12:
R13: dc00 R14: R15:
FS: 64010380() GS:8880b950() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 005fdeb8 CR3: 7bd96000 CR4: 003506f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
alloc_hugetlb_folio_nodemask+0xae/0x3f0 mm/hugetlb.c:2603
memfd_alloc_folio+0x15e/0x390 mm/memfd.c:75
memfd_pin_folios+0x1066/0x1720 mm/gup.c:3864
udmabuf_create+0x658/0x11c0 drivers/dma-buf/udmabuf.c:353
udmabuf_ioctl_create drivers/dma-buf/udmabuf.c:420 [inline]
udmabuf_ioctl+0x304/0x4f0 drivers/dma-buf/udmabuf.c:451
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:907 [inline]
__se_sys_ioctl+0xfc/0x170 fs/ioctl.c:893
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f5151a7a369
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6
48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48
c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7ffd962ee9e8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 7ffd962eebb8 RCX: 7f5151a7a369
RDX: 22c0 RSI: 40187542 RDI: 0003
RBP: 7f5151aed610 R08: 7ffd962eebb8 R09: 7ffd962eebb8
R10: 7ffd962eebb8 R11: 0246 R12: 0001
R13: 7ffd962eeba8 R14: 0001 R15: 0001
Modules linked in:
---[ end trace ]---
RIP: 0010:zonelist_zone_idx include/linux/mmzone.h:1613 [inline]
RIP: 0010:next_zones_zonelist include/linux/mmzone.h:1644 [inline]
RIP: 0010:first_zones_zonelist include/linux/mmzone.h:1670 [inline]
RIP: 0010:dequeue_hugetlb_folio_nodemask+0x193/0xe40 mm/hugetlb.c:1362
Code: 13 9b a0 ff c7 44 24 14 00 00 00 00 83 7c 24 40 00 0f 85 97 0c 00 00 48 83 7c
24 20 00 0f 85 45 09 00 00 48 89 d8 48 c1 e8 03 <42> 0f b6 04 28 84 c0 0f 85 58
09 00 00 44 8b 33 44 89 f7 8b 5c 24
RSP: 0018:c900035ef720 EFLAGS: 00010002
RAX: 04
[Bug 2067997] Re: Lenovo T14 Gen3 AMD laptop freezes shortly after suspend resume
Should be a duplicate of that. It was fixed in upstream 6.8.5, Ubuntu's 6.8.0-35 is still on 6.8.3. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2067997 Title: Lenovo T14 Gen3 AMD laptop freezes shortly after suspend resume To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2067997/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[PATCH v3 2/2] drm/amd: Add power_saving_policy drm property to eDP connectors
When the `power_saving_policy` property is set to bit mask
"Require color accuracy" ABM should be disabled immediately and
any requests by sysfs to update will return an -EBUSY error.
When the `power_saving_policy` property is set to bit mask
"Require low latency" PSR should be disabled.
When the property is restored to an empty bit mask the previous
value of ABM and pSR will be restored.
Signed-off-by: Mario Limonciello
---
v2->v3:
* Use `disallow_edp_enter_psr` instead
* Drop case in dm_update_crtc_state()
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
3 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..cfb5220cf182 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,10 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_power_saving_policy_property(adev_to_drm(adev),
+
DRM_MODE_POWER_SAVING_POLICY_ALL);
+
return 0;
}
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 f1d67c6f4b98..5fd7210b2479 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6436,6 +6436,13 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ dm_new_state->abm_forbidden = val &
DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ dm_new_state->abm_level = (dm_new_state->abm_forbidden ||
!amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ dm_new_state->psr_forbidden = val &
DRM_MODE_REQUIRE_LOW_LATENCY;
+ ret = 0;
}
return ret;
@@ -6478,6 +6485,13 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ *val = 0;
+ if (dm_state->psr_forbidden)
+ *val |= DRM_MODE_REQUIRE_LOW_LATENCY;
+ if (dm_state->abm_forbidden)
+ *val |= DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ ret = 0;
}
return ret;
@@ -6504,9 +6518,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6530,10 +6547,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7704,6 +7727,13 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.
[PATCH v3 0/2] Add support for 'power saving policy' property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured with requests of "Require color accuracy" or "Require low latency" that can be configured by the compositor. When a driver advertises support for this property and the compositor sets it to "Require color accuracy" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Require color accuracy" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. Compositors can also request that low latency is critical which in practice should cause PSR and PSR2 to be disabled. When the compositor has restored the value back to no requirements then the previous value that would have been programmed will be restored. v2->v3: * Updates from Leo's comments (see individual patches) The matching changes for the igt are here: https://lore.kernel.org/dri-devel/2024050849.33343-1-mario.limoncie...@amd.com/ Mario Limonciello (2): drm: Introduce 'power saving policy' drm property drm/amd: Add power_saving_policy drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + drivers/gpu/drm/drm_connector.c | 46 + include/drm/drm_connector.h | 2 + include/drm/drm_mode_config.h | 5 ++ include/uapi/drm/drm_mode.h | 7 +++ 7 files changed, 111 insertions(+), 5 deletions(-) -- 2.43.0
[PATCH v3 0/2] Add support for 'power saving policy' property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured with requests of "Require color accuracy" or "Require low latency" that can be configured by the compositor. When a driver advertises support for this property and the compositor sets it to "Require color accuracy" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Require color accuracy" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. Compositors can also request that low latency is critical which in practice should cause PSR and PSR2 to be disabled. When the compositor has restored the value back to no requirements then the previous value that would have been programmed will be restored. v2->v3: * Updates from Leo's comments (see individual patches) The matching changes for the igt are here: https://lore.kernel.org/dri-devel/2024050849.33343-1-mario.limoncie...@amd.com/ Mario Limonciello (2): drm: Introduce 'power saving policy' drm property drm/amd: Add power_saving_policy drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + drivers/gpu/drm/drm_connector.c | 46 + include/drm/drm_connector.h | 2 + include/drm/drm_mode_config.h | 5 ++ include/uapi/drm/drm_mode.h | 7 +++ 7 files changed, 111 insertions(+), 5 deletions(-) -- 2.43.0
[PATCH v3 2/2] drm/amd: Add power_saving_policy drm property to eDP connectors
When the `power_saving_policy` property is set to bit mask
"Require color accuracy" ABM should be disabled immediately and
any requests by sysfs to update will return an -EBUSY error.
When the `power_saving_policy` property is set to bit mask
"Require low latency" PSR should be disabled.
When the property is restored to an empty bit mask the previous
value of ABM and pSR will be restored.
Signed-off-by: Mario Limonciello
---
v2->v3:
* Use `disallow_edp_enter_psr` instead
* Drop case in dm_update_crtc_state()
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
3 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..cfb5220cf182 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,10 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_power_saving_policy_property(adev_to_drm(adev),
+
DRM_MODE_POWER_SAVING_POLICY_ALL);
+
return 0;
}
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 f1d67c6f4b98..5fd7210b2479 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6436,6 +6436,13 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ dm_new_state->abm_forbidden = val &
DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ dm_new_state->abm_level = (dm_new_state->abm_forbidden ||
!amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ dm_new_state->psr_forbidden = val &
DRM_MODE_REQUIRE_LOW_LATENCY;
+ ret = 0;
}
return ret;
@@ -6478,6 +6485,13 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ *val = 0;
+ if (dm_state->psr_forbidden)
+ *val |= DRM_MODE_REQUIRE_LOW_LATENCY;
+ if (dm_state->abm_forbidden)
+ *val |= DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ ret = 0;
}
return ret;
@@ -6504,9 +6518,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6530,10 +6547,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7704,6 +7727,13 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.
[PATCH v3 1/2] drm: Introduce 'power saving policy' drm property
The `power saving policy` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of policy of
whether a driver can use power saving features that may compromise
the experience intended by the compositor.
Acked-by: Leo Li
Signed-off-by: Mario Limonciello
---
v2->v3:
* Add tag
---
drivers/gpu/drm/drm_connector.c | 46 +
include/drm/drm_connector.h | 2 ++
include/drm/drm_mode_config.h | 5
include/uapi/drm/drm_mode.h | 7 +
4 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..088a5874c7a4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_power_saving_policy_enum_list[] = {
+ { __builtin_ffs(DRM_MODE_REQUIRE_COLOR_ACCURACY) - 1, "Require color
accuracy" },
+ { __builtin_ffs(DRM_MODE_REQUIRE_LOW_LATENCY) - 1, "Require low
latency" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1499,6 +1504,16 @@ static const u32 dp_colorspaces =
*
* Drivers can set up these properties by calling
* drm_mode_create_tv_margin_properties().
+ * power saving policy:
+ * This property is used to set the power saving policy for the connector.
+ * This property is populated with a bitmask of optional requirements set
+ * by the drm master for the drm driver to respect:
+ * - "Require color accuracy": Disable power saving features that will
+ * affect color fidelity.
+ * For example: Hardware assisted backlight modulation.
+ * - "Require low latency": Disable power saving features that will
+ * affect latency.
+ * For example: Panel self refresh (PSR)
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1963,6 +1978,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_power_saving_policy_property - create power saving policy
property
+ * @dev: DRM device
+ * @supported_policies: bitmask of supported power saving policies
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies)
+{
+ struct drm_property *power_saving;
+
+ if (dev->mode_config.power_saving_policy)
+ return 0;
+ WARN_ON((supported_policies & DRM_MODE_POWER_SAVING_POLICY_ALL) == 0);
+
+ power_saving =
+ drm_property_create_bitmask(dev, 0, "power saving policy",
+ drm_power_saving_policy_enum_list,
+
ARRAY_SIZE(drm_power_saving_policy_enum_list),
+ supported_policies);
+
+ dev->mode_config.power_saving_policy = power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_power_saving_policy_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..b0ec2ec48de7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,8 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 8de3c9a5f61b..eefcbf6c5377 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -969,6 +969,11 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @power_saving_policy: bitmask for power saving policy requests.
+*/
+ struct drm_property *power_saving_policy;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 1ca5c7e4
[PATCH v3 1/2] drm: Introduce 'power saving policy' drm property
The `power saving policy` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of policy of
whether a driver can use power saving features that may compromise
the experience intended by the compositor.
Acked-by: Leo Li
Signed-off-by: Mario Limonciello
---
v2->v3:
* Add tag
---
drivers/gpu/drm/drm_connector.c | 46 +
include/drm/drm_connector.h | 2 ++
include/drm/drm_mode_config.h | 5
include/uapi/drm/drm_mode.h | 7 +
4 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..088a5874c7a4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_power_saving_policy_enum_list[] = {
+ { __builtin_ffs(DRM_MODE_REQUIRE_COLOR_ACCURACY) - 1, "Require color
accuracy" },
+ { __builtin_ffs(DRM_MODE_REQUIRE_LOW_LATENCY) - 1, "Require low
latency" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1499,6 +1504,16 @@ static const u32 dp_colorspaces =
*
* Drivers can set up these properties by calling
* drm_mode_create_tv_margin_properties().
+ * power saving policy:
+ * This property is used to set the power saving policy for the connector.
+ * This property is populated with a bitmask of optional requirements set
+ * by the drm master for the drm driver to respect:
+ * - "Require color accuracy": Disable power saving features that will
+ * affect color fidelity.
+ * For example: Hardware assisted backlight modulation.
+ * - "Require low latency": Disable power saving features that will
+ * affect latency.
+ * For example: Panel self refresh (PSR)
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1963,6 +1978,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_power_saving_policy_property - create power saving policy
property
+ * @dev: DRM device
+ * @supported_policies: bitmask of supported power saving policies
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies)
+{
+ struct drm_property *power_saving;
+
+ if (dev->mode_config.power_saving_policy)
+ return 0;
+ WARN_ON((supported_policies & DRM_MODE_POWER_SAVING_POLICY_ALL) == 0);
+
+ power_saving =
+ drm_property_create_bitmask(dev, 0, "power saving policy",
+ drm_power_saving_policy_enum_list,
+
ARRAY_SIZE(drm_power_saving_policy_enum_list),
+ supported_policies);
+
+ dev->mode_config.power_saving_policy = power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_power_saving_policy_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..b0ec2ec48de7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,8 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 8de3c9a5f61b..eefcbf6c5377 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -969,6 +969,11 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @power_saving_policy: bitmask for power saving policy requests.
+*/
+ struct drm_property *power_saving_policy;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 1ca5c7e4
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
If you don't hook into some lid notify event how is one supposed to get the display back to life after opening the lid? I guess in my mind it's a tangential to the "initial modeset". The DRM master can issue a modeset to enable the combination as desired. This code is run whenever there's a hotplug/etc. Not sure why you're only thinking about the initial modeset. Got it; so in that case adding a notification chain for lid events to run it again should do the trick. When I tested I did confirm that with mutter such an event is received and it does the modeset to enable the eDP when lid is opened. This code isn't relevant when you have a userspace drm master calling the shots. Right. Let me ask this - what happens if no DRM master running and you hotplug a DP cable? Does a "new" clone configuration get done? Yes, this code reprobes the displays and comes up with a new config to suit the new situation. Got it; in this case you're right we should have some notification chain. Do you think it should be in the initial patch or a follow up? The other potential issue here is whether acpi_lid_open() is actually trustworthy. Some kms drivers have/had some lid handling in their own code, and I'm pretty sure those have often needed quirks/modparams to actually do sensible things on certain machines. FWIW I ripped out all the lid crap from i915 long ago since it was half backed, mostly broken, and ugly, and I'm not looking to add it back there. But I do think handling that in drm_client does seem somewhat sane, as that should more or less match what userspace clients would do. Just a question of how bad the quirk situation will get... If the lid reporting is wrong it's not just drm_client that would falter. There are other parts of the kernel that rely upon acpi_lid_open() being accurate and IMO it would be best to put any quirks to the effect in drivers/acpi/button.c. If it can't be relied upon then it's best to just report -EINVAL or -ENODEV. Also a direct acpi_lid_open() call seems a bit iffy. But I guess if someone needs this to work on non-ACPI system they get to figure out how to abstract it better. acpi_lid_open() does seem to return != 0 when ACPI is not supported, so at least it would err on the side of enabling everything. Yeah acpi_lid_open() seemed fine to me specifically because non ACPI hardcodes to open.
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
If you don't hook into some lid notify event how is one supposed to get the display back to life after opening the lid? I guess in my mind it's a tangential to the "initial modeset". The DRM master can issue a modeset to enable the combination as desired. This code is run whenever there's a hotplug/etc. Not sure why you're only thinking about the initial modeset. Got it; so in that case adding a notification chain for lid events to run it again should do the trick. When I tested I did confirm that with mutter such an event is received and it does the modeset to enable the eDP when lid is opened. This code isn't relevant when you have a userspace drm master calling the shots. Right. Let me ask this - what happens if no DRM master running and you hotplug a DP cable? Does a "new" clone configuration get done? Yes, this code reprobes the displays and comes up with a new config to suit the new situation. Got it; in this case you're right we should have some notification chain. Do you think it should be in the initial patch or a follow up? The other potential issue here is whether acpi_lid_open() is actually trustworthy. Some kms drivers have/had some lid handling in their own code, and I'm pretty sure those have often needed quirks/modparams to actually do sensible things on certain machines. FWIW I ripped out all the lid crap from i915 long ago since it was half backed, mostly broken, and ugly, and I'm not looking to add it back there. But I do think handling that in drm_client does seem somewhat sane, as that should more or less match what userspace clients would do. Just a question of how bad the quirk situation will get... If the lid reporting is wrong it's not just drm_client that would falter. There are other parts of the kernel that rely upon acpi_lid_open() being accurate and IMO it would be best to put any quirks to the effect in drivers/acpi/button.c. If it can't be relied upon then it's best to just report -EINVAL or -ENODEV. Also a direct acpi_lid_open() call seems a bit iffy. But I guess if someone needs this to work on non-ACPI system they get to figure out how to abstract it better. acpi_lid_open() does seem to return != 0 when ACPI is not supported, so at least it would err on the side of enabling everything. Yeah acpi_lid_open() seemed fine to me specifically because non ACPI hardcodes to open.
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
On 5/29/2024 09:14, Ville Syrjälä wrote:
On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
If you don't hook into some lid notify event how is one supposed to get
the display back to life after opening the lid?
I guess in my mind it's a tangential to the "initial modeset". The DRM
master can issue a modeset to enable the combination as desired.
When I tested I did confirm that with mutter such an event is received
and it does the modeset to enable the eDP when lid is opened.
Let me ask this - what happens if no DRM master running and you hotplug
a DP cable? Does a "new" clone configuration get done?
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
On 5/29/2024 09:14, Ville Syrjälä wrote:
On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
If you don't hook into some lid notify event how is one supposed to get
the display back to life after opening the lid?
I guess in my mind it's a tangential to the "initial modeset". The DRM
master can issue a modeset to enable the combination as desired.
When I tested I did confirm that with mutter such an event is received
and it does the modeset to enable the eDP when lid is opened.
Let me ask this - what happens if no DRM master running and you hotplug
a DP cable? Does a "new" clone configuration get done?
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
On 5/29/2024 08:55, Alex Deucher wrote:
On Wed, May 29, 2024 at 9:51 AM Jani Nikula wrote:
On Wed, 29 May 2024, Alex Deucher wrote:
On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
wrote:
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
Reviewed-by: Alex Deucher
Do you have drm-misc access or do you need someone to apply this for you?
I've bounced this to intel-gfx and intel-xe lists to get CI testing. I'd
appreciate holding off on merging until we have results.
Sure.
Thanks for the review and pushing it to CI testing infra.
I don't have any drm-misc access so if everything looks good then one of
you guys please merge it for me.
Thanks!
Alex
Thanks,
Jani.
Alex
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors,
connector_count, modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
--
Jani Nikula, Intel
Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
On 5/29/2024 08:55, Alex Deucher wrote:
On Wed, May 29, 2024 at 9:51 AM Jani Nikula wrote:
On Wed, 29 May 2024, Alex Deucher wrote:
On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
wrote:
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
Reviewed-by: Alex Deucher
Do you have drm-misc access or do you need someone to apply this for you?
I've bounced this to intel-gfx and intel-xe lists to get CI testing. I'd
appreciate holding off on merging until we have results.
Sure.
Thanks for the review and pushing it to CI testing infra.
I don't have any drm-misc access so if everything looks good then one of
you guys please merge it for me.
Thanks!
Alex
Thanks,
Jani.
Alex
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors,
connector_count, modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
--
Jani Nikula, Intel
[PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
[PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
[PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
Cc: hughsi...@gmail.com
v1->v2:
* Match LVDS as well
---
drivers/gpu/drm/drm_client_modeset.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..0b0411086e76 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ if (!enabled[i])
+ continue;
+ break;
+ default:
+ continue;
+ }
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
Re: Refine the firmware-updates exception
Ping? From: Mario Limonciello Sent: Wednesday, May 1, 2024 10:32:49 AM To: Robie Basak ; Mario Limonciello Cc: ubuntu-release@lists.ubuntu.com ; Richard Hughes Subject: Re: Refine the firmware-updates exception Robie, My apologies; my email didn't get a response a while and I was on leave a while when your response came back and I totally missed it. Your comment on bug 1979963 prompted me to find it though! On 3/7/24 08:00, Robie Basak wrote: > Hi Mario, > > Thank you for caring for the fwupd package in Ubuntu! > Sure. > One adminsitrative question: fwupd is in main and the Foundations team > are committed to looking after it. Is your proposal made on behalf of or > in coordination with the Foundations team? If not, what's the Foundation > team's view on your proposal? I am part of ~ubuntu-foundations-team, but admittedly I have not conferred closely with other members. I would invite discussion on any opposing views from other members. > > On Sun, Feb 18, 2024 at 12:11:48AM -0600, Mario Limonciello wrote: >> Considering all of that, I wanted to discuss with the release team to >> modify the exception used for the firmware updating stack to allow >> migrations from one stable branch to another; especially considering EOL >> dates. >> I've drafted an updated proposal in this wiki page and aligned it >> directionally upstream: >> https://wiki.ubuntu.com/firmware-updates-2.0 >> >> Can the release team please review and consider this? If approved we can >> just copy the text from the new page to the old page and delete the new >> page. > > In general we only grant general permission to make arbitrary feature > changes to packages in stable releases under specific circumstances[1]. > Probably the relevant reasons in the case of this package would be for > hardware enablement purposes, or in the case of "Internet protocol" > changes (eg. for fwupd, perhaps the mechanism to fetch updates is > changing and will no longer work). > > Notably, upstream advertising "EOL dates" is *not* a valid > justification. Consider this: Ubuntu already commits to supporting an > LTS release for 10 years. How many upstreams that "publish EOL dates" > would still be supporting their releases that we shipped by the end of > that period? I'd guess virtually zero. Clearly, we are committing to > "LTS-ness" for our entire stable release cycle *despite* "upstream EOL > dates" - in effect *extending* those dates for our users. Or, consider > what would happen if "upstream EOL dates" *were* justification for us to > make major version bumps to our packages. Near the end of our stable > release cycle, this would mean that we'd effectively become a rolling > release as every package for which upstream had published an "EOL date" > would need to be updated. Clearly this is not the intention, so clearly > "upstream EOL" cannot be a justification in itself for us to bump a > major version in a stable release. I think there's a big difference between "not touching" and "supporting". Due to the LVFS server changes that I've alluded to and Richard mentioned fwupd from 18.04 LTS and 16.04 are essentially dead software right now. In a sense due to the upstream EOL dates the difficulty of "maintaining" very old versions becomes exponentially greater as the software progresses. It's not my day job to maintain fwupd but it's something I like to work on upstream, and I do my best to keep it healthy where and when I can in Ubuntu. > >> I've drafted an updated proposal in this wiki page and aligned it >> directionally upstream: >> https://wiki.ubuntu.com/firmware-updates-2.0 > > One further note from your draft: > >> tarball releases only. No backported individual patches. > > I understand the benefit of aligning with upstream. However this is only > possible if upstream policies completely align with distribution policy > and expectations. There is no guarantee of this in the future, so from a > policy point of view, distribution-specific patches must never be ruled > out by a general policy like this. Instead, I expect uploaders and the > SRU team to continue considering such patches on a case-by-case basis. > It is normal and a common occurrence for the SRU team to push back on a > large set of updates and ask for a minimal cherry-pick to minimise risk > to users, and I don't see any reason that the fwupd should diverge from > this position. Therefore, IMHO this stipulation cannot form part for a > policy that the SRU team can accept. I understand the position; but I have been privy to patches that are backported "look" like they're compl
Re: [PATCH] drm/amd/amdgpu: Fix 'snprintf' output truncation warning
On 5/28/2024 10:24, Pratap Nirujogi wrote:
snprintf can truncate the output fw filename if the isp ucode_prefix
exceeds 29 characters. Knowing ISP ucode_prefix is in the format
isp_x_x_x, limiting the size of ucode_prefix[] to 10 characters
to fix the warning.
Fixes the below warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c: In function 'isp_early_init':
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c:192:58: warning: 'snprintf'
output may be truncated before the last format character
[-Wformat-truncation=]
192 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
| ^
In function 'isp_load_fw_by_psp',
inlined from 'isp_early_init' at
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c:218:8:
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c:192:9: note: 'snprintf' output
between 12 and 41 bytes into a destination of size 40
192 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
|
^
Signed-off-by: Pratap Nirujogi
As the kernel robot reported this you should add a "Reported-by:" tag
for it.
Otherwise LGTM (feel free to add that tag when committing).
Reviewed-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
index 240408486d6b..2a3f4668cb9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
@@ -96,7 +96,7 @@ static int isp_resume(void *handle)
static int isp_load_fw_by_psp(struct amdgpu_device *adev)
{
const struct common_firmware_header *hdr;
- char ucode_prefix[30];
+ char ucode_prefix[10];
char fw_name[40];
int r = 0;
[Bug 2063143] Re: sddm/simpledrm race conditions leads to frequent black display on bootup
I posted some idea over to the systemd bug on a way to approach this from systemd instead of each greeter. https://github.com/systemd/systemd/issues/32509#issuecomment-2134152084 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063143 Title: sddm/simpledrm race conditions leads to frequent black display on bootup To manage notifications about this bug go to: https://bugs.launchpad.net/plymouth/+bug/2063143/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2063143] Re: sddm/simpledrm race conditions leads to frequent black display on bootup
I posted some idea over to the systemd bug on a way to approach this from systemd instead of each greeter. https://github.com/systemd/systemd/issues/32509#issuecomment-2134152084 -- You received this bug notification because you are a member of Kubuntu Bugs, which is subscribed to sddm in Ubuntu. https://bugs.launchpad.net/bugs/2063143 Title: sddm/simpledrm race conditions leads to frequent black display on bootup To manage notifications about this bug go to: https://bugs.launchpad.net/plymouth/+bug/2063143/+subscriptions -- kubuntu-bugs mailing list kubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/kubuntu-bugs
[Bug 2063983] Re: simpledrm / lightdm race condition leads to black screen
The discussion upstream on dri-devel has mostly settled upon userspace greeters need to support hot-unplug. https://lore.kernel.org/dri-devel/ZkyZCmMU86nUV4TO@phenom.ffwll.local/ -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063983 Title: simpledrm / lightdm race condition leads to black screen To manage notifications about this bug go to: https://bugs.launchpad.net/systemd/+bug/2063983/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2063983] Re: simpledrm / lightdm race condition leads to black screen
Can someone with an affected system raise a bug report upstream to lightdm? There was a very similar bug that occurred in GDM last year: https://gitlab.gnome.org/GNOME/mutter/-/issues/2909 And there is a similar report opened with SDDM: https://github.com/sddm/sddm/issues/1917 ** Summary changed: - 24.04 Upgrade/Fresh Install results in black screen on reboot on AMDGPU system + simpledrm / lightdm race condition leads to black screen ** Bug watch added: github.com/systemd/systemd/issues #32509 https://github.com/systemd/systemd/issues/32509 ** Also affects: systemd via https://github.com/systemd/systemd/issues/32509 Importance: Unknown Status: Unknown ** Bug watch added: gitlab.gnome.org/GNOME/mutter/-/issues #2909 https://gitlab.gnome.org/GNOME/mutter/-/issues/2909 ** Bug watch added: github.com/sddm/sddm/issues #1917 https://github.com/sddm/sddm/issues/1917 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063983 Title: simpledrm / lightdm race condition leads to black screen To manage notifications about this bug go to: https://bugs.launchpad.net/systemd/+bug/2063983/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2063143] Re: sddm/lightdm race conditions leads to frequent black display on bootup
** Summary changed: - Frequent boot to black display + sddm/lightdm race conditions leads to frequent black display on bootup ** Summary changed: - sddm/lightdm race conditions leads to frequent black display on bootup + sddm/simpledrm race conditions leads to frequent black display on bootup -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063143 Title: sddm/simpledrm race conditions leads to frequent black display on bootup To manage notifications about this bug go to: https://bugs.launchpad.net/plymouth/+bug/2063143/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2063143] Re: sddm/lightdm race conditions leads to frequent black display on bootup
** Summary changed: - Frequent boot to black display + sddm/lightdm race conditions leads to frequent black display on bootup ** Summary changed: - sddm/lightdm race conditions leads to frequent black display on bootup + sddm/simpledrm race conditions leads to frequent black display on bootup -- You received this bug notification because you are a member of Kubuntu Bugs, which is subscribed to sddm in Ubuntu. https://bugs.launchpad.net/bugs/2063143 Title: sddm/simpledrm race conditions leads to frequent black display on bootup To manage notifications about this bug go to: https://bugs.launchpad.net/plymouth/+bug/2063143/+subscriptions -- kubuntu-bugs mailing list kubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/kubuntu-bugs
[Bug 2063983] Re: 24.04 Upgrade/Fresh Install results in black screen on reboot on AMDGPU system
** Package changed: linux (Ubuntu) => lightdm (Ubuntu) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063983 Title: 24.04 Upgrade/Fresh Install results in black screen on reboot on AMDGPU system To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/lightdm/+bug/2063983/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[PATCH] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Suggested-by: Alex Deucher
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_client_modeset.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..b76438c31761 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,27 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ if (connector->connector_type != DRM_MODE_CONNECTOR_eDP ||
!enabled[i])
+ continue;
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +866,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
[PATCH] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Suggested-by: Alex Deucher
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_client_modeset.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..b76438c31761 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,27 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ if (connector->connector_type != DRM_MODE_CONNECTOR_eDP ||
!enabled[i])
+ continue;
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +866,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
[PATCH] drm/client: Detect when ACPI lid is closed during initialization
If the lid on a laptop is closed when eDP connectors are populated
then it remains enabled when the initial framebuffer configuration
is built.
When creating the initial framebuffer configuration detect the ACPI
lid status and if it's closed disable any eDP connectors.
Suggested-by: Alex Deucher
Reported-by: Chris Bainbridge
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_client_modeset.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 31af5cf37a09..b76438c31761 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -8,6 +8,7 @@
*/
#include "drm/drm_modeset_lock.h"
+#include
#include
#include
#include
@@ -257,6 +258,27 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void drm_client_match_edp_lid(struct drm_device *dev,
+struct drm_connector **connectors,
+unsigned int connector_count,
+bool *enabled)
+{
+ int i;
+
+ for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
+
+ if (connector->connector_type != DRM_MODE_CONNECTOR_eDP ||
!enabled[i])
+ continue;
+
+ if (!acpi_lid_open()) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed,
disabling\n",
+ connector->base.id, connector->name);
+ enabled[i] = false;
+ }
+ }
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
@@ -844,6 +866,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client,
unsigned int width,
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
+ drm_client_match_edp_lid(dev, connectors, connector_count,
enabled);
if (!drm_client_target_cloned(dev, connectors, connector_count,
modes,
offsets, enabled, width, height)
&&
!drm_client_target_preferred(dev, connectors,
connector_count, modes,
--
2.43.0
[Bug 2065321] Re: Kubuntu 24.04 Second boot always black screen
*** This bug is a duplicate of bug 2063143 *** https://bugs.launchpad.net/bugs/2063143 ** This bug has been marked a duplicate of bug 2063143 Frequent boot to black display -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065321 Title: Kubuntu 24.04 Second boot always black screen To manage notifications about this bug go to: https://bugs.launchpad.net/sddm/+bug/2065321/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065321] Re: Kubuntu 24.04 Second boot always black screen
*** This bug is a duplicate of bug 2063143 *** https://bugs.launchpad.net/bugs/2063143 ** This bug has been marked a duplicate of bug 2063143 Frequent boot to black display -- You received this bug notification because you are a member of Kubuntu Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/2065321 Title: Kubuntu 24.04 Second boot always black screen To manage notifications about this bug go to: https://bugs.launchpad.net/sddm/+bug/2065321/+subscriptions -- kubuntu-bugs mailing list kubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/kubuntu-bugs
[PATCH] drm/amd: Fix shutdown (again) on some SMU v13.0.4/11 platforms
commit cd94d1b182d2 ("dm/amd/pm: Fix problems with reboot/shutdown for
some SMU 13.0.4/13.0.11 users") attempted to fix shutdown issues
that were reported since commit 31729e8c21ec ("drm/amd/pm: fixes a
random hang in S4 for SMU v13.0.4/11") but caused issues for some
people.
Adjust the workaround flow to properly only apply in the S4 case:
-> For shutdown go through SMU_MSG_PrepareMp1ForUnload
-> For S4 go through SMU_MSG_GfxDeviceDriverReset and
SMU_MSG_PrepareMp1ForUnload
Reported-and-tested-by: lectrode
Closes: https://github.com/void-linux/void-packages/issues/50417
Cc: sta...@vger.kernel.org
Fixes: cd94d1b182d2 ("dm/amd/pm: Fix problems with reboot/shutdown for some SMU
13.0.4/13.0.11 users")
Signed-off-by: Mario Limonciello
---
Cc: regressi...@lists.linux.dev
---
.../drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c | 20 ++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c
index 4abfcd32747d..c7ab0d7027d9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c
@@ -226,15 +226,17 @@ static int smu_v13_0_4_system_features_control(struct
smu_context *smu, bool en)
struct amdgpu_device *adev = smu->adev;
int ret = 0;
- if (!en && adev->in_s4) {
- /* Adds a GFX reset as workaround just before sending the
-* MP1_UNLOAD message to prevent GC/RLC/PMFW from entering
-* an invalid state.
-*/
- ret = smu_cmn_send_smc_msg_with_param(smu,
SMU_MSG_GfxDeviceDriverReset,
- SMU_RESET_MODE_2, NULL);
- if (ret)
- return ret;
+ if (!en && !adev->in_s0ix) {
+ if (adev->in_s4) {
+ /* Adds a GFX reset as workaround just before sending
the
+* MP1_UNLOAD message to prevent GC/RLC/PMFW from
entering
+* an invalid state.
+*/
+ ret = smu_cmn_send_smc_msg_with_param(smu,
SMU_MSG_GfxDeviceDriverReset,
+ SMU_RESET_MODE_2, NULL);
+ if (ret)
+ return ret;
+ }
ret = smu_cmn_send_smc_msg(smu, SMU_MSG_PrepareMp1ForUnload,
NULL);
}
--
2.43.0
[Bug 2066345] Re: Lenovo T14 Gen3 AMD laptop lid suspend freezes system
This should be a duplicate of https://bugs.launchpad.net/ubuntu/+source/linux-oem-6.5/+bug/2064595 The fix is https://github.com/torvalds/linux/commit/ca299b4512d4b4f516732a48ce9aa19d91f4473e -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2066345 Title: Lenovo T14 Gen3 AMD laptop lid suspend freezes system To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2066345/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065838] Re: System crash on resume from sleep
Do you have secure boot enabled? if so, turn it off and hopefully the kernel you built should be bootadble. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065838 Title: System crash on resume from sleep To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2065838/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[PATCH v2 3/4] tests/amdgpu/amd_abm: Add support for panel_power_saving property
From: Mario Limonciello
When the "panel power saving" property is set to forbidden the
compositor has indicated that userspace prefers to have color
accuracy and fidelity instead of power saving.
Verify that the sysfs file behaves as expected in this situation.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index f74c3012c..3fa1366fa 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -365,6 +365,43 @@ static void abm_gradual(data_t *data)
}
}
+
+static void abm_forbidden(data_t *data)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int target, r;
+
+ for_each_pipe_with_valid_output(>display, pipe, output) {
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ r = clear_power_saving_policy(data->drm_fd, output);
+ if (r == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_assert_eq(r, 0);
+
+ target = 3;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+
+ r = set_panel_power_saving_policy(data->drm_fd, output,
DRM_MODE_REQUIRE_COLOR_ACCURACY);
+ igt_assert_eq(r, 0);
+
+ target = 0;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, -1);
+
+ r = clear_power_saving_policy(data->drm_fd, output);
+ igt_assert_eq(r, 0);
+
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+ }
+}
+
igt_main
{
data_t data = {};
@@ -393,6 +430,8 @@ igt_main
abm_enabled();
igt_subtest("abm_gradual")
abm_gradual();
+ igt_subtest("abm_forbidden")
+ abm_forbidden();
igt_fixture {
igt_display_fini();
--
2.43.0
[PATCH v2 4/4] tests/amdgpu/amd_psr: Add support for `power saving policy` property
Verify that the property has disabled PSR
---
tests/amdgpu/amd_psr.c | 74 ++
1 file changed, 74 insertions(+)
diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 9da161a09..a9f4a6aa5 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -338,6 +338,78 @@ static void run_check_psr(data_t *data, bool
test_null_crtc) {
test_fini(data);
}
+static void psr_forbidden(data_t *data)
+{
+ int edp_idx, ret, i, psr_state;
+ igt_fb_t ref_fb, ref_fb2;
+ igt_fb_t *flip_fb;
+ igt_output_t *output;
+
+ test_init(data);
+
+ edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
+ igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
+
+ /* check if eDP support PSR1, PSR-SU.
+*/
+ igt_skip_on(!psr_mode_supported(data, PSR_MODE_1) &&
!psr_mode_supported(data, PSR_MODE_2));
+ for_each_connected_output(>display, output) {
+
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+ ret = clear_power_saving_policy(data->fd, output);
+ if (ret == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_require(ret == 0);
+
+ /* try to engage PSR */
+ ret = set_panel_power_saving_policy(data->fd, output,
DRM_MODE_REQUIRE_LOW_LATENCY);
+ igt_assert_eq(ret, 0);
+
+ igt_create_color_fb(data->fd, data->mode->hdisplay,
+ data->mode->vdisplay, DRM_FORMAT_XRGB,
0, 1.0,
+ 0.0, 0.0, _fb);
+ igt_create_color_fb(data->fd, data->mode->hdisplay,
+ data->mode->vdisplay, DRM_FORMAT_XRGB,
0, 0.0,
+ 1.0, 0.0, _fb2);
+
+ igt_plane_set_fb(data->primary, _fb);
+
+ igt_display_commit_atomic(>display,
DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
+
+ for (i = 0; i < N_FLIPS; i++) {
+ if (i % 2 == 0)
+ flip_fb = _fb2;
+ else
+ flip_fb = _fb;
+
+ ret = drmModePageFlip(data->fd,
output->config.crtc->crtc_id,
+ flip_fb->fb_id,
DRM_MODE_PAGE_FLIP_EVENT, NULL);
+ igt_require(ret == 0);
+ kmstest_wait_for_pageflip(data->fd);
+ }
+
+ /* PSR state takes some time to settle its value on static
screen */
+ sleep(PSR_SETTLE_DELAY);
+
+ psr_state = igt_amd_read_psr_state(data->fd, output->name);
+ igt_require(psr_state == PSR_STATE3);
+
+ igt_remove_fb(data->fd, _fb);
+ igt_remove_fb(data->fd, _fb2);
+
+ ret = clear_power_saving_policy(data->fd, output);
+ if (ret == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_require(ret == 0);
+
+ }
+}
+
static void run_check_psr_su_mpo(data_t *data, bool scaling, float
scaling_ratio)
{
int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
@@ -756,6 +828,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
igt_describe("Test to validate PSR SU enablement with Visual Confirm "
"and to validate PSR SU disable/re-enable w/ primary
scaling ratio 0.75");
igt_subtest("psr_su_mpo_scaling_0_75") run_check_psr_su_mpo(,
true, .75);
+ igt_describe("Test whether PSR can be forbidden");
+ igt_subtest("psr_forbidden") psr_forbidden();
igt_fixture
{
--
2.43.0
[PATCH v2 3/4] tests/amdgpu/amd_abm: Add support for panel_power_saving property
From: Mario Limonciello
When the "panel power saving" property is set to forbidden the
compositor has indicated that userspace prefers to have color
accuracy and fidelity instead of power saving.
Verify that the sysfs file behaves as expected in this situation.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index f74c3012c..3fa1366fa 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -365,6 +365,43 @@ static void abm_gradual(data_t *data)
}
}
+
+static void abm_forbidden(data_t *data)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int target, r;
+
+ for_each_pipe_with_valid_output(>display, pipe, output) {
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ r = clear_power_saving_policy(data->drm_fd, output);
+ if (r == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_assert_eq(r, 0);
+
+ target = 3;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+
+ r = set_panel_power_saving_policy(data->drm_fd, output,
DRM_MODE_REQUIRE_COLOR_ACCURACY);
+ igt_assert_eq(r, 0);
+
+ target = 0;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, -1);
+
+ r = clear_power_saving_policy(data->drm_fd, output);
+ igt_assert_eq(r, 0);
+
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+ }
+}
+
igt_main
{
data_t data = {};
@@ -393,6 +430,8 @@ igt_main
abm_enabled();
igt_subtest("abm_gradual")
abm_gradual();
+ igt_subtest("abm_forbidden")
+ abm_forbidden();
igt_fixture {
igt_display_fini();
--
2.43.0
[PATCH v2 2/4] tests/amdgpu/amd_abm: Make set_abm_level return type int
From: Mario Limonciello
In order to bubble of cases of expeted errors on set_abm_level()
change the return type to int.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index 2882c2c18..f74c3012c 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,10 +104,11 @@ static int backlight_write_brightness(int value)
return 0;
}
-static void set_abm_level(data_t *data, igt_output_t *output, int level)
+static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
int fd;
+ int ret;
igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
output->name) < PATH_MAX);
@@ -116,8 +117,12 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
igt_assert(fd != -1);
- igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
- write(fd, buf, 1));
+ snprintf(buf, sizeof(buf), "%d", level);
+ ret = write(fd, buf, 1);
+ if (ret < 0) {
+ close(fd);
+ return ret;
+ }
igt_assert_eq(close(fd), 0);
@@ -129,6 +134,7 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
DRM_MODE_DPMS_OFF);
kmstest_set_connector_dpms(data->drm_fd, output->config.connector,
DRM_MODE_DPMS_ON);
+ return 0;
}
static int backlight_read_max_brightness(int *result)
@@ -192,7 +198,8 @@ static void backlight_dpms_cycle(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness / 2);
usleep(10);
pwm_1 = read_target_backlight_pwm(data->drm_fd, output->name);
@@ -223,7 +230,8 @@ static void backlight_monotonic_basic(data_t *data)
brightness_step = max_brightness / 10;
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -257,7 +265,8 @@ static void backlight_monotonic_abm(data_t *data)
brightness_step = max_brightness / 10;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -289,14 +298,16 @@ static void abm_enabled(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
pwm_without_abm = prev_pwm;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, i);
+ ret = set_abm_level(data, output, i);
+ igt_assert_eq(ret, 0);
usleep(10);
pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
igt_assert(pwm <= prev_pwm);
@@ -323,7 +334,8 @@ static void abm_gradual(data_t *data)
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
sleep(convergence_delay);
@@ -331,7 +343,8 @@ static void abm_gradual(data_t *data)
curr = read_current_backlight_pwm(data->drm_fd, output->name);
igt_assert_eq(prev_pwm, curr);
- set_abm_level(data, output, 4);
+ ret = set_abm_level(data, output, 4);
+ igt_assert_eq(ret, 0);
for (i = 0; i < 10; i++) {
usleep(10);
pwm = read_current_backlight_pwm(data->drm_fd,
output->name);
--
2.43.0
[PATCH v2 2/4] tests/amdgpu/amd_abm: Make set_abm_level return type int
From: Mario Limonciello
In order to bubble of cases of expeted errors on set_abm_level()
change the return type to int.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index 2882c2c18..f74c3012c 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,10 +104,11 @@ static int backlight_write_brightness(int value)
return 0;
}
-static void set_abm_level(data_t *data, igt_output_t *output, int level)
+static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
int fd;
+ int ret;
igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
output->name) < PATH_MAX);
@@ -116,8 +117,12 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
igt_assert(fd != -1);
- igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
- write(fd, buf, 1));
+ snprintf(buf, sizeof(buf), "%d", level);
+ ret = write(fd, buf, 1);
+ if (ret < 0) {
+ close(fd);
+ return ret;
+ }
igt_assert_eq(close(fd), 0);
@@ -129,6 +134,7 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
DRM_MODE_DPMS_OFF);
kmstest_set_connector_dpms(data->drm_fd, output->config.connector,
DRM_MODE_DPMS_ON);
+ return 0;
}
static int backlight_read_max_brightness(int *result)
@@ -192,7 +198,8 @@ static void backlight_dpms_cycle(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness / 2);
usleep(10);
pwm_1 = read_target_backlight_pwm(data->drm_fd, output->name);
@@ -223,7 +230,8 @@ static void backlight_monotonic_basic(data_t *data)
brightness_step = max_brightness / 10;
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -257,7 +265,8 @@ static void backlight_monotonic_abm(data_t *data)
brightness_step = max_brightness / 10;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -289,14 +298,16 @@ static void abm_enabled(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
pwm_without_abm = prev_pwm;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, i);
+ ret = set_abm_level(data, output, i);
+ igt_assert_eq(ret, 0);
usleep(10);
pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
igt_assert(pwm <= prev_pwm);
@@ -323,7 +334,8 @@ static void abm_gradual(data_t *data)
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
sleep(convergence_delay);
@@ -331,7 +343,8 @@ static void abm_gradual(data_t *data)
curr = read_current_backlight_pwm(data->drm_fd, output->name);
igt_assert_eq(prev_pwm, curr);
- set_abm_level(data, output, 4);
+ ret = set_abm_level(data, output, 4);
+ igt_assert_eq(ret, 0);
for (i = 0; i < 10; i++) {
usleep(10);
pwm = read_current_backlight_pwm(data->drm_fd,
output->name);
--
2.43.0
[PATCH v2 4/4] tests/amdgpu/amd_psr: Add support for `power saving policy` property
Verify that the property has disabled PSR
---
tests/amdgpu/amd_psr.c | 74 ++
1 file changed, 74 insertions(+)
diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 9da161a09..a9f4a6aa5 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -338,6 +338,78 @@ static void run_check_psr(data_t *data, bool
test_null_crtc) {
test_fini(data);
}
+static void psr_forbidden(data_t *data)
+{
+ int edp_idx, ret, i, psr_state;
+ igt_fb_t ref_fb, ref_fb2;
+ igt_fb_t *flip_fb;
+ igt_output_t *output;
+
+ test_init(data);
+
+ edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
+ igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
+
+ /* check if eDP support PSR1, PSR-SU.
+*/
+ igt_skip_on(!psr_mode_supported(data, PSR_MODE_1) &&
!psr_mode_supported(data, PSR_MODE_2));
+ for_each_connected_output(>display, output) {
+
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+ ret = clear_power_saving_policy(data->fd, output);
+ if (ret == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_require(ret == 0);
+
+ /* try to engage PSR */
+ ret = set_panel_power_saving_policy(data->fd, output,
DRM_MODE_REQUIRE_LOW_LATENCY);
+ igt_assert_eq(ret, 0);
+
+ igt_create_color_fb(data->fd, data->mode->hdisplay,
+ data->mode->vdisplay, DRM_FORMAT_XRGB,
0, 1.0,
+ 0.0, 0.0, _fb);
+ igt_create_color_fb(data->fd, data->mode->hdisplay,
+ data->mode->vdisplay, DRM_FORMAT_XRGB,
0, 0.0,
+ 1.0, 0.0, _fb2);
+
+ igt_plane_set_fb(data->primary, _fb);
+
+ igt_display_commit_atomic(>display,
DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
+
+ for (i = 0; i < N_FLIPS; i++) {
+ if (i % 2 == 0)
+ flip_fb = _fb2;
+ else
+ flip_fb = _fb;
+
+ ret = drmModePageFlip(data->fd,
output->config.crtc->crtc_id,
+ flip_fb->fb_id,
DRM_MODE_PAGE_FLIP_EVENT, NULL);
+ igt_require(ret == 0);
+ kmstest_wait_for_pageflip(data->fd);
+ }
+
+ /* PSR state takes some time to settle its value on static
screen */
+ sleep(PSR_SETTLE_DELAY);
+
+ psr_state = igt_amd_read_psr_state(data->fd, output->name);
+ igt_require(psr_state == PSR_STATE3);
+
+ igt_remove_fb(data->fd, _fb);
+ igt_remove_fb(data->fd, _fb2);
+
+ ret = clear_power_saving_policy(data->fd, output);
+ if (ret == -ENODEV) {
+ igt_skip("No power saving policy prop\n");
+ return;
+ }
+ igt_require(ret == 0);
+
+ }
+}
+
static void run_check_psr_su_mpo(data_t *data, bool scaling, float
scaling_ratio)
{
int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
@@ -756,6 +828,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
igt_describe("Test to validate PSR SU enablement with Visual Confirm "
"and to validate PSR SU disable/re-enable w/ primary
scaling ratio 0.75");
igt_subtest("psr_su_mpo_scaling_0_75") run_check_psr_su_mpo(,
true, .75);
+ igt_describe("Test whether PSR can be forbidden");
+ igt_subtest("psr_forbidden") psr_forbidden();
igt_fixture
{
--
2.43.0
[PATCH v2 0/4] Add support for testing power saving policy DRM property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity or low latency is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured by the compositor to for it's requirements. When a driver advertises support for this property and the compositor sets requirements then the driver will disable any appplicable power saving features that can compromise the requirements. This set of IGT changes introduces a new subtest that will cover the expected kernel behavior when switching between requirements. Mario Limonciello (4): Add support for API for drivers to set power saving policy tests/amdgpu/amd_abm: Make set_abm_level return type int tests/amdgpu/amd_abm: Add support for panel_power_saving property tests/amdgpu/amd_psr: Add support for `power saving policy` property lib/igt_kms.c | 26 +++ lib/igt_kms.h | 6 tests/amdgpu/amd_abm.c | 72 ++-- tests/amdgpu/amd_psr.c | 74 ++ 4 files changed, 168 insertions(+), 10 deletions(-) -- 2.43.0
[PATCH v2 1/4] Add support for API for drivers to set power saving policy
---
lib/igt_kms.c | 26 ++
lib/igt_kms.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index af63d13b1..4ce5e4a95 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6581,3 +6581,29 @@ int get_num_scalers(int drm_fd, enum pipe pipe)
return num_scalers;
}
+
+static int toggle_power_saving_policy_prop(int drm_fd, igt_output_t *output,
uint64_t policy)
+{
+ uint32_t type = DRM_MODE_OBJECT_CONNECTOR;
+ bool prop_exists;
+ uint32_t prop_id;
+
+ prop_exists = kmstest_get_property(
+ drm_fd, output->id, type, "power saving policy",
+ _id, NULL, NULL);
+
+ if (!prop_exists)
+ return -ENODEV;
+
+ return drmModeObjectSetProperty(drm_fd, output->id, type, prop_id,
policy);
+}
+
+int clear_power_saving_policy(int drm_fd, igt_output_t *output)
+{
+ return toggle_power_saving_policy_prop(drm_fd, output, 0);
+}
+
+int set_panel_power_saving_policy(int drm_fd, igt_output_t *output, uint64_t
policy)
+{
+ return toggle_power_saving_policy_prop(drm_fd, output, policy);
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 01604dac9..129b88576 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1223,4 +1223,10 @@ bool igt_check_output_is_dp_mst(igt_output_t *output);
int igt_get_dp_mst_connector_id(igt_output_t *output);
int get_num_scalers(int drm_fd, enum pipe pipe);
+#define DRM_MODE_REQUIRE_COLOR_ACCURACYBIT(0) /* Compositor requires
color accuracy */
+#define DRM_MODE_REQUIRE_LOW_LATENCY BIT(1) /* Compositor requires low
latency */
+
+int clear_power_saving_policy(int drm_fd, igt_output_t *output);
+int set_panel_power_saving_policy(int drm_fd, igt_output_t *output, uint64_t
policy);
+
#endif /* __IGT_KMS_H__ */
--
2.43.0
[PATCH v2 1/4] Add support for API for drivers to set power saving policy
---
lib/igt_kms.c | 26 ++
lib/igt_kms.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index af63d13b1..4ce5e4a95 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6581,3 +6581,29 @@ int get_num_scalers(int drm_fd, enum pipe pipe)
return num_scalers;
}
+
+static int toggle_power_saving_policy_prop(int drm_fd, igt_output_t *output,
uint64_t policy)
+{
+ uint32_t type = DRM_MODE_OBJECT_CONNECTOR;
+ bool prop_exists;
+ uint32_t prop_id;
+
+ prop_exists = kmstest_get_property(
+ drm_fd, output->id, type, "power saving policy",
+ _id, NULL, NULL);
+
+ if (!prop_exists)
+ return -ENODEV;
+
+ return drmModeObjectSetProperty(drm_fd, output->id, type, prop_id,
policy);
+}
+
+int clear_power_saving_policy(int drm_fd, igt_output_t *output)
+{
+ return toggle_power_saving_policy_prop(drm_fd, output, 0);
+}
+
+int set_panel_power_saving_policy(int drm_fd, igt_output_t *output, uint64_t
policy)
+{
+ return toggle_power_saving_policy_prop(drm_fd, output, policy);
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 01604dac9..129b88576 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1223,4 +1223,10 @@ bool igt_check_output_is_dp_mst(igt_output_t *output);
int igt_get_dp_mst_connector_id(igt_output_t *output);
int get_num_scalers(int drm_fd, enum pipe pipe);
+#define DRM_MODE_REQUIRE_COLOR_ACCURACYBIT(0) /* Compositor requires
color accuracy */
+#define DRM_MODE_REQUIRE_LOW_LATENCY BIT(1) /* Compositor requires low
latency */
+
+int clear_power_saving_policy(int drm_fd, igt_output_t *output);
+int set_panel_power_saving_policy(int drm_fd, igt_output_t *output, uint64_t
policy);
+
#endif /* __IGT_KMS_H__ */
--
2.43.0
[PATCH v2 0/4] Add support for testing power saving policy DRM property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity or low latency is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured by the compositor to for it's requirements. When a driver advertises support for this property and the compositor sets requirements then the driver will disable any appplicable power saving features that can compromise the requirements. This set of IGT changes introduces a new subtest that will cover the expected kernel behavior when switching between requirements. Mario Limonciello (4): Add support for API for drivers to set power saving policy tests/amdgpu/amd_abm: Make set_abm_level return type int tests/amdgpu/amd_abm: Add support for panel_power_saving property tests/amdgpu/amd_psr: Add support for `power saving policy` property lib/igt_kms.c | 26 +++ lib/igt_kms.h | 6 tests/amdgpu/amd_abm.c | 72 ++-- tests/amdgpu/amd_psr.c | 74 ++ 4 files changed, 168 insertions(+), 10 deletions(-) -- 2.43.0
[PATCH v2 1/2] drm: Introduce 'power saving policy' drm property
The `power saving policy` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of policy of
whether a driver can use power saving features that may compromise
the experience intended by the compositor.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_connector.c | 46 +
include/drm/drm_connector.h | 2 ++
include/drm/drm_mode_config.h | 5
include/uapi/drm/drm_mode.h | 7 +
4 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..088a5874c7a4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_power_saving_policy_enum_list[] = {
+ { __builtin_ffs(DRM_MODE_REQUIRE_COLOR_ACCURACY) - 1, "Require color
accuracy" },
+ { __builtin_ffs(DRM_MODE_REQUIRE_LOW_LATENCY) - 1, "Require low
latency" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1499,6 +1504,16 @@ static const u32 dp_colorspaces =
*
* Drivers can set up these properties by calling
* drm_mode_create_tv_margin_properties().
+ * power saving policy:
+ * This property is used to set the power saving policy for the connector.
+ * This property is populated with a bitmask of optional requirements set
+ * by the drm master for the drm driver to respect:
+ * - "Require color accuracy": Disable power saving features that will
+ * affect color fidelity.
+ * For example: Hardware assisted backlight modulation.
+ * - "Require low latency": Disable power saving features that will
+ * affect latency.
+ * For example: Panel self refresh (PSR)
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1963,6 +1978,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_power_saving_policy_property - create power saving policy
property
+ * @dev: DRM device
+ * @supported_policies: bitmask of supported power saving policies
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies)
+{
+ struct drm_property *power_saving;
+
+ if (dev->mode_config.power_saving_policy)
+ return 0;
+ WARN_ON((supported_policies & DRM_MODE_POWER_SAVING_POLICY_ALL) == 0);
+
+ power_saving =
+ drm_property_create_bitmask(dev, 0, "power saving policy",
+ drm_power_saving_policy_enum_list,
+
ARRAY_SIZE(drm_power_saving_policy_enum_list),
+ supported_policies);
+
+ dev->mode_config.power_saving_policy = power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_power_saving_policy_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..b0ec2ec48de7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,8 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..32077e701e2f 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -954,6 +954,11 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @power_saving_policy: bitmask for power saving policy requests.
+*/
+ struct drm_property *power_saving_policy;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 7040e7ea80c7..c2c86623552c 100644
--- a/include/uapi/d
[PATCH v2 0/2] Add support for 'power saving policy' property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured with requests of "Require color accuracy" or "Require low latency" that can be configured by the compositor. When a driver advertises support for this property and the compositor sets it to "Require color accuracy" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Require color accuracy" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. Compositors can also request that low latency is critical which in practice should cause PSR and PSR2 to be disabled. When the compositor has restored the value back to no requirements then the previous value that would have been programmed will be restored. --- v1->v2: * New property as a bitmask * Handle both ABM and PSR/PSR2 * Add documentation Mario Limonciello (2): drm: Introduce 'power saving policy' drm property drm/amd: Add power_saving_policy drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + drivers/gpu/drm/drm_connector.c | 46 + include/drm/drm_connector.h | 2 + include/drm/drm_mode_config.h | 5 ++ include/uapi/drm/drm_mode.h | 7 +++ 7 files changed, 112 insertions(+), 5 deletions(-) -- 2.43.0
[PATCH v2 2/2] drm/amd: Add power_saving_policy drm property to eDP connectors
When the `power_saving_policy` property is set to bit mask
"Require color accuracy" ABM should be disabled immediately and
any requests by sysfs to update will return an -EBUSY error.
When the `power_saving_policy` property is set to bit mask
"Require low latency" PSR should be disabled.
When the property is restored to an empty bit mask the previous
value of ABM and pSR will be restored.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
3 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..cfb5220cf182 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,10 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_power_saving_policy_property(adev_to_drm(adev),
+
DRM_MODE_POWER_SAVING_POLICY_ALL);
+
return 0;
}
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 01b0a331e4a6..a480e86892de 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6421,6 +6421,13 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ dm_new_state->abm_forbidden = val &
DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ dm_new_state->abm_level = (dm_new_state->abm_forbidden ||
!amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ dm_new_state->psr_forbidden = val &
DRM_MODE_REQUIRE_LOW_LATENCY;
+ ret = 0;
}
return ret;
@@ -6463,6 +6470,13 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ *val = 0;
+ if (dm_state->psr_forbidden)
+ *val |= DRM_MODE_REQUIRE_LOW_LATENCY;
+ if (dm_state->abm_forbidden)
+ *val |= DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ ret = 0;
}
return ret;
@@ -6489,9 +6503,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6515,10 +6532,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7689,6 +7712,13 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP &&
+
[PATCH v2 2/2] drm/amd: Add power_saving_policy drm property to eDP connectors
When the `power_saving_policy` property is set to bit mask
"Require color accuracy" ABM should be disabled immediately and
any requests by sysfs to update will return an -EBUSY error.
When the `power_saving_policy` property is set to bit mask
"Require low latency" PSR should be disabled.
When the property is restored to an empty bit mask the previous
value of ABM and pSR will be restored.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
3 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..cfb5220cf182 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,10 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_power_saving_policy_property(adev_to_drm(adev),
+
DRM_MODE_POWER_SAVING_POLICY_ALL);
+
return 0;
}
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 01b0a331e4a6..a480e86892de 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6421,6 +6421,13 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ dm_new_state->abm_forbidden = val &
DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ dm_new_state->abm_level = (dm_new_state->abm_forbidden ||
!amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ dm_new_state->psr_forbidden = val &
DRM_MODE_REQUIRE_LOW_LATENCY;
+ ret = 0;
}
return ret;
@@ -6463,6 +6470,13 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.power_saving_policy) {
+ *val = 0;
+ if (dm_state->psr_forbidden)
+ *val |= DRM_MODE_REQUIRE_LOW_LATENCY;
+ if (dm_state->abm_forbidden)
+ *val |= DRM_MODE_REQUIRE_COLOR_ACCURACY;
+ ret = 0;
}
return ret;
@@ -6489,9 +6503,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6515,10 +6532,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7689,6 +7712,13 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP &&
+
[PATCH v2 1/2] drm: Introduce 'power saving policy' drm property
The `power saving policy` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of policy of
whether a driver can use power saving features that may compromise
the experience intended by the compositor.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_connector.c | 46 +
include/drm/drm_connector.h | 2 ++
include/drm/drm_mode_config.h | 5
include/uapi/drm/drm_mode.h | 7 +
4 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..088a5874c7a4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_power_saving_policy_enum_list[] = {
+ { __builtin_ffs(DRM_MODE_REQUIRE_COLOR_ACCURACY) - 1, "Require color
accuracy" },
+ { __builtin_ffs(DRM_MODE_REQUIRE_LOW_LATENCY) - 1, "Require low
latency" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1499,6 +1504,16 @@ static const u32 dp_colorspaces =
*
* Drivers can set up these properties by calling
* drm_mode_create_tv_margin_properties().
+ * power saving policy:
+ * This property is used to set the power saving policy for the connector.
+ * This property is populated with a bitmask of optional requirements set
+ * by the drm master for the drm driver to respect:
+ * - "Require color accuracy": Disable power saving features that will
+ * affect color fidelity.
+ * For example: Hardware assisted backlight modulation.
+ * - "Require low latency": Disable power saving features that will
+ * affect latency.
+ * For example: Panel self refresh (PSR)
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1963,6 +1978,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_power_saving_policy_property - create power saving policy
property
+ * @dev: DRM device
+ * @supported_policies: bitmask of supported power saving policies
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies)
+{
+ struct drm_property *power_saving;
+
+ if (dev->mode_config.power_saving_policy)
+ return 0;
+ WARN_ON((supported_policies & DRM_MODE_POWER_SAVING_POLICY_ALL) == 0);
+
+ power_saving =
+ drm_property_create_bitmask(dev, 0, "power saving policy",
+ drm_power_saving_policy_enum_list,
+
ARRAY_SIZE(drm_power_saving_policy_enum_list),
+ supported_policies);
+
+ dev->mode_config.power_saving_policy = power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_power_saving_policy_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..b0ec2ec48de7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,8 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_power_saving_policy_property(struct drm_device *dev,
+uint64_t supported_policies);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..32077e701e2f 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -954,6 +954,11 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @power_saving_policy: bitmask for power saving policy requests.
+*/
+ struct drm_property *power_saving_policy;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 7040e7ea80c7..c2c86623552c 100644
--- a/include/uapi/d
[PATCH v2 0/2] Add support for 'power saving policy' property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "power saving policy". This property is a bit mask that can be configured with requests of "Require color accuracy" or "Require low latency" that can be configured by the compositor. When a driver advertises support for this property and the compositor sets it to "Require color accuracy" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Require color accuracy" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. Compositors can also request that low latency is critical which in practice should cause PSR and PSR2 to be disabled. When the compositor has restored the value back to no requirements then the previous value that would have been programmed will be restored. --- v1->v2: * New property as a bitmask * Handle both ABM and PSR/PSR2 * Add documentation Mario Limonciello (2): drm: Introduce 'power saving policy' drm property drm/amd: Add power_saving_policy drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + drivers/gpu/drm/drm_connector.c | 46 + include/drm/drm_connector.h | 2 + include/drm/drm_mode_config.h | 5 ++ include/uapi/drm/drm_mode.h | 7 +++ 7 files changed, 112 insertions(+), 5 deletions(-) -- 2.43.0
Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs
+ Simon
On 5/22/2024 05:07, Rino André Johnsen wrote:
To be perfectly honest with you, I haven't given that much though. I
used the 'bpc' and 'colorspace' property in debugfs, since I could not
find that information anywhere else. And since I also needed to verify
the pixel encoding being used, I added it where those other values
were. That made for a simple and easy addition for this property.
If you want me to do this differently, let me know. And please point
me to the standardized DRM property where I should expose the values.
Here's a pointer to where the colorspace property is created:
https://github.com/torvalds/linux/blob/v6.9/drivers/gpu/drm/drm_connector.c#L2147
I would expect you can make another property for the information you're
looking for and then can get it from userspace using standard property
APIs.
Rino
On Tue, May 21, 2024 at 10:55 PM Mario Limonciello
wrote:
On 5/21/2024 15:06, Rino André Johnsen wrote:
What is already there in debugfs is 'bpc' and 'colorspace', but not
the pixel encoding/format.
I have searched high and low for that to be able to verify that my
monitor and computer are using my preferred combination of all those
three values.
I do think it should be available as a standard DRM CRTC property, but
for the time being, I figured that a simple debugfs property would be
sufficient for time being.
It's just about as much work either way to populate it though, why do it
twice instead of just doing it right the first time?
Rino
On Tue, May 21, 2024 at 9:04 PM Christian König
wrote:
Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Why isn't that available as standard DRM CRTC property in either sysfs
or debugfs?
I think the format specifiers should already be available somewhere there.
Regards,
Christian.
Signed-off-by: Rino Andre Johnsen
---
Changes in v2:
1. Do not initialize dm_crtc_state to NULL.
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..4254d4a4b56b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state;
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs
+ Simon
On 5/22/2024 05:07, Rino André Johnsen wrote:
To be perfectly honest with you, I haven't given that much though. I
used the 'bpc' and 'colorspace' property in debugfs, since I could not
find that information anywhere else. And since I also needed to verify
the pixel encoding being used, I added it where those other values
were. That made for a simple and easy addition for this property.
If you want me to do this differently, let me know. And please point
me to the standardized DRM property where I should expose the values.
Here's a pointer to where the colorspace property is created:
https://github.com/torvalds/linux/blob/v6.9/drivers/gpu/drm/drm_connector.c#L2147
I would expect you can make another property for the information you're
looking for and then can get it from userspace using standard property
APIs.
Rino
On Tue, May 21, 2024 at 10:55 PM Mario Limonciello
wrote:
On 5/21/2024 15:06, Rino André Johnsen wrote:
What is already there in debugfs is 'bpc' and 'colorspace', but not
the pixel encoding/format.
I have searched high and low for that to be able to verify that my
monitor and computer are using my preferred combination of all those
three values.
I do think it should be available as a standard DRM CRTC property, but
for the time being, I figured that a simple debugfs property would be
sufficient for time being.
It's just about as much work either way to populate it though, why do it
twice instead of just doing it right the first time?
Rino
On Tue, May 21, 2024 at 9:04 PM Christian König
wrote:
Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Why isn't that available as standard DRM CRTC property in either sysfs
or debugfs?
I think the format specifiers should already be available somewhere there.
Regards,
Christian.
Signed-off-by: Rino Andre Johnsen
---
Changes in v2:
1. Do not initialize dm_crtc_state to NULL.
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..4254d4a4b56b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state;
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs
On 5/21/2024 15:06, Rino André Johnsen wrote:
What is already there in debugfs is 'bpc' and 'colorspace', but not
the pixel encoding/format.
I have searched high and low for that to be able to verify that my
monitor and computer are using my preferred combination of all those
three values.
I do think it should be available as a standard DRM CRTC property, but
for the time being, I figured that a simple debugfs property would be
sufficient for time being.
It's just about as much work either way to populate it though, why do it
twice instead of just doing it right the first time?
Rino
On Tue, May 21, 2024 at 9:04 PM Christian König
wrote:
Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Why isn't that available as standard DRM CRTC property in either sysfs
or debugfs?
I think the format specifiers should already be available somewhere there.
Regards,
Christian.
Signed-off-by: Rino Andre Johnsen
---
Changes in v2:
1. Do not initialize dm_crtc_state to NULL.
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..4254d4a4b56b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state;
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs
On 5/21/2024 15:06, Rino André Johnsen wrote:
What is already there in debugfs is 'bpc' and 'colorspace', but not
the pixel encoding/format.
I have searched high and low for that to be able to verify that my
monitor and computer are using my preferred combination of all those
three values.
I do think it should be available as a standard DRM CRTC property, but
for the time being, I figured that a simple debugfs property would be
sufficient for time being.
It's just about as much work either way to populate it though, why do it
twice instead of just doing it right the first time?
Rino
On Tue, May 21, 2024 at 9:04 PM Christian König
wrote:
Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Why isn't that available as standard DRM CRTC property in either sysfs
or debugfs?
I think the format specifiers should already be available somewhere there.
Regards,
Christian.
Signed-off-by: Rino Andre Johnsen
---
Changes in v2:
1. Do not initialize dm_crtc_state to NULL.
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..4254d4a4b56b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state;
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
[Bug 2063002] Re: Add support for DCN 3.5
As the firmware is on it's own stable and by the time this hardware is in market that kernel fix should be picked up adding verification done tag. ** Tags added: verification-done-noble -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063002 Title: Add support for DCN 3.5 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux-firmware/+bug/2063002/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2063002] Re: Add support for DCN 3.5
Internal team tested this SRU against current generic and OEM kernel and it fails on both because they're missing the fix for https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2066233. After adding in that fix it works. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2063002 Title: Add support for DCN 3.5 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux-firmware/+bug/2063002/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065839] Re: UBSAN: array-index-out-of-bounds
** Changed in: linux (Ubuntu) Status: New => Invalid ** No longer affects: linux (Ubuntu) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065839 Title: UBSAN: array-index-out-of-bounds To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/broadcom-sta/+bug/2065839/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 12:27, Leo Li wrote: On 2024-05-21 12:21, Mario Limonciello wrote: On 5/21/2024 11:14, Xaver Hugl wrote: Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello : On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" It's not just about colors though, is it? The compositor might want to disable it to increase the backlight brightness in bright environments, so "color fidelity" doesn't really sound right Either of these better? "power saving policy" --> "power saving" --> "accuracy" "power saving policy" --> "allowed" --> "forbidden" Or any other idea? Another consideration in addition to accuracy is latency. I suppose a compositor may want to disable features such as PSR for use-cases requiring low latency. Touch and stylus input are some examples. I wonder if flags would work better than enums? A compositor can set something like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. I thought we had said the PSR latency issue discussed at the hackfest was likely just a bug and that nominally it won't matter? If it really will matter enough then yeah I think you're right that flags would be better. More drivers would probably want to opt into the property for the purpose of turning off PSR/PSR2/panel replay as well then. - Leo Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 12:27, Leo Li wrote: On 2024-05-21 12:21, Mario Limonciello wrote: On 5/21/2024 11:14, Xaver Hugl wrote: Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello : On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" It's not just about colors though, is it? The compositor might want to disable it to increase the backlight brightness in bright environments, so "color fidelity" doesn't really sound right Either of these better? "power saving policy" --> "power saving" --> "accuracy" "power saving policy" --> "allowed" --> "forbidden" Or any other idea? Another consideration in addition to accuracy is latency. I suppose a compositor may want to disable features such as PSR for use-cases requiring low latency. Touch and stylus input are some examples. I wonder if flags would work better than enums? A compositor can set something like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. I thought we had said the PSR latency issue discussed at the hackfest was likely just a bug and that nominally it won't matter? If it really will matter enough then yeah I think you're right that flags would be better. More drivers would probably want to opt into the property for the purpose of turning off PSR/PSR2/panel replay as well then. - Leo Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH v1 0/3] Add support for ISP interrupts and disable prefetch
On 5/21/2024 11:48, Pratap Nirujogi wrote: Add support for ISP interrupts and disable MMHUB prefetch for ISP v4.1.1 Pratap Nirujogi (3): drm/amd/amdgpu: Map ISP interrupts as generic IRQs drm/amd/amdgpu: Add ISP4.1.0 and ISP4.1.1 modules drm/amd/amdgpu: Disable MMHUB prefetch for ISP v4.1.1 drivers/gpu/drm/amd/amdgpu/Makefile | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 144 + drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h | 13 +- drivers/gpu/drm/amd/amdgpu/ih_v6_1.c | 6 + drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c | 149 ++ drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.h | 46 ++ drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c | 149 ++ drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.h | 46 ++ .../amd/include/ivsrcid/isp/irqsrcs_isp_4_1.h | 62 11 files changed, 517 insertions(+), 110 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c create mode 100644 drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.h create mode 100644 drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c create mode 100644 drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.h create mode 100644 drivers/gpu/drm/amd/include/ivsrcid/isp/irqsrcs_isp_4_1.h Reviewed-by: Mario Limonciello
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 11:14, Xaver Hugl wrote: Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello : On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" It's not just about colors though, is it? The compositor might want to disable it to increase the backlight brightness in bright environments, so "color fidelity" doesn't really sound right Either of these better? "power saving policy" --> "power saving" --> "accuracy" "power saving policy" --> "allowed" --> "forbidden" Or any other idea? Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 11:14, Xaver Hugl wrote: Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello : On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" It's not just about colors though, is it? The compositor might want to disable it to increase the backlight brightness in bright environments, so "color fidelity" doesn't really sound right Either of these better? "power saving policy" --> "power saving" --> "accuracy" "power saving policy" --> "allowed" --> "forbidden" Or any other idea? Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH 0/2] Add support for Panel Power Savings property
On 5/21/2024 08:43, Simon Ser wrote: This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" Would be nice to add documentation for the property in the "standard connector properties" section. Ack.
Re: [PATCH] drm/amd/display: Add pixel encoding info to debugfs
On 5/20/2024 16:07, Rino Andre Johnsen wrote:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Signed-off-by: Rino Andre Johnsen
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..d275e5522335 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state = NULL;
There is no need to initialize dm_crtc_state to NULL. You set it before
its first use.
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
Re: [PATCH] drm/amd/display: Add pixel encoding info to debugfs
On 5/20/2024 16:07, Rino Andre Johnsen wrote:
[Why]
For debugging and testing purposes.
[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
Signed-off-by: Rino Andre Johnsen
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..d275e5522335 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct
seq_file *m, void *data)
}
DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat
/sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+ struct drm_crtc *crtc = m->private;
+ struct drm_device *dev = crtc->dev;
+ struct dm_crtc_state *dm_crtc_state = NULL;
There is no need to initialize dm_crtc_state to NULL. You set it before
its first use.
+ int res = -ENODEV;
+
+ mutex_lock(>mode_config.mutex);
+ drm_modeset_lock(>mutex, NULL);
+ if (crtc->state == NULL)
+ goto unlock;
+
+ dm_crtc_state = to_dm_crtc_state(crtc->state);
+ if (dm_crtc_state->stream == NULL)
+ goto unlock;
+
+ switch (dm_crtc_state->stream->timing.pixel_encoding) {
+ case PIXEL_ENCODING_RGB:
+ seq_puts(m, "RGB");
+ break;
+ case PIXEL_ENCODING_YCBCR422:
+ seq_puts(m, "YCBCR422");
+ break;
+ case PIXEL_ENCODING_YCBCR444:
+ seq_puts(m, "YCBCR444");
+ break;
+ case PIXEL_ENCODING_YCBCR420:
+ seq_puts(m, "YCBCR420");
+ break;
+ default:
+ goto unlock;
+ }
+ res = 0;
+
+unlock:
+ drm_modeset_unlock(>mutex);
+ mutex_unlock(>mode_config.mutex);
+
+ return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
/*
* Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
crtc, _current_bpc_fops);
debugfs_create_file("amdgpu_current_colorspace", 0644,
crtc->debugfs_entry,
crtc, _current_colorspace_fops);
+ debugfs_create_file("amdgpu_current_pixelencoding", 0644,
crtc->debugfs_entry,
+ crtc, _current_pixelencoding_fops);
}
/*
[Bug 2065838] Re: System crash on resume from sleep
Save the below as a patch file and then apply using "patch -p1 < FILE".
Build your kernel and see if it has helped.
diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c
index 8907b8bf4267..ca060ec6936e 100644
--- a/drivers/acpi/acpica/exregion.c
+++ b/drivers/acpi/acpica/exregion.c
@@ -44,7 +44,6 @@ acpi_ex_system_memory_space_handler(u32 function,
struct acpi_mem_mapping *mm = mem_info->cur_mm;
u32 length;
acpi_size map_length;
- acpi_size page_boundary_map_length;
#ifdef ACPI_MISALIGNMENT_NOT_SUPPORTED
u32 remainder;
#endif
@@ -138,25 +137,8 @@ acpi_ex_system_memory_space_handler(u32 function,
map_length = (acpi_size)
((mem_info->address + mem_info->length) - address);
- /*
-* If mapping the entire remaining portion of the region will
cross
-* a page boundary, just map up to the page boundary, do not
cross.
-* On some systems, crossing a page boundary while mapping
regions
-* can cause warnings if the pages have different attributes
-* due to resource management.
-*
-* This has the added benefit of constraining a single mapping
to
-* one page, which is similar to the original code that used a
4k
-* maximum window.
-*/
- page_boundary_map_length = (acpi_size)
- (ACPI_ROUND_UP(address, ACPI_DEFAULT_PAGE_SIZE) - address);
- if (page_boundary_map_length == 0) {
- page_boundary_map_length = ACPI_DEFAULT_PAGE_SIZE;
- }
-
- if (map_length > page_boundary_map_length) {
- map_length = page_boundary_map_length;
+ if (map_length > ACPI_DEFAULT_PAGE_SIZE) {
+ map_length = ACPI_DEFAULT_PAGE_SIZE;
}
/* Create a new mapping starting at the address given */
--
2.34.1
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2065838
Title:
System crash on resume from sleep
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2065838/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065838] Re: System crash on resume from sleep
Yes since you didn't clone using git you can't use git revert. Once you can successfully build and test that kernel I'll post you a revert patch' with explanation how to use it. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065838 Title: System crash on resume from sleep To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2065838/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065838] Re: System crash on resume from sleep
Yeah I know they're different symptoms but the reason for that revert might have a similar root cause. I'm saying this because I've got a different system that fails to boot up that reverting that helps. In terms of specific instructions, I'd start with this: https://itsfoss.com/compile-linux-kernel/ Once you can get that compiling on your own I can help you with a applying a revert patch to see if it helps. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065838 Title: System crash on resume from sleep To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2065838/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[PATCH 2/2] tests/amdgpu/amd_abm: Add support for panel_power_saving property
From: Mario Limonciello
When the "panel power saving" property is set to forbidden the
compositor has indicated that userspace prefers to have color
accuracy and fidelity instead of power saving.
Verify that the sysfs file behaves as expected in this situation.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 65 ++
1 file changed, 65 insertions(+)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index f74c3012c..971cda153 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,6 +104,32 @@ static int backlight_write_brightness(int value)
return 0;
}
+static int toggle_panel_power_saving_prop(data_t *data, igt_output_t *output,
bool forbid)
+{
+ uint32_t type = DRM_MODE_OBJECT_CONNECTOR;
+ bool prop_exists;
+ uint32_t prop_id;
+
+ prop_exists = kmstest_get_property(
+ data->drm_fd, output->id, type, "panel power saving",
+ _id, NULL, NULL);
+
+ if (!prop_exists)
+ return -ENODEV;
+
+ return drmModeObjectSetProperty(data->drm_fd, output->id, type,
prop_id, forbid);
+}
+
+static int allow_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, false);
+}
+
+static int forbid_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, true);
+}
+
static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
@@ -365,6 +391,43 @@ static void abm_gradual(data_t *data)
}
}
+
+static void abm_forbidden(data_t *data)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int target, r;
+
+ for_each_pipe_with_valid_output(>display, pipe, output) {
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ r = allow_panel_power_saving(data, output);
+ if (r == -ENODEV) {
+ igt_skip("No panel power saving prop\n");
+ return;
+ }
+ igt_assert_eq(r, 0);
+
+ target = 3;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+
+ r = forbid_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ target = 0;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, -1);
+
+ r = allow_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+ }
+}
+
igt_main
{
data_t data = {};
@@ -393,6 +456,8 @@ igt_main
abm_enabled();
igt_subtest("abm_gradual")
abm_gradual();
+ igt_subtest("abm_forbidden")
+ abm_forbidden();
igt_fixture {
igt_display_fini();
--
2.45.0
[PATCH 2/2] tests/amdgpu/amd_abm: Add support for panel_power_saving property
From: Mario Limonciello
When the "panel power saving" property is set to forbidden the
compositor has indicated that userspace prefers to have color
accuracy and fidelity instead of power saving.
Verify that the sysfs file behaves as expected in this situation.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 65 ++
1 file changed, 65 insertions(+)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index f74c3012c..971cda153 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,6 +104,32 @@ static int backlight_write_brightness(int value)
return 0;
}
+static int toggle_panel_power_saving_prop(data_t *data, igt_output_t *output,
bool forbid)
+{
+ uint32_t type = DRM_MODE_OBJECT_CONNECTOR;
+ bool prop_exists;
+ uint32_t prop_id;
+
+ prop_exists = kmstest_get_property(
+ data->drm_fd, output->id, type, "panel power saving",
+ _id, NULL, NULL);
+
+ if (!prop_exists)
+ return -ENODEV;
+
+ return drmModeObjectSetProperty(data->drm_fd, output->id, type,
prop_id, forbid);
+}
+
+static int allow_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, false);
+}
+
+static int forbid_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, true);
+}
+
static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
@@ -365,6 +391,43 @@ static void abm_gradual(data_t *data)
}
}
+
+static void abm_forbidden(data_t *data)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int target, r;
+
+ for_each_pipe_with_valid_output(>display, pipe, output) {
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ r = allow_panel_power_saving(data, output);
+ if (r == -ENODEV) {
+ igt_skip("No panel power saving prop\n");
+ return;
+ }
+ igt_assert_eq(r, 0);
+
+ target = 3;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+
+ r = forbid_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ target = 0;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, -1);
+
+ r = allow_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+ }
+}
+
igt_main
{
data_t data = {};
@@ -393,6 +456,8 @@ igt_main
abm_enabled();
igt_subtest("abm_gradual")
abm_gradual();
+ igt_subtest("abm_forbidden")
+ abm_forbidden();
igt_fixture {
igt_display_fini();
--
2.45.0
[PATCH 0/2] Add support for testing panel power saving DRM property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. This set of IGT changes introduces a new subtest that will cover the expected kernel behavior when switching between Allowed and Forbidden. Mario Limonciello (2): tests/amdgpu/amd_abm: Make set_abm_level return type int tests/amdgpu/amd_abm: Add support for panel_power_saving property tests/amdgpu/amd_abm.c | 98 +- 1 file changed, 88 insertions(+), 10 deletions(-) -- 2.45.0
[PATCH 0/2] Add support for testing panel power saving DRM property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. This set of IGT changes introduces a new subtest that will cover the expected kernel behavior when switching between Allowed and Forbidden. Mario Limonciello (2): tests/amdgpu/amd_abm: Make set_abm_level return type int tests/amdgpu/amd_abm: Add support for panel_power_saving property tests/amdgpu/amd_abm.c | 98 +- 1 file changed, 88 insertions(+), 10 deletions(-) -- 2.45.0
[PATCH 1/2] tests/amdgpu/amd_abm: Make set_abm_level return type int
From: Mario Limonciello
In order to bubble of cases of expeted errors on set_abm_level()
change the return type to int.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index 2882c2c18..f74c3012c 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,10 +104,11 @@ static int backlight_write_brightness(int value)
return 0;
}
-static void set_abm_level(data_t *data, igt_output_t *output, int level)
+static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
int fd;
+ int ret;
igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
output->name) < PATH_MAX);
@@ -116,8 +117,12 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
igt_assert(fd != -1);
- igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
- write(fd, buf, 1));
+ snprintf(buf, sizeof(buf), "%d", level);
+ ret = write(fd, buf, 1);
+ if (ret < 0) {
+ close(fd);
+ return ret;
+ }
igt_assert_eq(close(fd), 0);
@@ -129,6 +134,7 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
DRM_MODE_DPMS_OFF);
kmstest_set_connector_dpms(data->drm_fd, output->config.connector,
DRM_MODE_DPMS_ON);
+ return 0;
}
static int backlight_read_max_brightness(int *result)
@@ -192,7 +198,8 @@ static void backlight_dpms_cycle(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness / 2);
usleep(10);
pwm_1 = read_target_backlight_pwm(data->drm_fd, output->name);
@@ -223,7 +230,8 @@ static void backlight_monotonic_basic(data_t *data)
brightness_step = max_brightness / 10;
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -257,7 +265,8 @@ static void backlight_monotonic_abm(data_t *data)
brightness_step = max_brightness / 10;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -289,14 +298,16 @@ static void abm_enabled(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
pwm_without_abm = prev_pwm;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, i);
+ ret = set_abm_level(data, output, i);
+ igt_assert_eq(ret, 0);
usleep(10);
pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
igt_assert(pwm <= prev_pwm);
@@ -323,7 +334,8 @@ static void abm_gradual(data_t *data)
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
sleep(convergence_delay);
@@ -331,7 +343,8 @@ static void abm_gradual(data_t *data)
curr = read_current_backlight_pwm(data->drm_fd, output->name);
igt_assert_eq(prev_pwm, curr);
- set_abm_level(data, output, 4);
+ ret = set_abm_level(data, output, 4);
+ igt_assert_eq(ret, 0);
for (i = 0; i < 10; i++) {
usleep(10);
pwm = read_current_backlight_pwm(data->drm_fd,
output->name);
--
2.45.0
[PATCH 1/2] tests/amdgpu/amd_abm: Make set_abm_level return type int
From: Mario Limonciello
In order to bubble of cases of expeted errors on set_abm_level()
change the return type to int.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index 2882c2c18..f74c3012c 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,10 +104,11 @@ static int backlight_write_brightness(int value)
return 0;
}
-static void set_abm_level(data_t *data, igt_output_t *output, int level)
+static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
int fd;
+ int ret;
igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
output->name) < PATH_MAX);
@@ -116,8 +117,12 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
igt_assert(fd != -1);
- igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
- write(fd, buf, 1));
+ snprintf(buf, sizeof(buf), "%d", level);
+ ret = write(fd, buf, 1);
+ if (ret < 0) {
+ close(fd);
+ return ret;
+ }
igt_assert_eq(close(fd), 0);
@@ -129,6 +134,7 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
DRM_MODE_DPMS_OFF);
kmstest_set_connector_dpms(data->drm_fd, output->config.connector,
DRM_MODE_DPMS_ON);
+ return 0;
}
static int backlight_read_max_brightness(int *result)
@@ -192,7 +198,8 @@ static void backlight_dpms_cycle(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness / 2);
usleep(10);
pwm_1 = read_target_backlight_pwm(data->drm_fd, output->name);
@@ -223,7 +230,8 @@ static void backlight_monotonic_basic(data_t *data)
brightness_step = max_brightness / 10;
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -257,7 +265,8 @@ static void backlight_monotonic_abm(data_t *data)
brightness_step = max_brightness / 10;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -289,14 +298,16 @@ static void abm_enabled(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
pwm_without_abm = prev_pwm;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, i);
+ ret = set_abm_level(data, output, i);
+ igt_assert_eq(ret, 0);
usleep(10);
pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
igt_assert(pwm <= prev_pwm);
@@ -323,7 +334,8 @@ static void abm_gradual(data_t *data)
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
sleep(convergence_delay);
@@ -331,7 +343,8 @@ static void abm_gradual(data_t *data)
curr = read_current_backlight_pwm(data->drm_fd, output->name);
igt_assert_eq(prev_pwm, curr);
- set_abm_level(data, output, 4);
+ ret = set_abm_level(data, output, 4);
+ igt_assert_eq(ret, 0);
for (i = 0; i < 10; i++) {
usleep(10);
pwm = read_current_backlight_pwm(data->drm_fd,
output->name);
--
2.45.0
[PATCH 0/2] Add support for Panel Power Savings property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Forbidden" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. When the compositor has restored the value back to "Allowed" then the previous value that would have been programmed will be restored. Mario Limonciello (2): drm: Introduce panel_power_saving drm property drm/amd: Add panel_power_saving drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 ++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + drivers/gpu/drm/drm_connector.c | 37 +++ include/drm/drm_connector.h | 1 + include/drm/drm_mode_config.h | 6 +++ include/uapi/drm/drm_mode.h | 4 ++ 7 files changed, 81 insertions(+), 5 deletions(-) -- 2.45.0
[PATCH 2/2] drm/amd: Add panel_power_saving drm property to eDP connectors
When the `panel_power_saving` property is set to "Forbidden" ABM
should be disabled immediately and any requests by sysfs to update
will return an -EBUSY error.
When the property is restored to "Allowed" the previous value of
ABM will be restored.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..6e6531c93d81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,9 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_panel_power_saving_property(adev_to_drm(adev));
+
return 0;
}
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 01b0a331e4a6..f6b80018b136 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6421,6 +6421,12 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ dm_new_state->abm_forbidden = val;
+ dm_new_state->abm_level = (val || !amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ ret = 0;
}
return ret;
@@ -6463,6 +6469,9 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ *val = dm_state->abm_forbidden;
+ ret = 0;
}
return ret;
@@ -6489,9 +6498,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6515,10 +6527,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7689,6 +7707,14 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP &&
+ (dc_is_dmcu_initialized(adev->dm.dc) ||
+adev->dm.dc->ctx->dmub_srv) && amdgpu_dm_abm_level < 0) {
+ drm_object_attach_property(>base.base,
+ dm->ddev->mode_config.panel_power_saving,
+ DRM_MODE_PANEL_POWER_SAVING_ALLOWED);
+ }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
/* Content Type is currently only implemented for HDMI. */
drm_connector_attach_content_type_property(>base);
diff --git a/drivers/gpu/drm/amd/display/amdgp
[PATCH 1/2] drm: Introduce panel_power_saving drm property
The `panel_power_saving` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of allowing
policy for the driver to use power saving features that may
compromise color fidelity.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_connector.c | 36 +
include/drm/drm_connector.h | 1 +
include/drm/drm_mode_config.h | 6 ++
include/uapi/drm/drm_mode.h | 4
4 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..ccf672c55e0c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_panel_power_saving_enum_list[] = {
+ { DRM_MODE_PANEL_POWER_SAVING_ALLOWED, "Allowed" },
+ { DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN, "Forbidden" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1963,6 +1968,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_panel_power_saving_property - create panel power saving
property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Atomic drivers should use drm_mode_create_panel_power_saving_property()
+ * instead to correctly assign _connector_state.panel_power_saving
+ * in the atomic state.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev)
+{
+ struct drm_property *panel_power_saving;
+
+ if (dev->mode_config.panel_power_saving)
+ return 0;
+
+ panel_power_saving =
+ drm_property_create_enum(dev, 0, "panel power saving",
+ drm_panel_power_saving_enum_list,
+
ARRAY_SIZE(drm_panel_power_saving_enum_list));
+
+ dev->mode_config.panel_power_saving = panel_power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_panel_power_saving_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..4ea3f912c641 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,7 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..099ad2d8c5c1 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -954,6 +954,12 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @panel_power_saving: DRM ENUM property for type of
+* Panel Power Saving.
+*/
+ struct drm_property *panel_power_saving;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 7040e7ea80c7..82e565cc76fb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -152,6 +152,10 @@ extern "C" {
#define DRM_MODE_SCALE_CENTER 2 /* Centered, no scaling */
#define DRM_MODE_SCALE_ASPECT 3 /* Full screen, preserve aspect */
+/* Panel power saving options */
+#define DRM_MODE_PANEL_POWER_SAVING_ALLOWED0 /* Panel power savings
features allowed */
+#define DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN 1 /* Panel power savings
features not allowed */
+
/* Dithering mode options */
#define DRM_MODE_DITHERING_OFF 0
#define DRM_MODE_DITHERING_ON 1
--
2.45.0
[PATCH 1/2] drm: Introduce panel_power_saving drm property
The `panel_power_saving` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of allowing
policy for the driver to use power saving features that may
compromise color fidelity.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_connector.c | 36 +
include/drm/drm_connector.h | 1 +
include/drm/drm_mode_config.h | 6 ++
include/uapi/drm/drm_mode.h | 4
4 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..ccf672c55e0c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_panel_power_saving_enum_list[] = {
+ { DRM_MODE_PANEL_POWER_SAVING_ALLOWED, "Allowed" },
+ { DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN, "Forbidden" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1963,6 +1968,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_panel_power_saving_property - create panel power saving
property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Atomic drivers should use drm_mode_create_panel_power_saving_property()
+ * instead to correctly assign _connector_state.panel_power_saving
+ * in the atomic state.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev)
+{
+ struct drm_property *panel_power_saving;
+
+ if (dev->mode_config.panel_power_saving)
+ return 0;
+
+ panel_power_saving =
+ drm_property_create_enum(dev, 0, "panel power saving",
+ drm_panel_power_saving_enum_list,
+
ARRAY_SIZE(drm_panel_power_saving_enum_list));
+
+ dev->mode_config.panel_power_saving = panel_power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_panel_power_saving_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..4ea3f912c641 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,7 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..099ad2d8c5c1 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -954,6 +954,12 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @panel_power_saving: DRM ENUM property for type of
+* Panel Power Saving.
+*/
+ struct drm_property *panel_power_saving;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 7040e7ea80c7..82e565cc76fb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -152,6 +152,10 @@ extern "C" {
#define DRM_MODE_SCALE_CENTER 2 /* Centered, no scaling */
#define DRM_MODE_SCALE_ASPECT 3 /* Full screen, preserve aspect */
+/* Panel power saving options */
+#define DRM_MODE_PANEL_POWER_SAVING_ALLOWED0 /* Panel power savings
features allowed */
+#define DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN 1 /* Panel power savings
features not allowed */
+
/* Dithering mode options */
#define DRM_MODE_DITHERING_OFF 0
#define DRM_MODE_DITHERING_ON 1
--
2.45.0
[PATCH 2/2] drm/amd: Add panel_power_saving drm property to eDP connectors
When the `panel_power_saving` property is set to "Forbidden" ABM
should be disabled immediately and any requests by sysfs to update
will return an -EBUSY error.
When the property is restored to "Allowed" the previous value of
ABM will be restored.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..6e6531c93d81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,9 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_panel_power_saving_property(adev_to_drm(adev));
+
return 0;
}
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 01b0a331e4a6..f6b80018b136 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6421,6 +6421,12 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ dm_new_state->abm_forbidden = val;
+ dm_new_state->abm_level = (val || !amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ ret = 0;
}
return ret;
@@ -6463,6 +6469,9 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ *val = dm_state->abm_forbidden;
+ ret = 0;
}
return ret;
@@ -6489,9 +6498,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6515,10 +6527,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7689,6 +7707,14 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP &&
+ (dc_is_dmcu_initialized(adev->dm.dc) ||
+adev->dm.dc->ctx->dmub_srv) && amdgpu_dm_abm_level < 0) {
+ drm_object_attach_property(>base.base,
+ dm->ddev->mode_config.panel_power_saving,
+ DRM_MODE_PANEL_POWER_SAVING_ALLOWED);
+ }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
/* Content Type is currently only implemented for HDMI. */
drm_connector_attach_content_type_property(>base);
diff --git a/drivers/gpu/drm/amd/display/amdgp
[PATCH 0/2] Add support for Panel Power Savings property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Forbidden" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. When the compositor has restored the value back to "Allowed" then the previous value that would have been programmed will be restored. Mario Limonciello (2): drm: Introduce panel_power_saving drm property drm/amd: Add panel_power_saving drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 ++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + drivers/gpu/drm/drm_connector.c | 37 +++ include/drm/drm_connector.h | 1 + include/drm/drm_mode_config.h | 6 +++ include/uapi/drm/drm_mode.h | 4 ++ 7 files changed, 81 insertions(+), 5 deletions(-) -- 2.45.0
[PATCH] drm/amd/display: Pass errors from amdgpu_dm_init() up
Errors in amdgpu_dm_init() are silently ignored and dm_hw_init()
will succeed. However often these are fatal errors and it would
be better to pass them up.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
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 d6e71aa808d8..01b0a331e4a6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2556,8 +2556,12 @@ static int amdgpu_dm_smu_write_watermarks_table(struct
amdgpu_device *adev)
static int dm_hw_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+ int r;
+
/* Create DAL display manager */
- amdgpu_dm_init(adev);
+ r = amdgpu_dm_init(adev);
+ if (r)
+ return r;
amdgpu_dm_hpd_init(adev);
return 0;
--
2.43.0
[Bug 2065838] Re: System crash on resume from sleep
As a random guess; could this be the same as
https://bugzilla.kernel.org/show_bug.cgi?id=218849?
Try reverting d410ee5109a1 ("ACPICA: avoid "Info: mapping multiple BARs.
Your kernel is fine."")
** Bug watch added: Linux Kernel Bug Tracker #218849
https://bugzilla.kernel.org/show_bug.cgi?id=218849
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2065838
Title:
System crash on resume from sleep
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2065838/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065948] Re: fwupdmgr enable/disable-remote auto-complete is buggy
Fixed upstream. https://github.com/fwupd/fwupd/pull/7264 ** Changed in: fwupd (Ubuntu) Status: New => Fix Committed -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065948 Title: fwupdmgr enable/disable-remote auto-complete is buggy To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/fwupd/+bug/2065948/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2065874] Re: Can't update firmware on TPM-backed FDE systems
This is the original bug for deb fwupd: https://github.com/canonical/ubuntu-desktop-installer/issues/2371 This is the original bug for snap fwupd: https://github.com/fwupd/fwupd/issues/6264 The problem is that fwupd (both deb and snap) don't understand the layout that TPM FDE uses. As mentioned in https://github.com/canonical/ubuntu-desktop- installer/issues/2371#issue-1940392263 about the deb problem: > fwupd is not aware of this layout. In order for a firmware update to work, fwupd expects to be able to create a new NVRAM boot entry using shim to chainload fwupdx64.efi. As mentioned in https://github.com/fwupd/fwupd/issues/6264#issuecomment-1764898120 about the snap problem: > My take on this issue is that it's because the Ubuntu 23.10 FDE mounts stuff in a weird location. The ESP is at /run/mnt which isn't something that the fwupd snap interface understands. It fully expects it to be in /boot/efi. ** Also affects: ubuntu-desktop-provision Importance: Undecided Status: New ** Changed in: fwupd (Ubuntu) Status: New => Triaged ** Bug watch added: github.com/canonical/ubuntu-desktop-installer/issues #2371 https://github.com/canonical/ubuntu-desktop-installer/issues/2371 ** Bug watch added: github.com/fwupd/fwupd/issues #6264 https://github.com/fwupd/fwupd/issues/6264 ** Also affects: snapd Importance: Undecided Status: New -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2065874 Title: Can't update firmware on TPM-backed FDE systems To manage notifications about this bug go to: https://bugs.launchpad.net/snapd/+bug/2065874/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 1942616] Re: Settings Power says high hardware temperature
It's saying the same thing the GUI does. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1942616 Title: Settings Power says high hardware temperature To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/power-profiles-daemon/+bug/1942616/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 1942616] Re: Settings Power says high hardware temperature
How about the command line tool (powerprofilesctl)? Can you switch using that? If it really is a pure GCC bug then you can file it here: https://launchpad.net/ubuntu/+source/gnome-control-center for Ubuntu and here: https://gitlab.gnome.org/GNOME/gnome-control-center for upstream. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1942616 Title: Settings Power says high hardware temperature To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/power-profiles-daemon/+bug/1942616/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 1942616] Re: Settings Power says high hardware temperature
You can file it here: https://bugzilla.kernel.org/ Mention your reproduction using a mainline kernel and add your logs. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1942616 Title: Settings Power says high hardware temperature To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/power-profiles-daemon/+bug/1942616/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
