| Message ID | 20240812122312.1567046-3-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,1/3] drm/mst: switch to guid_t type for GUID | expand |
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 >
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 >>
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);
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
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);
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
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);
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); >
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.
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);
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(-)