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 <m...@igalia.com>
---
.../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->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
@@ -7345,10 +7345,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;
}
@@ -7444,12 +7444,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 (!edid)
+ if (!drm_edid)
return;
if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
@@ -7462,23 +7462,23 @@ 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->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_valid(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);
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);
@@ -11038,7 +11038,7 @@ static bool parse_edid_cea(struct amdgpu_dm_connector *aconnector,
}
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;
@@ -11073,7 +11073,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;
@@ -11115,7 +11116,7 @@ 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;
@@ -11125,9 +11126,9 @@ void amdgpu_dm_update_freesync_caps(struct
drm_connector *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 = drm_edid_raw(drm_edid);
I got below compile errors:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function
‘amdgpu_dm_update_freesync_caps’:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:11176:41:
error: assignment discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
11176 | timing =
&edid->detailed_timings[i];
| ^
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:11221:33:
error: assignment discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
11221 | timing = &edid->detailed_timings[i];
and the following changes fixes it:
- 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;
bool freesync_capable = false;
enum adaptive_sync_type as_type = ADAPTIVE_SYNC_TYPE_NONE;
@@ -11140,7 +11141,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;
@@ -11162,7 +11163,7 @@ void amdgpu_dm_update_freesync_caps(struct
drm_connector *connector,
|| sink->sink_signal == SIGNAL_TYPE_EDP) {
bool edid_check_required = false;
- if (edid) {
+ if (drm_edid) {
edid_check_required = is_dp_capable_without_timing_msa(
adev->dm.dc,
amdgpu_dm_connector);
@@ -11214,7 +11215,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 9c1871b866cc..b81cf6f3713b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -637,7 +637,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 *edid;
/* shared with amdgpu */
struct amdgpu_hpd hpd;
@@ -908,7 +908,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 e3915c4f8566..91006326ce6d 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
@@ -898,7 +898,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,8 +910,8 @@ enum dc_edid_status dm_helpers_read_local_edid(
* do check sum and retry to make sure read correct edid.
*/
do {
-
- edid = drm_get_edid(&aconnector->base, ddc);
+ drm_edid = drm_edid_read_ddc(connector, ddc);
+ edid = drm_edid_raw(drm_edid);
/* DP Compliance Test 4.2.2.6 */
if (link->aux_mode && connector->edid_corrupt)
@@ -928,7 +929,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
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 7075a85dd16e..cdebd0e74b26 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
@@ -127,7 +127,7 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
dc_sink_release(aconnector->dc_sink);
}
- kfree(aconnector->edid);
+ drm_edid_free(aconnector->edid);
drm_connector_cleanup(connector);
drm_dp_mst_put_port_malloc(aconnector->mst_output_port);
@@ -297,15 +297,15 @@ static int dm_dp_mst_get_modes(struct drm_connector
*connector)
return drm_add_edid_modes(connector, NULL);
if (!aconnector->edid) {
- struct edid *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);
@@ -339,7 +339,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
return ret;
}
- aconnector->edid = edid;
+ aconnector->edid = drm_edid;
amdgpu_dm_set_mst_status(&aconnector->mst_status,
MST_REMOTE_EDID, true);
}
@@ -354,10 +354,12 @@ 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 = drm_edid_raw(aconnector->edid);
+
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) {
@@ -411,10 +413,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->edid);
- ret = drm_add_edid_modes(connector, aconnector->edid);
+ ret = drm_edid_connector_add_modes(connector);
return ret;
}
