[Intel-gfx] [PATCH] drm/dp_mst: Use the correct DPCD space in Synaptics quirk

[Lyude Paul]

mhhhhhhh, probably should do this a bit differently. Also adding Koba since
this involves using extended DPCD caps in the MST topology mgr.

On Fri, 2021-04-23 at 16:05 -0400, Nikola Cornij wrote:
> [why]
> Two conditions that were part of this fix did not go through:
> 
> 1. DPCD revision has to be v1.4 and up
>    This was because wrong DPCD space was used to get the values
> 
> 2. Downstream port must not be VGA converter
>    This was because for MST the topology manager AUX has to be used,
>    due to the way MST AUX reads are done.
> 
> [how]
> - Use Extended Receiver Capability DPCD space if
> DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is set
> - Use MST topology manager AUX to get port DPCD
> 
> Signed-off-by: Nikola Cornij <nikola.cornij at amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 33 ++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index de5124ce42cb..69fd16ce2cb3 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -5878,18 +5878,35 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct
> drm_dp_mst_port *port)
>                 return NULL;
>  
>         if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) &&
> -           port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 &&
>             port->parent == port->mgr->mst_primary) {
> -               u8 downstreamport;
> +               u8 training_aux_rd_interval = 0;
> +               u8 dpcd_rev = 0;
> +               unsigned int dpcd_caps_offset = 0;
>  
> -               if (drm_dp_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT,
> -                                    &downstreamport, 1) < 0)
> +               if (drm_dp_dpcd_read(port->mgr->aux,
> DP_TRAINING_AUX_RD_INTERVAL,
> +                                    &training_aux_rd_interval, 1) < 1)
>                         return NULL;
>  
> -               if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) &&
> -                  ((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK)
> -                    != DP_DWN_STRM_PORT_TYPE_ANALOG))
> -                       return port->mgr->aux;
> +               /* If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is set, the
> Extended Receiver Capability field has to be used */
> +               if (training_aux_rd_interval &
> DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT)
> +                       dpcd_caps_offset = 0x02200;

If we need to read the extended DPCD caps then we should give another go at
teaching the MST helpers how to read them by default instead of hacking around
it like this. We attempted to do this in the past:

https://patchwork.kernel.org/project/dri-devel/patch/20200911034431.29059-1-koba.ko@canonical.com/

But it ended up causing issues that I didn't have the time to look into, so we
had to revert it:

https://patchwork.freedesktop.org/series/83408/

Looking at this now I think the proper solution here shouldn't be very
difficult. In order to make it so drm_dp_mst_topology_mgr uses
drm_dp_dpcd_read_caps() we just need to make it so that drivers need to
provide their maximum link rate and lane count when setting up
drm_dp_mst_topology_set_mst(). From there, we can convert
drm_dp_mst_topology_mgr to drm_dp_dpcd_read_caps() and simply limit the
maximum link rate/lane count we cache in mgr->dpcd to the max link rate/lane
count limitations provided by the DRM driver.

Would you write up some patches to do this instead?

> +
> +               if (drm_dp_dpcd_read(port->mgr->aux, dpcd_caps_offset +
> DP_DPCD_REV,
> +                                    &dpcd_rev, 1) < 1)
> +                       return NULL;
> +
> +               if (dpcd_rev >= DP_DPCD_REV_14) {
> +                       u8 downstreamport = 0;
> +
> +                       if (drm_dp_dpcd_read(port->mgr->aux, dpcd_caps_offset
> + DP_DOWNSTREAMPORT_PRESENT,
> +                                            &downstreamport, 1) < 1)
> +                               return NULL;
> +
> +                       if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) &&
> +                          ((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK)
> +                            != DP_DWN_STRM_PORT_TYPE_ANALOG))
> +                               return port->mgr->aux;
> +               }
>         }
>  
>         /*

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat