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

[Jani Nikula <jani.nikula@intel.com>]

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