[PATCH 00/12] drm/amd: Switch over to struct drm_edid

[PATCH 00/12] drm/amd: Switch over to struct drm_edid

[Thomas Weißschuh]

The AMD DRM drivers use 'struct edid', raw pointers and even custom
structs to represent EDID data.
Uniformly switch to the safe and recommended "struct drm_edid".

Some uses of "struct edid" are left because some ad-hoc parsing is still
being done inside the drivers.

The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
will conflict with my backlight quirk series [0].
The conflict will result in an obvious and easy to fix build failure.

Patches 1 and 2 delete some dead code.
Patches 3 to 6 constify some arguments and shuffle around some code.
The remaining patches perform the actual conversion in steps.

[0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (12):
      drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
      drm/amd/display: Remove EDID members of ddc_service
      drm/edid: constify argument of drm_edid_is_valid()
      drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
      drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
      drm/amd/display: Constify 'struct edid' in parsing functions
      drm/amd/display: Use struct edid in dc_link_add_remote_sink()
      drm/amdgpu: Switch amdgpu_connector to struct drm_edid
      drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
      drm/edid: add a helper to compare two EDIDs
      drm/amd/display: Switch dc_sink to struct drm_edid
      drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid

 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
 .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
 drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
 drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
 drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
 drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
 drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
 .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
 .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
 drivers/gpu/drm/drm_edid.c                         | 20 +++++-
 include/drm/drm_edid.h                             |  3 +-
 20 files changed, 155 insertions(+), 171 deletions(-)
---
base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
change-id: 20240615-amdgpu-drm_edid-32d969dfb899

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 01/12] drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()

[Thomas Weißschuh]

The prototype is the whole content of commit
575d0df6dae4 ("drm/amd/display: refine the EDID override").
Apparently the definition was never added.

Fixes: 575d0df6dae4 ("drm/amd/display: refine the EDID override")
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
index 2e4a46f1b499..483d8c292618 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
@@ -208,8 +208,6 @@ int dm_helpers_dmub_set_config_sync(struct dc_context *ctx,
 		enum set_config_status *operation_result);
 enum adaptive_sync_type dm_get_adaptive_sync_support_type(struct dc_link *link);
 
-enum dc_edid_status dm_helpers_get_sbios_edid(struct dc_link *link, struct dc_edid *edid);
-
 bool dm_helpers_is_fullscreen(struct dc_context *ctx, struct dc_stream_state *stream);
 bool dm_helpers_is_hdr_on(struct dc_context *ctx, struct dc_stream_state *stream);
 

-- 
2.46.0

[PATCH 02/12] drm/amd/display: Remove EDID members of ddc_service

[Thomas Weißschuh]

All usages of these fields have been removed.

Fixes: 7c7f5b15be65 ("drm/amd/display: Refactor edid read.")
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
index 428e3a9ab65a..7dd1cfb9ab76 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
@@ -189,10 +189,6 @@ struct ddc_service {
 	enum display_dongle_type dongle_type;
 	struct dc_context *ctx;
 	struct dc_link *link;
-
-	uint32_t address;
-	uint32_t edid_buf_len;
-	uint8_t edid_buf[DC_MAX_EDID_BUFFER_SIZE];
 };
 
 #endif /* DC_DDC_TYPES_H_ */

-- 
2.46.0

[PATCH 03/12] drm/edid: constify argument of drm_edid_is_valid()

[Thomas Weißschuh]

drm_edid_is_valid() does not modify its argument, so mark it as const.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/drm_edid.c | 2 +-
 include/drm/drm_edid.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f68a41eeb1fa..69fb11741abd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2034,7 +2034,7 @@ EXPORT_SYMBOL(drm_edid_block_valid);
  *
  * Return: True if the EDID data is valid, false otherwise.
  */
-bool drm_edid_is_valid(struct edid *edid)
+bool drm_edid_is_valid(const struct edid *edid)
 {
 	int i;
 
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 6bdfa254a1c1..a5b377c4a342 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -442,7 +442,7 @@ int drm_add_modes_noedid(struct drm_connector *connector,
 int drm_edid_header_is_valid(const void *edid);
 bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
 			  bool *edid_corrupt);
-bool drm_edid_is_valid(struct edid *edid);
+bool drm_edid_is_valid(const struct edid *edid);
 void drm_edid_get_monitor_name(const struct edid *edid, char *name,
 			       int buflen);
 struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,

-- 
2.46.0

Re: [PATCH 03/12] drm/edid: constify argument of drm_edid_is_valid()

[Jani Nikula]

On Sun, 18 Aug 2024, Thomas Weißschuh <linux@weissschuh.net> wrote:
> drm_edid_is_valid() does not modify its argument, so mark it as const.

That's not true.

BR,
Jani.

>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  drivers/gpu/drm/drm_edid.c | 2 +-
>  include/drm/drm_edid.h     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f68a41eeb1fa..69fb11741abd 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2034,7 +2034,7 @@ EXPORT_SYMBOL(drm_edid_block_valid);
>   *
>   * Return: True if the EDID data is valid, false otherwise.
>   */
> -bool drm_edid_is_valid(struct edid *edid)
> +bool drm_edid_is_valid(const struct edid *edid)
>  {
>  	int i;
>  
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 6bdfa254a1c1..a5b377c4a342 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -442,7 +442,7 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>  int drm_edid_header_is_valid(const void *edid);
>  bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>  			  bool *edid_corrupt);
> -bool drm_edid_is_valid(struct edid *edid);
> +bool drm_edid_is_valid(const struct edid *edid);
>  void drm_edid_get_monitor_name(const struct edid *edid, char *name,
>  			       int buflen);
>  struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,

-- 
Jani Nikula, Intel

Re: [PATCH 03/12] drm/edid: constify argument of drm_edid_is_valid()

[Thomas Weißschuh]

On 2024-08-19 11:21:21+0000, Jani Nikula wrote:
> On Sun, 18 Aug 2024, Thomas Weißschuh <linux@weissschuh.net> wrote:
> > drm_edid_is_valid() does not modify its argument, so mark it as const.
> 
> That's not true.

Indeed, thanks for noticing.
It turns out this patch is not necessary anyways and I dropped it for
the next revision.


Thomas

[PATCH 04/12] drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()

[Thomas Weißschuh]

Reduce the number of casts needed by reusing the edid_buf variable.
Also initialize edid_buf after the !edid case has been handled to avoid
the ternary expression.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 8 +++++---
 1 file changed, 5 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 165e010fe69c..3cc0808f391a 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
@@ -94,7 +94,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 {
 	struct amdgpu_dm_connector *aconnector = link->priv;
 	struct drm_connector *connector = &aconnector->base;
-	struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
+	struct edid *edid_buf;
 	struct cea_sad *sads;
 	int sad_count = -1;
 	int sadb_count = -1;
@@ -106,6 +106,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	if (!edid_caps || !edid)
 		return EDID_BAD_INPUT;
 
+	edid_buf = (struct edid *)edid->raw_edid;
+
 	if (!drm_edid_is_valid(edid_buf))
 		result = EDID_BAD_CHECKSUM;
 
@@ -125,7 +127,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 
 	apply_edid_quirks(edid_buf, edid_caps);
 
-	sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+	sad_count = drm_edid_to_sad(edid_buf, &sads);
 	if (sad_count <= 0)
 		return result;
 
@@ -139,7 +141,7 @@ 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);
+	sadb_count = drm_edid_to_speaker_allocation(edid_buf, &sadb);
 
 	if (sadb_count < 0) {
 		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sadb_count);

-- 
2.46.0

[PATCH 05/12] drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()

[Thomas Weißschuh]

The argument edid is passed in as const.
Preserve this constness through the edid_buf variable and the used
helper functions.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 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 3cc0808f391a..98d1d5abafa7 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
@@ -45,14 +45,14 @@
 #include "dm_helpers.h"
 #include "ddc_service_types.h"
 
-static u32 edid_extract_panel_id(struct edid *edid)
+static u32 edid_extract_panel_id(const struct edid *edid)
 {
 	return (u32)edid->mfg_id[0] << 24   |
 	       (u32)edid->mfg_id[1] << 16   |
 	       (u32)EDID_PRODUCT_ID(edid);
 }
 
-static void apply_edid_quirks(struct edid *edid, struct dc_edid_caps *edid_caps)
+static void apply_edid_quirks(const struct edid *edid, struct dc_edid_caps *edid_caps)
 {
 	uint32_t panel_id = edid_extract_panel_id(edid);
 
@@ -94,7 +94,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 {
 	struct amdgpu_dm_connector *aconnector = link->priv;
 	struct drm_connector *connector = &aconnector->base;
-	struct edid *edid_buf;
+	const struct edid *edid_buf;
 	struct cea_sad *sads;
 	int sad_count = -1;
 	int sadb_count = -1;
@@ -106,7 +106,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 	if (!edid_caps || !edid)
 		return EDID_BAD_INPUT;
 
-	edid_buf = (struct edid *)edid->raw_edid;
+	edid_buf = (const struct edid *)edid->raw_edid;
 
 	if (!drm_edid_is_valid(edid_buf))
 		result = EDID_BAD_CHECKSUM;

-- 
2.46.0

[PATCH 06/12] drm/amd/display: Constify 'struct edid' in parsing functions

[Thomas Weißschuh]

The parsing functions do not modify their edid argument.
Mark the const to reflect this to the caller.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++++++-------
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +-
 2 files changed, 8 insertions(+), 8 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 7d999e352df3..4e7f40481379 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -11834,7 +11834,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;
@@ -11877,7 +11877,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;
@@ -11912,7 +11912,7 @@ 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;
@@ -11954,12 +11954,12 @@ 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 edid *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;
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 2d7755e2b6c3..27c0017707dd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -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 edid *edid);
 
 void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
 

-- 
2.46.0

[PATCH 07/12] drm/amd/display: Use struct edid in dc_link_add_remote_sink()

[Thomas Weißschuh]

The callers of dc_link_add_remote_sink() are using 'struct edid' which
they all need to cast to uint8_t *.
Allow the direct passing of 'struct edid' to avoid these cast and also
move the length calculation so it does not need to be duplicated
everywhere.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c           | 3 +--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 4 +---
 drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c       | 5 ++---
 drivers/gpu/drm/amd/display/dc/dc.h                         | 6 ++----
 drivers/gpu/drm/amd/display/dc/inc/link.h                   | 3 +--
 drivers/gpu/drm/amd/display/dc/link/link_detection.c        | 6 ++++--
 drivers/gpu/drm/amd/display/dc/link/link_detection.h        | 3 +--
 7 files changed, 12 insertions(+), 18 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 4e7f40481379..bd9a1a21720e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7103,8 +7103,7 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 
 	aconnector->dc_em_sink = dc_link_add_remote_sink(
 		aconnector->dc_link,
-		(uint8_t *)edid,
-		(edid->extensions + 1) * EDID_LENGTH,
+		edid,
 		&init_params);
 
 	if (aconnector->base.force == DRM_FORCE_ON) {
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 2e9f6da1acdc..25e98d248c21 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
@@ -325,7 +325,6 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 				dc_sink = dc_link_add_remote_sink(
 					aconnector->dc_link,
 					NULL,
-					0,
 					&init_params);
 
 				if (!dc_sink) {
@@ -362,8 +361,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 				.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
 		dc_sink = dc_link_add_remote_sink(
 			aconnector->dc_link,
-			(uint8_t *)aconnector->edid,
-			(aconnector->edid->extensions + 1) * EDID_LENGTH,
+			aconnector->edid,
 			&init_params);
 
 		if (!dc_sink) {
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
index dfdfe22d9e85..5fb7bf1d9034 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
@@ -266,11 +266,10 @@ unsigned int dc_dp_trace_get_link_loss_count(struct dc_link *link)
 
 struct dc_sink *dc_link_add_remote_sink(
 		struct dc_link *link,
-		const uint8_t *edid,
-		int len,
+		const struct edid *edid,
 		struct dc_sink_init_data *init_data)
 {
-	return link->dc->link_srv->add_remote_sink(link, edid, len, init_data);
+	return link->dc->link_srv->add_remote_sink(link, edid, init_data);
 }
 
 void dc_link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 6b036417a73a..4b2abb25ca3c 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1827,14 +1827,12 @@ struct dc_sink_init_data;
  * called.
  *
  * @dc_link - link the remote sink will be added to.
- * @edid - byte array of EDID raw data.
- * @len - size of the edid in byte
+ * @edid - EDID data.
  * @init_data -
  */
 struct dc_sink *dc_link_add_remote_sink(
 		struct dc_link *dc_link,
-		const uint8_t *edid,
-		int len,
+		const struct edid *edid,
 		struct dc_sink_init_data *init_data);
 
 /* Remove remote sink from a link with dc_connection_mst_branch connection type.
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link.h b/drivers/gpu/drm/amd/display/dc/inc/link.h
index 72a8479e1f2d..828b0bd71261 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link.h
@@ -109,8 +109,7 @@ struct link_service {
 			enum dc_connection_type *type);
 	struct dc_sink *(*add_remote_sink)(
 			struct dc_link *link,
-			const uint8_t *edid,
-			int len,
+			const struct edid *edid,
 			struct dc_sink_init_data *init_data);
 	void (*remove_remote_sink)(struct dc_link *link, struct dc_sink *sink);
 	bool (*get_hpd_state)(struct dc_link *link);
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 391dbe81534d..6a190d084a94 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -48,6 +48,8 @@
 #include "dm_helpers.h"
 #include "clk_mgr.h"
 
+#include <drm/drm_edid.h>
+
 #define DC_LOGGER \
 	link->ctx->logger
 #define DC_LOGGER_INIT(logger)
@@ -1379,10 +1381,10 @@ static bool link_add_remote_sink_helper(struct dc_link *dc_link, struct dc_sink
 
 struct dc_sink *link_add_remote_sink(
 		struct dc_link *link,
-		const uint8_t *edid,
-		int len,
+		const struct edid *edid,
 		struct dc_sink_init_data *init_data)
 {
+	int len = edid ? (edid->extensions + 1) * EDID_LENGTH : 0;
 	struct dc_sink *dc_sink;
 	enum dc_edid_status edid_status;
 
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.h b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
index 7da05078721e..dec5001411be 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.h
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
@@ -31,8 +31,7 @@ bool link_detect_connection_type(struct dc_link *link,
 		enum dc_connection_type *type);
 struct dc_sink *link_add_remote_sink(
 		struct dc_link *link,
-		const uint8_t *edid,
-		int len,
+		const struct edid *edid,
 		struct dc_sink_init_data *init_data);
 void link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink);
 bool link_reset_cur_dp_mst_topology(struct dc_link *link);

-- 
2.46.0

[PATCH 08/12] drm/amdgpu: Switch amdgpu_connector to struct drm_edid

[Thomas Weißschuh]

"struct drm_edid" is the safe and recommended alternative to "struct edid".

Rename the member to make sure that no usage sites are missed,
as "struct drm_edid" has some restrictions, for example it can not be
used with kfree().

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 56 +++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h       |  3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c         |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c         |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c          |  4 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c          |  4 +-
 6 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 344e0a9ee08a..75eac808b9e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -246,10 +246,10 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
 	return NULL;
 }
 
-static struct edid *
+static const struct drm_edid *
 amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev)
 {
-	return drm_edid_duplicate(drm_edid_raw(adev->mode_info.bios_hardcoded_edid));
+	return drm_edid_dup(adev->mode_info.bios_hardcoded_edid);
 }
 
 static void amdgpu_connector_get_edid(struct drm_connector *connector)
@@ -258,7 +258,7 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector)
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 
-	if (amdgpu_connector->edid)
+	if (amdgpu_connector->drm_edid)
 		return;
 
 	/* on hw with routers, select right port */
@@ -268,8 +268,8 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector)
 	if ((amdgpu_connector_encoder_get_dp_bridge_encoder_id(connector) !=
 	     ENCODER_OBJECT_ID_NONE) &&
 	    amdgpu_connector->ddc_bus->has_aux) {
-		amdgpu_connector->edid = drm_get_edid(connector,
-						      &amdgpu_connector->ddc_bus->aux.ddc);
+		amdgpu_connector->drm_edid = drm_edid_read_ddc(
+				connector, &amdgpu_connector->ddc_bus->aux.ddc);
 	} else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
 		   (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
 		struct amdgpu_connector_atom_dig *dig = amdgpu_connector->con_priv;
@@ -277,32 +277,34 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector)
 		if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT ||
 		     dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) &&
 		    amdgpu_connector->ddc_bus->has_aux)
-			amdgpu_connector->edid = drm_get_edid(connector,
-							      &amdgpu_connector->ddc_bus->aux.ddc);
+			amdgpu_connector->drm_edid = drm_edid_read_ddc(
+					connector, &amdgpu_connector->ddc_bus->aux.ddc);
 		else if (amdgpu_connector->ddc_bus)
-			amdgpu_connector->edid = drm_get_edid(connector,
-							      &amdgpu_connector->ddc_bus->adapter);
+			amdgpu_connector->drm_edid = drm_edid_read_ddc(
+					connector, &amdgpu_connector->ddc_bus->adapter);
 	} else if (amdgpu_connector->ddc_bus) {
-		amdgpu_connector->edid = drm_get_edid(connector,
-						      &amdgpu_connector->ddc_bus->adapter);
+		amdgpu_connector->drm_edid = drm_edid_read_ddc(
+				connector, &amdgpu_connector->ddc_bus->adapter);
 	}
 
-	if (!amdgpu_connector->edid) {
+	if (!amdgpu_connector->drm_edid) {
 		/* some laptops provide a hardcoded edid in rom for LCDs */
 		if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
 		     (connector->connector_type == DRM_MODE_CONNECTOR_eDP))) {
-			amdgpu_connector->edid = amdgpu_connector_get_hardcoded_edid(adev);
-			drm_connector_update_edid_property(connector, amdgpu_connector->edid);
+			amdgpu_connector->drm_edid = amdgpu_connector_get_hardcoded_edid(adev);
+			drm_edid_connector_update(connector, amdgpu_connector->drm_edid);
 		}
 	}
+
+	drm_edid_connector_update(connector, amdgpu_connector->drm_edid);
 }
 
 static void amdgpu_connector_free_edid(struct drm_connector *connector)
 {
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 
-	kfree(amdgpu_connector->edid);
-	amdgpu_connector->edid = NULL;
+	drm_edid_free(amdgpu_connector->drm_edid);
+	amdgpu_connector->drm_edid = NULL;
 }
 
 static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
@@ -310,12 +312,12 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 	int ret;
 
-	if (amdgpu_connector->edid) {
-		drm_connector_update_edid_property(connector, amdgpu_connector->edid);
-		ret = drm_add_edid_modes(connector, amdgpu_connector->edid);
+	if (amdgpu_connector->drm_edid) {
+		drm_edid_connector_update(connector, amdgpu_connector->drm_edid);
+		ret = drm_edid_connector_add_modes(connector);
 		return ret;
 	}
-	drm_connector_update_edid_property(connector, NULL);
+	drm_edid_connector_update(connector, NULL);
 	return 0;
 }
 
@@ -731,7 +733,7 @@ amdgpu_connector_lvds_detect(struct drm_connector *connector, bool force)
 
 	/* check for edid as well */
 	amdgpu_connector_get_edid(connector);
-	if (amdgpu_connector->edid)
+	if (amdgpu_connector->drm_edid)
 		ret = connector_status_connected;
 	/* check acpi lid status ??? */
 
@@ -881,13 +883,13 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force)
 		amdgpu_connector_free_edid(connector);
 		amdgpu_connector_get_edid(connector);
 
-		if (!amdgpu_connector->edid) {
+		if (!amdgpu_connector->drm_edid) {
 			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
 					connector->name);
 			ret = connector_status_connected;
 		} else {
-			amdgpu_connector->use_digital =
-				!!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
+			amdgpu_connector->use_digital = drm_edid_is_digital(
+					amdgpu_connector->drm_edid);
 
 			/* some oems have boards with separate digital and analog connectors
 			 * with a shared ddc line (often vga + hdmi)
@@ -1062,14 +1064,14 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 		amdgpu_connector_free_edid(connector);
 		amdgpu_connector_get_edid(connector);
 
-		if (!amdgpu_connector->edid) {
+		if (!amdgpu_connector->drm_edid) {
 			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
 					connector->name);
 			ret = connector_status_connected;
 			broken_edid = true; /* defer use_digital to later */
 		} else {
-			amdgpu_connector->use_digital =
-				!!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
+			amdgpu_connector->use_digital = drm_edid_is_digital(
+					amdgpu_connector->drm_edid);
 
 			/* some oems have boards with separate digital and analog connectors
 			 * with a shared ddc line (often vga + hdmi)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 5e3faefc5510..e584997c7777 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -50,7 +50,6 @@ struct amdgpu_device;
 struct amdgpu_encoder;
 struct amdgpu_router;
 struct amdgpu_hpd;
-struct edid;
 struct drm_edid;
 
 #define to_amdgpu_crtc(x) container_of(x, struct amdgpu_crtc, base)
@@ -623,7 +622,7 @@ struct amdgpu_connector {
 	bool use_digital;
 	/* 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;
 	void *con_priv;
 	bool dac_load_detect;
 	bool detected_by_load; /* if the connection status was determined by load */
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 742adbc460c9..3ad9e58979ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1299,7 +1299,7 @@ static void dce_v10_0_audio_write_speaker_allocation(struct drm_encoder *encoder
 		return;
 	}
 
-	sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb);
+	sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->drm_edid), &sadb);
 	if (sad_count < 0) {
 		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count);
 		sad_count = 0;
@@ -1369,7 +1369,7 @@ static void dce_v10_0_audio_write_sad_regs(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads);
+	sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->drm_edid), &sads);
 	if (sad_count < 0)
 		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
 	if (sad_count <= 0)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 8d46ebadfa46..5df1d69a2759 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1331,7 +1331,7 @@ static void dce_v11_0_audio_write_speaker_allocation(struct drm_encoder *encoder
 		return;
 	}
 
-	sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb);
+	sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->drm_edid), &sadb);
 	if (sad_count < 0) {
 		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count);
 		sad_count = 0;
@@ -1401,7 +1401,7 @@ static void dce_v11_0_audio_write_sad_regs(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads);
+	sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->drm_edid), &sads);
 	if (sad_count < 0)
 		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
 	if (sad_count <= 0)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index f08dc6a3886f..dbdb060a578c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1217,7 +1217,7 @@ static void dce_v6_0_audio_write_speaker_allocation(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb);
+	sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->drm_edid), &sadb);
 	if (sad_count < 0) {
 		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count);
 		sad_count = 0;
@@ -1292,7 +1292,7 @@ static void dce_v6_0_audio_write_sad_regs(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads);
+	sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->drm_edid), &sads);
 	if (sad_count < 0)
 		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
 	if (sad_count <= 0)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index a6a3adf2ae13..f6b8deab9b49 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1272,7 +1272,7 @@ static void dce_v8_0_audio_write_speaker_allocation(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb);
+	sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->drm_edid), &sadb);
 	if (sad_count < 0) {
 		DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count);
 		sad_count = 0;
@@ -1340,7 +1340,7 @@ static void dce_v8_0_audio_write_sad_regs(struct drm_encoder *encoder)
 		return;
 	}
 
-	sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads);
+	sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->drm_edid), &sads);
 	if (sad_count < 0)
 		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
 	if (sad_count <= 0)

-- 
2.46.0

[PATCH 09/12] drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid

[Thomas Weißschuh]

"struct drm_edid" is the safe and recommended alternative to "struct edid".

Rename the member to make sure that no usage sites are missed,
as "struct drm_edid" has some restrictions, for example it can not be
used with kfree().

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 70 ++++++++++++----------
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 26 ++++----
 3 files changed, 53 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 bd9a1a21720e..961f4f308dc7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3426,7 +3426,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) {
@@ -3485,18 +3485,18 @@ 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;
+			aconnector->drm_edid = drm_edid_alloc(sink->dc_edid.raw_edid,
+							      sink->dc_edid.length);
 
 			if (aconnector->dc_link->aux_mode)
 				drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
-						    aconnector->edid);
+						    drm_edid_raw(aconnector->drm_edid));
 		}
 
 		if (!aconnector->timing_requested) {
@@ -3507,8 +3507,8 @@ 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);
@@ -3517,7 +3517,7 @@ void amdgpu_dm_update_connector_after_detect(
 		aconnector->num_modes = 0;
 		dc_sink_release(aconnector->dc_sink);
 		aconnector->dc_sink = NULL;
-		aconnector->edid = NULL;
+		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 */
@@ -7016,7 +7016,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;
+	const struct drm_edid *drm_edid;
 	struct i2c_adapter *ddc;
 
 	if (dc_link && dc_link->aux_mode)
@@ -7025,23 +7025,25 @@ static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
 		ddc = &aconnector->i2c->base;
 
 	/*
-	 * Note: drm_get_edid gets edid in the following order:
+	 * Note: drm_edid_read_ddc 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_ddc(connector, ddc);
+	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) {
 		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,
+			drm_edid_raw(drm_edid),
+			(drm_edid_raw(drm_edid)->extensions + 1) * EDID_LENGTH);
 		dm_helpers_parse_edid_caps(
 			dc_link,
 			&dc_em_sink->dc_edid,
@@ -7076,7 +7078,7 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 			.link = aconnector->dc_link,
 			.sink_signal = SIGNAL_TYPE_VIRTUAL
 	};
-	struct edid *edid;
+	const struct drm_edid *drm_edid;
 	struct i2c_adapter *ddc;
 
 	if (dc_link->aux_mode)
@@ -7085,25 +7087,25 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 		ddc = &aconnector->i2c->base;
 
 	/*
-	 * Note: drm_get_edid gets edid in the following order:
+	 * Note: drm_edid_read_ddc 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_ddc(connector, ddc);
+	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;
 
 	aconnector->dc_em_sink = dc_link_add_remote_sink(
 		aconnector->dc_link,
-		edid,
+		drm_edid_raw(drm_edid),
 		&init_params);
 
 	if (aconnector->base.force == DRM_FORCE_ON) {
@@ -7786,16 +7788,17 @@ 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);
+		drm_edid_connector_update(connector, drm_edid);
 		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
@@ -7809,10 +7812,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_connector_edid_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;
 	}
@@ -7908,12 +7911,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)
@@ -7926,24 +7929,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_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);
 		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);
 
@@ -11953,12 +11956,13 @@ static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
  * FreeSync parameters.
  */
 void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
-				    const struct edid *edid)
+				    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;
+	const struct edid *edid = drm_edid_raw(drm_edid);
 	struct amdgpu_dm_connector *amdgpu_dm_connector =
 			to_amdgpu_dm_connector(connector);
 	struct dm_connector_state *dm_con_state = NULL;
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 27c0017707dd..c49cc0170fc5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -81,6 +81,7 @@ struct amdgpu_bo;
 struct dmub_srv;
 struct dc_plane_state;
 struct dmub_notification;
+struct drm_edid;
 
 struct amd_vsdb_block {
 	unsigned char ieee_id[3];
@@ -673,7 +674,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 +952,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,
-					const 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_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 25e98d248c21..cd03108db28b 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);
+	kfree(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,12 +302,13 @@ 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);
 
@@ -344,7 +345,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);
 	}
@@ -361,7 +362,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 				.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
 		dc_sink = dc_link_add_remote_sink(
 			aconnector->dc_link,
-			aconnector->edid,
+			drm_edid_raw(aconnector->drm_edid),
 			&init_params);
 
 		if (!dc_sink) {
@@ -403,7 +404,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))
@@ -417,10 +418,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;
 }
@@ -498,7 +498,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.46.0

Re: [PATCH 09/12] drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid

[Jani Nikula]

On Sun, 18 Aug 2024, Thomas Weißschuh <linux@weissschuh.net> wrote:
> "struct drm_edid" is the safe and recommended alternative to "struct edid".
>
> Rename the member to make sure that no usage sites are missed,
> as "struct drm_edid" has some restrictions, for example it can not be
> used with kfree().
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 70 ++++++++++++----------
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 26 ++++----
>  3 files changed, 53 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 bd9a1a21720e..961f4f308dc7 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3426,7 +3426,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) {
> @@ -3485,18 +3485,18 @@ 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;
> +			aconnector->drm_edid = drm_edid_alloc(sink->dc_edid.raw_edid,
> +							      sink->dc_edid.length);
>  
>  			if (aconnector->dc_link->aux_mode)
>  				drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
> -						    aconnector->edid);
> +						    drm_edid_raw(aconnector->drm_edid));
>  		}
>  
>  		if (!aconnector->timing_requested) {
> @@ -3507,8 +3507,8 @@ 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);
> @@ -3517,7 +3517,7 @@ void amdgpu_dm_update_connector_after_detect(
>  		aconnector->num_modes = 0;
>  		dc_sink_release(aconnector->dc_sink);
>  		aconnector->dc_sink = NULL;
> -		aconnector->edid = NULL;
> +		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 */
> @@ -7016,7 +7016,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;
> +	const struct drm_edid *drm_edid;
>  	struct i2c_adapter *ddc;
>  
>  	if (dc_link && dc_link->aux_mode)
> @@ -7025,23 +7025,25 @@ static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
>  		ddc = &aconnector->i2c->base;
>  
>  	/*
> -	 * Note: drm_get_edid gets edid in the following order:
> +	 * Note: drm_edid_read_ddc 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_ddc(connector, ddc);
> +	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) {
>  		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,
> +			drm_edid_raw(drm_edid),
> +			(drm_edid_raw(drm_edid)->extensions + 1) * EDID_LENGTH);
>  		dm_helpers_parse_edid_caps(
>  			dc_link,
>  			&dc_em_sink->dc_edid,
> @@ -7076,7 +7078,7 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
>  			.link = aconnector->dc_link,
>  			.sink_signal = SIGNAL_TYPE_VIRTUAL
>  	};
> -	struct edid *edid;
> +	const struct drm_edid *drm_edid;
>  	struct i2c_adapter *ddc;
>  
>  	if (dc_link->aux_mode)
> @@ -7085,25 +7087,25 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
>  		ddc = &aconnector->i2c->base;
>  
>  	/*
> -	 * Note: drm_get_edid gets edid in the following order:
> +	 * Note: drm_edid_read_ddc 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_ddc(connector, ddc);
> +	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;

You need to drm_edid_connector_update() to update
connector->display_info with the EDID info.

BR,
Jani.

>  
> -	aconnector->edid = edid;
> +	aconnector->drm_edid = drm_edid;
>  
>  	aconnector->dc_em_sink = dc_link_add_remote_sink(
>  		aconnector->dc_link,
> -		edid,
> +		drm_edid_raw(drm_edid),
>  		&init_params);
>  
>  	if (aconnector->base.force == DRM_FORCE_ON) {
> @@ -7786,16 +7788,17 @@ 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);
> +		drm_edid_connector_update(connector, drm_edid);
>  		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
> @@ -7809,10 +7812,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_connector_edid_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;
>  	}
> @@ -7908,12 +7911,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)
> @@ -7926,24 +7929,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_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);
>  		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);
>  
> @@ -11953,12 +11956,13 @@ static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
>   * FreeSync parameters.
>   */
>  void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
> -				    const struct edid *edid)
> +				    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;
> +	const struct edid *edid = drm_edid_raw(drm_edid);
>  	struct amdgpu_dm_connector *amdgpu_dm_connector =
>  			to_amdgpu_dm_connector(connector);
>  	struct dm_connector_state *dm_con_state = NULL;
> 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 27c0017707dd..c49cc0170fc5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -81,6 +81,7 @@ struct amdgpu_bo;
>  struct dmub_srv;
>  struct dc_plane_state;
>  struct dmub_notification;
> +struct drm_edid;
>  
>  struct amd_vsdb_block {
>  	unsigned char ieee_id[3];
> @@ -673,7 +674,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 +952,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,
> -					const 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_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 25e98d248c21..cd03108db28b 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);
> +	kfree(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,12 +302,13 @@ 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);
>  
> @@ -344,7 +345,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);
>  	}
> @@ -361,7 +362,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
>  				.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
>  		dc_sink = dc_link_add_remote_sink(
>  			aconnector->dc_link,
> -			aconnector->edid,
> +			drm_edid_raw(aconnector->drm_edid),
>  			&init_params);
>  
>  		if (!dc_sink) {
> @@ -403,7 +404,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))
> @@ -417,10 +418,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;
>  }
> @@ -498,7 +498,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;

-- 
Jani Nikula, Intel

[PATCH 10/12] drm/edid: add a helper to compare two EDIDs

[Thomas Weißschuh]

As struct drm_edid is opaque, drivers can't directly memcmp() the
contained data. Add a helper to provide this functionality.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
 include/drm/drm_edid.h     |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 69fb11741abd..c2493c983a64 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1840,6 +1840,24 @@ static bool drm_edid_eq(const struct drm_edid *drm_edid,
 	return true;
 }
 
+/**
+ * drm_edid_equal - compare two EDID
+ * @drm_edid_a: First EDID data
+ * @drm_edid_b: Second EDID data
+ *
+ * Compare two EDIDs for equality (including extensions)
+ *
+ * Return: True if the EDIDs are equal, false otherwise.
+ */
+bool drm_edid_equal(const struct drm_edid *drm_edid_a, const struct drm_edid *drm_edid_b)
+{
+	if (!drm_edid_b)
+		return !drm_edid_a;
+
+	return drm_edid_eq(drm_edid_a, drm_edid_b->edid, drm_edid_b->size);
+}
+EXPORT_SYMBOL(drm_edid_equal);
+
 enum edid_block_status {
 	EDID_BLOCK_OK = 0,
 	EDID_BLOCK_READ_FAIL,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index a5b377c4a342..35b40a9d3350 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -456,6 +456,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
 const struct drm_edid *drm_edid_alloc(const void *edid, size_t size);
 const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid);
 void drm_edid_free(const struct drm_edid *drm_edid);
+bool drm_edid_equal(const struct drm_edid *drm_edid_a, const struct drm_edid *drm_edid_b);
 bool drm_edid_valid(const struct drm_edid *drm_edid);
 const struct edid *drm_edid_raw(const struct drm_edid *drm_edid);
 const struct drm_edid *drm_edid_read(struct drm_connector *connector);

-- 
2.46.0

Re: [PATCH 10/12] drm/edid: add a helper to compare two EDIDs

[Jani Nikula]

On Sun, 18 Aug 2024, Thomas Weißschuh <linux@weissschuh.net> wrote:
> As struct drm_edid is opaque, drivers can't directly memcmp() the
> contained data. Add a helper to provide this functionality.

I'm not sure why drivers would need to compare EDIDs.

The only user was added in commit eb815442e840 ("drm/amd/display: don't
create new dc_sink if nothing changed at detection") with absolutely no
explanation why.

Other drivers use connector->epoch_counter to see if the EDID or status
changed.


BR,
Jani.


>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
>  include/drm/drm_edid.h     |  1 +
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 69fb11741abd..c2493c983a64 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1840,6 +1840,24 @@ static bool drm_edid_eq(const struct drm_edid *drm_edid,
>  	return true;
>  }
>  
> +/**
> + * drm_edid_equal - compare two EDID
> + * @drm_edid_a: First EDID data
> + * @drm_edid_b: Second EDID data
> + *
> + * Compare two EDIDs for equality (including extensions)
> + *
> + * Return: True if the EDIDs are equal, false otherwise.
> + */
> +bool drm_edid_equal(const struct drm_edid *drm_edid_a, const struct drm_edid *drm_edid_b)
> +{
> +	if (!drm_edid_b)
> +		return !drm_edid_a;
> +
> +	return drm_edid_eq(drm_edid_a, drm_edid_b->edid, drm_edid_b->size);
> +}
> +EXPORT_SYMBOL(drm_edid_equal);
> +
>  enum edid_block_status {
>  	EDID_BLOCK_OK = 0,
>  	EDID_BLOCK_READ_FAIL,
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index a5b377c4a342..35b40a9d3350 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -456,6 +456,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
>  const struct drm_edid *drm_edid_alloc(const void *edid, size_t size);
>  const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid);
>  void drm_edid_free(const struct drm_edid *drm_edid);
> +bool drm_edid_equal(const struct drm_edid *drm_edid_a, const struct drm_edid *drm_edid_b);
>  bool drm_edid_valid(const struct drm_edid *drm_edid);
>  const struct edid *drm_edid_raw(const struct drm_edid *drm_edid);
>  const struct drm_edid *drm_edid_read(struct drm_connector *connector);

-- 
Jani Nikula, Intel

[PATCH 11/12] drm/amd/display: Switch dc_sink to struct drm_edid

[Thomas Weißschuh]

The custom "struct dc_edid" can be replaced by the standard "struct
drm_edid.

Rename the member to make sure that no usage sites are missed,
as "struct drm_edid" has some restrictions, for example it can not be
used with kfree().

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 11 +++----
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 24 ++++++--------
 drivers/gpu/drm/amd/display/dc/dc.h                |  2 +-
 drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  3 --
 drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 ---
 drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  2 +-
 .../gpu/drm/amd/display/dc/link/link_detection.c   | 37 +++++++---------------
 7 files changed, 27 insertions(+), 57 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 961f4f308dc7..e5ac1e6eca80 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3484,15 +3484,14 @@ void amdgpu_dm_update_connector_after_detect(
 
 		aconnector->dc_sink = sink;
 		dc_sink_retain(aconnector->dc_sink);
-		if (sink->dc_edid.length == 0) {
+		if (!sink->drm_edid) {
 			aconnector->drm_edid = NULL;
 			if (aconnector->dc_link->aux_mode) {
 				drm_dp_cec_unset_edid(
 					&aconnector->dm_dp_aux.aux);
 			}
 		} else {
-			aconnector->drm_edid = drm_edid_alloc(sink->dc_edid.raw_edid,
-							      sink->dc_edid.length);
+			aconnector->drm_edid = drm_edid_dup(sink->drm_edid);
 
 			if (aconnector->dc_link->aux_mode)
 				drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux,
@@ -7041,12 +7040,10 @@ static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
 	/* Update emulated (virtual) sink's EDID */
 	if (dc_em_sink && dc_link) {
 		memset(&dc_em_sink->edid_caps, 0, sizeof(struct dc_edid_caps));
-		memmove(dc_em_sink->dc_edid.raw_edid,
-			drm_edid_raw(drm_edid),
-			(drm_edid_raw(drm_edid)->extensions + 1) * EDID_LENGTH);
+		dc_em_sink->drm_edid = drm_edid_dup(drm_edid);
 		dm_helpers_parse_edid_caps(
 			dc_link,
-			&dc_em_sink->dc_edid,
+			dc_em_sink->drm_edid,
 			&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 98d1d5abafa7..f3bf664ffc90 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
@@ -89,7 +89,7 @@ static void apply_edid_quirks(const struct edid *edid, struct dc_edid_caps *edid
  */
 enum dc_edid_status dm_helpers_parse_edid_caps(
 		struct dc_link *link,
-		const struct dc_edid *edid,
+		const struct drm_edid *drm_edid,
 		struct dc_edid_caps *edid_caps)
 {
 	struct amdgpu_dm_connector *aconnector = link->priv;
@@ -103,10 +103,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
 
 	enum dc_edid_status result = EDID_OK;
 
-	if (!edid_caps || !edid)
+	if (!edid_caps || !drm_edid)
 		return EDID_BAD_INPUT;
 
-	edid_buf = (const struct edid *)edid->raw_edid;
+	edid_buf = drm_edid_raw(drm_edid);
 
 	if (!drm_edid_is_valid(edid_buf))
 		result = EDID_BAD_CHECKSUM;
@@ -899,7 +899,7 @@ 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;
 
 	if (link->aux_mode)
 		ddc = &aconnector->dm_dp_aux.aux.ddc;
@@ -911,29 +911,25 @@ enum dc_edid_status dm_helpers_read_local_edid(
 	 */
 	do {
 
-		edid = drm_get_edid(&aconnector->base, ddc);
+		drm_edid = drm_edid_read_ddc(&aconnector->base, ddc);
 
 		/* 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;
 
-		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);
+		sink->drm_edid = drm_edid_dup(drm_edid);
 
 		edid_status = dm_helpers_parse_edid_caps(
 						link,
-						&sink->dc_edid,
+						sink->drm_edid,
 						&sink->edid_caps);
 
 	} while (edid_status == EDID_BAD_CHECKSUM && --retry > 0);
@@ -960,7 +956,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
 		dm_helpers_dp_write_dpcd(ctx,
 					link,
 					DP_TEST_EDID_CHECKSUM,
-					&sink->dc_edid.raw_edid[sink->dc_edid.length-1],
+					&drm_edid_raw(sink->drm_edid)->checksum,
 					1);
 
 		dm_helpers_dp_write_dpcd(ctx,
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 4b2abb25ca3c..eb151a637f1a 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -2371,7 +2371,7 @@ struct scdc_caps {
  */
 struct dc_sink {
 	enum signal_type sink_signal;
-	struct dc_edid dc_edid; /* raw edid */
+	const struct drm_edid *drm_edid; /* raw edid */
 	struct dc_edid_caps edid_caps; /* parse display caps */
 	struct dc_container_id *dc_container_id;
 	uint32_t dongle_max_pix_clk;
diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
index 7dd1cfb9ab76..6e0a457b7551 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
@@ -178,9 +178,6 @@ enum display_dongle_type {
 	DISPLAY_DONGLE_DP_HDMI_MISMATCHED_DONGLE,
 };
 
-#define DC_MAX_EDID_BUFFER_SIZE 2048
-#define DC_EDID_BLOCK_SIZE 128
-
 struct ddc_service {
 	struct ddc *ddc_pin;
 	struct ddc_flags flags;
diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h
index 97279b080f3e..0906a0bae399 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -149,11 +149,6 @@ struct dc_cea_audio_mode {
 	};
 };
 
-struct dc_edid {
-	uint32_t length;
-	uint8_t raw_edid[DC_MAX_EDID_BUFFER_SIZE];
-};
-
 /* When speaker location data block is not available, DEFAULT_SPEAKER_LOCATION
  * is used. In this case we assume speaker location are: front left, front
  * right and front center. */
diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
index 483d8c292618..5a3d4bf7c370 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
@@ -61,7 +61,7 @@ void dm_helpers_free_gpu_mem(
 
 enum dc_edid_status dm_helpers_parse_edid_caps(
 	struct dc_link *link,
-	const struct dc_edid *edid,
+	const struct drm_edid *drm_edid,
 	struct dc_edid_caps *edid_caps);
 
 
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 6a190d084a94..cdbf6bcc8f68 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -615,18 +615,6 @@ static bool detect_dp(struct dc_link *link,
 	return true;
 }
 
-static bool is_same_edid(struct dc_edid *old_edid, struct dc_edid *new_edid)
-{
-	if (old_edid->length != new_edid->length)
-		return false;
-
-	if (new_edid->length == 0)
-		return false;
-
-	return (memcmp(old_edid->raw_edid,
-		       new_edid->raw_edid, new_edid->length) == 0);
-}
-
 static bool wait_for_entering_dp_alt_mode(struct dc_link *link)
 {
 
@@ -866,6 +854,7 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 	struct dpcd_caps prev_dpcd_caps;
 	enum dc_connection_type new_connection_type = dc_connection_none;
 	const uint32_t post_oui_delay = 30; // 30ms
+	const struct edid *edid;
 
 	DC_LOGGER_INIT(link->ctx->logger);
 
@@ -1100,8 +1089,8 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 		// Check if edid is the same
 		if ((prev_sink) &&
 		    (edid_status == EDID_THE_SAME || edid_status == EDID_OK))
-			same_edid = is_same_edid(&prev_sink->dc_edid,
-						 &sink->dc_edid);
+			same_edid = drm_edid_equal(prev_sink->drm_edid,
+						   sink->drm_edid);
 
 		if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
 			link->ctx->dc->debug.hdmi20_disable = true;
@@ -1138,11 +1127,12 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 		if (link->local_sink && dc_is_dp_signal(sink_caps.signal))
 			dp_trace_init(link);
 
+		edid = drm_edid_raw(sink->drm_edid);
 		/* Connectivity log: detection */
-		for (i = 0; i < sink->dc_edid.length / DC_EDID_BLOCK_SIZE; i++) {
+		for (i = 0; i < edid->extensions + 1; i++) {
 			CONN_DATA_DETECT(link,
-					 &sink->dc_edid.raw_edid[i * DC_EDID_BLOCK_SIZE],
-					 DC_EDID_BLOCK_SIZE,
+					 edid + i,
+					 EDID_LENGTH,
 					 "%s: [Block %d] ", sink->edid_caps.display_name, i);
 		}
 
@@ -1388,11 +1378,6 @@ struct dc_sink *link_add_remote_sink(
 	struct dc_sink *dc_sink;
 	enum dc_edid_status edid_status;
 
-	if (len > DC_MAX_EDID_BUFFER_SIZE) {
-		dm_error("Max EDID buffer size breached!\n");
-		return NULL;
-	}
-
 	if (!init_data) {
 		BREAK_TO_DEBUGGER();
 		return NULL;
@@ -1408,8 +1393,7 @@ struct dc_sink *link_add_remote_sink(
 	if (!dc_sink)
 		return NULL;
 
-	memmove(dc_sink->dc_edid.raw_edid, edid, len);
-	dc_sink->dc_edid.length = len;
+	dc_sink->drm_edid = drm_edid_alloc(edid, len);
 
 	if (!link_add_remote_sink_helper(
 			link,
@@ -1418,7 +1402,7 @@ struct dc_sink *link_add_remote_sink(
 
 	edid_status = dm_helpers_parse_edid_caps(
 			link,
-			&dc_sink->dc_edid,
+			dc_sink->drm_edid,
 			&dc_sink->edid_caps);
 
 	/*
@@ -1426,7 +1410,8 @@ struct dc_sink *link_add_remote_sink(
 	 * parsing fails
 	 */
 	if (edid_status != EDID_OK && edid_status != EDID_PARTIAL_VALID) {
-		dc_sink->dc_edid.length = 0;
+		drm_edid_free(dc_sink->drm_edid);
+		dc_sink->drm_edid = NULL;
 		dm_error("Bad EDID, status%d!\n", edid_status);
 	}
 

-- 
2.46.0

[PATCH 12/12] drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid

[Thomas Weißschuh]

"struct drm_edid" is the safe and recommended alternative to "struct edid".
Now that all callers of dc_link_add_remote_sink() have access to a
validate struct drm_edid, pass it around directly.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c           | 2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c       | 4 ++--
 drivers/gpu/drm/amd/display/dc/dc.h                         | 2 +-
 drivers/gpu/drm/amd/display/dc/inc/link.h                   | 2 +-
 drivers/gpu/drm/amd/display/dc/link/link_detection.c        | 5 ++---
 drivers/gpu/drm/amd/display/dc/link/link_detection.h        | 2 +-
 7 files changed, 9 insertions(+), 10 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 e5ac1e6eca80..23582fa4fd2b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7102,7 +7102,7 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 
 	aconnector->dc_em_sink = dc_link_add_remote_sink(
 		aconnector->dc_link,
-		drm_edid_raw(drm_edid),
+		drm_edid,
 		&init_params);
 
 	if (aconnector->base.force == DRM_FORCE_ON) {
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 cd03108db28b..f49af060c0e7 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
@@ -362,7 +362,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 				.sink_signal = SIGNAL_TYPE_DISPLAY_PORT_MST };
 		dc_sink = dc_link_add_remote_sink(
 			aconnector->dc_link,
-			drm_edid_raw(aconnector->drm_edid),
+			aconnector->drm_edid,
 			&init_params);
 
 		if (!dc_sink) {
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
index 5fb7bf1d9034..e35e6763dcbb 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c
@@ -266,10 +266,10 @@ unsigned int dc_dp_trace_get_link_loss_count(struct dc_link *link)
 
 struct dc_sink *dc_link_add_remote_sink(
 		struct dc_link *link,
-		const struct edid *edid,
+		const struct drm_edid *drm_edid,
 		struct dc_sink_init_data *init_data)
 {
-	return link->dc->link_srv->add_remote_sink(link, edid, init_data);
+	return link->dc->link_srv->add_remote_sink(link, drm_edid, init_data);
 }
 
 void dc_link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index eb151a637f1a..ce1cde89c621 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1832,7 +1832,7 @@ struct dc_sink_init_data;
  */
 struct dc_sink *dc_link_add_remote_sink(
 		struct dc_link *dc_link,
-		const struct edid *edid,
+		const struct drm_edid *drm_edid,
 		struct dc_sink_init_data *init_data);
 
 /* Remove remote sink from a link with dc_connection_mst_branch connection type.
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link.h b/drivers/gpu/drm/amd/display/dc/inc/link.h
index 828b0bd71261..062109823f32 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link.h
@@ -109,7 +109,7 @@ struct link_service {
 			enum dc_connection_type *type);
 	struct dc_sink *(*add_remote_sink)(
 			struct dc_link *link,
-			const struct edid *edid,
+			const struct drm_edid *drm_edid,
 			struct dc_sink_init_data *init_data);
 	void (*remove_remote_sink)(struct dc_link *link, struct dc_sink *sink);
 	bool (*get_hpd_state)(struct dc_link *link);
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index cdbf6bcc8f68..64d30ba476dd 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1371,10 +1371,9 @@ static bool link_add_remote_sink_helper(struct dc_link *dc_link, struct dc_sink
 
 struct dc_sink *link_add_remote_sink(
 		struct dc_link *link,
-		const struct edid *edid,
+		const struct drm_edid *drm_edid,
 		struct dc_sink_init_data *init_data)
 {
-	int len = edid ? (edid->extensions + 1) * EDID_LENGTH : 0;
 	struct dc_sink *dc_sink;
 	enum dc_edid_status edid_status;
 
@@ -1393,7 +1392,7 @@ struct dc_sink *link_add_remote_sink(
 	if (!dc_sink)
 		return NULL;
 
-	dc_sink->drm_edid = drm_edid_alloc(edid, len);
+	dc_sink->drm_edid = drm_edid_dup(drm_edid);
 
 	if (!link_add_remote_sink_helper(
 			link,
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.h b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
index dec5001411be..c35b4f5304c6 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.h
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.h
@@ -31,7 +31,7 @@ bool link_detect_connection_type(struct dc_link *link,
 		enum dc_connection_type *type);
 struct dc_sink *link_add_remote_sink(
 		struct dc_link *link,
-		const struct edid *edid,
+		const struct drm_edid *drm_edid,
 		struct dc_sink_init_data *init_data);
 void link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink);
 bool link_reset_cur_dp_mst_topology(struct dc_link *link);

-- 
2.46.0

Re: [PATCH 00/12] drm/amd: Switch over to struct drm_edid

[Melissa Wen]

On 08/18, Thomas Weißschuh wrote:
> The AMD DRM drivers use 'struct edid', raw pointers and even custom
> structs to represent EDID data.
> Uniformly switch to the safe and recommended "struct drm_edid".
> 
> Some uses of "struct edid" are left because some ad-hoc parsing is still
> being done inside the drivers.

Hi Thomas,

It's great to see more people working on removing raw edid from amd
display driver in favor of drm_edid.

I glanced over your series and I found it similar to my recent proposal
to migrate amdgpu_dm_connector from edid to drm_edid. You can find the
v5 of this work here:
https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/

I believe it's more productive if we can join efforts and improve that
proposal instead of duplicating work. I'll look at your patches more
carefully this week. If you can review my work, I'd be happy to hear
your feedback too.

Thanks,

Melissa

> 
> The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
> will conflict with my backlight quirk series [0].
> The conflict will result in an obvious and easy to fix build failure.
> 
> Patches 1 and 2 delete some dead code.
> Patches 3 to 6 constify some arguments and shuffle around some code.
> The remaining patches perform the actual conversion in steps.
> 
> [0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Thomas Weißschuh (12):
>       drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
>       drm/amd/display: Remove EDID members of ddc_service
>       drm/edid: constify argument of drm_edid_is_valid()
>       drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
>       drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
>       drm/amd/display: Constify 'struct edid' in parsing functions
>       drm/amd/display: Use struct edid in dc_link_add_remote_sink()
>       drm/amdgpu: Switch amdgpu_connector to struct drm_edid
>       drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
>       drm/edid: add a helper to compare two EDIDs
>       drm/amd/display: Switch dc_sink to struct drm_edid
>       drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
>  .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
>  drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
>  drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
>  drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
>  drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
>  drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
>  .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
>  .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
>  drivers/gpu/drm/drm_edid.c                         | 20 +++++-
>  include/drm/drm_edid.h                             |  3 +-
>  20 files changed, 155 insertions(+), 171 deletions(-)
> ---
> base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
> change-id: 20240615-amdgpu-drm_edid-32d969dfb899
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 

Re: [PATCH 00/12] drm/amd: Switch over to struct drm_edid

[Thomas Weißschuh]

Hi Melissa,

On 2024-08-19 11:31:44+0000, Melissa Wen wrote:
> On 08/18, Thomas Weißschuh wrote:
> > The AMD DRM drivers use 'struct edid', raw pointers and even custom
> > structs to represent EDID data.
> > Uniformly switch to the safe and recommended "struct drm_edid".
> > 
> > Some uses of "struct edid" are left because some ad-hoc parsing is still
> > being done inside the drivers.
> 
> Hi Thomas,
> 
> It's great to see more people working on removing raw edid from amd
> display driver in favor of drm_edid.
> 
> I glanced over your series and I found it similar to my recent proposal
> to migrate amdgpu_dm_connector from edid to drm_edid. You can find the
> v5 of this work here:
> https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/

thanks for the pointer.

> I believe it's more productive if we can join efforts and improve that
> proposal instead of duplicating work. I'll look at your patches more
> carefully this week. If you can review my work, I'd be happy to hear
> your feedback too.

Indeed. I'll take a look at your patches.
Let's see how they can be combined.

> Thanks,
> 
> Melissa
> 
> > 
> > The patch "drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid"
> > will conflict with my backlight quirk series [0].
> > The conflict will result in an obvious and easy to fix build failure.
> > 
> > Patches 1 and 2 delete some dead code.
> > Patches 3 to 6 constify some arguments and shuffle around some code.
> > The remaining patches perform the actual conversion in steps.
> > 
> > [0] https://lore.kernel.org/lkml/20240818-amdgpu-min-backlight-quirk-v5-0-b6c0ead0c73d@weissschuh.net/
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Thomas Weißschuh (12):
> >       drm/amd/display: remove spurious definition for dm_helpers_get_sbios_edid()
> >       drm/amd/display: Remove EDID members of ddc_service
> >       drm/edid: constify argument of drm_edid_is_valid()
> >       drm/amd/display: Simplify raw_edid handling in dm_helpers_parse_edid_caps()
> >       drm/amd/display: Constify raw_edid handling in dm_helpers_parse_edid_caps()
> >       drm/amd/display: Constify 'struct edid' in parsing functions
> >       drm/amd/display: Use struct edid in dc_link_add_remote_sink()
> >       drm/amdgpu: Switch amdgpu_connector to struct drm_edid
> >       drm/amd/display: Switch amdgpu_dm_connector to struct drm_edid
> >       drm/edid: add a helper to compare two EDIDs
> >       drm/amd/display: Switch dc_sink to struct drm_edid
> >       drm/amd/display: Switch dc_link_add_remote_sink() to struct drm_edid
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c     | 56 ++++++++-------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c             |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c             |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c              |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c              |  4 +-
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 84 +++++++++++-----------
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  5 +-
> >  .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 34 +++++----
> >  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 28 ++++----
> >  .../gpu/drm/amd/display/dc/core/dc_link_exports.c  |  5 +-
> >  drivers/gpu/drm/amd/display/dc/dc.h                |  8 +--
> >  drivers/gpu/drm/amd/display/dc/dc_ddc_types.h      |  7 --
> >  drivers/gpu/drm/amd/display/dc/dc_types.h          |  5 --
> >  drivers/gpu/drm/amd/display/dc/dm_helpers.h        |  4 +-
> >  drivers/gpu/drm/amd/display/dc/inc/link.h          |  3 +-
> >  .../gpu/drm/amd/display/dc/link/link_detection.c   | 42 ++++-------
> >  .../gpu/drm/amd/display/dc/link/link_detection.h   |  3 +-
> >  drivers/gpu/drm/drm_edid.c                         | 20 +++++-
> >  include/drm/drm_edid.h                             |  3 +-
> >  20 files changed, 155 insertions(+), 171 deletions(-)
> > ---
> > base-commit: 207565ee2594ac47261cdfc8a5048f4dc322c878
> > change-id: 20240615-amdgpu-drm_edid-32d969dfb899
> > 
> > Best regards,
> > -- 
> > Thomas Weißschuh <linux@weissschuh.net>
> >