* [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code
@ 2024-09-27 23:05 Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 1/5] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid Mario Limonciello
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:05 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Mario Limonciello <mario.limonciello@amd.com>
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-mwen@igalia.com/ [1]
Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limonciello@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(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v8 1/5] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
@ 2024-09-27 23:05 ` Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 2/5] drm/amd/display: switch to setting physical address directly Mario Limonciello
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:05 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Melissa Wen <mwen@igalia.com>
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.
Signed-off-by: Melissa Wen <mwen@igalia.com>
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v7: fix LKP robot issue
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 123 ++++++++----------
.../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 | 34 ++---
4 files changed, 84 insertions(+), 90 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 6f4b753f5f51..b7d93787667c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3487,7 +3487,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) {
@@ -3546,18 +3546,19 @@ 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);
if (aconnector->dc_link->aux_mode)
- drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
- aconnector->edid);
+ drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux, edid);
}
if (!aconnector->timing_requested) {
@@ -3568,17 +3569,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 */
@@ -7105,32 +7107,24 @@ 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 i2c_adapter *ddc;
-
- if (dc_link && dc_link->aux_mode)
- ddc = &aconnector->dm_dp_aux.aux.ddc;
- else
- ddc = &aconnector->i2c->base;
+ const struct drm_edid *drm_edid;
- /*
- * 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, ddc);
- if (!edid) {
+ drm_edid = drm_edid_read(connector);
+ drm_edid_connector_update(connector, drm_edid);
+ if (!drm_edid) {
DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
}
- aconnector->edid = edid;
-
+ aconnector->drm_edid = drm_edid;
/* Update emulated (virtual) sink's EDID */
if (dc_em_sink && dc_link) {
+ // FIXME: Get rid of drm_edid_raw()
+ 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, edid, (edid->extensions + 1) * EDID_LENGTH);
+ 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,
@@ -7160,36 +7154,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 dc_link *dc_link = aconnector->dc_link;
struct dc_sink_init_data init_params = {
.link = aconnector->dc_link,
.sink_signal = SIGNAL_TYPE_VIRTUAL
};
- struct edid *edid;
- struct i2c_adapter *ddc;
-
- if (dc_link->aux_mode)
- ddc = &aconnector->dm_dp_aux.aux.ddc;
- else
- ddc = &aconnector->i2c->base;
+ const struct drm_edid *drm_edid;
+ const struct edid *edid;
- /*
- * 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, ddc);
- if (!edid) {
+ drm_edid = drm_edid_read(connector);
+ drm_edid_connector_update(connector, drm_edid);
+ if (!drm_edid) {
DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
}
- 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->drm_edid = drm_edid;
+ edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
aconnector->dc_em_sink = dc_link_add_remote_sink(
aconnector->dc_link,
(uint8_t *)edid,
@@ -7876,16 +7860,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->probed_modes);
amdgpu_dm_connector->num_modes =
- drm_add_edid_modes(connector, edid);
+ drm_edid_connector_add_modes(connector);
/* sorting the probed modes before calling function
* amdgpu_dm_get_native_mode() since EDID can have
@@ -7899,10 +7883,10 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
amdgpu_dm_get_native_mode(connector);
/* Freesync capabilities are reset by calling
- * drm_add_edid_modes() and need to be
+ * drm_edid_connector_add_modes() and need to be
* restored here.
*/
- amdgpu_dm_update_freesync_caps(connector, edid);
+ amdgpu_dm_update_freesync_caps(connector, drm_edid);
} else {
amdgpu_dm_connector->num_modes = 0;
}
@@ -7998,12 +7982,12 @@ static uint add_fs_modes(struct amdgpu_dm_connector *aconnector)
}
static void amdgpu_dm_connector_add_freesync_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 (!(amdgpu_freesync_vid_mode && edid))
+ if (!(amdgpu_freesync_vid_mode && drm_edid))
return;
if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
@@ -8016,24 +8000,24 @@ static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
struct drm_encoder *encoder;
- struct edid *edid = amdgpu_dm_connector->edid;
+ const struct drm_edid *drm_edid = amdgpu_dm_connector->drm_edid;
struct dc_link_settings *verified_link_cap =
&amdgpu_dm_connector->dc_link->verified_link_cap;
const struct dc *dc = amdgpu_dm_connector->dc_link->dc;
encoder = amdgpu_dm_connector_to_encoder(connector);
- if (!drm_edid_is_valid(edid)) {
+ if (!drm_edid) {
amdgpu_dm_connector->num_modes =
drm_add_modes_noedid(connector, 640, 480);
if (dc->link_srv->dp_get_encoding_format(verified_link_cap) == DP_128b_132b_ENCODING)
amdgpu_dm_connector->num_modes +=
drm_add_modes_noedid(connector, 1920, 1080);
} else {
- amdgpu_dm_connector_ddc_get_modes(connector, edid);
+ amdgpu_dm_connector_ddc_get_modes(connector, drm_edid);
if (encoder)
amdgpu_dm_connector_add_common_modes(encoder, connector);
- amdgpu_dm_connector_add_freesync_modes(connector, edid);
+ amdgpu_dm_connector_add_freesync_modes(connector, drm_edid);
}
amdgpu_dm_fbc_init(connector);
@@ -11958,7 +11942,7 @@ static bool parse_edid_cea(struct amdgpu_dm_connector *aconnector,
}
static void parse_edid_displayid_vrr(struct drm_connector *connector,
- struct edid *edid)
+ const struct edid *edid)
{
u8 *edid_ext = NULL;
int i;
@@ -12001,7 +11985,7 @@ static void parse_edid_displayid_vrr(struct drm_connector *connector,
}
static int parse_amd_vsdb(struct amdgpu_dm_connector *aconnector,
- struct edid *edid, struct amdgpu_hdmi_vsdb_info *vsdb_info)
+ const struct edid *edid, struct amdgpu_hdmi_vsdb_info *vsdb_info)
{
u8 *edid_ext = NULL;
int i;
@@ -12036,7 +12020,8 @@ static int parse_amd_vsdb(struct amdgpu_dm_connector *aconnector,
}
static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
- struct edid *edid, struct amdgpu_hdmi_vsdb_info *vsdb_info)
+ const struct edid *edid,
+ struct amdgpu_hdmi_vsdb_info *vsdb_info)
{
u8 *edid_ext = NULL;
int i;
@@ -12070,7 +12055,7 @@ static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
* amdgpu_dm_update_freesync_caps - Update Freesync capabilities
*
* @connector: Connector to query.
- * @edid: EDID from monitor
+ * @drm_edid: DRM EDID from monitor
*
* Amdgpu supports Freesync in DP and HDMI displays, and it is required to keep
* track of some of the display information in the internal data struct used by
@@ -12078,19 +12063,19 @@ static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
* FreeSync parameters.
*/
void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
- struct edid *edid)
+ const struct drm_edid *drm_edid)
{
int i = 0;
- struct detailed_timing *timing;
- struct detailed_non_pixel *data;
- struct detailed_data_monitor_range *range;
+ const struct detailed_timing *timing;
+ const struct detailed_non_pixel *data;
+ const struct detailed_data_monitor_range *range;
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
struct dm_connector_state *dm_con_state = NULL;
struct dc_sink *sink;
-
struct amdgpu_device *adev = drm_to_adev(connector->dev);
struct amdgpu_hdmi_vsdb_info vsdb_info = {0};
+ const struct edid *edid;
bool freesync_capable = false;
enum adaptive_sync_type as_type = ADAPTIVE_SYNC_TYPE_NONE;
@@ -12103,7 +12088,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
amdgpu_dm_connector->dc_sink :
amdgpu_dm_connector->dc_em_sink;
- if (!edid || !sink) {
+ if (!drm_edid || !sink) {
dm_con_state = to_dm_connector_state(connector->state);
amdgpu_dm_connector->min_vfreq = 0;
@@ -12120,6 +12105,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
if (!adev->dm.freesync_module)
goto update;
+ edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
+
/* Some eDP panels only have the refresh rate range info in DisplayID */
if ((connector->display_info.monitor_range.min_vfreq == 0 ||
connector->display_info.monitor_range.max_vfreq == 0))
@@ -12196,7 +12183,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
}
- } else if (edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
+ } else if (drm_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];
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 15d4690c74d6..5e49b97b3d46 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -673,7 +673,7 @@ struct amdgpu_dm_connector {
/* we need to mind the EDID between detect
and get modes due to analog/digital/tvencoder */
- struct edid *edid;
+ const struct drm_edid *drm_edid;
/* shared with amdgpu */
struct amdgpu_hpd hpd;
@@ -951,7 +951,7 @@ void dm_restore_drm_connector_state(struct drm_device *dev,
struct drm_connector *connector);
void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
- struct edid *edid);
+ const struct drm_edid *drm_edid);
void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
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 50109d13d967..b8004ccdcc33 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
@@ -897,7 +897,8 @@ enum dc_edid_status dm_helpers_read_local_edid(
struct i2c_adapter *ddc;
int retry = 3;
enum dc_edid_status edid_status;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
+ const struct edid *edid;
if (link->aux_mode)
ddc = &aconnector->dm_dp_aux.aux.ddc;
@@ -909,25 +910,27 @@ enum dc_edid_status dm_helpers_read_local_edid(
*/
do {
- edid = drm_get_edid(&aconnector->base, ddc);
+ drm_edid = drm_edid_read_ddc(connector, ddc);
+ drm_edid_connector_update(connector, drm_edid);
/* DP Compliance Test 4.2.2.6 */
if (link->aux_mode && connector->edid_corrupt)
drm_dp_send_real_edid_checksum(&aconnector->dm_dp_aux.aux, connector->real_edid_checksum);
- if (!edid && connector->edid_corrupt) {
+ if (!drm_edid && connector->edid_corrupt) {
connector->edid_corrupt = false;
return EDID_BAD_CHECKSUM;
}
- if (!edid)
+ if (!drm_edid)
return EDID_NO_RESPONSE;
+ edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
sink->dc_edid.length = EDID_LENGTH * (edid->extensions + 1);
memmove(sink->dc_edid.raw_edid, (uint8_t *)edid, sink->dc_edid.length);
/* We don't need the original edid anymore */
- kfree(edid);
+ drm_edid_free(drm_edid);
edid_status = dm_helpers_parse_edid_caps(
link,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 83a31b97e96b..5e3d3eda3c9f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -129,7 +129,7 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
dc_sink_release(aconnector->dc_sink);
}
- kfree(aconnector->edid);
+ drm_edid_free(aconnector->drm_edid);
drm_connector_cleanup(connector);
drm_dp_mst_put_port_malloc(aconnector->mst_output_port);
@@ -182,7 +182,7 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
dc_sink_release(dc_sink);
aconnector->dc_sink = NULL;
- aconnector->edid = NULL;
+ aconnector->drm_edid = NULL;
aconnector->dsc_aux = NULL;
port->passthrough_aux = NULL;
}
@@ -302,16 +302,18 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
if (!aconnector)
return drm_add_edid_modes(connector, NULL);
- if (!aconnector->edid) {
- struct edid *edid;
+ if (!aconnector->drm_edid) {
+ const struct drm_edid *drm_edid;
- edid = drm_dp_mst_get_edid(connector, &aconnector->mst_root->mst_mgr, aconnector->mst_output_port);
+ drm_edid = drm_dp_mst_edid_read(connector,
+ &aconnector->mst_root->mst_mgr,
+ aconnector->mst_output_port);
- if (!edid) {
+ if (!drm_edid) {
amdgpu_dm_set_mst_status(&aconnector->mst_status,
MST_REMOTE_EDID, false);
- drm_connector_update_edid_property(
+ drm_edid_connector_update(
&aconnector->base,
NULL);
@@ -345,7 +347,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
return ret;
}
- aconnector->edid = edid;
+ aconnector->drm_edid = drm_edid;
amdgpu_dm_set_mst_status(&aconnector->mst_status,
MST_REMOTE_EDID, true);
}
@@ -360,10 +362,13 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
struct dc_sink_init_data init_params = {
.link = aconnector->dc_link,
.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
+ const struct edid *edid;
+
+ edid = drm_edid_raw(aconnector->drm_edid); // FIXME: Get rid of drm_edid_raw()
dc_sink = dc_link_add_remote_sink(
aconnector->dc_link,
- (uint8_t *)aconnector->edid,
- (aconnector->edid->extensions + 1) * EDID_LENGTH,
+ (uint8_t *)edid,
+ (edid->extensions + 1) * EDID_LENGTH,
&init_params);
if (!dc_sink) {
@@ -405,7 +410,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(
- connector, aconnector->edid);
+ connector, aconnector->drm_edid);
#if defined(CONFIG_DRM_AMD_DC_FP)
if (!validate_dsc_caps_on_connector(aconnector))
@@ -419,10 +424,9 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
}
}
- drm_connector_update_edid_property(
- &aconnector->base, aconnector->edid);
+ drm_edid_connector_update(&aconnector->base, aconnector->drm_edid);
- ret = drm_add_edid_modes(connector, aconnector->edid);
+ ret = drm_edid_connector_add_modes(connector);
return ret;
}
@@ -500,7 +504,7 @@ dm_dp_mst_detect(struct drm_connector *connector,
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
- aconnector->edid = NULL;
+ aconnector->drm_edid = NULL;
aconnector->dsc_aux = NULL;
port->passthrough_aux = NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 2/5] drm/amd/display: switch to setting physical address directly
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 1/5] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid Mario Limonciello
@ 2024-09-27 23:05 ` Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 3/5] drm/amd/display: always call connector_update when parsing freesync_caps Mario Limonciello
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:05 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Melissa Wen <mwen@igalia.com>
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 <mwen@igalia.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 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 b7d93787667c..0b6c936be7a6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3548,8 +3548,7 @@ void amdgpu_dm_update_connector_after_detect(
if (sink->dc_edid.length == 0) {
aconnector->drm_edid = NULL;
if (aconnector->dc_link->aux_mode) {
- drm_dp_cec_unset_edid(
- &aconnector->dm_dp_aux.aux);
+ drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
}
} else {
const struct edid *edid = (const struct edid *)sink->dc_edid.raw_edid;
@@ -3558,7 +3557,8 @@ void amdgpu_dm_update_connector_after_detect(
drm_edid_connector_update(connector, aconnector->drm_edid);
if (aconnector->dc_link->aux_mode)
- drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux, edid);
+ drm_dp_cec_attach(&aconnector->dm_dp_aux.aux,
+ connector->display_info.source_physical_address);
}
if (!aconnector->timing_requested) {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 3/5] drm/amd/display: always call connector_update when parsing freesync_caps
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 1/5] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 2/5] drm/amd/display: switch to setting physical address directly Mario Limonciello
@ 2024-09-27 23:05 ` Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 4/5] drm/amd/display: remove redundant freesync parser for DP Mario Limonciello
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:05 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Melissa Wen <mwen@igalia.com>
Update connector caps with drm_edid data before parsing info for
freesync.
Signed-off-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 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 0b6c936be7a6..efc1609ff26f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3569,13 +3569,11 @@ void amdgpu_dm_update_connector_after_detect(
"failed to create aconnector->requested_timing\n");
}
- 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_edid_connector_update(connector, NULL);
aconnector->num_modes = 0;
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
@@ -12088,6 +12086,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
amdgpu_dm_connector->dc_sink :
amdgpu_dm_connector->dc_em_sink;
+ drm_edid_connector_update(connector, drm_edid);
+
if (!drm_edid || !sink) {
dm_con_state = to_dm_connector_state(connector->state);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 4/5] drm/amd/display: remove redundant freesync parser for DP
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
` (2 preceding siblings ...)
2024-09-27 23:05 ` [PATCH v8 3/5] drm/amd/display: always call connector_update when parsing freesync_caps Mario Limonciello
@ 2024-09-27 23:05 ` Mario Limonciello
2024-09-27 23:06 ` [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP Mario Limonciello
2024-10-04 16:16 ` [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Alex Hung
5 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:05 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Melissa Wen <mwen@igalia.com>
When updating connector under drm_edid infrastructure, many calculations
and validations are already done and become redundant inside AMD driver.
Remove those driver-specific code in favor of the DRM common code.
Signed-off-by: Melissa Wen <mwen@igalia.com>
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +------------------
1 file changed, 4 insertions(+), 70 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 efc1609ff26f..bd8fb9968c7c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -12064,9 +12064,6 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
int i = 0;
- const struct detailed_timing *timing;
- const struct detailed_non_pixel *data;
- const struct detailed_data_monitor_range *range;
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
struct dm_connector_state *dm_con_state = NULL;
@@ -12093,8 +12090,6 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
amdgpu_dm_connector->min_vfreq = 0;
amdgpu_dm_connector->max_vfreq = 0;
- connector->display_info.monitor_range.min_vfreq = 0;
- connector->display_info.monitor_range.max_vfreq = 0;
freesync_capable = false;
goto update;
@@ -12114,67 +12109,10 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
if (edid && (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT ||
sink->sink_signal == SIGNAL_TYPE_EDP)) {
- bool edid_check_required = false;
-
- if (amdgpu_dm_connector->dc_link &&
- amdgpu_dm_connector->dc_link->dpcd_caps.allow_invalid_MSA_timing_param) {
- if (edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ) {
- amdgpu_dm_connector->min_vfreq = connector->display_info.monitor_range.min_vfreq;
- amdgpu_dm_connector->max_vfreq = connector->display_info.monitor_range.max_vfreq;
- if (amdgpu_dm_connector->max_vfreq -
- amdgpu_dm_connector->min_vfreq > 10)
- freesync_capable = true;
- } else {
- edid_check_required = edid->version > 1 ||
- (edid->version == 1 &&
- edid->revision > 1);
- }
- }
-
- if (edid_check_required) {
- for (i = 0; i < 4; i++) {
-
- timing = &edid->detailed_timings[i];
- data = &timing->data.other_data;
- range = &data->data.range;
- /*
- * Check if monitor has continuous frequency mode
- */
- if (data->type != EDID_DETAIL_MONITOR_RANGE)
- continue;
- /*
- * Check for flag range limits only. If flag == 1 then
- * no additional timing information provided.
- * Default GTF, GTF Secondary curve and CVT are not
- * supported
- */
- if (range->flags != 1)
- continue;
-
- connector->display_info.monitor_range.min_vfreq = range->min_vfreq;
- connector->display_info.monitor_range.max_vfreq = range->max_vfreq;
-
- if (edid->revision >= 4) {
- if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
- connector->display_info.monitor_range.min_vfreq += 255;
- if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
- connector->display_info.monitor_range.max_vfreq += 255;
- }
-
- amdgpu_dm_connector->min_vfreq =
- connector->display_info.monitor_range.min_vfreq;
- amdgpu_dm_connector->max_vfreq =
- connector->display_info.monitor_range.max_vfreq;
-
- break;
- }
-
- if (amdgpu_dm_connector->max_vfreq -
- amdgpu_dm_connector->min_vfreq > 10) {
-
- freesync_capable = true;
- }
- }
+ amdgpu_dm_connector->min_vfreq = connector->display_info.monitor_range.min_vfreq;
+ amdgpu_dm_connector->max_vfreq = connector->display_info.monitor_range.max_vfreq;
+ if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
+ freesync_capable = true;
parse_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
if (vsdb_info.replay_mode) {
@@ -12182,13 +12120,9 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
amdgpu_dm_connector->vsdb_info.amd_vsdb_version = vsdb_info.amd_vsdb_version;
amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
}
-
} else if (drm_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)
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
` (3 preceding siblings ...)
2024-09-27 23:05 ` [PATCH v8 4/5] drm/amd/display: remove redundant freesync parser for DP Mario Limonciello
@ 2024-09-27 23:06 ` Mario Limonciello
2024-10-08 16:05 ` Mark Pearson
2024-10-04 16:16 ` [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Alex Hung
5 siblings, 1 reply; 9+ messages in thread
From: Mario Limonciello @ 2024-09-27 23:06 UTC (permalink / raw)
To: Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson
From: Mario Limonciello <mario.limonciello@amd.com>
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.
Reviewed-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3:
* Change message to INFO when using ACPI EDID
* rebase
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 65 ++++++++++++++++++-
drivers/gpu/drm/amd/include/amd_shared.h | 5 ++
2 files changed, 67 insertions(+), 3 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 b8004ccdcc33..7534e1624e4f 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 <acpi/video.h>
+
#include <linux/string.h>
#include <linux/acpi.h>
#include <linux/i2c.h>
@@ -887,6 +889,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_dbg(connector->dev, "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,
@@ -909,8 +965,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_info(connector->dev, "Using ACPI provided EDID for %s\n", connector->name);
+ else
+ drm_edid = drm_edid_read_ddc(connector, ddc);
drm_edid_connector_update(connector, drm_edid);
/* DP Compliance Test 4.2.2.6 */
@@ -1300,4 +1359,4 @@ bool dm_helpers_is_hdr_on(struct dc_context *ctx, struct dc_stream_state *stream
{
// TODO
return false;
-}
\ No newline at end of file
+}
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;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
` (4 preceding siblings ...)
2024-09-27 23:06 ` [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP Mario Limonciello
@ 2024-10-04 16:16 ` Alex Hung
5 siblings, 0 replies; 9+ messages in thread
From: Alex Hung @ 2024-10-04 16:16 UTC (permalink / raw)
To: Mario Limonciello, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Mario Limonciello, amd-gfx, dri-devel, harry.wentland,
sunpeng.li, Mark Pearson, Siqueira, Rodrigo
No regressed found on this patchset series.
Reviewed-by: Alex Hung <alex.hung@amd.com>
On 9/27/24 17:05, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> 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-mwen@igalia.com/ [1]
> Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limonciello@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(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP
2024-09-27 23:06 ` [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP Mario Limonciello
@ 2024-10-08 16:05 ` Mark Pearson
2024-10-08 17:01 ` Mario Limonciello
0 siblings, 1 reply; 9+ messages in thread
From: Mark Pearson @ 2024-10-08 16:05 UTC (permalink / raw)
To: Mario Limonciello, Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Limonciello, Mario, amd-gfx, dri-devel,
harry.wentland, sunpeng.li
On Fri, Sep 27, 2024, at 7:06 PM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> 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.
>
> Reviewed-by: Alex Hung <alex.hung@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v3:
> * Change message to INFO when using ACPI EDID
> * rebase
> ---
> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 65 ++++++++++++++++++-
> drivers/gpu/drm/amd/include/amd_shared.h | 5 ++
> 2 files changed, 67 insertions(+), 3 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 b8004ccdcc33..7534e1624e4f 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 <acpi/video.h>
> +
> #include <linux/string.h>
> #include <linux/acpi.h>
> #include <linux/i2c.h>
> @@ -887,6 +889,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_dbg(connector->dev, "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,
> @@ -909,8 +965,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_info(connector->dev, "Using ACPI provided EDID for %s\n",
> connector->name);
> + else
> + drm_edid = drm_edid_read_ddc(connector, ddc);
> drm_edid_connector_update(connector, drm_edid);
>
> /* DP Compliance Test 4.2.2.6 */
> @@ -1300,4 +1359,4 @@ bool dm_helpers_is_hdr_on(struct dc_context *ctx,
> struct dc_stream_state *stream
> {
> // TODO
> return false;
> -}
> \ No newline at end of file
> +}
> 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;
> --
> 2.43.0
For the series, we tested at Lenovo and it fixed a couple of different issues we had seen and reported on different HW models.
- issue with setting 1600 x 1200 on Z16 G2
- issue with frequency setting being incorrect on T14 G4 AMD with OLED panels
I didn't do the testing myself (I don't have the configurations on hand that reproduce the problem) but my colleagues did.
Can I do a:
Tested-by: Lenovo Linux team <mpearson-lenovo@squebb.ca>
Or is there some other protocol for this?
Thanks
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP
2024-10-08 16:05 ` Mark Pearson
@ 2024-10-08 17:01 ` Mario Limonciello
0 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2024-10-08 17:01 UTC (permalink / raw)
To: Mark Pearson, Alex Hung, Alexander Deucher, Melissa Wen
Cc: kernel-dev, Limonciello, Mario, amd-gfx, dri-devel,
harry.wentland, sunpeng.li
On 10/8/2024 11:05, Mark Pearson wrote:
>
> For the series, we tested at Lenovo and it fixed a couple of different issues we had seen and reported on different HW models.
> - issue with setting 1600 x 1200 on Z16 G2
> - issue with frequency setting being incorrect on T14 G4 AMD with OLED panels
> I didn't do the testing myself (I don't have the configurations on hand that reproduce the problem) but my colleagues did.
> Can I do a:
> Tested-by: Lenovo Linux team <mpearson-lenovo@squebb.ca>
> Or is there some other protocol for this?
>
> Thanks
> Mark
Thanks Mark! If we have any problems that arise from this patch that
cause us to need to change it or back it out we'll reach out to you for
further testing from your extended team on the models it helped.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-10-09 13:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-27 23:05 [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 1/5] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 2/5] drm/amd/display: switch to setting physical address directly Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 3/5] drm/amd/display: always call connector_update when parsing freesync_caps Mario Limonciello
2024-09-27 23:05 ` [PATCH v8 4/5] drm/amd/display: remove redundant freesync parser for DP Mario Limonciello
2024-09-27 23:06 ` [PATCH v8 5/5] drm/amd/display: Fetch the EDID from _DDC if available for eDP Mario Limonciello
2024-10-08 16:05 ` Mark Pearson
2024-10-08 17:01 ` Mario Limonciello
2024-10-04 16:16 ` [PATCH v8 0/5] drm/amd/display: Use drm_edid for more code Alex Hung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.