[RESEND 1/3] drm/mst: switch to guid_t type for GUID

[RESEND 1/3] drm/mst: switch to guid_t type for GUID

[Jani Nikula]

The kernel has a guid_t type for GUIDs. Switch to using it, but avoid
any functional changes here.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 67 +++++++++++--------
 include/drm/display/drm_dp_mst_helper.h       | 12 ++--
 3 files changed, 45 insertions(+), 36 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 7e7929f24ae4..72c10fc2c890 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2610,7 +2610,7 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
 		}
 	}
 
-	memcpy(mgr->mst_primary->guid, guid, 16);
+	import_guid(&mgr->mst_primary->guid, guid);
 
 out_fail:
 	mutex_unlock(&mgr->lock);
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 379a449a28a2..39f1dc45004e 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -89,7 +89,7 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
 					   struct drm_dp_mst_branch *mstb,
 					   struct drm_dp_mst_port *port);
 static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
-				 u8 *guid);
+				 guid_t *guid);
 
 static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
 static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
@@ -801,7 +801,7 @@ static bool drm_dp_sideband_parse_link_address(const struct drm_dp_mst_topology_
 	int idx = 1;
 	int i;
 
-	memcpy(repmsg->u.link_addr.guid, &raw->msg[idx], 16);
+	import_guid(&repmsg->u.link_addr.guid, &raw->msg[idx]);
 	idx += 16;
 	repmsg->u.link_addr.nports = raw->msg[idx] & 0xf;
 	idx++;
@@ -829,7 +829,7 @@ static bool drm_dp_sideband_parse_link_address(const struct drm_dp_mst_topology_
 			idx++;
 			if (idx > raw->curlen)
 				goto fail_len;
-			memcpy(repmsg->u.link_addr.ports[i].peer_guid, &raw->msg[idx], 16);
+			import_guid(&repmsg->u.link_addr.ports[i].peer_guid, &raw->msg[idx]);
 			idx += 16;
 			if (idx > raw->curlen)
 				goto fail_len;
@@ -1029,7 +1029,7 @@ static bool drm_dp_sideband_parse_reply(const struct drm_dp_mst_topology_mgr *mg
 	msg->req_type = (raw->msg[0] & 0x7f);
 
 	if (msg->reply_type == DP_SIDEBAND_REPLY_NAK) {
-		memcpy(msg->u.nak.guid, &raw->msg[1], 16);
+		import_guid(&msg->u.nak.guid, &raw->msg[1]);
 		msg->u.nak.reason = raw->msg[17];
 		msg->u.nak.nak_data = raw->msg[18];
 		return false;
@@ -1078,7 +1078,7 @@ drm_dp_sideband_parse_connection_status_notify(const struct drm_dp_mst_topology_
 	if (idx > raw->curlen)
 		goto fail_len;
 
-	memcpy(msg->u.conn_stat.guid, &raw->msg[idx], 16);
+	import_guid(&msg->u.conn_stat.guid, &raw->msg[idx]);
 	idx += 16;
 	if (idx > raw->curlen)
 		goto fail_len;
@@ -1107,7 +1107,7 @@ static bool drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
 	if (idx > raw->curlen)
 		goto fail_len;
 
-	memcpy(msg->u.resource_stat.guid, &raw->msg[idx], 16);
+	import_guid(&msg->u.resource_stat.guid, &raw->msg[idx]);
 	idx += 16;
 	if (idx > raw->curlen)
 		goto fail_len;
@@ -2174,20 +2174,24 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
 				      offset, size, buffer);
 }
 
-static int drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
+static int drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, guid_t *guid)
 {
 	int ret = 0;
 
-	memcpy(mstb->guid, guid, 16);
+	guid_copy(&mstb->guid, guid);
+
+	if (!drm_dp_validate_guid(mstb->mgr, &mstb->guid)) {
+		u8 buf[UUID_SIZE];
+
+		export_guid(buf, &mstb->guid);
 
-	if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
 		if (mstb->port_parent) {
 			ret = drm_dp_send_dpcd_write(mstb->mgr,
 						     mstb->port_parent,
-						     DP_GUID, 16, mstb->guid);
+						     DP_GUID, sizeof(buf), buf);
 		} else {
 			ret = drm_dp_dpcd_write(mstb->mgr->aux,
-						DP_GUID, mstb->guid, 16);
+						DP_GUID, buf, sizeof(buf));
 		}
 	}
 
@@ -2567,9 +2571,9 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_
 	return mstb;
 }
 
-static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
-	struct drm_dp_mst_branch *mstb,
-	const uint8_t *guid)
+static struct drm_dp_mst_branch *
+get_mst_branch_device_by_guid_helper(struct drm_dp_mst_branch *mstb,
+				     const guid_t *guid)
 {
 	struct drm_dp_mst_branch *found_mstb;
 	struct drm_dp_mst_port *port;
@@ -2577,10 +2581,9 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
 	if (!mstb)
 		return NULL;
 
-	if (memcmp(mstb->guid, guid, 16) == 0)
+	if (guid_equal(&mstb->guid, guid))
 		return mstb;
 
-
 	list_for_each_entry(port, &mstb->ports, next) {
 		found_mstb = get_mst_branch_device_by_guid_helper(port->mstb, guid);
 
@@ -2593,7 +2596,7 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
 
 static struct drm_dp_mst_branch *
 drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr,
-				     const uint8_t *guid)
+				     const guid_t *guid)
 {
 	struct drm_dp_mst_branch *mstb;
 	int ret;
@@ -2695,17 +2698,20 @@ static void drm_dp_mst_queue_probe_work(struct drm_dp_mst_topology_mgr *mgr)
 }
 
 static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
-				 u8 *guid)
+				 guid_t *guid)
 {
 	u64 salt;
+	u8 buf[UUID_SIZE];
 
-	if (memchr_inv(guid, 0, 16))
+	if (!guid_is_null(guid))
 		return true;
 
 	salt = get_jiffies_64();
 
-	memcpy(&guid[0], &salt, sizeof(u64));
-	memcpy(&guid[8], &salt, sizeof(u64));
+	memcpy(&buf[0], &salt, sizeof(u64));
+	memcpy(&buf[8], &salt, sizeof(u64));
+
+	import_guid(guid, buf);
 
 	return false;
 }
@@ -2945,7 +2951,7 @@ static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
 	drm_dbg_kms(mgr->dev, "link address reply: %d\n", reply->nports);
 	drm_dp_dump_link_address(mgr, reply);
 
-	ret = drm_dp_check_mstb_guid(mstb, reply->guid);
+	ret = drm_dp_check_mstb_guid(mstb, &reply->guid);
 	if (ret) {
 		char buf[64];
 
@@ -3799,8 +3805,9 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
 int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
 				   bool sync)
 {
+	u8 buf[UUID_SIZE];
+	guid_t guid;
 	int ret;
-	u8 guid[16];
 
 	mutex_lock(&mgr->lock);
 	if (!mgr->mst_primary)
@@ -3821,13 +3828,15 @@ int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
 	}
 
 	/* Some hubs forget their guids after they resume */
-	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
-	if (ret != 16) {
+	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
 		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
 		goto out_fail;
 	}
 
-	ret = drm_dp_check_mstb_guid(mgr->mst_primary, guid);
+	import_guid(&guid, buf);
+
+	ret = drm_dp_check_mstb_guid(mgr->mst_primary, &guid);
 	if (ret) {
 		drm_dbg_kms(mgr->dev, "check mstb failed - undocked during suspend?\n");
 		goto out_fail;
@@ -4005,12 +4014,12 @@ drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr,
 	bool hotplug = false, dowork = false;
 
 	if (hdr->broadcast) {
-		const u8 *guid = NULL;
+		const guid_t *guid = NULL;
 
 		if (msg->req_type == DP_CONNECTION_STATUS_NOTIFY)
-			guid = msg->u.conn_stat.guid;
+			guid = &msg->u.conn_stat.guid;
 		else if (msg->req_type == DP_RESOURCE_STATUS_NOTIFY)
-			guid = msg->u.resource_stat.guid;
+			guid = &msg->u.resource_stat.guid;
 
 		if (guid)
 			mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid);
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 02b037d3a93f..f6a1cbb0f600 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -244,18 +244,18 @@ struct drm_dp_mst_branch {
 	bool link_address_sent;
 
 	/* global unique identifier to identify branch devices */
-	u8 guid[16];
+	guid_t guid;
 };
 
 
 struct drm_dp_nak_reply {
-	u8 guid[16];
+	guid_t guid;
 	u8 reason;
 	u8 nak_data;
 };
 
 struct drm_dp_link_address_ack_reply {
-	u8 guid[16];
+	guid_t guid;
 	u8 nports;
 	struct drm_dp_link_addr_reply_port {
 		bool input_port;
@@ -265,7 +265,7 @@ struct drm_dp_link_address_ack_reply {
 		bool ddps;
 		bool legacy_device_plug_status;
 		u8 dpcd_revision;
-		u8 peer_guid[16];
+		guid_t peer_guid;
 		u8 num_sdp_streams;
 		u8 num_sdp_stream_sinks;
 	} ports[16];
@@ -348,7 +348,7 @@ struct drm_dp_allocate_payload_ack_reply {
 };
 
 struct drm_dp_connection_status_notify {
-	u8 guid[16];
+	guid_t guid;
 	u8 port_number;
 	bool legacy_device_plug_status;
 	bool displayport_device_plug_status;
@@ -425,7 +425,7 @@ struct drm_dp_query_payload {
 
 struct drm_dp_resource_status_notify {
 	u8 port_number;
-	u8 guid[16];
+	guid_t guid;
 	u16 available_pbn;
 };
 
-- 
2.39.2

[RESEND 2/3] drm/mst: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

Instead of just smashing jiffies into a GUID, use guid_gen() to generate
RFC 4122 compliant GUIDs.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 39f1dc45004e..38a9a1441e62 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2700,18 +2700,10 @@ static void drm_dp_mst_queue_probe_work(struct drm_dp_mst_topology_mgr *mgr)
 static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
 				 guid_t *guid)
 {
-	u64 salt;
-	u8 buf[UUID_SIZE];
-
 	if (!guid_is_null(guid))
 		return true;
 
-	salt = get_jiffies_64();
-
-	memcpy(&buf[0], &salt, sizeof(u64));
-	memcpy(&buf[8], &salt, sizeof(u64));
-
-	import_guid(guid, buf);
+	guid_gen(guid);
 
 	return false;
 }
-- 
2.39.2

[RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

Instead of just smashing jiffies into a GUID, use guid_gen() to generate
RFC 4122 compliant GUIDs.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Side note, it baffles me why amdgpu has a copy of this instead of
plumbing it into drm mst code.
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
 
 static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
 {
+	u8 buf[UUID_SIZE];
+	guid_t guid;
 	int ret;
-	u8 guid[16];
-	u64 tmp64;
 
 	mutex_lock(&mgr->lock);
 	if (!mgr->mst_primary)
@@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
 	}
 
 	/* Some hubs forget their guids after they resume */
-	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
-	if (ret != 16) {
+	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
 		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
 		goto out_fail;
 	}
 
-	if (memchr_inv(guid, 0, 16) == NULL) {
-		tmp64 = get_jiffies_64();
-		memcpy(&guid[0], &tmp64, sizeof(u64));
-		memcpy(&guid[8], &tmp64, sizeof(u64));
+	import_guid(&guid, buf);
 
-		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
+	if (guid_is_null(&guid)) {
+		guid_gen(&guid);
+		export_guid(buf, &guid);
 
-		if (ret != 16) {
+		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
+
+		if (ret != sizeof(buf)) {
 			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
 			goto out_fail;
 		}
 	}
 
-	import_guid(&mgr->mst_primary->guid, guid);
+	guid_copy(&mgr->mst_primary->guid, &guid);
 
 out_fail:
 	mutex_unlock(&mgr->lock);
-- 
2.39.2

Re: [RESEND 1/3] drm/mst: switch to guid_t type for GUID

[Daniel Vetter]

On Mon, Aug 12, 2024 at 03:23:10PM +0300, Jani Nikula wrote:
> The kernel has a guid_t type for GUIDs. Switch to using it, but avoid
> any functional changes here.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I didn't cross-check everything, I'll trust the compiler on this. But
functionally lgtm

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reading code a bit I did wonder whether we could have send/receive macros
that just work for compile-time statically sized types ... but not even
kmalloc is there yet I think, at least haven't seen anything.
-Sima

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 67 +++++++++++--------
>  include/drm/display/drm_dp_mst_helper.h       | 12 ++--
>  3 files changed, 45 insertions(+), 36 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 7e7929f24ae4..72c10fc2c890 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2610,7 +2610,7 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>  		}
>  	}
>  
> -	memcpy(mgr->mst_primary->guid, guid, 16);
> +	import_guid(&mgr->mst_primary->guid, guid);
>  
>  out_fail:
>  	mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 379a449a28a2..39f1dc45004e 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -89,7 +89,7 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
>  					   struct drm_dp_mst_branch *mstb,
>  					   struct drm_dp_mst_port *port);
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> -				 u8 *guid);
> +				 guid_t *guid);
>  
>  static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
>  static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
> @@ -801,7 +801,7 @@ static bool drm_dp_sideband_parse_link_address(const struct drm_dp_mst_topology_
>  	int idx = 1;
>  	int i;
>  
> -	memcpy(repmsg->u.link_addr.guid, &raw->msg[idx], 16);
> +	import_guid(&repmsg->u.link_addr.guid, &raw->msg[idx]);
>  	idx += 16;
>  	repmsg->u.link_addr.nports = raw->msg[idx] & 0xf;
>  	idx++;
> @@ -829,7 +829,7 @@ static bool drm_dp_sideband_parse_link_address(const struct drm_dp_mst_topology_
>  			idx++;
>  			if (idx > raw->curlen)
>  				goto fail_len;
> -			memcpy(repmsg->u.link_addr.ports[i].peer_guid, &raw->msg[idx], 16);
> +			import_guid(&repmsg->u.link_addr.ports[i].peer_guid, &raw->msg[idx]);
>  			idx += 16;
>  			if (idx > raw->curlen)
>  				goto fail_len;
> @@ -1029,7 +1029,7 @@ static bool drm_dp_sideband_parse_reply(const struct drm_dp_mst_topology_mgr *mg
>  	msg->req_type = (raw->msg[0] & 0x7f);
>  
>  	if (msg->reply_type == DP_SIDEBAND_REPLY_NAK) {
> -		memcpy(msg->u.nak.guid, &raw->msg[1], 16);
> +		import_guid(&msg->u.nak.guid, &raw->msg[1]);
>  		msg->u.nak.reason = raw->msg[17];
>  		msg->u.nak.nak_data = raw->msg[18];
>  		return false;
> @@ -1078,7 +1078,7 @@ drm_dp_sideband_parse_connection_status_notify(const struct drm_dp_mst_topology_
>  	if (idx > raw->curlen)
>  		goto fail_len;
>  
> -	memcpy(msg->u.conn_stat.guid, &raw->msg[idx], 16);
> +	import_guid(&msg->u.conn_stat.guid, &raw->msg[idx]);
>  	idx += 16;
>  	if (idx > raw->curlen)
>  		goto fail_len;
> @@ -1107,7 +1107,7 @@ static bool drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
>  	if (idx > raw->curlen)
>  		goto fail_len;
>  
> -	memcpy(msg->u.resource_stat.guid, &raw->msg[idx], 16);
> +	import_guid(&msg->u.resource_stat.guid, &raw->msg[idx]);
>  	idx += 16;
>  	if (idx > raw->curlen)
>  		goto fail_len;
> @@ -2174,20 +2174,24 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
>  				      offset, size, buffer);
>  }
>  
> -static int drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> +static int drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, guid_t *guid)
>  {
>  	int ret = 0;
>  
> -	memcpy(mstb->guid, guid, 16);
> +	guid_copy(&mstb->guid, guid);
> +
> +	if (!drm_dp_validate_guid(mstb->mgr, &mstb->guid)) {
> +		u8 buf[UUID_SIZE];
> +
> +		export_guid(buf, &mstb->guid);
>  
> -	if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
>  		if (mstb->port_parent) {
>  			ret = drm_dp_send_dpcd_write(mstb->mgr,
>  						     mstb->port_parent,
> -						     DP_GUID, 16, mstb->guid);
> +						     DP_GUID, sizeof(buf), buf);
>  		} else {
>  			ret = drm_dp_dpcd_write(mstb->mgr->aux,
> -						DP_GUID, mstb->guid, 16);
> +						DP_GUID, buf, sizeof(buf));
>  		}
>  	}
>  
> @@ -2567,9 +2571,9 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_
>  	return mstb;
>  }
>  
> -static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
> -	struct drm_dp_mst_branch *mstb,
> -	const uint8_t *guid)
> +static struct drm_dp_mst_branch *
> +get_mst_branch_device_by_guid_helper(struct drm_dp_mst_branch *mstb,
> +				     const guid_t *guid)
>  {
>  	struct drm_dp_mst_branch *found_mstb;
>  	struct drm_dp_mst_port *port;
> @@ -2577,10 +2581,9 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
>  	if (!mstb)
>  		return NULL;
>  
> -	if (memcmp(mstb->guid, guid, 16) == 0)
> +	if (guid_equal(&mstb->guid, guid))
>  		return mstb;
>  
> -
>  	list_for_each_entry(port, &mstb->ports, next) {
>  		found_mstb = get_mst_branch_device_by_guid_helper(port->mstb, guid);
>  
> @@ -2593,7 +2596,7 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper(
>  
>  static struct drm_dp_mst_branch *
>  drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr,
> -				     const uint8_t *guid)
> +				     const guid_t *guid)
>  {
>  	struct drm_dp_mst_branch *mstb;
>  	int ret;
> @@ -2695,17 +2698,20 @@ static void drm_dp_mst_queue_probe_work(struct drm_dp_mst_topology_mgr *mgr)
>  }
>  
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> -				 u8 *guid)
> +				 guid_t *guid)
>  {
>  	u64 salt;
> +	u8 buf[UUID_SIZE];
>  
> -	if (memchr_inv(guid, 0, 16))
> +	if (!guid_is_null(guid))
>  		return true;
>  
>  	salt = get_jiffies_64();
>  
> -	memcpy(&guid[0], &salt, sizeof(u64));
> -	memcpy(&guid[8], &salt, sizeof(u64));
> +	memcpy(&buf[0], &salt, sizeof(u64));
> +	memcpy(&buf[8], &salt, sizeof(u64));
> +
> +	import_guid(guid, buf);
>  
>  	return false;
>  }
> @@ -2945,7 +2951,7 @@ static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>  	drm_dbg_kms(mgr->dev, "link address reply: %d\n", reply->nports);
>  	drm_dp_dump_link_address(mgr, reply);
>  
> -	ret = drm_dp_check_mstb_guid(mstb, reply->guid);
> +	ret = drm_dp_check_mstb_guid(mstb, &reply->guid);
>  	if (ret) {
>  		char buf[64];
>  
> @@ -3799,8 +3805,9 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
>  int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
>  				   bool sync)
>  {
> +	u8 buf[UUID_SIZE];
> +	guid_t guid;
>  	int ret;
> -	u8 guid[16];
>  
>  	mutex_lock(&mgr->lock);
>  	if (!mgr->mst_primary)
> @@ -3821,13 +3828,15 @@ int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
>  	}
>  
>  	/* Some hubs forget their guids after they resume */
> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> -	if (ret != 16) {
> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
> +	if (ret != sizeof(buf)) {
>  		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>  		goto out_fail;
>  	}
>  
> -	ret = drm_dp_check_mstb_guid(mgr->mst_primary, guid);
> +	import_guid(&guid, buf);
> +
> +	ret = drm_dp_check_mstb_guid(mgr->mst_primary, &guid);
>  	if (ret) {
>  		drm_dbg_kms(mgr->dev, "check mstb failed - undocked during suspend?\n");
>  		goto out_fail;
> @@ -4005,12 +4014,12 @@ drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr,
>  	bool hotplug = false, dowork = false;
>  
>  	if (hdr->broadcast) {
> -		const u8 *guid = NULL;
> +		const guid_t *guid = NULL;
>  
>  		if (msg->req_type == DP_CONNECTION_STATUS_NOTIFY)
> -			guid = msg->u.conn_stat.guid;
> +			guid = &msg->u.conn_stat.guid;
>  		else if (msg->req_type == DP_RESOURCE_STATUS_NOTIFY)
> -			guid = msg->u.resource_stat.guid;
> +			guid = &msg->u.resource_stat.guid;
>  
>  		if (guid)
>  			mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid);
> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> index 02b037d3a93f..f6a1cbb0f600 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -244,18 +244,18 @@ struct drm_dp_mst_branch {
>  	bool link_address_sent;
>  
>  	/* global unique identifier to identify branch devices */
> -	u8 guid[16];
> +	guid_t guid;
>  };
>  
>  
>  struct drm_dp_nak_reply {
> -	u8 guid[16];
> +	guid_t guid;
>  	u8 reason;
>  	u8 nak_data;
>  };
>  
>  struct drm_dp_link_address_ack_reply {
> -	u8 guid[16];
> +	guid_t guid;
>  	u8 nports;
>  	struct drm_dp_link_addr_reply_port {
>  		bool input_port;
> @@ -265,7 +265,7 @@ struct drm_dp_link_address_ack_reply {
>  		bool ddps;
>  		bool legacy_device_plug_status;
>  		u8 dpcd_revision;
> -		u8 peer_guid[16];
> +		guid_t peer_guid;
>  		u8 num_sdp_streams;
>  		u8 num_sdp_stream_sinks;
>  	} ports[16];
> @@ -348,7 +348,7 @@ struct drm_dp_allocate_payload_ack_reply {
>  };
>  
>  struct drm_dp_connection_status_notify {
> -	u8 guid[16];
> +	guid_t guid;
>  	u8 port_number;
>  	bool legacy_device_plug_status;
>  	bool displayport_device_plug_status;
> @@ -425,7 +425,7 @@ struct drm_dp_query_payload {
>  
>  struct drm_dp_resource_status_notify {
>  	u8 port_number;
> -	u8 guid[16];
> +	guid_t guid;
>  	u16 available_pbn;
>  };
>  
> -- 
> 2.39.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RESEND 2/3] drm/mst: switch to guid_gen() to generate valid GUIDs

[Daniel Vetter]

On Mon, Aug 12, 2024 at 03:23:11PM +0300, Jani Nikula wrote:
> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
> RFC 4122 compliant GUIDs.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Read a bit the RFC, definitely sounds better than stuffing jiffies into
the guid ...

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 39f1dc45004e..38a9a1441e62 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -2700,18 +2700,10 @@ static void drm_dp_mst_queue_probe_work(struct drm_dp_mst_topology_mgr *mgr)
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
>  				 guid_t *guid)
>  {
> -	u64 salt;
> -	u8 buf[UUID_SIZE];
> -
>  	if (!guid_is_null(guid))
>  		return true;
>  
> -	salt = get_jiffies_64();
> -
> -	memcpy(&buf[0], &salt, sizeof(u64));
> -	memcpy(&buf[8], &salt, sizeof(u64));
> -
> -	import_guid(guid, buf);
> +	guid_gen(guid);
>  
>  	return false;
>  }
> -- 
> 2.39.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Daniel Vetter]

On Mon, Aug 12, 2024 at 03:23:12PM +0300, Jani Nikula wrote:
> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
> RFC 4122 compliant GUIDs.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> Side note, it baffles me why amdgpu has a copy of this instead of
> plumbing it into drm mst code.

Yeah ec5fa9fcdeca ("drm/amd/display: Adjust the MST resume flow") promised
a follow-up, but that seems to have never materialized. Really should
materialize though. Patch lgtm

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>  1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>  
>  static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>  {
> +	u8 buf[UUID_SIZE];
> +	guid_t guid;
>  	int ret;
> -	u8 guid[16];
> -	u64 tmp64;
>  
>  	mutex_lock(&mgr->lock);
>  	if (!mgr->mst_primary)
> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>  	}
>  
>  	/* Some hubs forget their guids after they resume */
> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> -	if (ret != 16) {
> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
> +	if (ret != sizeof(buf)) {
>  		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>  		goto out_fail;
>  	}
>  
> -	if (memchr_inv(guid, 0, 16) == NULL) {
> -		tmp64 = get_jiffies_64();
> -		memcpy(&guid[0], &tmp64, sizeof(u64));
> -		memcpy(&guid[8], &tmp64, sizeof(u64));
> +	import_guid(&guid, buf);
>  
> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
> +	if (guid_is_null(&guid)) {
> +		guid_gen(&guid);
> +		export_guid(buf, &guid);
>  
> -		if (ret != 16) {
> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
> +
> +		if (ret != sizeof(buf)) {
>  			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>  			goto out_fail;
>  		}
>  	}
>  
> -	import_guid(&mgr->mst_primary->guid, guid);
> +	guid_copy(&mgr->mst_primary->guid, &guid);
>  
>  out_fail:
>  	mutex_unlock(&mgr->lock);
> -- 
> 2.39.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

On Wed, 28 Aug 2024, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Mon, Aug 12, 2024 at 03:23:12PM +0300, Jani Nikula wrote:
>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>> RFC 4122 compliant GUIDs.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> ---
>> 
>> Side note, it baffles me why amdgpu has a copy of this instead of
>> plumbing it into drm mst code.
>
> Yeah ec5fa9fcdeca ("drm/amd/display: Adjust the MST resume flow") promised
> a follow-up, but that seems to have never materialized. Really should
> materialize though. Patch lgtm
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks!

Cc: AMD folks, ack for merging the series via drm-misc-next?

BR,
Jani.


>
>
>> ---
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>>  1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>>  
>>  static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>  {
>> +	u8 buf[UUID_SIZE];
>> +	guid_t guid;
>>  	int ret;
>> -	u8 guid[16];
>> -	u64 tmp64;
>>  
>>  	mutex_lock(&mgr->lock);
>>  	if (!mgr->mst_primary)
>> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>  	}
>>  
>>  	/* Some hubs forget their guids after they resume */
>> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
>> -	if (ret != 16) {
>> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
>> +	if (ret != sizeof(buf)) {
>>  		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>>  		goto out_fail;
>>  	}
>>  
>> -	if (memchr_inv(guid, 0, 16) == NULL) {
>> -		tmp64 = get_jiffies_64();
>> -		memcpy(&guid[0], &tmp64, sizeof(u64));
>> -		memcpy(&guid[8], &tmp64, sizeof(u64));
>> +	import_guid(&guid, buf);
>>  
>> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
>> +	if (guid_is_null(&guid)) {
>> +		guid_gen(&guid);
>> +		export_guid(buf, &guid);
>>  
>> -		if (ret != 16) {
>> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
>> +
>> +		if (ret != sizeof(buf)) {
>>  			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>>  			goto out_fail;
>>  		}
>>  	}
>>  
>> -	import_guid(&mgr->mst_primary->guid, guid);
>> +	guid_copy(&mgr->mst_primary->guid, &guid);
>>  
>>  out_fail:
>>  	mutex_unlock(&mgr->lock);
>> -- 
>> 2.39.2
>> 

-- 
Jani Nikula, Intel

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Hamza Mahfooz]

On 8/12/24 08:23, Jani Nikula wrote:
> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
> RFC 4122 compliant GUIDs.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---

Acked-by: Hamza Mahfooz <hamza.mahfooz@amd.com>

I would prefer to take this series through the amdgpu tree though,
assuming nobody minds.

> 
> Side note, it baffles me why amdgpu has a copy of this instead of
> plumbing it into drm mst code.
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>   1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>   
>   static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>   {
> +	u8 buf[UUID_SIZE];
> +	guid_t guid;
>   	int ret;
> -	u8 guid[16];
> -	u64 tmp64;
>   
>   	mutex_lock(&mgr->lock);
>   	if (!mgr->mst_primary)
> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>   	}
>   
>   	/* Some hubs forget their guids after they resume */
> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> -	if (ret != 16) {
> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
> +	if (ret != sizeof(buf)) {
>   		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>   		goto out_fail;
>   	}
>   
> -	if (memchr_inv(guid, 0, 16) == NULL) {
> -		tmp64 = get_jiffies_64();
> -		memcpy(&guid[0], &tmp64, sizeof(u64));
> -		memcpy(&guid[8], &tmp64, sizeof(u64));
> +	import_guid(&guid, buf);
>   
> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
> +	if (guid_is_null(&guid)) {
> +		guid_gen(&guid);
> +		export_guid(buf, &guid);
>   
> -		if (ret != 16) {
> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
> +
> +		if (ret != sizeof(buf)) {
>   			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>   			goto out_fail;
>   		}
>   	}
>   
> -	import_guid(&mgr->mst_primary->guid, guid);
> +	guid_copy(&mgr->mst_primary->guid, &guid);
>   
>   out_fail:
>   	mutex_unlock(&mgr->lock);
-- 
Hamza

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Alex Deucher]

On Wed, Aug 28, 2024 at 9:53 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
> On Wed, 28 Aug 2024, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Mon, Aug 12, 2024 at 03:23:12PM +0300, Jani Nikula wrote:
> >> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
> >> RFC 4122 compliant GUIDs.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>
> >> ---
> >>
> >> Side note, it baffles me why amdgpu has a copy of this instead of
> >> plumbing it into drm mst code.
> >
> > Yeah ec5fa9fcdeca ("drm/amd/display: Adjust the MST resume flow") promised
> > a follow-up, but that seems to have never materialized. Really should
> > materialize though. Patch lgtm
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks!
>
> Cc: AMD folks, ack for merging the series via drm-misc-next?

Unless Harry has any objections,
Acked-by: Alex Deucher <alexander.deucher@amd.com>

>
> BR,
> Jani.
>
>
> >
> >
> >> ---
> >>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
> >>  1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
> >>
> >>  static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
> >>  {
> >> +    u8 buf[UUID_SIZE];
> >> +    guid_t guid;
> >>      int ret;
> >> -    u8 guid[16];
> >> -    u64 tmp64;
> >>
> >>      mutex_lock(&mgr->lock);
> >>      if (!mgr->mst_primary)
> >> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
> >>      }
> >>
> >>      /* Some hubs forget their guids after they resume */
> >> -    ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> >> -    if (ret != 16) {
> >> +    ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
> >> +    if (ret != sizeof(buf)) {
> >>              drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
> >>              goto out_fail;
> >>      }
> >>
> >> -    if (memchr_inv(guid, 0, 16) == NULL) {
> >> -            tmp64 = get_jiffies_64();
> >> -            memcpy(&guid[0], &tmp64, sizeof(u64));
> >> -            memcpy(&guid[8], &tmp64, sizeof(u64));
> >> +    import_guid(&guid, buf);
> >>
> >> -            ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
> >> +    if (guid_is_null(&guid)) {
> >> +            guid_gen(&guid);
> >> +            export_guid(buf, &guid);
> >>
> >> -            if (ret != 16) {
> >> +            ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
> >> +
> >> +            if (ret != sizeof(buf)) {
> >>                      drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
> >>                      goto out_fail;
> >>              }
> >>      }
> >>
> >> -    import_guid(&mgr->mst_primary->guid, guid);
> >> +    guid_copy(&mgr->mst_primary->guid, &guid);
> >>
> >>  out_fail:
> >>      mutex_unlock(&mgr->lock);
> >> --
> >> 2.39.2
> >>
>
> --
> Jani Nikula, Intel

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

On Wed, 28 Aug 2024, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
> On 8/12/24 08:23, Jani Nikula wrote:
>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>> RFC 4122 compliant GUIDs.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> ---
>
> Acked-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>
> I would prefer to take this series through the amdgpu tree though,
> assuming nobody minds.

How long is it going to take for that to get synced back to
drm-misc-next though?

BR,
Jani.


>
>> 
>> Side note, it baffles me why amdgpu has a copy of this instead of
>> plumbing it into drm mst code.
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>>   1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>>   
>>   static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>   {
>> +	u8 buf[UUID_SIZE];
>> +	guid_t guid;
>>   	int ret;
>> -	u8 guid[16];
>> -	u64 tmp64;
>>   
>>   	mutex_lock(&mgr->lock);
>>   	if (!mgr->mst_primary)
>> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>   	}
>>   
>>   	/* Some hubs forget their guids after they resume */
>> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
>> -	if (ret != 16) {
>> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
>> +	if (ret != sizeof(buf)) {
>>   		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>>   		goto out_fail;
>>   	}
>>   
>> -	if (memchr_inv(guid, 0, 16) == NULL) {
>> -		tmp64 = get_jiffies_64();
>> -		memcpy(&guid[0], &tmp64, sizeof(u64));
>> -		memcpy(&guid[8], &tmp64, sizeof(u64));
>> +	import_guid(&guid, buf);
>>   
>> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
>> +	if (guid_is_null(&guid)) {
>> +		guid_gen(&guid);
>> +		export_guid(buf, &guid);
>>   
>> -		if (ret != 16) {
>> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
>> +
>> +		if (ret != sizeof(buf)) {
>>   			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>>   			goto out_fail;
>>   		}
>>   	}
>>   
>> -	import_guid(&mgr->mst_primary->guid, guid);
>> +	guid_copy(&mgr->mst_primary->guid, &guid);
>>   
>>   out_fail:
>>   	mutex_unlock(&mgr->lock);

-- 
Jani Nikula, Intel

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Harry Wentland]

On 2024-08-28 09:58, Alex Deucher wrote:
> On Wed, Aug 28, 2024 at 9:53 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> On Wed, 28 Aug 2024, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> On Mon, Aug 12, 2024 at 03:23:12PM +0300, Jani Nikula wrote:
>>>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>>>> RFC 4122 compliant GUIDs.
>>>>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>
>>>> ---
>>>>
>>>> Side note, it baffles me why amdgpu has a copy of this instead of
>>>> plumbing it into drm mst code.
>>>
>>> Yeah ec5fa9fcdeca ("drm/amd/display: Adjust the MST resume flow") promised
>>> a follow-up, but that seems to have never materialized. Really should
>>> materialize though. Patch lgtm
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Thanks!
>>
>> Cc: AMD folks, ack for merging the series via drm-misc-next?
> 
> Unless Harry has any objections,
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 

Acked-by: Harry Wentland <harry.wentland@amd.com>

Harry

>>
>> BR,
>> Jani.
>>
>>
>>>
>>>
>>>> ---
>>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>>>>  1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>>>>
>>>>  static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>>  {
>>>> +    u8 buf[UUID_SIZE];
>>>> +    guid_t guid;
>>>>      int ret;
>>>> -    u8 guid[16];
>>>> -    u64 tmp64;
>>>>
>>>>      mutex_lock(&mgr->lock);
>>>>      if (!mgr->mst_primary)
>>>> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>>      }
>>>>
>>>>      /* Some hubs forget their guids after they resume */
>>>> -    ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
>>>> -    if (ret != 16) {
>>>> +    ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
>>>> +    if (ret != sizeof(buf)) {
>>>>              drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>>>>              goto out_fail;
>>>>      }
>>>>
>>>> -    if (memchr_inv(guid, 0, 16) == NULL) {
>>>> -            tmp64 = get_jiffies_64();
>>>> -            memcpy(&guid[0], &tmp64, sizeof(u64));
>>>> -            memcpy(&guid[8], &tmp64, sizeof(u64));
>>>> +    import_guid(&guid, buf);
>>>>
>>>> -            ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
>>>> +    if (guid_is_null(&guid)) {
>>>> +            guid_gen(&guid);
>>>> +            export_guid(buf, &guid);
>>>>
>>>> -            if (ret != 16) {
>>>> +            ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
>>>> +
>>>> +            if (ret != sizeof(buf)) {
>>>>                      drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>>>>                      goto out_fail;
>>>>              }
>>>>      }
>>>>
>>>> -    import_guid(&mgr->mst_primary->guid, guid);
>>>> +    guid_copy(&mgr->mst_primary->guid, &guid);
>>>>
>>>>  out_fail:
>>>>      mutex_unlock(&mgr->lock);
>>>> --
>>>> 2.39.2
>>>>
>>
>> --
>> Jani Nikula, Intel

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

On Wed, 28 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 28 Aug 2024, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>> On 8/12/24 08:23, Jani Nikula wrote:
>>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>>> RFC 4122 compliant GUIDs.
>>> 
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> 
>>> ---
>>
>> Acked-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>>
>> I would prefer to take this series through the amdgpu tree though,
>> assuming nobody minds.
>
> How long is it going to take for that to get synced back to
> drm-misc-next though?

Also getting acks from Alex and Harry to merge via drm-misc-next.

BR,
Jani.


>
> BR,
> Jani.
>
>
>>
>>> 
>>> Side note, it baffles me why amdgpu has a copy of this instead of
>>> plumbing it into drm mst code.
>>> ---
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>>>   1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>>>   
>>>   static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>   {
>>> +	u8 buf[UUID_SIZE];
>>> +	guid_t guid;
>>>   	int ret;
>>> -	u8 guid[16];
>>> -	u64 tmp64;
>>>   
>>>   	mutex_lock(&mgr->lock);
>>>   	if (!mgr->mst_primary)
>>> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>   	}
>>>   
>>>   	/* Some hubs forget their guids after they resume */
>>> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
>>> -	if (ret != 16) {
>>> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
>>> +	if (ret != sizeof(buf)) {
>>>   		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>>>   		goto out_fail;
>>>   	}
>>>   
>>> -	if (memchr_inv(guid, 0, 16) == NULL) {
>>> -		tmp64 = get_jiffies_64();
>>> -		memcpy(&guid[0], &tmp64, sizeof(u64));
>>> -		memcpy(&guid[8], &tmp64, sizeof(u64));
>>> +	import_guid(&guid, buf);
>>>   
>>> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
>>> +	if (guid_is_null(&guid)) {
>>> +		guid_gen(&guid);
>>> +		export_guid(buf, &guid);
>>>   
>>> -		if (ret != 16) {
>>> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
>>> +
>>> +		if (ret != sizeof(buf)) {
>>>   			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>>>   			goto out_fail;
>>>   		}
>>>   	}
>>>   
>>> -	import_guid(&mgr->mst_primary->guid, guid);
>>> +	guid_copy(&mgr->mst_primary->guid, &guid);
>>>   
>>>   out_fail:
>>>   	mutex_unlock(&mgr->lock);

-- 
Jani Nikula, Intel

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Hamza Mahfooz]

On 8/28/24 10:06, Jani Nikula wrote:
> On Wed, 28 Aug 2024, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>> On 8/12/24 08:23, Jani Nikula wrote:
>>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>>> RFC 4122 compliant GUIDs.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> ---
>>
>> Acked-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>>
>> I would prefer to take this series through the amdgpu tree though,
>> assuming nobody minds.
> 
> How long is it going to take for that to get synced back to
> drm-misc-next though?

It might take awhile, so it's probably best to merge it through
drm-misc-next.

> 
> BR,
> Jani.
> 
> 
>>
>>>
>>> Side note, it baffles me why amdgpu has a copy of this instead of
>>> plumbing it into drm mst code.
>>> ---
>>>    .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++++---------
>>>    1 file changed, 12 insertions(+), 11 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 72c10fc2c890..ce05e7e2a383 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -2568,9 +2568,9 @@ static int dm_late_init(void *handle)
>>>    
>>>    static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>    {
>>> +	u8 buf[UUID_SIZE];
>>> +	guid_t guid;
>>>    	int ret;
>>> -	u8 guid[16];
>>> -	u64 tmp64;
>>>    
>>>    	mutex_lock(&mgr->lock);
>>>    	if (!mgr->mst_primary)
>>> @@ -2591,26 +2591,27 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
>>>    	}
>>>    
>>>    	/* Some hubs forget their guids after they resume */
>>> -	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
>>> -	if (ret != 16) {
>>> +	ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, buf, sizeof(buf));
>>> +	if (ret != sizeof(buf)) {
>>>    		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>>>    		goto out_fail;
>>>    	}
>>>    
>>> -	if (memchr_inv(guid, 0, 16) == NULL) {
>>> -		tmp64 = get_jiffies_64();
>>> -		memcpy(&guid[0], &tmp64, sizeof(u64));
>>> -		memcpy(&guid[8], &tmp64, sizeof(u64));
>>> +	import_guid(&guid, buf);
>>>    
>>> -		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16);
>>> +	if (guid_is_null(&guid)) {
>>> +		guid_gen(&guid);
>>> +		export_guid(buf, &guid);
>>>    
>>> -		if (ret != 16) {
>>> +		ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, buf, sizeof(buf));
>>> +
>>> +		if (ret != sizeof(buf)) {
>>>    			drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n");
>>>    			goto out_fail;
>>>    		}
>>>    	}
>>>    
>>> -	import_guid(&mgr->mst_primary->guid, guid);
>>> +	guid_copy(&mgr->mst_primary->guid, &guid);
>>>    
>>>    out_fail:
>>>    	mutex_unlock(&mgr->lock);
> 
-- 
Hamza

Re: [RESEND 3/3] drm/amd/display: switch to guid_gen() to generate valid GUIDs

[Jani Nikula]

On Wed, 28 Aug 2024, Harry Wentland <harry.wentland@amd.com> wrote:
> On 2024-08-28 09:58, Alex Deucher wrote:
>> On Wed, Aug 28, 2024 at 9:53 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>>
>>> On Wed, 28 Aug 2024, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>> On Mon, Aug 12, 2024 at 03:23:12PM +0300, Jani Nikula wrote:
>>>>> Instead of just smashing jiffies into a GUID, use guid_gen() to generate
>>>>> RFC 4122 compliant GUIDs.
>>>>>
>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>>
>>>>> ---
>>>>>
>>>>> Side note, it baffles me why amdgpu has a copy of this instead of
>>>>> plumbing it into drm mst code.
>>>>
>>>> Yeah ec5fa9fcdeca ("drm/amd/display: Adjust the MST resume flow") promised
>>>> a follow-up, but that seems to have never materialized. Really should
>>>> materialize though. Patch lgtm
>>>>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>> Thanks!
>>>
>>> Cc: AMD folks, ack for merging the series via drm-misc-next?
>> 
>> Unless Harry has any objections,
>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>> 
>
> Acked-by: Harry Wentland <harry.wentland@amd.com>

Thanks for the reviews and acks, pushed the series to drm-misc-next.

BR,
Jani.


-- 
Jani Nikula, Intel