* [PATCH 0/5] Adjustments to common mode behavior
@ 2025-09-24 16:16 Mario Limonciello
2025-09-24 16:16 ` [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS Mario Limonciello
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
As part of enablement for SI and CIK in DC Timur pointed out some
differences in behavior for common mode handling for DC vs non DC
code paths. This series lines up the behavior between the two
implementations.
Cc: Timur Kristóf <timur.kristof@gmail.com>
Mario Limonciello (5):
drm/amd/display: Only enable common modes for eDP and LVDS
drm/amd: Drop unnecessary check in amdgpu_connector_add_common_modes()
drm/amd: Use dynamic array size declaration for
amdgpu_connector_add_common_modes()
drm/amd: Drop some common modes from
amdgpu_connector_add_common_modes()
drm/amd: Add name to modes from amdgpu_connector_add_common_modes()
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 41 +++++++++----------
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++
2 files changed, 23 insertions(+), 22 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
@ 2025-09-24 16:16 ` Mario Limonciello
2025-09-24 17:58 ` Harry Wentland
2025-09-24 16:16 ` [PATCH 2/5] drm/amd: Drop unnecessary check in amdgpu_connector_add_common_modes() Mario Limonciello
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
[Why]
The main reason common modes are added is for compatibility with
clone mode when a laptop is connected to a projector or external
monitor. Since commit 978fa2f6d0b12 ("drm/amd/display: Use scaling
for non-native resolutions on eDP") when non-native modes are picked
for eDP the GPU scalar will be used. This is because it is inconsistent
whether eDP panels have the capability to actually drive non-native
resolutions. With panels connected to other connectors this limitation
generally doesn't exist as we the EDID will advertise support for a
number of resolutions and monitors will use built in scaling hardware.
Comparing DC and non-DC code paths the non-DC code path only adds
common modes for LVDS and eDP whereas the DC codepath does it for
all connector types.
In the past there was an experiment done to disable common mode adding
for eDP and LVDS from commit 6d396e7ac1ce3 ("drm/amd/display: Disable
common modes for LVDS") and commit 7948afb46af92 ("drm/amd/display:
Disable common modes for eDP") but this was reverted in
commit a8b79b09185de ("drm/amd: Re-enable common modes for eDP and
LVDS") because it caused problems with Xorg.
[How]
Only add common modes for eDP and LVDS for DC, matching the behavior
of non-DC.
Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++
1 file changed, 4 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 97d9eba17963..5a05ee6746af 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8151,6 +8151,10 @@ static void amdgpu_dm_connector_add_common_modes(struct drm_encoder *encoder,
{"1920x1200", 1920, 1200}
};
+ if ((connector->connector_type != DRM_MODE_CONNECTOR_eDP) &&
+ (connector->connector_type != DRM_MODE_CONNECTOR_LVDS))
+ return;
+
n = ARRAY_SIZE(common_modes);
for (i = 0; i < n; i++) {
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/5] drm/amd: Drop unnecessary check in amdgpu_connector_add_common_modes()
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
2025-09-24 16:16 ` [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS Mario Limonciello
@ 2025-09-24 16:16 ` Mario Limonciello
2025-09-24 16:16 ` [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes() Mario Limonciello
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
[Why]
amdgpu_connector_add_common_modes() has a check for the width and height
of common modes being too small, but the array of common_modes[] has fixed
values. The check is dead code.
[How]
Drop unnecessary check.
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 5e375e9c4f5d..84a8af961531 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -434,8 +434,6 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
common_modes[i].h == native_mode->vdisplay))
continue;
}
- if (common_modes[i].w < 320 || common_modes[i].h < 200)
- continue;
mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h, 60, false, false, false);
if (!mode)
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes()
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
2025-09-24 16:16 ` [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS Mario Limonciello
2025-09-24 16:16 ` [PATCH 2/5] drm/amd: Drop unnecessary check in amdgpu_connector_add_common_modes() Mario Limonciello
@ 2025-09-24 16:16 ` Mario Limonciello
2025-09-24 17:05 ` Alex Deucher
2025-09-24 16:16 ` [PATCH 4/5] drm/amd: Drop some common modes from amdgpu_connector_add_common_modes() Mario Limonciello
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
[Why]
Adding or removing a mode from common_modes[] can be fragile if a user
forgot to update the for loop boundaries.
[How]
Use ARRAY_SIZE() to detect size of the array and use that instead.
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 84a8af961531..ba24a0eba2a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -398,10 +398,11 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
struct drm_display_mode *mode = NULL;
struct drm_display_mode *native_mode = &amdgpu_encoder->native_mode;
int i;
+ int n;
static const struct mode_size {
int w;
int h;
- } common_modes[17] = {
+ } common_modes[] = {
{ 640, 480},
{ 720, 480},
{ 800, 600},
@@ -421,7 +422,9 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
{1920, 1200}
};
- for (i = 0; i < 17; i++) {
+ n = ARRAY_SIZE(common_modes);
+
+ for (i = 0; i < n; i++) {
if (amdgpu_encoder->devices & (ATOM_DEVICE_TV_SUPPORT)) {
if (common_modes[i].w > 1024 ||
common_modes[i].h > 768)
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/5] drm/amd: Drop some common modes from amdgpu_connector_add_common_modes()
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
` (2 preceding siblings ...)
2025-09-24 16:16 ` [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes() Mario Limonciello
@ 2025-09-24 16:16 ` Mario Limonciello
2025-09-24 16:16 ` [PATCH 5/5] drm/amd: Add name to " Mario Limonciello
2025-09-24 17:13 ` [PATCH 0/5] Adjustments to common mode behavior Timur Kristóf
5 siblings, 0 replies; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
[Why]
DC and non-DC codepaths have different sets of common modes that are
added for eDP and LVDS cases. This can cause different behaviors for
turning on DC on hardware that can support both.
[How]
Drop extra modes from amdgpu_connector_add_common_modes() not present
in amdgpu_dm_connector_add_common_modes().
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index ba24a0eba2a4..8c085ed703dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -404,18 +404,12 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
int h;
} common_modes[] = {
{ 640, 480},
- { 720, 480},
{ 800, 600},
- { 848, 480},
{1024, 768},
- {1152, 768},
{1280, 720},
{1280, 800},
- {1280, 854},
- {1280, 960},
{1280, 1024},
{1440, 900},
- {1400, 1050},
{1680, 1050},
{1600, 1200},
{1920, 1080},
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/5] drm/amd: Add name to modes from amdgpu_connector_add_common_modes()
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
` (3 preceding siblings ...)
2025-09-24 16:16 ` [PATCH 4/5] drm/amd: Drop some common modes from amdgpu_connector_add_common_modes() Mario Limonciello
@ 2025-09-24 16:16 ` Mario Limonciello
2025-09-24 17:07 ` Alex Deucher
2025-09-24 17:13 ` [PATCH 0/5] Adjustments to common mode behavior Timur Kristóf
5 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 16:16 UTC (permalink / raw)
To: mario.limonciello, amd-gfx; +Cc: Timur Kristóf
[Why]
When DC adds common modes it adds modes with a string to match what
they are. Non-DC doesn't. This can be inconsistent when turning on/off
DC support.
[How]
Add a name member to common_modes[] and copy it into the drm display
mode.
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 26 ++++++++++---------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 8c085ed703dd..120d8017b6f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -399,21 +399,22 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
struct drm_display_mode *native_mode = &amdgpu_encoder->native_mode;
int i;
int n;
- static const struct mode_size {
+ struct mode_size {
+ char name[DRM_DISPLAY_MODE_LEN];
int w;
int h;
} common_modes[] = {
- { 640, 480},
- { 800, 600},
- {1024, 768},
- {1280, 720},
- {1280, 800},
- {1280, 1024},
- {1440, 900},
- {1680, 1050},
- {1600, 1200},
- {1920, 1080},
- {1920, 1200}
+ { "640x480", 640, 480},
+ { "800x600", 800, 600},
+ { "1024x768", 1024, 768},
+ { "1280x720", 1280, 720},
+ { "1280x800", 1280, 800},
+ {"1280x1024", 1280, 1024},
+ { "1440x900", 1440, 900},
+ {"1680x1050", 1680, 1050},
+ {"1600x1200", 1600, 1200},
+ {"1920x1080", 1920, 1080},
+ {"1920x1200", 1920, 1200}
};
n = ARRAY_SIZE(common_modes);
@@ -435,6 +436,7 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h, 60, false, false, false);
if (!mode)
return;
+ strscpy(mode->name, common_modes[i].name, DRM_DISPLAY_MODE_LEN);
drm_mode_probed_add(connector, mode);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes()
2025-09-24 16:16 ` [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes() Mario Limonciello
@ 2025-09-24 17:05 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2025-09-24 17:05 UTC (permalink / raw)
To: Mario Limonciello; +Cc: amd-gfx, Timur Kristóf
On Wed, Sep 24, 2025 at 12:54 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> [Why]
> Adding or removing a mode from common_modes[] can be fragile if a user
> forgot to update the for loop boundaries.
>
> [How]
> Use ARRAY_SIZE() to detect size of the array and use that instead.
>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 84a8af961531..ba24a0eba2a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -398,10 +398,11 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
> struct drm_display_mode *mode = NULL;
> struct drm_display_mode *native_mode = &amdgpu_encoder->native_mode;
> int i;
> + int n;
> static const struct mode_size {
> int w;
> int h;
> - } common_modes[17] = {
> + } common_modes[] = {
> { 640, 480},
> { 720, 480},
> { 800, 600},
> @@ -421,7 +422,9 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
> {1920, 1200}
> };
>
> - for (i = 0; i < 17; i++) {
> + n = ARRAY_SIZE(common_modes);
> +
> + for (i = 0; i < n; i++) {
Could drop n and just use ARRAY_SIZE() directly here. Either way:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> if (amdgpu_encoder->devices & (ATOM_DEVICE_TV_SUPPORT)) {
> if (common_modes[i].w > 1024 ||
> common_modes[i].h > 768)
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/5] drm/amd: Add name to modes from amdgpu_connector_add_common_modes()
2025-09-24 16:16 ` [PATCH 5/5] drm/amd: Add name to " Mario Limonciello
@ 2025-09-24 17:07 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2025-09-24 17:07 UTC (permalink / raw)
To: Mario Limonciello; +Cc: amd-gfx, Timur Kristóf
On Wed, Sep 24, 2025 at 12:24 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> [Why]
> When DC adds common modes it adds modes with a string to match what
> they are. Non-DC doesn't. This can be inconsistent when turning on/off
> DC support.
>
> [How]
> Add a name member to common_modes[] and copy it into the drm display
> mode.
>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 26 ++++++++++---------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 8c085ed703dd..120d8017b6f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -399,21 +399,22 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
> struct drm_display_mode *native_mode = &amdgpu_encoder->native_mode;
> int i;
> int n;
> - static const struct mode_size {
> + struct mode_size {
> + char name[DRM_DISPLAY_MODE_LEN];
> int w;
> int h;
> } common_modes[] = {
> - { 640, 480},
> - { 800, 600},
> - {1024, 768},
> - {1280, 720},
> - {1280, 800},
> - {1280, 1024},
> - {1440, 900},
> - {1680, 1050},
> - {1600, 1200},
> - {1920, 1080},
> - {1920, 1200}
> + { "640x480", 640, 480},
> + { "800x600", 800, 600},
> + { "1024x768", 1024, 768},
> + { "1280x720", 1280, 720},
> + { "1280x800", 1280, 800},
> + {"1280x1024", 1280, 1024},
> + { "1440x900", 1440, 900},
> + {"1680x1050", 1680, 1050},
> + {"1600x1200", 1600, 1200},
> + {"1920x1080", 1920, 1080},
> + {"1920x1200", 1920, 1200}
> };
>
> n = ARRAY_SIZE(common_modes);
> @@ -435,6 +436,7 @@ static void amdgpu_connector_add_common_modes(struct drm_encoder *encoder,
> mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h, 60, false, false, false);
> if (!mode)
> return;
> + strscpy(mode->name, common_modes[i].name, DRM_DISPLAY_MODE_LEN);
>
> drm_mode_probed_add(connector, mode);
> }
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
` (4 preceding siblings ...)
2025-09-24 16:16 ` [PATCH 5/5] drm/amd: Add name to " Mario Limonciello
@ 2025-09-24 17:13 ` Timur Kristóf
2025-09-24 17:21 ` Mario Limonciello
5 siblings, 1 reply; 20+ messages in thread
From: Timur Kristóf @ 2025-09-24 17:13 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx
On 9/24/25 18:16, Mario Limonciello wrote:
> As part of enablement for SI and CIK in DC Timur pointed out some
> differences in behavior for common mode handling for DC vs non DC
> code paths. This series lines up the behavior between the two
> implementations.
>
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
Thank you Mario, this series makes good sense to me.
My only worry is this: is it possible that removing the common modes
from connectors like DP, HDMI, etc. will regress somebody's setup?
Two possible cases come to mind:
1. When we are unable to read the EDID for some reason
2. When the EDID is buggy and/or doesn't contain any modes
Are these issues real or am I overthinking it?
Thanks & best regards,
Timur
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 17:13 ` [PATCH 0/5] Adjustments to common mode behavior Timur Kristóf
@ 2025-09-24 17:21 ` Mario Limonciello
2025-09-24 17:33 ` Timur Kristóf
0 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 17:21 UTC (permalink / raw)
To: Timur Kristóf, amd-gfx, Wentland, Harry
On 9/24/25 12:13 PM, Timur Kristóf wrote:
>
>
> On 9/24/25 18:16, Mario Limonciello wrote:
>> As part of enablement for SI and CIK in DC Timur pointed out some
>> differences in behavior for common mode handling for DC vs non DC
>> code paths. This series lines up the behavior between the two
>> implementations.
>>
> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>
> Thank you Mario, this series makes good sense to me.
> My only worry is this: is it possible that removing the common modes
> from connectors like DP, HDMI, etc. will regress somebody's setup?
Possibly. We're not going to know until we try. I generally prefer not
to add common modes (hence why I tried to drop them before until we hit
the Xorg bug report).
If someone complains about this then I see two other directions we can go.
1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP,
HDMI. Make them not apply common modes to VGA and DVI.
2) Enabling common modes /across the board/ but anything not in the EDID
gets the GPU scalar turned on.
>
> Two possible cases come to mind:
> 1. When we are unable to read the EDID for some reason
> 2. When the EDID is buggy and/or doesn't contain any modes
> Are these issues real or am I overthinking it?
>
> Thanks & best regards,
> Timur
Failing to read EDID has happened in the past, but I think with the
deferred aux message handling that should be cleared up now.
I don't think it's a bug if a monitor doesn't advertise support for
certain modes. Honestly I think we've gotten quite lucky that eDP that
panels worked with non-native resolutions not in the EDID in the first
place.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 17:21 ` Mario Limonciello
@ 2025-09-24 17:33 ` Timur Kristóf
2025-09-24 17:48 ` Mario Limonciello
0 siblings, 1 reply; 20+ messages in thread
From: Timur Kristóf @ 2025-09-24 17:33 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx, Wentland, Harry
On 9/24/25 19:21, Mario Limonciello wrote:
>
> On 9/24/25 12:13 PM, Timur Kristóf wrote:
>>
>>
>> On 9/24/25 18:16, Mario Limonciello wrote:
>>> As part of enablement for SI and CIK in DC Timur pointed out some
>>> differences in behavior for common mode handling for DC vs non DC
>>> code paths. This series lines up the behavior between the two
>>> implementations.
>>>
>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>
>> Thank you Mario, this series makes good sense to me.
>> My only worry is this: is it possible that removing the common modes
>> from connectors like DP, HDMI, etc. will regress somebody's setup?
>
> Possibly. We're not going to know until we try. I generally prefer not
> to add common modes (hence why I tried to drop them before until we hit
> the Xorg bug report).
>
> If someone complains about this then I see two other directions we can go.
Sounds good.
Considering the non-DC code already didn't add those common modes, I
think it's reasonable to assume that we would have already heard about
it if somebody had issues with it.
>
> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP,
> HDMI. Make them not apply common modes to VGA and DVI
>
> 2) Enabling common modes /across the board/ but anything not in the EDID
> gets the GPU scalar turned on.
I guess we'll see if any of those are necessary. For now, I'd propose to
just consider adding the common modes if there are 0 modes probed. But
I'm also OK with leaving that for later if you feel it isn't necessary.
A slightly related question, would you be OK with changing the link
detection code to return dc_connection_none when DDC cannot read an EDID
header on digital signals, similar to how the non-DC code does it?
>>
>> Two possible cases come to mind:
>> 1. When we are unable to read the EDID for some reason
>> 2. When the EDID is buggy and/or doesn't contain any modes
>> Are these issues real or am I overthinking it?
>>
>> Thanks & best regards,
>> Timur
>
> Failing to read EDID has happened in the past, but I think with the
> deferred aux message handling that should be cleared up now.
I was actually curious about that. I saw that issue while I was working
on something else. How is it deferred now? Can you point me to the
series that fixed it?
>
> I don't think it's a bug if a monitor doesn't advertise support for
> certain modes. Honestly I think we've gotten quite lucky that eDP that
> panels worked with non-native resolutions not in the EDID in the first
> place.
Agreed.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 17:33 ` Timur Kristóf
@ 2025-09-24 17:48 ` Mario Limonciello
2025-09-24 18:11 ` Timur Kristóf
2025-09-24 18:24 ` Harry Wentland
0 siblings, 2 replies; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 17:48 UTC (permalink / raw)
To: Timur Kristóf, amd-gfx, Wentland, Harry, Alex Hung
On 9/24/25 12:33 PM, Timur Kristóf wrote:
>
>
> On 9/24/25 19:21, Mario Limonciello wrote:
>>
>> On 9/24/25 12:13 PM, Timur Kristóf wrote:
>>>
>>>
>>> On 9/24/25 18:16, Mario Limonciello wrote:
>>>> As part of enablement for SI and CIK in DC Timur pointed out some
>>>> differences in behavior for common mode handling for DC vs non DC
>>>> code paths. This series lines up the behavior between the two
>>>> implementations.
>>>>
>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>
>>> Thank you Mario, this series makes good sense to me.
>>> My only worry is this: is it possible that removing the common modes
>>> from connectors like DP, HDMI, etc. will regress somebody's setup?
>>
>> Possibly. We're not going to know until we try. I generally prefer
>> not to add common modes (hence why I tried to drop them before until
>> we hit the Xorg bug report).
>>
>> If someone complains about this then I see two other directions we can
>> go.
>
> Sounds good.
>
> Considering the non-DC code already didn't add those common modes, I
> think it's reasonable to assume that we would have already heard about
> it if somebody had issues with it.
>
>>
>> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS,
>> DP, HDMI. Make them not apply common modes to VGA and DVI
>>
>> 2) Enabling common modes /across the board/ but anything not in the
>> EDID gets the GPU scalar turned on.
>
> I guess we'll see if any of those are necessary. For now, I'd propose to
> just consider adding the common modes if there are 0 modes probed. But
> I'm also OK with leaving that for later if you feel it isn't necessary.
>
Yeah if something comes up and we need to weight it out we have this
thread to refer back to for our ideas on what to do.
> A slightly related question, would you be OK with changing the link
> detection code to return dc_connection_none when DDC cannot read an EDID
> header on digital signals, similar to how the non-DC code does it?
>
I personally think lining up all these nuances that are different
between the two is a good idea.e e
But for that specific question that's probably more of a Harry/Alex Hung
question.
>>>
>>> Two possible cases come to mind:
>>> 1. When we are unable to read the EDID for some reason
>>> 2. When the EDID is buggy and/or doesn't contain any modes
>>> Are these issues real or am I overthinking it?
>>>
>>> Thanks & best regards,
>>> Timur
>>
>> Failing to read EDID has happened in the past, but I think with the
>> deferred aux message handling that should be cleared up now.
>
> I was actually curious about that. I saw that issue while I was working
> on something else. How is it deferred now? Can you point me to the
> series that fixed it?
>
There's more patches than this one, but I believe this was the 💰 patch.
https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS
2025-09-24 16:16 ` [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS Mario Limonciello
@ 2025-09-24 17:58 ` Harry Wentland
0 siblings, 0 replies; 20+ messages in thread
From: Harry Wentland @ 2025-09-24 17:58 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx; +Cc: Timur Kristóf
On 2025-09-24 12:16, Mario Limonciello wrote:
> [Why]
> The main reason common modes are added is for compatibility with
> clone mode when a laptop is connected to a projector or external
> monitor. Since commit 978fa2f6d0b12 ("drm/amd/display: Use scaling
> for non-native resolutions on eDP") when non-native modes are picked
> for eDP the GPU scalar will be used. This is because it is inconsistent
> whether eDP panels have the capability to actually drive non-native
> resolutions. With panels connected to other connectors this limitation
> generally doesn't exist as we the EDID will advertise support for a
> number of resolutions and monitors will use built in scaling hardware.
>
> Comparing DC and non-DC code paths the non-DC code path only adds
> common modes for LVDS and eDP whereas the DC codepath does it for
> all connector types.
>
> In the past there was an experiment done to disable common mode adding
> for eDP and LVDS from commit 6d396e7ac1ce3 ("drm/amd/display: Disable
> common modes for LVDS") and commit 7948afb46af92 ("drm/amd/display:
> Disable common modes for eDP") but this was reverted in
> commit a8b79b09185de ("drm/amd: Re-enable common modes for eDP and
> LVDS") because it caused problems with Xorg.
>
> [How]
> Only add common modes for eDP and LVDS for DC, matching the behavior
> of non-DC.
>
> Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Harry
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++
> 1 file changed, 4 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 97d9eba17963..5a05ee6746af 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8151,6 +8151,10 @@ static void amdgpu_dm_connector_add_common_modes(struct drm_encoder *encoder,
> {"1920x1200", 1920, 1200}
> };
>
> + if ((connector->connector_type != DRM_MODE_CONNECTOR_eDP) &&
> + (connector->connector_type != DRM_MODE_CONNECTOR_LVDS))
> + return;
> +
> n = ARRAY_SIZE(common_modes);
>
> for (i = 0; i < n; i++) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 17:48 ` Mario Limonciello
@ 2025-09-24 18:11 ` Timur Kristóf
2025-09-24 18:31 ` Mario Limonciello
2025-09-24 18:24 ` Harry Wentland
1 sibling, 1 reply; 20+ messages in thread
From: Timur Kristóf @ 2025-09-24 18:11 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx, Wentland, Harry, Alex Hung
On 9/24/25 19:48, Mario Limonciello wrote:
>
>> A slightly related question, would you be OK with changing the link
>> detection code to return dc_connection_none when DDC cannot read an
>> EDID header on digital signals, similar to how the non-DC code does it?
>>
>
> I personally think lining up all these nuances that are different
> between the two is a good idea.e e
>
> But for that specific question that's probably more of a Harry/Alex Hung
> question.
@Harry and @Alex, what do you guys think about this?
>>>>
>>>> Two possible cases come to mind:
>>>> 1. When we are unable to read the EDID for some reason
>>>> 2. When the EDID is buggy and/or doesn't contain any modes
>>>> Are these issues real or am I overthinking it?
>>>>
>>>> Thanks & best regards,
>>>> Timur
>>>
>>> Failing to read EDID has happened in the past, but I think with the
>>> deferred aux message handling that should be cleared up now.
>>
>> I was actually curious about that. I saw that issue while I was
>> working on something else. How is it deferred now? Can you point me to
>> the series that fixed it?
>>
>
> There's more patches than this one, but I believe this was the 💰 patch.
>
> https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
>
I'm not sure if that deals with the same issue that I'm referring to.
Basically what I notice is that when plugging in a cable, it can happen
that the HPD pins make contact before the DDC pins, and hence DC fails
to read the EDID. This can happen often with DVI ports and rarely with
HDMI (and never happened to me with DP).
The non-DC code has a solution for this to retry in a few seconds.
I wasn't able to find anything for this in DC.
I found some code to set up some HPD filters, but that code is not
called from anywhere and I was told the HPD filter may not be the right
solution.
So my question is, what would be the right solution? Can we use a HPD
filter? Or better to do what the non-DC code does and just retry again
later?
Thanks,
Timur
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 17:48 ` Mario Limonciello
2025-09-24 18:11 ` Timur Kristóf
@ 2025-09-24 18:24 ` Harry Wentland
2025-09-24 19:11 ` Alex Deucher
1 sibling, 1 reply; 20+ messages in thread
From: Harry Wentland @ 2025-09-24 18:24 UTC (permalink / raw)
To: Mario Limonciello, Timur Kristóf, amd-gfx, Alex Hung
On 2025-09-24 13:48, Mario Limonciello wrote:
> On 9/24/25 12:33 PM, Timur Kristóf wrote:
>>
>>
>> On 9/24/25 19:21, Mario Limonciello wrote:
>>>
>>> On 9/24/25 12:13 PM, Timur Kristóf wrote:
>>>>
>>>>
>>>> On 9/24/25 18:16, Mario Limonciello wrote:
>>>>> As part of enablement for SI and CIK in DC Timur pointed out some
>>>>> differences in behavior for common mode handling for DC vs non DC
>>>>> code paths. This series lines up the behavior between the two
>>>>> implementations.
>>>>>
>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>
>>>> Thank you Mario, this series makes good sense to me.
>>>> My only worry is this: is it possible that removing the common modes from connectors like DP, HDMI, etc. will regress somebody's setup?
>>>
>>> Possibly. We're not going to know until we try. I generally prefer not to add common modes (hence why I tried to drop them before until we hit the Xorg bug report).
>>>
>>> If someone complains about this then I see two other directions we can go.
>>
>> Sounds good.
>>
>> Considering the non-DC code already didn't add those common modes, I think it's reasonable to assume that we would have already heard about it if somebody had issues with it.
>>
>>>
>>> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP, HDMI. Make them not apply common modes to VGA and DVI
>>>
>>> 2) Enabling common modes /across the board/ but anything not in the EDID gets the GPU scalar turned on.
I was surprised the previous approach failed, which seems
to indicate GPU scaling isn't already happening. I wonder
why. I think this would make a better default behavior
instead of relying on monitor scalers to deal with
non-advertised modes.
Harry
>>
>> I guess we'll see if any of those are necessary. For now, I'd propose to just consider adding the common modes if there are 0 modes probed. But I'm also OK with leaving that for later if you feel it isn't necessary.
>>
>
> Yeah if something comes up and we need to weight it out we have this thread to refer back to for our ideas on what to do.
>
>> A slightly related question, would you be OK with changing the link detection code to return dc_connection_none when DDC cannot read an EDID header on digital signals, similar to how the non-DC code does it?
>>
>
> I personally think lining up all these nuances that are different between the two is a good idea.e e
>
> But for that specific question that's probably more of a Harry/Alex Hung question.
>
>>>>
>>>> Two possible cases come to mind:
>>>> 1. When we are unable to read the EDID for some reason
>>>> 2. When the EDID is buggy and/or doesn't contain any modes
>>>> Are these issues real or am I overthinking it?
>>>>
>>>> Thanks & best regards,
>>>> Timur
>>>
>>> Failing to read EDID has happened in the past, but I think with the deferred aux message handling that should be cleared up now.
>>
>> I was actually curious about that. I saw that issue while I was working on something else. How is it deferred now? Can you point me to the series that fixed it?
>>
>
> There's more patches than this one, but I believe this was the 💰 patch.
>
> https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 18:11 ` Timur Kristóf
@ 2025-09-24 18:31 ` Mario Limonciello
2025-09-24 21:04 ` Timur Kristóf
0 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2025-09-24 18:31 UTC (permalink / raw)
To: Timur Kristóf, amd-gfx, Wentland, Harry, Alex Hung
On 9/24/25 1:11 PM, Timur Kristóf wrote:
>
>
> On 9/24/25 19:48, Mario Limonciello wrote:
>
>>
>>> A slightly related question, would you be OK with changing the link
>>> detection code to return dc_connection_none when DDC cannot read an
>>> EDID header on digital signals, similar to how the non-DC code does it?
>>>
>>
>> I personally think lining up all these nuances that are different
>> between the two is a good idea.e e
>>
>> But for that specific question that's probably more of a Harry/Alex
>> Hung question.
>
> @Harry and @Alex, what do you guys think about this?
>
>>>>>
>>>>> Two possible cases come to mind:
>>>>> 1. When we are unable to read the EDID for some reason
>>>>> 2. When the EDID is buggy and/or doesn't contain any modes
>>>>> Are these issues real or am I overthinking it?
>>>>>
>>>>> Thanks & best regards,
>>>>> Timur
>>>>
>>>> Failing to read EDID has happened in the past, but I think with the
>>>> deferred aux message handling that should be cleared up now.
>>>
>>> I was actually curious about that. I saw that issue while I was
>>> working on something else. How is it deferred now? Can you point me
>>> to the series that fixed it?
>>>
>>
>> There's more patches than this one, but I believe this was the 💰 patch.
>>
>> https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
>>
>
> I'm not sure if that deals with the same issue that I'm referring to.
>
> Basically what I notice is that when plugging in a cable, it can happen
> that the HPD pins make contact before the DDC pins, and hence DC fails
> to read the EDID. This can happen often with DVI ports and rarely with
> HDMI (and never happened to me with DP).
>
> The non-DC code has a solution for this to retry in a few seconds.
> I wasn't able to find anything for this in DC.
> I found some code to set up some HPD filters, but that code is not
> called from anywhere and I was told the HPD filter may not be the right
> solution.
>
> So my question is, what would be the right solution? Can we use a HPD
> filter? Or better to do what the non-DC code does and just retry again
> later?
>
> Thanks,
> Timur
Ah yes this is a totally different problem. It doesn't turn into a
storm does it? I would think a filter only makes sense if there is an
interrupt storm.
As long as it's just one HPD and no EDID found from what you've
described I would think spinning up a timer to try again in a few
seconds sounds like a reasonable approach.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 18:24 ` Harry Wentland
@ 2025-09-24 19:11 ` Alex Deucher
2025-09-24 20:00 ` Harry Wentland
0 siblings, 1 reply; 20+ messages in thread
From: Alex Deucher @ 2025-09-24 19:11 UTC (permalink / raw)
To: Harry Wentland; +Cc: Mario Limonciello, Timur Kristóf, amd-gfx, Alex Hung
On Wed, Sep 24, 2025 at 2:44 PM Harry Wentland <harry.wentland@amd.com> wrote:
>
>
>
> On 2025-09-24 13:48, Mario Limonciello wrote:
> > On 9/24/25 12:33 PM, Timur Kristóf wrote:
> >>
> >>
> >> On 9/24/25 19:21, Mario Limonciello wrote:
> >>>
> >>> On 9/24/25 12:13 PM, Timur Kristóf wrote:
> >>>>
> >>>>
> >>>> On 9/24/25 18:16, Mario Limonciello wrote:
> >>>>> As part of enablement for SI and CIK in DC Timur pointed out some
> >>>>> differences in behavior for common mode handling for DC vs non DC
> >>>>> code paths. This series lines up the behavior between the two
> >>>>> implementations.
> >>>>>
> >>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> >>>>
> >>>> Thank you Mario, this series makes good sense to me.
> >>>> My only worry is this: is it possible that removing the common modes from connectors like DP, HDMI, etc. will regress somebody's setup?
> >>>
> >>> Possibly. We're not going to know until we try. I generally prefer not to add common modes (hence why I tried to drop them before until we hit the Xorg bug report).
> >>>
> >>> If someone complains about this then I see two other directions we can go.
> >>
> >> Sounds good.
> >>
> >> Considering the non-DC code already didn't add those common modes, I think it's reasonable to assume that we would have already heard about it if somebody had issues with it.
> >>
> >>>
> >>> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP, HDMI. Make them not apply common modes to VGA and DVI
> >>>
> >>> 2) Enabling common modes /across the board/ but anything not in the EDID gets the GPU scalar turned on.
>
> I was surprised the previous approach failed, which seems
> to indicate GPU scaling isn't already happening. I wonder
> why. I think this would make a better default behavior
> instead of relying on monitor scalers to deal with
> non-advertised modes.
My thinking with the original logic in radeon and the amdgpu non-DC
code was to only add the common modes to eDP/LVDS because the EDIDs
for those panels usually only had one mode in them and users almost
always wanted to do clone mode with an external monitor. For external
monitors they often supported multiple modes already so there was less
incentive to add additional modes. The default setting of the scaler
property was also different on eDP/LVDS (on) and external displays
(off). If a user wanted to use the GPU scaler on an external display,
they could enable the scaler property and then manually add whatever
mode they wanted. If they wanted to use the modes from the EDID, they
would just disable the scaler property and pick the mode from the
EDID.
Alex
>
> Harry
>
> >>
> >> I guess we'll see if any of those are necessary. For now, I'd propose to just consider adding the common modes if there are 0 modes probed. But I'm also OK with leaving that for later if you feel it isn't necessary.
> >>
> >
> > Yeah if something comes up and we need to weight it out we have this thread to refer back to for our ideas on what to do.
> >
> >> A slightly related question, would you be OK with changing the link detection code to return dc_connection_none when DDC cannot read an EDID header on digital signals, similar to how the non-DC code does it?
> >>
> >
> > I personally think lining up all these nuances that are different between the two is a good idea.e e
> >
> > But for that specific question that's probably more of a Harry/Alex Hung question.
> >
> >>>>
> >>>> Two possible cases come to mind:
> >>>> 1. When we are unable to read the EDID for some reason
> >>>> 2. When the EDID is buggy and/or doesn't contain any modes
> >>>> Are these issues real or am I overthinking it?
> >>>>
> >>>> Thanks & best regards,
> >>>> Timur
> >>>
> >>> Failing to read EDID has happened in the past, but I think with the deferred aux message handling that should be cleared up now.
> >>
> >> I was actually curious about that. I saw that issue while I was working on something else. How is it deferred now? Can you point me to the series that fixed it?
> >>
> >
> > There's more patches than this one, but I believe this was the 💰 patch.
> >
> > https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
> >
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 19:11 ` Alex Deucher
@ 2025-09-24 20:00 ` Harry Wentland
2025-09-24 20:07 ` Alex Deucher
0 siblings, 1 reply; 20+ messages in thread
From: Harry Wentland @ 2025-09-24 20:00 UTC (permalink / raw)
To: Alex Deucher; +Cc: Mario Limonciello, Timur Kristóf, amd-gfx, Alex Hung
On 2025-09-24 15:11, Alex Deucher wrote:
> On Wed, Sep 24, 2025 at 2:44 PM Harry Wentland <harry.wentland@amd.com> wrote:
>>
>>
>>
>> On 2025-09-24 13:48, Mario Limonciello wrote:
>>> On 9/24/25 12:33 PM, Timur Kristóf wrote:
>>>>
>>>>
>>>> On 9/24/25 19:21, Mario Limonciello wrote:
>>>>>
>>>>> On 9/24/25 12:13 PM, Timur Kristóf wrote:
>>>>>>
>>>>>>
>>>>>> On 9/24/25 18:16, Mario Limonciello wrote:
>>>>>>> As part of enablement for SI and CIK in DC Timur pointed out some
>>>>>>> differences in behavior for common mode handling for DC vs non DC
>>>>>>> code paths. This series lines up the behavior between the two
>>>>>>> implementations.
>>>>>>>
>>>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>>>
>>>>>> Thank you Mario, this series makes good sense to me.
>>>>>> My only worry is this: is it possible that removing the common modes from connectors like DP, HDMI, etc. will regress somebody's setup?
>>>>>
>>>>> Possibly. We're not going to know until we try. I generally prefer not to add common modes (hence why I tried to drop them before until we hit the Xorg bug report).
>>>>>
>>>>> If someone complains about this then I see two other directions we can go.
>>>>
>>>> Sounds good.
>>>>
>>>> Considering the non-DC code already didn't add those common modes, I think it's reasonable to assume that we would have already heard about it if somebody had issues with it.
>>>>
>>>>>
>>>>> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP, HDMI. Make them not apply common modes to VGA and DVI
>>>>>
>>>>> 2) Enabling common modes /across the board/ but anything not in the EDID gets the GPU scalar turned on.
>>
>> I was surprised the previous approach failed, which seems
>> to indicate GPU scaling isn't already happening. I wonder
>> why. I think this would make a better default behavior
>> instead of relying on monitor scalers to deal with
>> non-advertised modes.
>
> My thinking with the original logic in radeon and the amdgpu non-DC
> code was to only add the common modes to eDP/LVDS because the EDIDs
> for those panels usually only had one mode in them and users almost
> always wanted to do clone mode with an external monitor. For external
> monitors they often supported multiple modes already so there was less
> incentive to add additional modes. The default setting of the scaler
> property was also different on eDP/LVDS (on) and external displays
> (off). If a user wanted to use the GPU scaler on an external display,
> they could enable the scaler property and then manually add whatever
> mode they wanted. If they wanted to use the modes from the EDID, they
> would just disable the scaler property and pick the mode from the
> EDID.
>
Makes sense. I forgot usermode controls scaling via the "scaling mode"
property.
Harry
> Alex
>
>>
>> Harry
>>
>>>>
>>>> I guess we'll see if any of those are necessary. For now, I'd propose to just consider adding the common modes if there are 0 modes probed. But I'm also OK with leaving that for later if you feel it isn't necessary.
>>>>
>>>
>>> Yeah if something comes up and we need to weight it out we have this thread to refer back to for our ideas on what to do.
>>>
>>>> A slightly related question, would you be OK with changing the link detection code to return dc_connection_none when DDC cannot read an EDID header on digital signals, similar to how the non-DC code does it?
>>>>
>>>
>>> I personally think lining up all these nuances that are different between the two is a good idea.e e
>>>
>>> But for that specific question that's probably more of a Harry/Alex Hung question.
>>>
>>>>>>
>>>>>> Two possible cases come to mind:
>>>>>> 1. When we are unable to read the EDID for some reason
>>>>>> 2. When the EDID is buggy and/or doesn't contain any modes
>>>>>> Are these issues real or am I overthinking it?
>>>>>>
>>>>>> Thanks & best regards,
>>>>>> Timur
>>>>>
>>>>> Failing to read EDID has happened in the past, but I think with the deferred aux message handling that should be cleared up now.
>>>>
>>>> I was actually curious about that. I saw that issue while I was working on something else. How is it deferred now? Can you point me to the series that fixed it?
>>>>
>>>
>>> There's more patches than this one, but I believe this was the 💰 patch.
>>>
>>> https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
>>>
>>>
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 20:00 ` Harry Wentland
@ 2025-09-24 20:07 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2025-09-24 20:07 UTC (permalink / raw)
To: Harry Wentland; +Cc: Mario Limonciello, Timur Kristóf, amd-gfx, Alex Hung
On Wed, Sep 24, 2025 at 4:00 PM Harry Wentland <harry.wentland@amd.com> wrote:
>
>
>
> On 2025-09-24 15:11, Alex Deucher wrote:
> > On Wed, Sep 24, 2025 at 2:44 PM Harry Wentland <harry.wentland@amd.com> wrote:
> >>
> >>
> >>
> >> On 2025-09-24 13:48, Mario Limonciello wrote:
> >>> On 9/24/25 12:33 PM, Timur Kristóf wrote:
> >>>>
> >>>>
> >>>> On 9/24/25 19:21, Mario Limonciello wrote:
> >>>>>
> >>>>> On 9/24/25 12:13 PM, Timur Kristóf wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 9/24/25 18:16, Mario Limonciello wrote:
> >>>>>>> As part of enablement for SI and CIK in DC Timur pointed out some
> >>>>>>> differences in behavior for common mode handling for DC vs non DC
> >>>>>>> code paths. This series lines up the behavior between the two
> >>>>>>> implementations.
> >>>>>>>
> >>>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> >>>>>>
> >>>>>> Thank you Mario, this series makes good sense to me.
> >>>>>> My only worry is this: is it possible that removing the common modes from connectors like DP, HDMI, etc. will regress somebody's setup?
> >>>>>
> >>>>> Possibly. We're not going to know until we try. I generally prefer not to add common modes (hence why I tried to drop them before until we hit the Xorg bug report).
> >>>>>
> >>>>> If someone complains about this then I see two other directions we can go.
> >>>>
> >>>> Sounds good.
> >>>>
> >>>> Considering the non-DC code already didn't add those common modes, I think it's reasonable to assume that we would have already heard about it if somebody had issues with it.
> >>>>
> >>>>>
> >>>>> 1) to make both DC and non-DC paths apply common modes to eDP,LVDS, DP, HDMI. Make them not apply common modes to VGA and DVI
> >>>>>
> >>>>> 2) Enabling common modes /across the board/ but anything not in the EDID gets the GPU scalar turned on.
> >>
> >> I was surprised the previous approach failed, which seems
> >> to indicate GPU scaling isn't already happening. I wonder
> >> why. I think this would make a better default behavior
> >> instead of relying on monitor scalers to deal with
> >> non-advertised modes.
> >
> > My thinking with the original logic in radeon and the amdgpu non-DC
> > code was to only add the common modes to eDP/LVDS because the EDIDs
> > for those panels usually only had one mode in them and users almost
> > always wanted to do clone mode with an external monitor. For external
> > monitors they often supported multiple modes already so there was less
> > incentive to add additional modes. The default setting of the scaler
> > property was also different on eDP/LVDS (on) and external displays
> > (off). If a user wanted to use the GPU scaler on an external display,
> > they could enable the scaler property and then manually add whatever
> > mode they wanted. If they wanted to use the modes from the EDID, they
> > would just disable the scaler property and pick the mode from the
> > EDID.
> >
>
> Makes sense. I forgot usermode controls scaling via the "scaling mode"
> property.
Now that compositors generally control all of this, unless you are
running X, there's probably not a good way to mess with this.
Alex
>
> Harry
>
> > Alex
> >
> >>
> >> Harry
> >>
> >>>>
> >>>> I guess we'll see if any of those are necessary. For now, I'd propose to just consider adding the common modes if there are 0 modes probed. But I'm also OK with leaving that for later if you feel it isn't necessary.
> >>>>
> >>>
> >>> Yeah if something comes up and we need to weight it out we have this thread to refer back to for our ideas on what to do.
> >>>
> >>>> A slightly related question, would you be OK with changing the link detection code to return dc_connection_none when DDC cannot read an EDID header on digital signals, similar to how the non-DC code does it?
> >>>>
> >>>
> >>> I personally think lining up all these nuances that are different between the two is a good idea.e e
> >>>
> >>> But for that specific question that's probably more of a Harry/Alex Hung question.
> >>>
> >>>>>>
> >>>>>> Two possible cases come to mind:
> >>>>>> 1. When we are unable to read the EDID for some reason
> >>>>>> 2. When the EDID is buggy and/or doesn't contain any modes
> >>>>>> Are these issues real or am I overthinking it?
> >>>>>>
> >>>>>> Thanks & best regards,
> >>>>>> Timur
> >>>>>
> >>>>> Failing to read EDID has happened in the past, but I think with the deferred aux message handling that should be cleared up now.
> >>>>
> >>>> I was actually curious about that. I saw that issue while I was working on something else. How is it deferred now? Can you point me to the series that fixed it?
> >>>>
> >>>
> >>> There's more patches than this one, but I believe this was the 💰 patch.
> >>>
> >>> https://lore.kernel.org/amd-gfx/20250428135514.20775-27-ray.wu@amd.com/
> >>>
> >>>
> >>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Adjustments to common mode behavior
2025-09-24 18:31 ` Mario Limonciello
@ 2025-09-24 21:04 ` Timur Kristóf
0 siblings, 0 replies; 20+ messages in thread
From: Timur Kristóf @ 2025-09-24 21:04 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx, Wentland, Harry, Alex Hung
On 9/24/25 20:31, Mario Limonciello wrote:
>>
>> The non-DC code has a solution for this to retry in a few seconds.
>> I wasn't able to find anything for this in DC.
>> I found some code to set up some HPD filters, but that code is not
>> called from anywhere and I was told the HPD filter may not be the
>> right solution.
>>
>> So my question is, what would be the right solution? Can we use a HPD
>> filter? Or better to do what the non-DC code does and just retry again
>> later?
>>
>> Thanks,
>> Timur
>
> Ah yes this is a totally different problem. It doesn't turn into a
> storm does it? I would think a filter only makes sense if there is an
> interrupt storm.
>
> As long as it's just one HPD and no EDID found from what you've
> described I would think spinning up a timer to try again in a few
> seconds sounds like a reasonable approach.
Yes, it sounds like a different problem.
There is no storm, just 1 interrupt but at a time when the DDC pins
haven't made contact yet so we can't read the EDID yet.
I also asked Harry about it and he agrees that a SW delay would be the
best solution. I'll try to come up with a patch to implement that.
(Another alternative would be the HPD filter, but that can apparently
cause issues and is not recommended.)
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-09-24 21:04 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 16:16 [PATCH 0/5] Adjustments to common mode behavior Mario Limonciello
2025-09-24 16:16 ` [PATCH 1/5] drm/amd/display: Only enable common modes for eDP and LVDS Mario Limonciello
2025-09-24 17:58 ` Harry Wentland
2025-09-24 16:16 ` [PATCH 2/5] drm/amd: Drop unnecessary check in amdgpu_connector_add_common_modes() Mario Limonciello
2025-09-24 16:16 ` [PATCH 3/5] drm/amd: Use dynamic array size declaration for amdgpu_connector_add_common_modes() Mario Limonciello
2025-09-24 17:05 ` Alex Deucher
2025-09-24 16:16 ` [PATCH 4/5] drm/amd: Drop some common modes from amdgpu_connector_add_common_modes() Mario Limonciello
2025-09-24 16:16 ` [PATCH 5/5] drm/amd: Add name to " Mario Limonciello
2025-09-24 17:07 ` Alex Deucher
2025-09-24 17:13 ` [PATCH 0/5] Adjustments to common mode behavior Timur Kristóf
2025-09-24 17:21 ` Mario Limonciello
2025-09-24 17:33 ` Timur Kristóf
2025-09-24 17:48 ` Mario Limonciello
2025-09-24 18:11 ` Timur Kristóf
2025-09-24 18:31 ` Mario Limonciello
2025-09-24 21:04 ` Timur Kristóf
2025-09-24 18:24 ` Harry Wentland
2025-09-24 19:11 ` Alex Deucher
2025-09-24 20:00 ` Harry Wentland
2025-09-24 20:07 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.