Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume

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

On Tue, 14 Jun 2022, Imre Deak <imre.deak@intel.com> wrote:
> On Tue, Jun 14, 2022 at 03:32:59PM +0300, Jani Nikula wrote:
>> On Tue, 14 Jun 2022, Imre Deak <imre.deak@intel.com> wrote:
>> > The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
>> > from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
>> > registers are not read subsequently also as required.
>> 
>> Does it actually change the behaviour depending on whether the dpcd is
>> read or not, or is this just about the resume path overwriting mgr->dpcd
>> with stuff from DP_DPCD_REV?
>
> Yes, the reading out DP_DP13_DPCD_REV has a side-effect, see
> https://gitlab.freedesktop.org/drm/intel/-/issues/5292#note_1343399

:o

Okay, the commit message is fine as-is.

>
>> drm_dp_mst_topology_mgr_set_mst() does use drm_dp_read_dpcd_caps() for
>> reading the caps, which would normally set mgr->dpcd from
>> DP_DP13_DPCD_REV.
>
> Right, but at that point DP_DP13_DPCD_REV returns HBR2 w/o this change.
>
>> BR,
>> Jani.
>> 
>> >
>> > Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
>> > done during connector detection. While at it also fix up the same call
>> > in drm_dp_mst_dump_topology().
>> >
>> > Cc: Lyude Paul <lyude@redhat.com>
>> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
>> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> > ---
>> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > index 67b3b9697da7f..18f2b6075b780 100644
>> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
>> >  	if (!mgr->mst_primary)
>> >  		goto out_fail;
>> >  
>> > -	ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
>> > -			       DP_RECEIVER_CAP_SIZE);
>> > -	if (ret != DP_RECEIVER_CAP_SIZE) {
>> > +	if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
>> >  		drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n");
>> >  		goto out_fail;
>> >  	}
>> > @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>> >  		u8 buf[DP_PAYLOAD_TABLE_SIZE];
>> >  		int ret;
>> >  
>> > -		ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
>> > -		if (ret) {
>> > +		if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
>> >  			seq_printf(m, "dpcd read failed\n");
>> >  			goto out;
>> >  		}
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center