Re: [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code
No regressed found on this patchset series. Reviewed-by: Alex Hung On 9/27/24 17:05, Mario Limonciello wrote: From: Mario Limonciello This is the successor of Melissa's v5 series that was posted [1] as well as my series that was posted [2]. Melissa's patches are mostly unmodified from v5, but the series has been rebase on the new 6.10 based amd-staging-drm-next. As were both touching similar code for fetching the EDID, I've merged the pertinent parts of my series into this one in allowing the connector to fetch the EDID from _DDC if available for eDP as well. There are still some remaining uses of drm_edid_raw() but they touch pure DC code, so a wrapper or macro will probably be needed to convert them. This can be for follow ups later on. Link: https://lore.kernel.org/amd-gfx/20240807203207.2830-1-m...@igalia.com/ [1] Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limoncie...@amd.com/ [2] v8: * Drop patches 5-9 as they cause regressions and will be future followups * Rebase patch 10 on patches 1-4 Mario Limonciello (1): drm/amd/display: Fetch the EDID from _DDC if available for eDP Melissa Wen (4): drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid drm/amd/display: switch to setting physical address directly drm/amd/display: always call connector_update when parsing freesync_caps drm/amd/display: remove redundant freesync parser for DP .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 195 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 76 ++- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 34 +-- drivers/gpu/drm/amd/include/amd_shared.h | 5 + 5 files changed, 153 insertions(+), 161 deletions(-)
Re: [PATCH v7 00/10] drm/amd/display: Use drm_edid for more code
On 9/27/24 14:45, Melissa Wen wrote: Hi Alex, Thanks for the intensive testing. I'll need some time to reproduce and debug these regressions. So, we can divide this series into four steps: 1-2 are the basis for drm_edid migration 3-4 are code cleanups 5-9 are drm_edid_product_id migration 10 is for ACPI EDID feature. Bearing this and the reported regressions around the product_id part in mind, I wonder if we can start by applying the drm_edid migration and the ACPI EDID feature, which means applying patches 1-4 and 10. And then let the second part of product_id migration for the next iteration. IMHO it seems a smoother transition. WDYT? This sounds like a good plan. Not all tests were completed on 1-4 + 10 as the series were dropped due to the regressions. I can send the 5 patches for tests next week again; however, 10 cannot be applied cleanly on top of 1-4, and help from Mario to rebase is probably needed. As for the rest, let me know if you cannot reproduce these issues since you may or may not have the same hardware configs. Melissa On 27/09/2024 15:48, Alex Hung wrote: Hi Mario and Melissa, There are three regressions identified during the test, and improvement is required before the patches can be merged. Please see details below. 1. null pointer when hot-plugging a dsc hub (+ three 4k60 monitors). This may point to "drm/amd/display: use drm_edid_product_id for parsing EDID product info" since that's the only patch calling drm_edid_get_product_id. [ 227.797361] RIP: 0010:drm_edid_get_product_id+0x1d/0x50 [drm] [ 227.797388] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5 48 85 ff 74 24 48 8b 47 08 48 85 c0 74 1b 48 83 3f 7f 76 15 <48> 8b 50 08 5d 48 89 16 0f b7 40 10 66 89 46 08 e9 a9 47 ee ce 31 [ 227.797391] RSP: 0018:ac58126f7930 EFLAGS: 00010216 [ 227.797394] RAX: 372d RBX: 8eaeaf8ac808 RCX: 8eaeaf8ac107 [ 227.797396] RDX: 0002 RSI: ac58126f7944 RDI: 8eaeeeaf9f80 [ 227.797398] RBP: ac58126f7930 R08: 8eae8e500d00 R09: 0001 [ 227.797400] R10: ac58126f7978 R11: 0017f81b R12: 8eae84cb [ 227.797402] R13: 8eaeeeaf9f80 R14: R15: 0100 [ 227.797405] FS: 7f031a616ac0() GS:8eb57cd8() knlGS: [ 227.797407] CS: 0010 DS: ES: CR0: 80050033 [ 227.797409] CR2: 3735 CR3: 00014decc000 CR4: 00750ef0 [ 227.797411] PKRU: 5554 [ 227.797413] Call Trace: [ 227.797415] [ 227.797417] ? show_regs+0x64/0x70 [ 227.797423] ? __die+0x24/0x70 [ 227.797427] ? page_fault_oops+0x160/0x520 [ 227.797435] ? do_user_addr_fault+0x2e9/0x6a0 [ 227.797438] ? dc_link_add_remote_sink+0x20/0x30 [amdgpu] [ 227.797654] ? dm_dp_mst_get_modes+0xee/0x520 [amdgpu] [ 227.797882] ? drm_helper_probe_single_connector_modes+0x1b5/0x670 [drm_kms_helper] [ 227.797894] ? exc_page_fault+0x7f/0x190 [ 227.797899] ? asm_exc_page_fault+0x27/0x30 [ 227.797906] ? drm_edid_get_product_id+0x1d/0x50 [drm] [ 227.797932] dm_helpers_parse_edid_caps+0x69/0x260 [amdgpu] [ 227.798139] amdgpu :0b:00.0: [drm:drm_dp_dpcd_write [drm_display_helper]] AMDGPU DM aux hw bus 2: 0x02003 AUX <- (ret= 1) 10 [ 227.798158] link_add_remote_sink+0xa8/0x1a0 [amdgpu] [ 227.798402] dc_link_add_remote_sink+0x20/0x30 [amdgpu] [ 227.798615] dm_dp_mst_get_modes+0xee/0x520 [amdgpu] [ 227.798843] ? srso_alias_return_thunk+0x5/0xfbef5 [ 227.798848] drm_helper_probe_single_connector_modes+0x1b5/0x670 [drm_kms_helper] 2. DP2 Display is not listed as an audio device Steps to reproduce: - Attach display to system. - Boot to Desktop (Wayland) - Attempt to select display as an audio device. This points to patch "drm/amd/display: get SAD from drm_eld when parsing EDID caps" 3. IGT amd_mem_leak'sconnector-suspend-resume fails on Navi 31. This points to the patch "drm/amd/display: remove dc_edid handler from dm_helpers_parse_edid_caps". Using IGT_SRANDOM=1727192429 for randomisation Opened device: /dev/dri/card0 Starting subtest: connector-suspend-resume [cmd] rtcwake: assuming RTC uses UTC ... rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Sep 24 15:40:50 2024 unreferenced object 0x95c683517b00 (size 256): comm "kworker/u64:30", pid 3636, jiffies 4295452761 hex dump (first 32 bytes): 00 ff ff ff ff ff ff 00 22 0e c2 36 00 00 00 00 "..6 33 1d 01 04 b5 3c 22 78 3b 28 91 a7 55 4e a3 26 3<"x;(..UN.& backtrace (crc 5800a91d): [] kmemleak_alloc+0x4a/0x80 [] kmalloc_node_track_caller_noprof+0x337/0x410 [] krealloc_noprof+0x58/0xb0 [] _drm_do_get_edid+0x138/0x410 [drm] [] drm_edid_read_custom+0x35/0xd0 [drm] [] drm_edid_read_ddc+0x44/0x80 [drm] [] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
Re: [PATCH v7 00/10] drm/amd/display: Use drm_edid for more code
Hi Mario and Melissa,
There are three regressions identified during the test, and improvement
is required before the patches can be merged. Please see details below.
1. null pointer when hot-plugging a dsc hub (+ three 4k60 monitors).
This may point to "drm/amd/display: use drm_edid_product_id for parsing
EDID product info" since that's the only patch calling
drm_edid_get_product_id.
[ 227.797361] RIP: 0010:drm_edid_get_product_id+0x1d/0x50 [drm]
[ 227.797388] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00
00 55 48 89 e5 48 85 ff 74 24 48 8b 47 08 48 85 c0 74 1b 48 83 3f 7f 76
15 <48> 8b 50 08 5d 48 89 16 0f b7 40 10 66 89 46 08 e9 a9 47 ee ce 31
[ 227.797391] RSP: 0018:ac58126f7930 EFLAGS: 00010216
[ 227.797394] RAX: 372d RBX: 8eaeaf8ac808 RCX:
8eaeaf8ac107
[ 227.797396] RDX: 0002 RSI: ac58126f7944 RDI:
8eaeeeaf9f80
[ 227.797398] RBP: ac58126f7930 R08: 8eae8e500d00 R09:
0001
[ 227.797400] R10: ac58126f7978 R11: 0017f81b R12:
8eae84cb
[ 227.797402] R13: 8eaeeeaf9f80 R14: R15:
0100
[ 227.797405] FS: 7f031a616ac0() GS:8eb57cd8()
knlGS:
[ 227.797407] CS: 0010 DS: ES: CR0: 80050033
[ 227.797409] CR2: 3735 CR3: 00014decc000 CR4:
00750ef0
[ 227.797411] PKRU: 5554
[ 227.797413] Call Trace:
[ 227.797415]
[ 227.797417] ? show_regs+0x64/0x70
[ 227.797423] ? __die+0x24/0x70
[ 227.797427] ? page_fault_oops+0x160/0x520
[ 227.797435] ? do_user_addr_fault+0x2e9/0x6a0
[ 227.797438] ? dc_link_add_remote_sink+0x20/0x30 [amdgpu]
[ 227.797654] ? dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
[ 227.797882] ? drm_helper_probe_single_connector_modes+0x1b5/0x670
[drm_kms_helper]
[ 227.797894] ? exc_page_fault+0x7f/0x190
[ 227.797899] ? asm_exc_page_fault+0x27/0x30
[ 227.797906] ? drm_edid_get_product_id+0x1d/0x50 [drm]
[ 227.797932] dm_helpers_parse_edid_caps+0x69/0x260 [amdgpu]
[ 227.798139] amdgpu :0b:00.0: [drm:drm_dp_dpcd_write
[drm_display_helper]] AMDGPU DM aux hw bus 2: 0x02003 AUX <- (ret= 1) 10
[ 227.798158] link_add_remote_sink+0xa8/0x1a0 [amdgpu]
[ 227.798402] dc_link_add_remote_sink+0x20/0x30 [amdgpu]
[ 227.798615] dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
[ 227.798843] ? srso_alias_return_thunk+0x5/0xfbef5
[ 227.798848] drm_helper_probe_single_connector_modes+0x1b5/0x670
[drm_kms_helper]
2. DP2 Display is not listed as an audio device
Steps to reproduce:
- Attach display to system.
- Boot to Desktop (Wayland)
- Attempt to select display as an audio device.
This points to patch "drm/amd/display: get SAD from drm_eld when parsing
EDID caps"
3. IGT amd_mem_leak'sconnector-suspend-resume fails on Navi 31.
This points to the patch "drm/amd/display: remove dc_edid handler from
dm_helpers_parse_edid_caps".
Using IGT_SRANDOM=1727192429 for randomisation
Opened device: /dev/dri/card0
Starting subtest: connector-suspend-resume
[cmd] rtcwake: assuming RTC uses UTC ...
rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Sep 24 15:40:50 2024
unreferenced object 0x95c683517b00 (size 256):
comm "kworker/u64:30", pid 3636, jiffies 4295452761
hex dump (first 32 bytes):
00 ff ff ff ff ff ff 00 22 0e c2 36 00 00 00 00 "..6
33 1d 01 04 b5 3c 22 78 3b 28 91 a7 55 4e a3 26 3<"x;(..UN.&
backtrace (crc 5800a91d):
[] kmemleak_alloc+0x4a/0x80
[] kmalloc_node_track_caller_noprof+0x337/0x410
[] krealloc_noprof+0x58/0xb0
[] _drm_do_get_edid+0x138/0x410 [drm]
[] drm_edid_read_custom+0x35/0xd0 [drm]
[] drm_edid_read_ddc+0x44/0x80 [drm]
[] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
[] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
[] link_detect+0x36/0x4f0 [amdgpu]
[] dc_link_detect+0x20/0x30 [amdgpu]
[] dm_resume+0x1e0/0x7d0 [amdgpu]
[] amdgpu_device_ip_resume_phase2+0x58/0xd0 [amdgpu]
[] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
[] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
[] pci_pm_resume+0x71/0x100
[] dpm_run_callback+0x91/0x1e0
unreferenced object 0x95c6c58dd0c0 (size 16):
comm "kworker/u64:30", pid 3636, jiffies 4295452764
hex dump (first 16 bytes):
00 01 00 00 00 00 00 00 00 7b 51 83 c6 95 ff ff .{Q.
backtrace (crc 70d89717):
[] kmemleak_alloc+0x4a/0x80
[] kmalloc_trace_noprof+0x28e/0x300
[] _drm_edid_alloc+0x35/0x60 [drm]
[] drm_edid_read_custom+0x55/0xd0 [drm]
[] drm_edid_read_ddc+0x44/0x80 [drm]
[] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
[] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
[] link_detect+0x36/0x4f0 [amdgpu]
[] dc_link_detect+0x20/0x30 [amdgpu]
[] dm_resume+0x1e0/0x7d0 [amdgpu]
[] amdgpu_device_ip_resume_phase2+0x58/0xd0 [amdgpu]
[] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
[] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
[] pci_pm_resume+0x71/0x100
Re: [PATCH v7 07/10] drm/amd/display: get SAD from drm_eld when parsing EDID caps
Mario and Melissa,
Another regression identified on this patch - DP Display is not listed
as an audio device after this patch is applied.
Cheers,
Alex Hung
On 9/18/24 15:38, Mario Limonciello wrote:
From: Melissa Wen
drm_edid_connector_update() updates display info, filling ELD with audio
info from Short-Audio Descriptors in the last step of
update_dislay_info(). Our goal is stopping using raw edid, so we can
extract SAD from drm_eld instead of access raw edid to get audio caps.
Signed-off-by: Melissa Wen
Signed-off-by: Mario Limonciello
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 +--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 854e51c0b3fb..e0326127fd9a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -98,9 +98,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
- struct cea_sad *sads;
- int sad_count = -1;
- int sadb_count = -1;
+ int sad_count, sadb_count;
int i = 0;
uint8_t *sadb = NULL;
@@ -129,18 +127,21 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(&product_id, edid_caps);
- sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+ sad_count = drm_eld_sad_count(connector->eld);
if (sad_count <= 0)
return result;
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
- struct cea_sad *sad = &sads[i];
+ struct cea_sad sad;
- edid_caps->audio_modes[i].format_code = sad->format;
- edid_caps->audio_modes[i].channel_count = sad->channels + 1;
- edid_caps->audio_modes[i].sample_rate = sad->freq;
- edid_caps->audio_modes[i].sample_size = sad->byte2;
+ if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+ continue;
+
+ edid_caps->audio_modes[i].format_code = sad.format;
+ edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+ edid_caps->audio_modes[i].sample_rate = sad.freq;
+ edid_caps->audio_modes[i].sample_size = sad.byte2;
}
sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid, &sadb);
@@ -155,7 +156,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sads);
kfree(sadb);
return result;
Re: [PATCH v7 09/10] drm/amd/display: remove dc_edid handler from dm_helpers_parse_edid_caps
Mario and Melissa,
This patch causes a regrerssion on 7900 XTX in an IGT test:
amd_mem_leak's connector-suspend-resume.
Is this patch necessary on this series or is it independent from other
patches, i.e. can it be dropped from this series until fixed??
Cheers,
Alex Hung
On 9/18/24 15:38, Mario Limonciello wrote:
From: Melissa Wen
We can parse edid caps from drm_edid and drm_eld and any calls of
dm_helpers_parse_edid_caps is made in a state that we have drm_edid set
to amdgpu connector.
Signed-off-by: Melissa Wen
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 30 ---
drivers/gpu/drm/amd/display/dc/dm_helpers.h | 1 -
.../drm/amd/display/dc/link/link_detection.c | 6 ++--
4 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index bd8fb9968c7c..bf847ac55475 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7123,10 +7123,7 @@ static void amdgpu_dm_connector_funcs_force(struct
drm_connector *connector)
memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
memmove(dc_em_sink->dc_edid.raw_edid, edid,
(edid->extensions + 1) * EDID_LENGTH);
- dm_helpers_parse_edid_caps(
- dc_link,
- &dc_em_sink->dc_edid,
- &dc_em_sink->edid_caps);
+ dm_helpers_parse_edid_caps(dc_link, &dc_em_sink->edid_caps);
}
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d43ed3ea000b..8f4be7a1ec45 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -83,32 +83,24 @@ static void apply_edid_quirks(struct drm_edid_product_id
*product_id,
* dm_helpers_parse_edid_caps() - Parse edid caps
*
* @link: current detected link
- * @edid: [in] pointer to edid
* @edid_caps:[in] pointer to edid caps
*
* Return: void
*/
-enum dc_edid_status dm_helpers_parse_edid_caps(
- struct dc_link *link,
- const struct dc_edid *edid,
- struct dc_edid_caps *edid_caps)
+enum dc_edid_status dm_helpers_parse_edid_caps(struct dc_link *link,
+ struct dc_edid_caps *edid_caps)
{
struct amdgpu_dm_connector *aconnector = link->priv;
struct drm_connector *connector = &aconnector->base;
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
- struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
int sad_count;
int i = 0;
-
enum dc_edid_status result = EDID_OK;
- if (!edid_caps || !edid)
+ if (!edid_caps || !drm_edid)
return EDID_BAD_INPUT;
- if (!drm_edid_is_valid(edid_buf))
- result = EDID_BAD_CHECKSUM;
-
drm_edid_get_product_id(drm_edid, &product_id);
edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
@@ -920,19 +912,23 @@ enum dc_edid_status dm_helpers_read_local_edid(
if (!drm_edid)
return EDID_NO_RESPONSE;
- edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
+ /* FIXME: Get rid of drm_edid_raw()
+* Raw edid is still needed for dm_helpers_dp_write_dpcd()
+*/
+ edid = drm_edid_raw(drm_edid);
sink->dc_edid.length = EDID_LENGTH * (edid->extensions + 1);
memmove(sink->dc_edid.raw_edid, (uint8_t *)edid,
sink->dc_edid.length);
edid_status = dm_helpers_parse_edid_caps(
link,
- &sink->dc_edid,
&sink->edid_caps);
- /* We don't need the original edid anymore */
- drm_edid_free(drm_edid);
-
- } while (edid_status == EDID_BAD_CHECKSUM && --retry > 0);
+ if (edid_status != EDID_OK) {
+ /* We can discard the drm_edid and retry */
+ drm_edid_free(drm_edid);
+ drm_edid_connector_update(connector, drm_edid);
+ }
+ } while (edid_status != EDID_OK && --retry > 0);
if (edid_status != EDID_OK)
DRM_ERROR("EDID err: %d, on connector: %s",
diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
b/drivers/gpu/drm/amd/di
Re: [PATCH v7 10/10] drm/amd/display: Fetch the EDID from _DDC if available for eDP
A minor comment (see inline below).
Otherwise
Reviewed-by: Alex Hung
On 2024-09-18 15:38, Mario Limonciello wrote:
Some manufacturers have intentionally put an EDID that differs from
the EDID on the internal panel on laptops.
Attempt to fetch this EDID if it exists and prefer it over the EDID
that is provided by the panel. If a user prefers to use the EDID from
the panel, offer a DC debugging parameter that would disable this.
Signed-off-by: Mario Limonciello
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 62 ++-
drivers/gpu/drm/amd/include/amd_shared.h | 5 ++
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 8f4be7a1ec45..05d3e00ecce0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -23,6 +23,8 @@
*
*/
+#include
+
#include
#include
#include
@@ -874,6 +876,60 @@ bool dm_helpers_is_dp_sink_present(struct dc_link *link)
return dp_sink_present;
}
+static int
+dm_helpers_probe_acpi_edid(void *data, u8 *buf, unsigned int block, size_t len)
+{
+ struct drm_connector *connector = data;
+ struct acpi_device *acpidev = ACPI_COMPANION(connector->dev->dev);
+ unsigned char start = block * EDID_LENGTH;
+ void *edid;
+ int r;
+
+ if (!acpidev)
+ return -ENODEV;
+
+ /* fetch the entire edid from BIOS */
+ r = acpi_video_get_edid(acpidev, ACPI_VIDEO_DISPLAY_LCD, -1, &edid);
+ if (r < 0) {
+ DRM_DEBUG_KMS("Failed to get EDID from ACPI: %d\n", r);
+ return r;
+ }
+ if (len > r || start > r || start + len > r) {
+ r = -EINVAL;
+ goto cleanup;
+ }
+
+ memcpy(buf, edid + start, len);
+ r = 0;
+
+cleanup:
+ kfree(edid);
+
+ return r;
+}
+
+static const struct drm_edid *
+dm_helpers_read_acpi_edid(struct amdgpu_dm_connector *aconnector)
+{
+ struct drm_connector *connector = &aconnector->base;
+
+ if (amdgpu_dc_debug_mask & DC_DISABLE_ACPI_EDID)
+ return NULL;
+
+ switch (connector->connector_type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ break;
+ default:
+ return NULL;
+ }
+
+ if (connector->force == DRM_FORCE_OFF)
+ return NULL;
+
+ return drm_edid_read_custom(connector, dm_helpers_probe_acpi_edid,
connector);
+}
+
enum dc_edid_status dm_helpers_read_local_edid(
struct dc_context *ctx,
struct dc_link *link,
@@ -896,7 +952,11 @@ enum dc_edid_status dm_helpers_read_local_edid(
* do check sum and retry to make sure read correct edid.
*/
do {
- drm_edid = drm_edid_read_ddc(connector, ddc);
+ drm_edid = dm_helpers_read_acpi_edid(aconnector);
+ if (drm_edid)
+ DRM_DEBUG_KMS("Using ACPI provided EDID for %s\n",
connector->name);
It is better to always output a message when ACPI's EDID is used without
enabling any debug options. How about DRM_INFO?
+ else
+ drm_edid = drm_edid_read_ddc(connector, ddc);
drm_edid_connector_update(connector, drm_edid);
aconnector->drm_edid = drm_edid;
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index 3f91926a50e9..1ec7c5e5249e 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -337,6 +337,11 @@ enum DC_DEBUG_MASK {
* @DC_FORCE_IPS_ENABLE: If set, force enable all IPS, all the time.
*/
DC_FORCE_IPS_ENABLE = 0x4000,
+ /**
+* @DC_DISABLE_ACPI_EDID: If set, don't attempt to fetch EDID for
+* eDP display from ACPI _DDC method.
+*/
+ DC_DISABLE_ACPI_EDID = 0x8000,
};
enum amd_dpm_forced_level;
Re: [PATCH v4 09/11] drm/amd/display: get SAD from drm_eld when parsing EDID caps
On 2024-07-28 20:02, Melissa Wen wrote:
On 07/25, Alex Hung wrote:
On 2024-07-05 21:35, Melissa Wen wrote:
instead of parsing struct edid.
A more informative commit message will be helpful.
sure. I'll improve it in the next version.
A soft reminder - a few other patches need improved commit messages too.
Signed-off-by: Melissa Wen
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 7657b1051c54..45c04de08c65 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -97,7 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
- struct cea_sad *sads;
int sad_count, sadb_count;
int i = 0;
uint8_t *sadb = NULL;
@@ -127,18 +126,21 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(&product_id, edid_caps);
- sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+ sad_count = drm_eld_sad_count(connector->eld);
if (sad_count <= 0)
return result;
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
- struct cea_sad *sad = &sads[i];
+ struct cea_sad sad;
- edid_caps->audio_modes[i].format_code = sad->format;
- edid_caps->audio_modes[i].channel_count = sad->channels + 1;
- edid_caps->audio_modes[i].sample_rate = sad->freq;
- edid_caps->audio_modes[i].sample_size = sad->byte2;
+ if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+ continue;
+
+ edid_caps->audio_modes[i].format_code = sad.format;
+ edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+ edid_caps->audio_modes[i].sample_rate = sad.freq;
+ edid_caps->audio_modes[i].sample_size = sad.byte2;
}
sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid,
&sadb);
@@ -153,7 +155,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sads);
kfree(sadb);
return result;
Re: [PATCH v4 01/11] drm/amd/display: clean unused variables for hdmi freesync parser
On 2024-07-28 19:32, Melissa Wen wrote:
On 07/25, Alex Hung wrote:
Hi Melissa,
There are no commit messages in this patch.
Also, do you think this can be merged with Patch 5 "drm/amd/display: remove
redundant freesync parser for DP"?
Hi Alex,
Thanks for your feedback.
I'll add a brief description in the next version.
Regarding merging it into patch 5, I'd prefer to keep it detached
because here we have a non-functional change. I can send it as a
separate, single patch from this series to reduce noise and make
validation faster. WDYT?
Make thanks. Thanks for clarification.
Melissa
On 2024-07-05 21:35, Melissa Wen wrote:
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 98cf523a629e..1dfa7ec9af35 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -12108,9 +12108,6 @@ void amdgpu_dm_update_freesync_caps(struct
drm_connector *connector,
} else if (edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
if (i >= 0 && vsdb_info.freesync_supported) {
- timing = &edid->detailed_timings[i];
- data= &timing->data.other_data;
-
amdgpu_dm_connector->min_vfreq =
vsdb_info.min_refresh_rate_hz;
amdgpu_dm_connector->max_vfreq =
vsdb_info.max_refresh_rate_hz;
if (amdgpu_dm_connector->max_vfreq -
amdgpu_dm_connector->min_vfreq > 10)
Re: [PATCH v4 10/11] drm/amd/display: get SADB from drm_eld when parsing EDID caps
On 2024-07-05 21:35, Melissa Wen wrote:
instead of parsing struct edid.
A more informative commit message will be helpful.
Signed-off-by: Melissa Wen
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 45c04de08c65..3fb07f437793 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -97,9 +97,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
- int sad_count, sadb_count;
+ int sad_count;
int i = 0;
- uint8_t *sadb = NULL;
enum dc_edid_status result = EDID_OK;
@@ -143,20 +142,12 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
edid_caps->audio_modes[i].sample_size = sad.byte2;
}
- sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid, &sadb);
- if (sadb_count < 0) {
- DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n",
sadb_count);
- sadb_count = 0;
- }
-
- if (sadb_count)
- edid_caps->speaker_flags = sadb[0];
+ if (connector->eld[DRM_ELD_SPEAKER])
+ edid_caps->speaker_flags = connector->eld[DRM_ELD_SPEAKER];
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sadb);
-
return result;
}
Re: [PATCH v4 09/11] drm/amd/display: get SAD from drm_eld when parsing EDID caps
On 2024-07-05 21:35, Melissa Wen wrote:
instead of parsing struct edid.
A more informative commit message will be helpful.
Signed-off-by: Melissa Wen
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 7657b1051c54..45c04de08c65 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -97,7 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
const struct drm_edid *drm_edid = aconnector->drm_edid;
struct drm_edid_product_id product_id;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
- struct cea_sad *sads;
int sad_count, sadb_count;
int i = 0;
uint8_t *sadb = NULL;
@@ -127,18 +126,21 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(&product_id, edid_caps);
- sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+ sad_count = drm_eld_sad_count(connector->eld);
if (sad_count <= 0)
return result;
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
- struct cea_sad *sad = &sads[i];
+ struct cea_sad sad;
- edid_caps->audio_modes[i].format_code = sad->format;
- edid_caps->audio_modes[i].channel_count = sad->channels + 1;
- edid_caps->audio_modes[i].sample_rate = sad->freq;
- edid_caps->audio_modes[i].sample_size = sad->byte2;
+ if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+ continue;
+
+ edid_caps->audio_modes[i].format_code = sad.format;
+ edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+ edid_caps->audio_modes[i].sample_rate = sad.freq;
+ edid_caps->audio_modes[i].sample_size = sad.byte2;
}
sadb_count = drm_edid_to_speaker_allocation((struct edid *) edid->raw_edid, &sadb);
@@ -153,7 +155,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sads);
kfree(sadb);
return result;
Re: [PATCH v4 07/11] drm/amd/display: don't give initial values for sad/b_count
Can this be merged with [PATCH 10/11] drm/amd/display: get SADB from drm_eld when parsing EDID caps On 2024-07-05 21:35, Melissa Wen wrote: Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 85704fd75ee4..6df55161d6df 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -97,8 +97,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps( struct drm_edid_product_id product_id; struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL; struct cea_sad *sads; - int sad_count = -1; - int sadb_count = -1; + int sad_count, sadb_count; int i = 0; uint8_t *sadb = NULL;
Re: [PATCH v4 03/11] drm/amd/display: switch to setting physical address directly
On 2024-07-05 21:35, Melissa Wen wrote:
Connectors have source physical address available in display
info. Use drm_dp_cec_attach() to use it instead of parsing the EDID
again.
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 49b8c5b00728..163850aeac4a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3477,10 +3477,9 @@ void amdgpu_dm_update_connector_after_detect(
aconnector->drm_edid = drm_edid_alloc(edid,
sink->dc_edid.length);
drm_edid_connector_update(connector,
aconnector->drm_edid);
- /* FIXME: Get rid of drm_edid_raw() */
if (aconnector->dc_link->aux_mode)
- drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
-
drm_edid_raw(aconnector->drm_edid));
drm_dp_cec_set_edid is removed here. I guess it doesn't matter if edid
or drm_edid_raw(aconnector->drm_edid) is past here.
+ drm_dp_cec_attach(&aconnector->dm_dp_aux.aux,
+
connector->display_info.source_physical_address);
}
if (!aconnector->timing_requested) {
Re: [PATCH v4 02/11] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
Please see inline comments.
On 2024-07-05 21:35, Melissa Wen wrote:
Replace raw edid handling (struct edid) with the opaque EDID type
(struct drm_edid) on amdgpu_dm_connector for consistency. It may also
prevent mismatch of approaches in different parts of the driver code.
Working in progress. It was only exercised with IGT tests.
v2: use const to fix warnings (Alex Hung)
v3: fix general protection fault on mst
v4: rename edid to drm_edid in amdgpu_connector (Jani)
call drm_edid_connector_update to clear edid in case of NULL (Jani)
keep setting NULL instead of free drm_edid (Jani)
check drm_edid not NULL, instead of valid (Jani)
Signed-off-by: Melissa Wen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 106 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +-
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 13 ++-
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +++---
4 files changed, 79 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1dfa7ec9af35..49b8c5b00728 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3408,7 +3408,7 @@ void amdgpu_dm_update_connector_after_detect(
aconnector->dc_sink = sink;
dc_sink_retain(aconnector->dc_sink);
amdgpu_dm_update_freesync_caps(connector,
- aconnector->edid);
+ aconnector->drm_edid);
} else {
amdgpu_dm_update_freesync_caps(connector, NULL);
if (!aconnector->dc_sink) {
@@ -3467,18 +3467,20 @@ void amdgpu_dm_update_connector_after_detect(
aconnector->dc_sink = sink;
dc_sink_retain(aconnector->dc_sink);
if (sink->dc_edid.length == 0) {
- aconnector->edid = NULL;
+ aconnector->drm_edid = NULL;
if (aconnector->dc_link->aux_mode) {
drm_dp_cec_unset_edid(
&aconnector->dm_dp_aux.aux);
}
} else {
- aconnector->edid =
- (struct edid *)sink->dc_edid.raw_edid;
+ const struct edid *edid = (const struct edid
*)sink->dc_edid.raw_edid;
+ aconnector->drm_edid = drm_edid_alloc(edid,
sink->dc_edid.length);
+ drm_edid_connector_update(connector,
aconnector->drm_edid);
+ /* FIXME: Get rid of drm_edid_raw() */
if (aconnector->dc_link->aux_mode)
drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
- aconnector->edid);
Why not pass edid but drm_edid_raw(aconnector->drm_edid)?
+
drm_edid_raw(aconnector->drm_edid));
}
if (!aconnector->timing_requested) {
@@ -3489,17 +3491,18 @@ void amdgpu_dm_update_connector_after_detect(
"failed to create
aconnector->requested_timing\n");
}
- drm_connector_update_edid_property(connector, aconnector->edid);
- amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
+ drm_edid_connector_update(connector, aconnector->drm_edid);
+ amdgpu_dm_update_freesync_caps(connector, aconnector->drm_edid);
update_connector_ext_caps(aconnector);
} else {
drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
amdgpu_dm_update_freesync_caps(connector, NULL);
- drm_connector_update_edid_property(connector, NULL);
+ drm_edid_connector_update(connector, NULL);
aconnector->num_modes = 0;
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
- aconnector->edid = NULL;
+ drm_edid_free(aconnector->drm_edid);
+ aconnector->drm_edid = NULL;
kfree(aconnector->timing_requested);
aconnector->timing_requested = NULL;
/* Set CP to DESIRED if it was ENABLED, so we can re-enable it
again on hotplug */
@@ -7002,13 +7005,7 @@ static void amdgpu_dm_connector_funcs_force(struct
drm_connector *connector)
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
- struct edid *edid;
- struct
Re: [PATCH v4 01/11] drm/amd/display: clean unused variables for hdmi freesync parser
Hi Melissa,
There are no commit messages in this patch.
Also, do you think this can be merged with Patch 5 "drm/amd/display:
remove redundant freesync parser for DP"?
On 2024-07-05 21:35, Melissa Wen wrote:
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 98cf523a629e..1dfa7ec9af35 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -12108,9 +12108,6 @@ void amdgpu_dm_update_freesync_caps(struct
drm_connector *connector,
} else if (edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
if (i >= 0 && vsdb_info.freesync_supported) {
- timing = &edid->detailed_timings[i];
- data= &timing->data.other_data;
-
amdgpu_dm_connector->min_vfreq =
vsdb_info.min_refresh_rate_hz;
amdgpu_dm_connector->max_vfreq =
vsdb_info.max_refresh_rate_hz;
if (amdgpu_dm_connector->max_vfreq -
amdgpu_dm_connector->min_vfreq > 10)
Re: [RFC PATCH 2/2] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
On 2024-01-26 09:28, Melissa Wen wrote:
Replace raw edid handling (struct edid) with the opaque EDID type
(struct drm_edid) on amdgpu_dm_connector for consistency. It may also
prevent mismatch of approaches in different parts of the driver code.
Working in progress. There are a couple of cast warnings and it was only
tested with IGT.
Signed-off-by: Melissa Wen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 63 ++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +-
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 9 +--
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 23 +++
4 files changed, 51 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 68e71e2ea472..741081d73bb3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3277,12 +3277,12 @@ void amdgpu_dm_update_connector_after_detect(
&aconnector->dm_dp_aux.aux);
}
} else {
- aconnector->edid =
- (struct edid *)sink->dc_edid.raw_edid;
+ const struct edid *edid = (const struct edid
*)sink->dc_edid.raw_edid;
+ aconnector->edid = drm_edid_alloc(edid,
(edid->extensions + 1) * EDID_LENGTH);
if (aconnector->dc_link->aux_mode)
drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
- aconnector->edid);
+
drm_edid_raw(aconnector->edid));
}
if (!aconnector->timing_requested) {
@@ -3293,13 +3293,13 @@ void amdgpu_dm_update_connector_after_detect(
"failed to create
aconnector->requested_timing\n");
}
- drm_connector_update_edid_property(connector, aconnector->edid);
+ drm_edid_connector_update(connector, aconnector->edid);
amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
update_connector_ext_caps(aconnector);
} else {
drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
amdgpu_dm_update_freesync_caps(connector, NULL);
- drm_connector_update_edid_property(connector, NULL);
+ drm_edid_connector_update(connector, NULL);
aconnector->num_modes = 0;
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
@@ -6564,7 +6564,6 @@ static void amdgpu_dm_connector_funcs_force(struct
drm_connector *connector)
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
const struct drm_edid *drm_edid;
- const struct edid *edid;
/*
* Note: drm_get_edid gets edid in the following order:
@@ -6578,11 +6577,12 @@ static void amdgpu_dm_connector_funcs_force(struct
drm_connector *connector)
DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
}
- edid = drm_edid_raw(drm_edid);
- aconnector->edid = edid;
-
+ aconnector->edid = drm_edid;
+ drm_edid_connector_update(connector, drm_edid);
/* Update emulated (virtual) sink's EDID */
if (dc_em_sink && dc_link) {
+ const struct edid *edid = drm_edid_raw(drm_edid);
+
memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
memmove(dc_em_sink->dc_edid.raw_edid, (uint8_t *)edid,
(edid->extensions + 1) * EDID_LENGTH);
dm_helpers_parse_edid_caps(
@@ -6633,13 +6633,13 @@ static void create_eml_sink(struct amdgpu_dm_connector
*aconnector)
return;
}
- edid = drm_edid_raw(drm_edid);
-
- if (drm_detect_hdmi_monitor(edid))
+ if (connector->display_info.is_hdmi)
init_params.sink_signal = SIGNAL_TYPE_HDMI_TYPE_A;
- aconnector->edid = edid;
+ aconnector->edid = drm_edid;
+ drm_edid_connector_update(connector, drm_edid);
+ edid = drm_edid_raw(drm_edid);
aconnector->dc_em_sink = dc_link_add_remote_sink(
aconnector->dc_link,
(uint8_t *)edid,
@@ -7322,16 +7322,16 @@ static void amdgpu_set_panel_orientation(struct
drm_connector *connector)
}
static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
- struct edid *edid)
+ const struct drm_edid *drm_edid)
{
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
- if (edid) {
+ if (drm_edid) {
/* empty probed_modes */
INIT_LIST_HEAD(&connector->pro
Re: [PATCH] drm/amd/display: Fix memory leak in dm_set_writeback()
Thanks for catching this.
Reviewed-by: Alex Hung
On 2023-12-08 02:58, Harshit Mogalapalli wrote:
'wb_info' needs to be freed on error paths or it would leak the memory.
Smatch pointed this out.
Fixes: c81e13b929df ("drm/amd/display: Hande writeback request from userspace")
Signed-off-by: Harshit Mogalapalli
---
This is based on static analysis and only compile tested
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
1 file changed, 2 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 afdcc43ea06c..333995f70239 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8871,12 +8871,14 @@ static void dm_set_writeback(struct
amdgpu_display_manager *dm,
acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
if (!acrtc) {
DRM_ERROR("no amdgpu_crtc found\n");
+ kfree(wb_info);
return;
}
afb = to_amdgpu_framebuffer(new_con_state->writeback_job->fb);
if (!afb) {
DRM_ERROR("No amdgpu_framebuffer found\n");
+ kfree(wb_info);
return;
}
[PATCH][V3] drm/amd/display: Remove unwanted drm edid references
[WHY]
edid_override and drm_edid_override_connector_update, according to drm
documentation, should not be referred outside drm_edid.
[HOW]
Remove and replace them accordingly. This can tested by IGT's
kms_hdmi_inject test.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5efebc06296b..3968dd9cef7f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6444,15 +6444,23 @@ amdgpu_dm_connector_late_register(struct drm_connector
*connector)
static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
struct edid *edid;
- if (!connector->edid_override)
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
+ }
- drm_edid_override_connector_update(&aconnector->base);
- edid = aconnector->base.edid_blob_ptr->data;
aconnector->edid = edid;
/* Update emulated (virtual) sink's EDID */
@@ -6487,30 +6495,26 @@ static int get_modes(struct drm_connector *connector)
static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
{
+ struct drm_connector *connector = &aconnector->base;
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(&aconnector->base);
struct dc_sink_init_data init_params = {
.link = aconnector->dc_link,
.sink_signal = SIGNAL_TYPE_VIRTUAL
};
struct edid *edid;
- if (!aconnector->base.edid_blob_ptr) {
- /* if connector->edid_override valid, pass
-* it to edid_override to edid_blob_ptr
-*/
-
- drm_edid_override_connector_update(&aconnector->base);
-
- if (!aconnector->base.edid_blob_ptr) {
- DRM_ERROR("No EDID firmware found on connector: %s
,forcing to OFF!\n",
- aconnector->base.name);
-
- aconnector->base.force = DRM_FORCE_OFF;
- return;
- }
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
+ return;
}
- edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
-
aconnector->edid = edid;
aconnector->dc_em_sink = dc_link_add_remote_sink(
--
2.42.0
Re: [PATCH][V2] drm/amd/display: Remove unwanted drm edid references
On 2023-09-18 04:25, Jani Nikula wrote:
On Fri, 15 Sep 2023, Alex Hung wrote:
[WHY]
edid_override and drm_edid_override_connector_update, according to drm
documentation, should not be referred outside drm_edid.
[HOW]
Remove and replace them accordingly. This can tested by IGT's
kms_hdmi_inject test.
Signed-off-by: Alex Hung
---
V2 - add comments for drm_get_edid and check edid before use.
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 44 +++
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5efebc06296b..b29ef87c27a9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6444,15 +6444,24 @@ amdgpu_dm_connector_late_register(struct drm_connector
*connector)
static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
struct edid *edid;
- if (!connector->edid_override)
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s, forcing to OFF!\n",
connector->name);
+ connector->force = DRM_FORCE_OFF;
Drivers aren't supposed to modify connector->force.
Why would you do that anyway? This connects EDID probe success with
connector forcing, and I've been repeatedly saying these are two
separate things that should not be conflated.
Maybe the user has set connector->force, because the display probe is
flaky. This switches connector->force off if the display does not
respond, undermining one of the main purposes of the whole thing.
Thanks. I will removed this and sent V3.
return;
+ }
- drm_edid_override_connector_update(&aconnector->base);
- edid = aconnector->base.edid_blob_ptr->data;
aconnector->edid = edid;
/* Update emulated (virtual) sink's EDID */
@@ -6487,30 +6496,27 @@ static int get_modes(struct drm_connector *connector)
static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
{
+ struct drm_connector *connector = &aconnector->base;
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(&aconnector->base);
struct dc_sink_init_data init_params = {
.link = aconnector->dc_link,
.sink_signal = SIGNAL_TYPE_VIRTUAL
};
struct edid *edid;
- if (!aconnector->base.edid_blob_ptr) {
- /* if connector->edid_override valid, pass
-* it to edid_override to edid_blob_ptr
-*/
-
- drm_edid_override_connector_update(&aconnector->base);
-
- if (!aconnector->base.edid_blob_ptr) {
- DRM_ERROR("No EDID firmware found on connector: %s ,forcing
to OFF!\n",
- aconnector->base.name);
-
- aconnector->base.force = DRM_FORCE_OFF;
- return;
- }
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s, forcing to OFF!\n",
connector->name);
+ connector->force = DRM_FORCE_OFF;
Ditto.
BR,
Jani.
+ return;
}
- edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
-
aconnector->edid = edid;
aconnector->dc_em_sink = dc_link_add_remote_sink(
[PATCH][V2] drm/amd/display: Remove unwanted drm edid references
[WHY]
edid_override and drm_edid_override_connector_update, according to drm
documentation, should not be referred outside drm_edid.
[HOW]
Remove and replace them accordingly. This can tested by IGT's
kms_hdmi_inject test.
Signed-off-by: Alex Hung
---
V2 - add comments for drm_get_edid and check edid before use.
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 44 +++
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5efebc06296b..b29ef87c27a9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6444,15 +6444,24 @@ amdgpu_dm_connector_late_register(struct drm_connector
*connector)
static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
struct edid *edid;
- if (!connector->edid_override)
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s, forcing to OFF!\n",
connector->name);
+ connector->force = DRM_FORCE_OFF;
return;
+ }
- drm_edid_override_connector_update(&aconnector->base);
- edid = aconnector->base.edid_blob_ptr->data;
aconnector->edid = edid;
/* Update emulated (virtual) sink's EDID */
@@ -6487,30 +6496,27 @@ static int get_modes(struct drm_connector *connector)
static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
{
+ struct drm_connector *connector = &aconnector->base;
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(&aconnector->base);
struct dc_sink_init_data init_params = {
.link = aconnector->dc_link,
.sink_signal = SIGNAL_TYPE_VIRTUAL
};
struct edid *edid;
- if (!aconnector->base.edid_blob_ptr) {
- /* if connector->edid_override valid, pass
-* it to edid_override to edid_blob_ptr
-*/
-
- drm_edid_override_connector_update(&aconnector->base);
-
- if (!aconnector->base.edid_blob_ptr) {
- DRM_ERROR("No EDID firmware found on connector: %s
,forcing to OFF!\n",
- aconnector->base.name);
-
- aconnector->base.force = DRM_FORCE_OFF;
- return;
- }
+ /*
+* Note: drm_get_edid gets edid in the following order:
+* 1) override EDID if set via edid_override debugfs,
+* 2) firmware EDID if set via edid_firmware module parameter
+* 3) regular DDC read.
+*/
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+ if (!edid) {
+ DRM_ERROR("No EDID found on connector: %s, forcing to OFF!\n",
connector->name);
+ connector->force = DRM_FORCE_OFF;
+ return;
}
- edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
-
aconnector->edid = edid;
aconnector->dc_em_sink = dc_link_add_remote_sink(
--
2.42.0
[PATCH] drm/amd/display: Remove unwanted drm edid references
[WHY]
edid_override and drm_edid_override_connector_update, according to drm
documentation, should not be referred outside drm_edid.
[HOW]
Remove and replace them accordingly.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1bb1a394f55f..f6a255773242 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6372,15 +6372,12 @@ amdgpu_dm_connector_late_register(struct drm_connector
*connector)
static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
struct edid *edid;
- if (!connector->edid_override)
- return;
-
- drm_edid_override_connector_update(&aconnector->base);
- edid = aconnector->base.edid_blob_ptr->data;
+ edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
aconnector->edid = edid;
/* Update emulated (virtual) sink's EDID */
@@ -6421,22 +6418,6 @@ static void create_eml_sink(struct amdgpu_dm_connector
*aconnector)
};
struct edid *edid;
- if (!aconnector->base.edid_blob_ptr) {
- /* if connector->edid_override valid, pass
-* it to edid_override to edid_blob_ptr
-*/
-
- drm_edid_override_connector_update(&aconnector->base);
-
- if (!aconnector->base.edid_blob_ptr) {
- DRM_ERROR("No EDID firmware found on connector: %s
,forcing to OFF!\n",
- aconnector->base.name);
-
- aconnector->base.force = DRM_FORCE_OFF;
- return;
- }
- }
-
edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
aconnector->edid = edid;
--
2.42.0
Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()
On 2023-08-30 01:29, Jani Nikula wrote: On Tue, 29 Aug 2023, Alex Hung wrote: On 2023-08-29 11:03, Jani Nikula wrote: On Tue, 29 Aug 2023, Jani Nikula wrote: On Tue, 29 Aug 2023, Alex Deucher wrote: On Tue, Aug 29, 2023 at 6:48 AM Jani Nikula wrote: On Wed, 23 Aug 2023, Jani Nikula wrote: On Tue, 22 Aug 2023, Alex Hung wrote: On 2023-08-22 06:01, Jani Nikula wrote: Over the past years I've been trying to unify the override and firmware EDID handling as well as EDID property updates. It won't work if drivers do their own random things. Let's check how to replace these references by appropriate ones or fork the function as reverting these patches causes regressions. I think the fundamental problem you have is conflating connector forcing with EDID override. They're orthogonal. The .force callback has no business basing the decisions on connector->edid_override. Force is force, override is override. The driver isn't even supposed to know or care if the EDID originates from the firmware loader or override EDID debugfs. drm_get_edid() will handle that for you transparently. It'll return the EDID, and you shouldn't look at connector->edid_blob_ptr either. Using that will make future work in drm_edid.c harder. You can't fix that with minor tweaks. I think you'll be better off starting from scratch. Also, connector->edid_override is debugfs. You actually can change the behaviour. If your userspace, whatever it is, has been written to assume connector forcing if EDID override is set, you *do* have to fix that, and set both. Any updates on fixing this, or shall we proceed with the reverts? There is a patch under internal reviews. It removes calls edid_override and drm_edid_override_connector_update as intended in this patchset but does not remove the functionality. While I am happy to hear there's progress, I'm somewhat baffled the review is internal. The commits that I suggested to revert were also only reviewed internally, as far as I can see... And that's kind of the problem. Upstream code should be reviewed in public. Hi Jani, All patches are sent for public reviews, the progress is summarized as the followings: == internal == 1. a patch or patches are tested by CI. 2. internal technical and IP reviews are performed to ensure no concerns before patches are merged to internal branch. == public == 3. a regression test and IP reviews are performed by engineers before sending to public mailing lists. 4. the patchset is sent for public reviews ex. https://patchwork.freedesktop.org/series/122498/ 5. patches are merged to public repo. BR, Jani. With the patch. both following git grep commands return nothing in amd-staging-drm-next. $ git grep drm_edid_override_connector_update -- drivers/gpu/drm/amd $ git grep edid_override -- drivers/gpu/drm/amd Best regards, Alex Hung What is the goal of the reverts? I don't disagree that we may be using the interfaces wrong, but reverting them will regess functionality in the driver. The commits are in v6.5-rc1, but not yet in a release. No user depends on them yet. I'd strongly prefer them not reaching v6.5 final and users. Sorry for confusion here, that's obviously come and gone already. :( The firmware EDID, override EDID, connector forcing, the EDID property, etc. have been and somewhat still are a hairy mess that we must keep untangling, and this isn't helping. I've put in crazy amounts of work on this, and I've added kernel-doc comments about stuff that should and should not be done, but they go unread and ignored. I really don't want to end up having to clean this up myself before I can embark on further cleanups and refactoring. And again, if the functionality in the driver depends on conflating two things that should be separate, it's probably not such a hot idea to let it reach users either. Even if it's just debugfs. BR, Jani.
Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()
On 2023-08-29 11:03, Jani Nikula wrote: On Tue, 29 Aug 2023, Jani Nikula wrote: On Tue, 29 Aug 2023, Alex Deucher wrote: On Tue, Aug 29, 2023 at 6:48 AM Jani Nikula wrote: On Wed, 23 Aug 2023, Jani Nikula wrote: On Tue, 22 Aug 2023, Alex Hung wrote: On 2023-08-22 06:01, Jani Nikula wrote: Over the past years I've been trying to unify the override and firmware EDID handling as well as EDID property updates. It won't work if drivers do their own random things. Let's check how to replace these references by appropriate ones or fork the function as reverting these patches causes regressions. I think the fundamental problem you have is conflating connector forcing with EDID override. They're orthogonal. The .force callback has no business basing the decisions on connector->edid_override. Force is force, override is override. The driver isn't even supposed to know or care if the EDID originates from the firmware loader or override EDID debugfs. drm_get_edid() will handle that for you transparently. It'll return the EDID, and you shouldn't look at connector->edid_blob_ptr either. Using that will make future work in drm_edid.c harder. You can't fix that with minor tweaks. I think you'll be better off starting from scratch. Also, connector->edid_override is debugfs. You actually can change the behaviour. If your userspace, whatever it is, has been written to assume connector forcing if EDID override is set, you *do* have to fix that, and set both. Any updates on fixing this, or shall we proceed with the reverts? There is a patch under internal reviews. It removes calls edid_override and drm_edid_override_connector_update as intended in this patchset but does not remove the functionality. With the patch. both following git grep commands return nothing in amd-staging-drm-next. $ git grep drm_edid_override_connector_update -- drivers/gpu/drm/amd $ git grep edid_override -- drivers/gpu/drm/amd Best regards, Alex Hung What is the goal of the reverts? I don't disagree that we may be using the interfaces wrong, but reverting them will regess functionality in the driver. The commits are in v6.5-rc1, but not yet in a release. No user depends on them yet. I'd strongly prefer them not reaching v6.5 final and users. Sorry for confusion here, that's obviously come and gone already. :( The firmware EDID, override EDID, connector forcing, the EDID property, etc. have been and somewhat still are a hairy mess that we must keep untangling, and this isn't helping. I've put in crazy amounts of work on this, and I've added kernel-doc comments about stuff that should and should not be done, but they go unread and ignored. I really don't want to end up having to clean this up myself before I can embark on further cleanups and refactoring. And again, if the functionality in the driver depends on conflating two things that should be separate, it's probably not such a hot idea to let it reach users either. Even if it's just debugfs. BR, Jani.
Re: [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()
On 2023-08-22 06:01, Jani Nikula wrote: Over the past years I've been trying to unify the override and firmware EDID handling as well as EDID property updates. It won't work if drivers do their own random things. Let's check how to replace these references by appropriate ones or fork the function as reverting these patches causes regressions. Cheers, Alex BR, Jani. Cc: Alex Deucher Cc: Alex Hung Cc: Chao-kai Wang Cc: Daniel Wheeler Cc: Harry Wentland Cc: Hersen Wu Cc: Leo Li Cc: Rodrigo Siqueira Cc: Wenchieh Chien Cc: David Airlie Cc: Daniel Vetter Jani Nikula (4): Revert "drm/amd/display: drop unused count variable in create_eml_sink()" Revert "drm/amd/display: assign edid_blob_ptr with edid from debugfs" Revert "drm/amd/display: mark amdgpu_dm_connector_funcs_force static" Revert "drm/amd/display: implement force function in amdgpu_dm_connector_funcs" .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 44 +++ 1 file changed, 5 insertions(+), 39 deletions(-)
Re: [PATCH] drm/amd/display: check attr flag before set cursor degamma on DCN3+
Tested-by: Alex Hung
On 2023-07-31 02:35, Melissa Wen wrote:
Don't set predefined degamma curve to cursor plane if the cursor
attribute flag is not set. Applying a degamma curve to the cursor by
default breaks userspace expectation. Checking the flag before
performing any color transformation prevents too dark cursor gamma in
DCN3+ on many Linux desktop environment (KDE Plasma, GNOME,
wlroots-based, etc.) as reported at:
- https://gitlab.freedesktop.org/drm/amd/-/issues/1513
This is the same approach followed by DCN2 drivers where the issue is
not present.
Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
index e5b7ef7422b8..50dc83404644 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
@@ -357,8 +357,11 @@ void dpp3_set_cursor_attributes(
int cur_rom_en = 0;
if (color_format == CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA ||
- color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA)
- cur_rom_en = 1;
+ color_format == CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA) {
+ if
(cursor_attributes->attribute_flags.bits.ENABLE_CURSOR_DEGAMMA) {
+ cur_rom_en = 1;
+ }
+ }
REG_UPDATE_3(CURSOR0_CONTROL,
CUR0_MODE, color_format,
[PATCH] drm/amd/display: fix an incorrect comment
This is a copy-and-paste error. Fix the comment to match the macro definition. Signed-off-by: Alex Hung --- drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h index 63219ecd8478..1bf6b12f5663 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h @@ -29,4 +29,4 @@ void dcn10_resource_construct_fp(struct dc *dc); -#endif /* __DCN20_FPU_H__ */ +#endif /* __DCN10_FPU_H__ */ -- 2.38.1
[RFC PATCH 5/5] drm/amd/display: Fill 3D LUT from userspace
Implement the 3D LUT interface, convert and pass the data for amdgpu
driver.
Note: A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 181 ++
3 files changed, 195 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 7094578a683f..10e6dc5c8552 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5656,6 +5656,19 @@ static int fill_dc_plane_attributes(struct amdgpu_device
*adev,
dc_plane_state->in_transfer_func->type = TF_TYPE_HWPWL;
}
+ /* 3D LUT from userspace */
+ if (plane_state->color_mgmt_changed) {
+ if (plane_state->lut_3d && dc_plane_state->lut3d_func) {
+ ret = amdgpu_dm_fill_3dlut_data(plane_state,
&dc_plane_state->lut3d_func->lut_3d);
+ if (!ret)
+
dc_plane_state->lut3d_func->state.bits.initialized = 1;
+ else
+ return ret;
+ } else {
+ /* TODO disable 3D LUT */
+ }
+ }
+
return 0;
}
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 667957087ccf..644c5ff6ee9a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -726,6 +726,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state
*crtc,
struct dc_plane_state *dc_plane_state);
void amdgpu_dm_fill_pwl_data(struct drm_property_blob *lut_blob, struct
pwl_params *lut_params, struct drm_color_lut_range *pwl_definition, int
pwl_size);
+int amdgpu_dm_fill_3dlut_data(const struct drm_plane_state *plane_state,
struct tetrahedral_params *param);
void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *aconnector);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index ae633fe52525..705852bf63e7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -22,6 +22,7 @@
* Authors: AMD
*
*/
+ #include
#include "amdgpu.h"
#include "amdgpu_mode.h"
#include "amdgpu_dm.h"
@@ -469,6 +470,186 @@ int amdgpu_dm_verify_lut_sizes(const struct
drm_crtc_state *crtc_state)
return 0;
}
+#define R_3DLUT0
+#define G_3DLUT1
+#define B_3DLUT2
+
+static __u16 extract_rgb_value(void *lut_3d, __u32 color_format, __u8 color)
+{
+ __u64 val = *(__u64 *) lut_3d;
+
+ switch (color_format) {
+ case DRM_FORMAT_XRGB16161616:
+ if (color == R_3DLUT)
+ return val & 0x;
+ else if (color == G_3DLUT)
+ return (val >> 16) & 0x;
+ else if (color == B_3DLUT)
+ return (val >> 32) & 0x;
+ break;
+ case DRM_FORMAT_XBGR16161616:
+ if (color == B_3DLUT)
+ return val & 0x;
+ else if (color == G_3DLUT)
+ return (val >> 16) & 0x;
+ else if (color == R_3DLUT)
+ return (val >> 32) & 0x;
+ break;
+ case DRM_FORMAT_XRGB:
+ if (color == R_3DLUT)
+ return val & 0xFF;
+ else if (color == G_3DLUT)
+ return (val >> 8) & 0xFF;
+ else if (color == B_3DLUT)
+ return (val >> 16) & 0xFF;
+ break;
+ case DRM_FORMAT_XBGR:
+ if (color == B_3DLUT)
+ return val & 0xFF;
+ else if (color == G_3DLUT)
+ return (val >> 8) & 0xFF;
+ else if (color == R_3DLUT)
+ return (val >> 16) & 0xFF;
+ break;
+ default:
+ return 0;
+ }
+
+ return 0;
+}
+
+static bool extract_rgb_data(const struct drm_plane_state *plane_state, struct
drm_mode_3dlut_mode *mode, __u16 *lut_data)
+{
+ __u16 i, lut_volume;
+ void *lut_3d = plane_state->lut_3d->data;
+ __u32 cfmt = mode->color_format;
+
+ /* copy RGB accordingly */
+ lut_volume = mode->lut_size * mode->lut_size * mode-&
[RFC PATCH 4/5] drm/amd/display: Enable plane 3DLUT mode
Enable the 3D LUT mode supported by amdgpu.
Note: A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++
drivers/gpu/drm/drm_color_mgmt.c | 31 +++
include/drm/drm_plane.h | 2 ++
3 files changed, 36 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 ee277f357140..7094578a683f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8008,6 +8008,9 @@ static int amdgpu_dm_plane_init(struct
amdgpu_display_manager *dm,
/* TODO need to check ASICs */
drm_plane_create_3d_lut_properties(plane->dev, plane, 1);
+ res = drm_plane_color_add_3dlut_mode(plane, "3dlut_17_12bit",
&lut_3d_mode_17_12bit, sizeof(lut_3d_mode_17_12bit));
+ if (res)
+ return res;
drm_plane_attach_3dlut_properties(plane);
/* Create (reset) the plane state */
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4bfe5b5c9670..5418ca24db73 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -743,6 +743,37 @@ void drm_plane_attach_3dlut_properties(struct drm_plane
*plane)
}
EXPORT_SYMBOL(drm_plane_attach_3dlut_properties);
+int drm_plane_color_add_3dlut_mode(struct drm_plane *plane,
+const char *name,
+const struct
drm_mode_3dlut_mode *mode_3dlut,
+size_t length)
+{
+ struct drm_property_blob *blob;
+ struct drm_property *prop = NULL;
+ int ret;
+
+ prop = plane->lut_3d_mode_property;
+
+ if (!prop)
+ return -EINVAL;
+
+ if (length == 0 && name)
+ return drm_property_add_enum(prop, 0, name);
+
+ blob = drm_property_create_blob(plane->dev, length, mode_3dlut);
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
+
+ ret = drm_property_add_enum(prop, blob->base.id, name);
+ if (ret) {
+ drm_property_blob_put(blob);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_add_3dlut_mode);
+
int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
const char *name,
const struct
drm_color_lut_range *ranges,
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 4e272144170f..f94f91466675 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -946,6 +946,8 @@ int drm_plane_create_3d_lut_properties(struct drm_device
*dev,
struct drm_plane *plane,
int num_values);
void drm_plane_attach_3dlut_properties(struct drm_plane *plane);
+int drm_plane_color_add_3dlut_mode(struct drm_plane *plane, const char *name,
+const struct
drm_mode_3dlut_mode *mode_3dlut, size_t length);
int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
const char *name,
const struct
drm_color_lut_range *ranges,
--
2.37.3
[RFC PATCH 3/5] drm/amd/display: Define 3D LUT struct for HDR planes
Add a 3D LUT mode supported by amdgpu driver.
Note: A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/modules/color/color_gamma.h | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.h
b/drivers/gpu/drm/amd/display/modules/color/color_gamma.h
index e06e0a8effc8..aceb23b03a4b 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.h
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.h
@@ -27,6 +27,7 @@
#define COLOR_MOD_COLOR_GAMMA_H_
#include "color_table.h"
+#include
struct dc_transfer_func;
struct dc_gamma;
@@ -35,6 +36,17 @@ struct dc_rgb_fixed;
struct dc_color_caps;
enum dc_transfer_func_predefined;
+/*
+ * 3D LUT mode for 17x17x17 LUT and 12 bits of color depth
+ */
+static const struct drm_mode_3dlut_mode lut_3d_mode_17_12bit = {
+ .lut_size = 17,
+ .lut_stride = {17, 17, 18},
+ .bit_depth = 12,
+ .color_format = DRM_FORMAT_XRGB16161616,
+ .flags = 0,
+};
+
static const struct drm_color_lut_range nonlinear_pwl[] = {
{
.flags = (DRM_MODE_LUT_GAMMA | DRM_MODE_LUT_REFLECT_NEGATIVE |
DRM_MODE_LUT_INTERPOLATE | DRM_MODE_LUT_REUSE_LAST |
DRM_MODE_LUT_NON_DECREASING),
--
2.37.3
[RFC PATCH 2/5] drm: Add Plane 3DLUT and 3DLUT mode properties
Add plane lut_3d mode and lut_3d as blob properties.
lut_3d mode is an enum property with values as blob_ids.
Userspace can get supported modes and also set one of the modes.
Note: A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Signed-off-by: Alex Hung
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++
drivers/gpu/drm/drm_atomic_state_helper.c | 3 ++
drivers/gpu/drm/drm_atomic_uapi.c | 11 ++
drivers/gpu/drm/drm_color_mgmt.c | 37 +++
include/drm/drm_mode_object.h | 2 +-
include/drm/drm_plane.h | 31
6 files changed, 87 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 f546c1326db3..ee277f357140 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8006,6 +8006,10 @@ static int amdgpu_dm_plane_init(struct
amdgpu_display_manager *dm,
drm_plane_attach_gamma_properties(plane);
drm_plane_attach_ctm_property(plane);
+ /* TODO need to check ASICs */
+ drm_plane_create_3d_lut_properties(plane->dev, plane, 1);
+ drm_plane_attach_3dlut_properties(plane);
+
/* Create (reset) the plane state */
if (plane->funcs->reset)
plane->funcs->reset(plane);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 7ddf6e4b956b..85900cd1bffe 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -318,6 +318,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
drm_property_blob_get(state->ctm);
if (state->gamma_lut)
drm_property_blob_get(state->gamma_lut);
+ if (state->lut_3d)
+ drm_property_blob_get(state->lut_3d);
state->color_mgmt_changed = false;
}
@@ -369,6 +371,7 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
drm_property_blob_put(state->degamma_lut);
drm_property_blob_put(state->ctm);
drm_property_blob_put(state->gamma_lut);
+ drm_property_blob_put(state->lut_3d);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index ba3e64cb184a..66e59e7c194d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -622,6 +622,13 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == plane->lut_3d_property) {
+ ret = drm_atomic_replace_property_blob_from_id(dev,
+ &state->lut_3d, val, -1, 8, &replaced);
+ state->color_mgmt_changed |= replaced;
+ return 0;
+ } else if (property == plane->lut_3d_mode_property) {
+ state->lut_3d_mode = val;
} else if (property == config->prop_fb_damage_clips) {
ret = drm_atomic_replace_property_blob_from_id(dev,
&state->fb_damage_clips,
@@ -700,6 +707,10 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
} else if (property == plane->gamma_lut_property) {
*val = (state->gamma_lut) ?
state->gamma_lut->base.id : 0;
+ } else if (property == plane->lut_3d_property) {
+ *val = (state->lut_3d) ? state->lut_3d->base.id : 0;
+ } else if (property == plane->lut_3d_mode_property) {
+ *val = state->lut_3d_mode;
} else if (property == config->prop_fb_damage_clips) {
*val = (state->fb_damage_clips) ?
state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index b5b3ff7f654d..4bfe5b5c9670 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -706,6 +706,43 @@ void drm_plane_attach_gamma_properties(struct drm_plane
*plane)
}
EXPORT_SYMBOL(drm_plane_attach_gamma_properties);
+int drm_plane_create_3d_lut_properties(struct drm_device *dev,
+ struct drm_plane *plane,
+ int num_values)
+{
+ struct drm_property *mode;
+ struct drm_property *blob;
+
+ mode = drm_property_create(dev, DRM_MODE_PROP_ENUM,
"PLANE_3D_LUT_MODE", num_values);
+ if (!mode)
+ return -ENOMEM;
+
+ plane->lut_3d_mod
[RFC PATCH 1/5] drm: Add 3D LUT mode and its attributes
A struct is defined for 3D LUT modes to be supported by hardware.
The elements includes lut_isze, lut_stride, bit_depth, color_format
and flags.
Note: A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Signed-off-by: Alex Hung
---
include/uapi/drm/drm_mode.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index d0ce48d2e732..334e8a9b49cc 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -877,6 +877,23 @@ struct drm_color_lut_ext {
__u64 reserved;
};
+/*
+ * struct drm_mode_3dlut_mode - 3D LUT mode information.
+ * @lut_size: number of valid points on every dimension of 3D LUT.
+ * @lut_stride: number of points on every dimension of 3D LUT.
+ * @bit_depth: number of bits of RGB. If color_mode defines entries with higher
+ * bit_depth the least significant bits will be truncated.
+ * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
DRM_FORMAT_XBGR16161616.
+ * @flags: flags for hardware-sepcific features
+ */
+struct drm_mode_3dlut_mode {
+ __u16 lut_size;
+ __u16 lut_stride[3];
+ __u16 bit_depth;
+ __u32 color_format;
+ __u32 flags;
+};
+
#define DRM_MODE_PAGE_FLIP_EVENT 0x01
#define DRM_MODE_PAGE_FLIP_ASYNC 0x02
#define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
--
2.37.3
[RFC PATCH 0/5] Proposal for Pre-blending 3D LUT interfaces
This is an proposal and a draft implementation to enable 3D LUT on
drm_plane. This proposal defines a new interface for userspace
applications to query hardware capabilities and to pass/enable 3D LUT
functions via this DRM/KMS APIs.
Overviews:
┌─┐┌─┐┌───┐┌──┐ ┌┐
│Userspace│◄──►│3DLUT API│◄──►│DRM│◄──►│GPU driver├──►│hardware│
└─┘└─┘└───┘└──┘ └┘
1. Userspace queries the 3DLUT mode (defined by drm_mode_3dlut_mode)
from the GPU drivers (ex. amdgpu).
2. The GPU Driver replies sizes and the color depth of the
3DLUT modes, such as defined by struct lut_3d_mode_17_12bit.
3. If applicable, userspace selects and sets one of preferred 3DLUT
modes by "lut_3d_mode" to driver.
4. Userspace passes 3D LUT via drm_property_blob "lut_3d". In the case
of the mode "lut_3d_mode_17_12bit", the 3D LUT should have a cube
size = 17x17x17 (lut_size), color depth = 12 bits (bit_depth), and
X/Y/Z axis = R/G/B (color_format).
5. The GPU driver parses 3D LUT and passes it to hardware pipeline, and
enables 3D LUT accordingly.
Notes:
1. The patchset is based on the previous work on
https://gitlab.freedesktop.org/hwentland/linux/-/tree/color-and-hdr
2. This interface can be part of the newly proposed DRM/KMS color pipeline
API (https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11). A
future integration to the new API may be required or preferred, such
as the followings:
struct drm_color_pipeline_element {
drm_color_pipeline_element_type;
drm_color_pipeline_element_lut3d
};
struct drm_mode_3dlut_mode -> struct drm_color_pipeline_lut3d_config
struct drm_color_pipeline_lut3d {
lut_3d_mode_17_12bit;
};
struct drm_color_pipeline_lut3d_data {
*lut_3d;
};
and etc.
3. A patchset "IGT tests for pre-blending 3D LUT interfaces" for this
proposal is sent to IGT mailing list.
Related Work:
- Enable 3D LUT to AMD display drivers
(https://www.spinics.net/lists/amd-gfx/msg83032.html)
Alex Hung (5):
drm: Add 3D LUT mode and its attributes
drm: Add Plane 3DLUT and 3DLUT mode properties
drm/amd/display: Define 3D LUT struct for HDR planes
drm/amd/display: Enable plane 3DLUT mode
drm/amd/display: Fill 3D LUT from userspace
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 181 ++
.../amd/display/modules/color/color_gamma.h | 12 ++
drivers/gpu/drm/drm_atomic_state_helper.c | 3 +
drivers/gpu/drm/drm_atomic_uapi.c | 11 ++
drivers/gpu/drm/drm_color_mgmt.c | 68 +++
include/drm/drm_mode_object.h | 2 +-
include/drm/drm_plane.h | 33
include/uapi/drm/drm_mode.h | 17 ++
10 files changed, 347 insertions(+), 1 deletion(-)
--
2.37.3
Re: [PATCH V3 46/47] drm/amd/display: Fix failures of disabling primary plans
On 2022-09-14 10:55, Michel Dänzer wrote:
[ Adding the dri-devel list ]
On 2022-09-14 18:30, Alex Hung wrote:
On 2022-09-14 07:40, Michel Dänzer wrote:
On 2022-09-14 15:31, Michel Dänzer wrote:
On 2022-09-14 07:10, Wayne Lin wrote:
From: Alex Hung
[Why & How]
This fixes kernel errors when IGT disables primary planes during the
tests kms_universal_plane::functional_test_pipe/pageflip_test_pipe.
NAK.
This essentially reverts commit b836a274b797 ("drm/amdgpu/dc: Require primary plane
to be enabled whenever the CRTC is") (except that it goes even further and
completely removes the requirement for any HW plane to be enabled when the HW cursor is),
so it would reintroduce the issues described in that commit log.
Actually not exactly the same issues, due to going even further than reverting
my fix.
Instead, the driver will claim that an atomic commit which enables the CRTC and
the cursor plane, while leaving all other KMS planes disabled, succeeds. But
the HW cursor will not actually be visible.
I did not observe problems with cursors (GNOME 42.4 - desktop and youtube/mpv
video playback: windowed/fullscreen). Are there steps to reproduce cursor
problems?
As described in my last follow-up e-mail: An atomic KMS commit which enables
the CRTC and the cursor plane, but disables all other KMS planes for the CRTC.
The commit will succeed, but will not result in the HW cursor being actually
visible. (I don't know of any specific application or test which exercises this)
Did you mean cursor plane depends on primary plane (i.e. no primary
plane = no visible HW cursor)? If there is no primary plane, what
scenario would it be required to draw a cursor?
If this is a rare case, would it still be a concern?
Also see the commit log of bc92c06525e5 ("drm/amd/display: Allow commits with no
planes active").
Does it mean dm_crtc_helper_atomic_check does not need to return -EINVAL
if there is no active cursor because there are no cursors to be shown
anyways, as shown in the below diff:
+static bool does_crtc_have_active_cursor(struct drm_crtc_state
*new_crtc_state)
+{
+ struct drm_device *dev = new_crtc_state->crtc->dev;
+ struct drm_plane *plane;
+
+ drm_for_each_plane_mask(plane, dev, new_crtc_state->plane_mask) {
+ if (plane->type == DRM_PLANE_TYPE_CURSOR)
+ return true;
+ }
+
+ return false;
+}
+
static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -383,7 +396,8 @@ static int dm_crtc_helper_atomic_check(struct
drm_crtc *crtc,
* userspace which stops using the HW cursor altogether in
response to the resulting EINVAL.
*/
if (crtc_state->enable &&
- !(crtc_state->plane_mask & drm_plane_mask(crtc->primary))) {
+ !(crtc_state->plane_mask & drm_plane_mask(crtc->primary)) &&
+ does_crtc_have_active_cursor(crtc_state)) {
Note: This would fix kms_universal_plane but not kms_cursor_legacy.
If IGT tests disable the primary plane while leaving the CRTC enabled, those
tests are broken and need to be fixed instead.
There are IGT cursor tests fixed by this:
igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions
igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size
It also reduces amdgpu workaround in IGT's kms_concurrent:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F499382%2F%3Fseries%3D107734%26rev%3D1&data=05%7C01%7Calex.hung%40amd.com%7Cc757c9e4fbda4f8474a308da9671f920%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637987713521806985%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XRRvilVZMBALIWHAOLArxjiAcgqQ%2FwNRnuUUJCTOYzY%3D&reserved=0
The change affect multiple IGT tests. Adding amdgpu-specific workarounds to
each of the IGT tests is not an ideal solution.
It's not amdgpu specific, other atomic KMS drivers have the same restriction. IGT tests
need to be able to handle this. See e.g. 88e379cef970 ("kms_cursor_legacy: Keep
primary plane enabled for XRGB overlay fallback").
Commit 88e379cef970 adds the following change to keep primary plane enabled:
+ igt_plane_set_fb(primary, prim_fb)
but kms_universal_plane fails at testing disabling primary plane, ex.
tests/kms_universal_plane.c:
192 /* Step 5: Universal API's, disable primary plane (CRC 5) */
193 igt_plane_set_fb(primary, NULL);
194 igt_display_commit2(display, COMMIT_UNIVERSAL);
195 igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_5);
...
230 /* Step 11: Disable primary plane */
231 igt_plane_set_fb(primary, NULL);
232 igt_display_commit2(display, COMMIT_UNIV
Re: [PATCH] RFC: ACPI / OSI: remove workarounds for hybrid graphics laptops
On 2020-07-19 1:50 p.m., Karol Herbst wrote:
> On Fri, Jul 17, 2020 at 9:52 PM Alex Hung wrote:
>>
>> On 2020-07-17 1:05 p.m., Karol Herbst wrote:
>>> It's hard to figure out what systems are actually affected and right now I
>>> don't see a good way of removing those...
>>>
>>> But I'd like to see thos getting removed and drivers fixed instead (which
>>> happened at least for nouveau).
>>>
>>> And as mentioned before, I prefer people working on fixing issues instead
>>> of spending time to add firmware level workarounds which are hard to know
>>> to which systems they apply to, hard to remove and basically a big huge
>>> pain to work with.> In the end I have no idea how to even figure out what
>>> systems are affected
>>> and which not by this, so I have no idea how to even verify we can safely
>>> remove this (which just means those are impossible to remove unless we risk
>>> breaking systems, which again makes those supper annoying to deal with).
>>>
>>> Also from the comments it's hard to get what those bits really do. Are they
>>> just preventing runtime pm or do the devices are powered down when booting?
>>> I am sure it's the former, still...
>>>
>>> Please, don't do this again.
>>>
>>> For now, those workaround prevent power savings on systems those workaround
>>> applies to, which might be any so those should get removed asap and if
>>> new issues arrise removing those please do a proper bug report and we can
>>> look into it and come up with a proper fix (and keep this patch out until
>>> we resolve all of those).
>>>
>>> Signed-off-by: Karol Herbst
>>> CC: Alex Hung
>>> CC: "Rafael J. Wysocki"
>>> CC: Len Brown
>>> CC: Lyude Paul
>>> CC: linux-ker...@vger.kernel.org
>>> CC: dri-devel@lists.freedesktop.org
>>> CC: nouv...@lists.freedesktop.org
>>> ---
>>> drivers/acpi/osi.c | 24
>>> 1 file changed, 24 deletions(-)
>>>
>>> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
>>> index 9f68538091384..d4405e1ca9b97 100644
>>> --- a/drivers/acpi/osi.c
>>> +++ b/drivers/acpi/osi.c
>>> @@ -44,30 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
>>> {"Processor Device", true},
>>> {"3.0 _SCP Extensions", true},
>>> {"Processor Aggregator Device", true},
>>> - /*
>>> - * Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia
>>> graphics
>>> - * cards as RTD3 is not supported by drivers now. Systems with NVidia
>>> - * cards will hang without RTD3 disabled.
>>> - *
>>> - * Once NVidia drivers officially support RTD3, this _OSI strings can
>>> - * be removed if both new and old graphics cards are supported.
>>> - */
>>> - {"Linux-Dell-Video", true},
>>> - /*
>>> - * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's
>>> HDMI
>>> - * audio device which is turned off for power-saving in Windows OS.
>>> - * This power management feature observed on some Lenovo Thinkpad
>>> - * systems which will not be able to output audio via HDMI without
>>> - * a BIOS workaround.
>>> - */
>>> - {"Linux-Lenovo-NV-HDMI-Audio", true},
>>> - /*
>>> - * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
>>> - * output video directly to external monitors on HP Inc. mobile
>>> - * workstations as Nvidia and AMD VGA drivers provide limited
>>> - * hybrid graphics supports.
>>> - */
>>> - {"Linux-HPI-Hybrid-Graphics", true},
>>> };
>>>
>>> static u32 acpi_osi_handler(acpi_string interface, u32 supported)
>>>
>>
>> The changes were discussed and tested a while ago, and no crashes were
>> observed. Thanks for solving PM issues in nouveau.
>>
>> Acked-by: Alex Hung
>>
>
> By any chance, do you have a list of systems implementing those workarounds?
>
I don't keep a list but the workaround, in theory, should only apply to
the systems with the specific nvidia hardware.
I reminded OEMs and ODMs that these _OSI strings were temporary
solutions, and highlighted we were going to remove them after our
discussion last year. If they were paying attentions recent systems
shouldn't have these _OSI strings.
--
Cheers,
Alex Hung
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] RFC: ACPI / OSI: remove workarounds for hybrid graphics laptops
On 2020-07-17 1:05 p.m., Karol Herbst wrote:
> It's hard to figure out what systems are actually affected and right now I
> don't see a good way of removing those...
>
> But I'd like to see thos getting removed and drivers fixed instead (which
> happened at least for nouveau).
>
> And as mentioned before, I prefer people working on fixing issues instead
> of spending time to add firmware level workarounds which are hard to know
> to which systems they apply to, hard to remove and basically a big huge
> pain to work with.> In the end I have no idea how to even figure out what
> systems are affected
> and which not by this, so I have no idea how to even verify we can safely
> remove this (which just means those are impossible to remove unless we risk
> breaking systems, which again makes those supper annoying to deal with).
>
> Also from the comments it's hard to get what those bits really do. Are they
> just preventing runtime pm or do the devices are powered down when booting?
> I am sure it's the former, still...
>
> Please, don't do this again.
>
> For now, those workaround prevent power savings on systems those workaround
> applies to, which might be any so those should get removed asap and if
> new issues arrise removing those please do a proper bug report and we can
> look into it and come up with a proper fix (and keep this patch out until
> we resolve all of those).
>
> Signed-off-by: Karol Herbst
> CC: Alex Hung
> CC: "Rafael J. Wysocki"
> CC: Len Brown
> CC: Lyude Paul
> CC: linux-ker...@vger.kernel.org
> CC: dri-devel@lists.freedesktop.org
> CC: nouv...@lists.freedesktop.org
> ---
> drivers/acpi/osi.c | 24
> 1 file changed, 24 deletions(-)
>
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index 9f68538091384..d4405e1ca9b97 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -44,30 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
> {"Processor Device", true},
> {"3.0 _SCP Extensions", true},
> {"Processor Aggregator Device", true},
> - /*
> - * Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia graphics
> - * cards as RTD3 is not supported by drivers now. Systems with NVidia
> - * cards will hang without RTD3 disabled.
> - *
> - * Once NVidia drivers officially support RTD3, this _OSI strings can
> - * be removed if both new and old graphics cards are supported.
> - */
> - {"Linux-Dell-Video", true},
> - /*
> - * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI
> - * audio device which is turned off for power-saving in Windows OS.
> - * This power management feature observed on some Lenovo Thinkpad
> - * systems which will not be able to output audio via HDMI without
> - * a BIOS workaround.
> - */
> - {"Linux-Lenovo-NV-HDMI-Audio", true},
> - /*
> - * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
> - * output video directly to external monitors on HP Inc. mobile
> - * workstations as Nvidia and AMD VGA drivers provide limited
> - * hybrid graphics supports.
> - */
> - {"Linux-HPI-Hybrid-Graphics", true},
> };
>
> static u32 acpi_osi_handler(acpi_string interface, u32 supported)
>
The changes were discussed and tested a while ago, and no crashes were
observed. Thanks for solving PM issues in nouveau.
Acked-by: Alex Hung
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
We have done some tests on three of Intel + nVidia configuration systems with OEM _OSI strings removed - while some bugs are still observed, ex. one out of three has suspend/resume issues, no system crashes were observed - the biggest issue that worries us. The positive results give us confident to ack the removal of the OEM _OSI strings. While our tests were not able to cover all possible I+N systems, we are sure we can fix issues along the way. If there aren't systems that cannot be fixed without these OEM _OSI strings, these strings should probably enable with DMI quirks (possible future patches) so they won't affect others. Acked-by: Alex Hung On Thu, Sep 5, 2019 at 10:26 AM Rafael J. Wysocki wrote: > > On Thursday, September 5, 2019 5:51:23 PM CEST Karol Herbst wrote: > > is there any update on the testing with my patches? On the hardware I > > had access to those patches helped, but I can't know if it also helped > > on the hardware for which those workarounds where actually added. > > Alex Hung and Mario need to answer this question I think. > > > On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki > > wrote: > > > > > > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote: > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst wrote: > > > > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > > > > > > > The original commit message didn't even make sense. AMD _does_ > > > > > support it and > > > > > it works with Nouveau as well. > > > > > > > > > > Also what was the issue being solved here? No references to any bugs > > > > > and not > > > > > even explaining any issue at all isn't the way we do things. > > > > > > > > > > And even if it means a muxed design, then the fix is to make it work > > > > > inside the > > > > > driver, not adding some hacky workaround through ACPI tricks. > > > > > > > > > > And what out of tree drivers do or do not support we don't care one > > > > > bit anyway. > > > > > > > > > > > > > I think the reverts should be merged via Rafael's tree as the original > > > > patches went in via there, and we should get them in asap. > > > > > > > > Acked-by: Dave Airlie > > > > > > The _OSI strings are to be dropped when all of the needed support is > > > there in > > > drivers, so they should go away along with the requisite driver changes. > > > > > > > that goes beside the point. firmware level workarounds for GPU driver > > issues were pushed without consulting with upstream GPU developers. > > That's something which shouldn't have happened in the first place. And > > yes, I am personally annoyed by the fact, that people know about > > issues, but instead of contacting the proper persons and working on a > > proper fix, we end up with stupid firmware level workarounds. I can't > > see why we ever would have wanted such workarounds in the first place. > > > > And I would be much happier if the next time something like that comes > > up, that the drm mailing list will be contacted as well or somebody > > involved. > > > > We could have also just disable the feature inside the driver (and > > probably we should have done that a long time ago, so that is > > essentially our fault, but still) > > > > > I'm all for dropping then when that's the case, so please feel free to > > > add ACKs > > > from me to the patches in question at that point. > > > > > > Cheers, > > > Rafael > > > > > > > > > > > > > > > -- Cheers, Alex Hung
Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
On Thu, Sep 5, 2019 at 5:26 PM Rafael J. Wysocki wrote: > > On Thursday, September 5, 2019 5:51:23 PM CEST Karol Herbst wrote: > > is there any update on the testing with my patches? On the hardware I > > had access to those patches helped, but I can't know if it also helped > > on the hardware for which those workarounds where actually added. > > Alex Hung and Mario need to answer this question I think. Sorry for taking a long time. I don't have full testing results yet but we found at least a regression occurred with _OSI string removed - it is not on nVidia hardware but on AMD PX one. I will try to collect and share more details. > > > On Mon, Aug 19, 2019 at 11:52 AM Rafael J. Wysocki > > wrote: > > > > > > On Thursday, August 15, 2019 12:47:35 AM CEST Dave Airlie wrote: > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst wrote: > > > > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > > > > > > > The original commit message didn't even make sense. AMD _does_ > > > > > support it and > > > > > it works with Nouveau as well. > > > > > > > > > > Also what was the issue being solved here? No references to any bugs > > > > > and not > > > > > even explaining any issue at all isn't the way we do things. > > > > > > > > > > And even if it means a muxed design, then the fix is to make it work > > > > > inside the > > > > > driver, not adding some hacky workaround through ACPI tricks. > > > > > > > > > > And what out of tree drivers do or do not support we don't care one > > > > > bit anyway. > > > > > > > > > > > > > I think the reverts should be merged via Rafael's tree as the original > > > > patches went in via there, and we should get them in asap. > > > > > > > > Acked-by: Dave Airlie > > > > > > The _OSI strings are to be dropped when all of the needed support is > > > there in > > > drivers, so they should go away along with the requisite driver changes. > > > > > > > that goes beside the point. firmware level workarounds for GPU driver > > issues were pushed without consulting with upstream GPU developers. > > That's something which shouldn't have happened in the first place. And > > yes, I am personally annoyed by the fact, that people know about > > issues, but instead of contacting the proper persons and working on a > > proper fix, we end up with stupid firmware level workarounds. I can't > > see why we ever would have wanted such workarounds in the first place. > > > > And I would be much happier if the next time something like that comes > > up, that the drm mailing list will be contacted as well or somebody > > involved. > > > > We could have also just disable the feature inside the driver (and > > probably we should have done that a long time ago, so that is > > essentially our fault, but still) > > > > > I'm all for dropping then when that's the case, so please feel free to > > > add ACKs > > > from me to the patches in question at that point. > > > > > > Cheers, > > > Rafael > > > > > > > > > > > > > > > -- Cheers, Alex Hung
Re: [PATCH 3/7] Revert "ACPI / OSI: Add OEM _OSI strings to disable NVidia RTD3"
On Wed, Aug 14, 2019 at 3:31 PM Karol Herbst wrote:
>
> This reverts commit 9251a71db62ca9cc7e7cf364218610b0f018c291.
>
> This was never discussed with anybody Nouveau related and we would have NACKed
> this change immediately.
>
> We have a better workaround, which makes it actually work with Nouveau. No
> idea
> why the comment mentions the Nvidia driver and assumes it gives any weight to
> the reasoning we don't care about out of tree drivers.
>
> Nouveau does support RTD3, but we had some issues with that. And we even have
> a better fix for this issue. Also, can we _please_ do it in a way worthy of an
> upstream community the next time?
>
> If some distribution feels like they have to please companies not wanting to
> be part of the linux community, please do so downstream and don't try to push
> something like this upstream.
Hi Karol,
A lot of appreciation for your hard-work on this issue, but unfriendly
comments aren't necessary. At the time this was discussed with
hardware vendors and platform vendors and it worked for many systems
and benefit for many people buying these platforms. Last but not
least, I do appreciate better fixes and want to retire the hacks too.
I am going to notify hardware owners to test these patches on the
original intended systems, and will report whether there are
regressions.
>
> Signed-off-by: Karol Herbst
> CC: Alex Hung
> CC: Rafael J. Wysocki
> CC: Dave Airlie
> CC: Lyude Paul
> CC: Ben Skeggs
> ---
> drivers/acpi/osi.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index 56cc95b6b724..f5d559a2ff14 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -44,15 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
> {"Processor Device", true},
> {"3.0 _SCP Extensions", true},
> {"Processor Aggregator Device", true},
> - /*
> -* Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia
> graphics
> -* cards as RTD3 is not supported by drivers now. Systems with NVidia
> -* cards will hang without RTD3 disabled.
> -*
> -* Once NVidia drivers officially support RTD3, this _OSI strings can
> -* be removed if both new and old graphics cards are supported.
> -*/
> - {"Linux-Dell-Video", true},
> };
>
> static u32 acpi_osi_handler(acpi_string interface, u32 supported)
> --
> 2.21.0
>
--
Cheers,
Alex Hung
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
Thanks for the series of fixes. I will check whether these fixes work
on the original intended systems.
On Wed, Aug 14, 2019 at 3:31 PM Karol Herbst wrote:
>
> This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c.
>
> The original commit message didn't even make sense. AMD _does_ support it and
> it works with Nouveau as well.
>
> Also what was the issue being solved here? No references to any bugs and not
> even explaining any issue at all isn't the way we do things.
>
> And even if it means a muxed design, then the fix is to make it work inside
> the
> driver, not adding some hacky workaround through ACPI tricks.
>
> And what out of tree drivers do or do not support we don't care one bit
> anyway.
>
> Signed-off-by: Karol Herbst
> CC: Alex Hung
> CC: Rafael J. Wysocki
> CC: Dave Airlie
> CC: Lyude Paul
> CC: Ben Skeggs
> ---
> drivers/acpi/osi.c | 7 ---
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index bec0bebc7f52..9b20ac4d79a0 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -61,13 +61,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
> * a BIOS workaround.
> */
> {"Linux-Lenovo-NV-HDMI-Audio", true},
> - /*
> -* Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
> -* output video directly to external monitors on HP Inc. mobile
> -* workstations as Nvidia and AMD VGA drivers provide limited
> -* hybrid graphics supports.
> -*/
> - {"Linux-HPI-Hybrid-Graphics", true},
> };
>
> static u32 acpi_osi_handler(acpi_string interface, u32 supported)
> --
> 2.21.0
>
--
Cheers,
Alex Hung
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
