* [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-08-27 5:30 ` Koba Ko
0 siblings, 0 replies; 14+ messages in thread
From: Koba Ko @ 2020-08-27 5:30 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel, linux-kernel
Cc: anthony.wong
Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
get the slower speed even the mst hub can run in the faster speed.
As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to
get the faster capability.
If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
Signed-off-by: Koba Ko <koba.ko@canonical.com>
---
drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 67dd72ea200e..3b84c6801281 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count)
int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state)
{
int ret = 0;
+ u8 dpcd_ext = 0;
+ unsigned int dpcd_offset = 0;
struct drm_dp_mst_branch *mstb = NULL;
mutex_lock(&mgr->payload_lock);
@@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
struct drm_dp_payload reset_pay;
WARN_ON(mgr->mst_primary);
+ drm_dp_dpcd_read(mgr->aux,
+ DP_TRAINING_AUX_RD_INTERVAL,
+ &dpcd_ext, sizeof(dpcd_ext));
+
+ dpcd_offset =
+ ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV : DP_DPCD_REV);
/* get dpcd info */
- ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
+ ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
if (ret != DP_RECEIVER_CAP_SIZE) {
DRM_DEBUG_KMS("failed to read DPCD\n");
goto out_unlock;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-08-27 5:30 ` Koba Ko
0 siblings, 0 replies; 14+ messages in thread
From: Koba Ko @ 2020-08-27 5:30 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel, linux-kernel
Cc: anthony.wong
Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
get the slower speed even the mst hub can run in the faster speed.
As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to
get the faster capability.
If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
Signed-off-by: Koba Ko <koba.ko@canonical.com>
---
drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 67dd72ea200e..3b84c6801281 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count)
int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state)
{
int ret = 0;
+ u8 dpcd_ext = 0;
+ unsigned int dpcd_offset = 0;
struct drm_dp_mst_branch *mstb = NULL;
mutex_lock(&mgr->payload_lock);
@@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
struct drm_dp_payload reset_pay;
WARN_ON(mgr->mst_primary);
+ drm_dp_dpcd_read(mgr->aux,
+ DP_TRAINING_AUX_RD_INTERVAL,
+ &dpcd_ext, sizeof(dpcd_ext));
+
+ dpcd_offset =
+ ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV : DP_DPCD_REV);
/* get dpcd info */
- ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
+ ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
if (ret != DP_RECEIVER_CAP_SIZE) {
DRM_DEBUG_KMS("failed to read DPCD\n");
goto out_unlock;
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-08-27 5:30 ` Koba Ko
@ 2020-09-09 6:31 ` Koba Ko
-1 siblings, 0 replies; 14+ messages in thread
From: Koba Ko @ 2020-09-09 6:31 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, lyude
Cc: Anthony Wong
On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
>
> Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> get the slower speed even the mst hub can run in the faster speed.
>
> As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to
> get the faster capability.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
>
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 67dd72ea200e..3b84c6801281 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count)
> int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state)
> {
> int ret = 0;
> + u8 dpcd_ext = 0;
> + unsigned int dpcd_offset = 0;
> struct drm_dp_mst_branch *mstb = NULL;
>
> mutex_lock(&mgr->payload_lock);
> @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
> struct drm_dp_payload reset_pay;
>
> WARN_ON(mgr->mst_primary);
> + drm_dp_dpcd_read(mgr->aux,
> + DP_TRAINING_AUX_RD_INTERVAL,
> + &dpcd_ext, sizeof(dpcd_ext));
> +
> + dpcd_offset =
> + ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV : DP_DPCD_REV);
>
> /* get dpcd info */
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
> + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
> if (ret != DP_RECEIVER_CAP_SIZE) {
> DRM_DEBUG_KMS("failed to read DPCD\n");
> goto out_unlock;
> --
> 2.25.1
>
Add Lyude Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 6:31 ` Koba Ko
0 siblings, 0 replies; 14+ messages in thread
From: Koba Ko @ 2020-09-09 6:31 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, lyude
Cc: Anthony Wong
On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
>
> Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> get the slower speed even the mst hub can run in the faster speed.
>
> As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to
> get the faster capability.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
>
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 67dd72ea200e..3b84c6801281 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count)
> int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state)
> {
> int ret = 0;
> + u8 dpcd_ext = 0;
> + unsigned int dpcd_offset = 0;
> struct drm_dp_mst_branch *mstb = NULL;
>
> mutex_lock(&mgr->payload_lock);
> @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
> struct drm_dp_payload reset_pay;
>
> WARN_ON(mgr->mst_primary);
> + drm_dp_dpcd_read(mgr->aux,
> + DP_TRAINING_AUX_RD_INTERVAL,
> + &dpcd_ext, sizeof(dpcd_ext));
> +
> + dpcd_offset =
> + ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV : DP_DPCD_REV);
>
> /* get dpcd info */
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
> + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, DP_RECEIVER_CAP_SIZE);
> if (ret != DP_RECEIVER_CAP_SIZE) {
> DRM_DEBUG_KMS("failed to read DPCD\n");
> goto out_unlock;
> --
> 2.25.1
>
Add Lyude Paul
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-09-09 6:31 ` Koba Ko
@ 2020-09-09 16:33 ` Lyude Paul
-1 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 16:33 UTC (permalink / raw)
To: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List
Cc: Anthony Wong
We just added a new helper for DPCD retrieval to drm_dp_helper.c (which also
handles grabbing the extended receiver caps), we should probably make use of
it here
On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> > get the slower speed even the mst hub can run in the faster speed.
> >
> > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV
> > to
> > get the faster capability.
> > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> >
> > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 67dd72ea200e..3b84c6801281 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw,
> > u8 dp_link_count)
> > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr,
> > bool mst_state)
> > {
> > int ret = 0;
> > + u8 dpcd_ext = 0;
> > + unsigned int dpcd_offset = 0;
> > struct drm_dp_mst_branch *mstb = NULL;
> >
> > mutex_lock(&mgr->payload_lock);
> > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > drm_dp_mst_topology_mgr *mgr, bool ms
> > struct drm_dp_payload reset_pay;
> >
> > WARN_ON(mgr->mst_primary);
> > + drm_dp_dpcd_read(mgr->aux,
> > + DP_TRAINING_AUX_RD_INTERVAL,
> > + &dpcd_ext, sizeof(dpcd_ext));
> > +
> > + dpcd_offset =
> > + ((dpcd_ext &
> > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > DP_DPCD_REV);
> >
> > /* get dpcd info */
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > DP_RECEIVER_CAP_SIZE);
> > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd,
> > DP_RECEIVER_CAP_SIZE);
> > if (ret != DP_RECEIVER_CAP_SIZE) {
> > DRM_DEBUG_KMS("failed to read DPCD\n");
> > goto out_unlock;
> > --
> > 2.25.1
> >
> Add Lyude Paul
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 16:33 ` Lyude Paul
0 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 16:33 UTC (permalink / raw)
To: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List
Cc: Anthony Wong
We just added a new helper for DPCD retrieval to drm_dp_helper.c (which also
handles grabbing the extended receiver caps), we should probably make use of
it here
On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> > get the slower speed even the mst hub can run in the faster speed.
> >
> > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV
> > to
> > get the faster capability.
> > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> >
> > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 67dd72ea200e..3b84c6801281 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw,
> > u8 dp_link_count)
> > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr,
> > bool mst_state)
> > {
> > int ret = 0;
> > + u8 dpcd_ext = 0;
> > + unsigned int dpcd_offset = 0;
> > struct drm_dp_mst_branch *mstb = NULL;
> >
> > mutex_lock(&mgr->payload_lock);
> > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > drm_dp_mst_topology_mgr *mgr, bool ms
> > struct drm_dp_payload reset_pay;
> >
> > WARN_ON(mgr->mst_primary);
> > + drm_dp_dpcd_read(mgr->aux,
> > + DP_TRAINING_AUX_RD_INTERVAL,
> > + &dpcd_ext, sizeof(dpcd_ext));
> > +
> > + dpcd_offset =
> > + ((dpcd_ext &
> > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > DP_DPCD_REV);
> >
> > /* get dpcd info */
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > DP_RECEIVER_CAP_SIZE);
> > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd,
> > DP_RECEIVER_CAP_SIZE);
> > if (ret != DP_RECEIVER_CAP_SIZE) {
> > DRM_DEBUG_KMS("failed to read DPCD\n");
> > goto out_unlock;
> > --
> > 2.25.1
> >
> Add Lyude Paul
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-09-09 16:33 ` Lyude Paul
@ 2020-09-09 18:20 ` Ville Syrjälä
-1 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2020-09-09 18:20 UTC (permalink / raw)
To: Lyude Paul
Cc: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, Anthony Wong
On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> We just added a new helper for DPCD retrieval to drm_dp_helper.c (which also
> handles grabbing the extended receiver caps), we should probably make use of
> it here
Someone should really rework this whole thing so that the driver can
pass in the link rate+lane count when setting up the link. There's no
reason to assume that the source device capabilities match or exceed
the MST device caps. It would also allow the driver the dynamically
adjust these in response to link failures.
> On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> > > get the slower speed even the mst hub can run in the faster speed.
> > >
> > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV
> > > to
> > > get the faster capability.
> > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > >
> > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > ---
> > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 67dd72ea200e..3b84c6801281 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw,
> > > u8 dp_link_count)
> > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr,
> > > bool mst_state)
> > > {
> > > int ret = 0;
> > > + u8 dpcd_ext = 0;
> > > + unsigned int dpcd_offset = 0;
> > > struct drm_dp_mst_branch *mstb = NULL;
> > >
> > > mutex_lock(&mgr->payload_lock);
> > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > struct drm_dp_payload reset_pay;
> > >
> > > WARN_ON(mgr->mst_primary);
> > > + drm_dp_dpcd_read(mgr->aux,
> > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > + &dpcd_ext, sizeof(dpcd_ext));
> > > +
> > > + dpcd_offset =
> > > + ((dpcd_ext &
> > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > DP_DPCD_REV);
> > >
> > > /* get dpcd info */
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > > DP_RECEIVER_CAP_SIZE);
> > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd,
> > > DP_RECEIVER_CAP_SIZE);
> > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > goto out_unlock;
> > > --
> > > 2.25.1
> > >
> > Add Lyude Paul
> >
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 18:20 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2020-09-09 18:20 UTC (permalink / raw)
To: Lyude Paul
Cc: Thomas Zimmermann, David Airlie, Linux Kernel Mailing List,
Anthony Wong, Koba Ko, dri-devel
On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> We just added a new helper for DPCD retrieval to drm_dp_helper.c (which also
> handles grabbing the extended receiver caps), we should probably make use of
> it here
Someone should really rework this whole thing so that the driver can
pass in the link rate+lane count when setting up the link. There's no
reason to assume that the source device capabilities match or exceed
the MST device caps. It would also allow the driver the dynamically
adjust these in response to link failures.
> On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV and
> > > get the slower speed even the mst hub can run in the faster speed.
> > >
> > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV
> > > to
> > > get the faster capability.
> > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > >
> > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > ---
> > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 67dd72ea200e..3b84c6801281 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw,
> > > u8 dp_link_count)
> > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr,
> > > bool mst_state)
> > > {
> > > int ret = 0;
> > > + u8 dpcd_ext = 0;
> > > + unsigned int dpcd_offset = 0;
> > > struct drm_dp_mst_branch *mstb = NULL;
> > >
> > > mutex_lock(&mgr->payload_lock);
> > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > struct drm_dp_payload reset_pay;
> > >
> > > WARN_ON(mgr->mst_primary);
> > > + drm_dp_dpcd_read(mgr->aux,
> > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > + &dpcd_ext, sizeof(dpcd_ext));
> > > +
> > > + dpcd_offset =
> > > + ((dpcd_ext &
> > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > DP_DPCD_REV);
> > >
> > > /* get dpcd info */
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > > DP_RECEIVER_CAP_SIZE);
> > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd,
> > > DP_RECEIVER_CAP_SIZE);
> > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > goto out_unlock;
> > > --
> > > 2.25.1
> > >
> > Add Lyude Paul
> >
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-09-09 18:20 ` Ville Syrjälä
@ 2020-09-09 18:26 ` Lyude Paul
-1 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 18:26 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, Anthony Wong
On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > We just added a new helper for DPCD retrieval to drm_dp_helper.c (which
> > also
> > handles grabbing the extended receiver caps), we should probably make use
> > of
> > it here
>
> Someone should really rework this whole thing so that the driver can
> pass in the link rate+lane count when setting up the link. There's no
> reason to assume that the source device capabilities match or exceed
> the MST device caps. It would also allow the driver the dynamically
> adjust these in response to link failures.
I'm a bit confused, I also think we should pass the link rate+lane count in
(especially since it'll be very helpful for when we start optimizing PBN
handling in regards to bw constraints), but I'm not sure what you mean by
"There's no reason to assume that the source device capabilities match or
exceed the MST device caps", aren't we doing the opposite here by checking the
MST device caps?
>
> > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV
> > > > and
> > > > get the slower speed even the mst hub can run in the faster speed.
> > > >
> > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > DP_DP13_DPCD_REV
> > > > to
> > > > get the faster capability.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > >
> > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > ---
> > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index 67dd72ea200e..3b84c6801281 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > dp_link_bw,
> > > > u8 dp_link_count)
> > > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr
> > > > *mgr,
> > > > bool mst_state)
> > > > {
> > > > int ret = 0;
> > > > + u8 dpcd_ext = 0;
> > > > + unsigned int dpcd_offset = 0;
> > > > struct drm_dp_mst_branch *mstb = NULL;
> > > >
> > > > mutex_lock(&mgr->payload_lock);
> > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > struct drm_dp_payload reset_pay;
> > > >
> > > > WARN_ON(mgr->mst_primary);
> > > > + drm_dp_dpcd_read(mgr->aux,
> > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > +
> > > > + dpcd_offset =
> > > > + ((dpcd_ext &
> > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > DP_DPCD_REV);
> > > >
> > > > /* get dpcd info */
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > >dpcd,
> > > > DP_RECEIVER_CAP_SIZE);
> > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > >dpcd,
> > > > DP_RECEIVER_CAP_SIZE);
> > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > goto out_unlock;
> > > > --
> > > > 2.25.1
> > > >
> > > Add Lyude Paul
> > >
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 18:26 ` Lyude Paul
0 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 18:26 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Thomas Zimmermann, David Airlie, Linux Kernel Mailing List,
Anthony Wong, Koba Ko, dri-devel
On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > We just added a new helper for DPCD retrieval to drm_dp_helper.c (which
> > also
> > handles grabbing the extended receiver caps), we should probably make use
> > of
> > it here
>
> Someone should really rework this whole thing so that the driver can
> pass in the link rate+lane count when setting up the link. There's no
> reason to assume that the source device capabilities match or exceed
> the MST device caps. It would also allow the driver the dynamically
> adjust these in response to link failures.
I'm a bit confused, I also think we should pass the link rate+lane count in
(especially since it'll be very helpful for when we start optimizing PBN
handling in regards to bw constraints), but I'm not sure what you mean by
"There's no reason to assume that the source device capabilities match or
exceed the MST device caps", aren't we doing the opposite here by checking the
MST device caps?
>
> > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV
> > > > and
> > > > get the slower speed even the mst hub can run in the faster speed.
> > > >
> > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > DP_DP13_DPCD_REV
> > > > to
> > > > get the faster capability.
> > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > >
> > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > ---
> > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index 67dd72ea200e..3b84c6801281 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > dp_link_bw,
> > > > u8 dp_link_count)
> > > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr
> > > > *mgr,
> > > > bool mst_state)
> > > > {
> > > > int ret = 0;
> > > > + u8 dpcd_ext = 0;
> > > > + unsigned int dpcd_offset = 0;
> > > > struct drm_dp_mst_branch *mstb = NULL;
> > > >
> > > > mutex_lock(&mgr->payload_lock);
> > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > struct drm_dp_payload reset_pay;
> > > >
> > > > WARN_ON(mgr->mst_primary);
> > > > + drm_dp_dpcd_read(mgr->aux,
> > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > +
> > > > + dpcd_offset =
> > > > + ((dpcd_ext &
> > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > DP_DPCD_REV);
> > > >
> > > > /* get dpcd info */
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > >dpcd,
> > > > DP_RECEIVER_CAP_SIZE);
> > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > >dpcd,
> > > > DP_RECEIVER_CAP_SIZE);
> > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > goto out_unlock;
> > > > --
> > > > 2.25.1
> > > >
> > > Add Lyude Paul
> > >
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-09-09 18:26 ` Lyude Paul
@ 2020-09-09 18:36 ` Ville Syrjälä
-1 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2020-09-09 18:36 UTC (permalink / raw)
To: Lyude Paul
Cc: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, Anthony Wong
On Wed, Sep 09, 2020 at 02:26:11PM -0400, Lyude Paul wrote:
> On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> > On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > > We just added a new helper for DPCD retrieval to drm_dp_helper.c (which
> > > also
> > > handles grabbing the extended receiver caps), we should probably make use
> > > of
> > > it here
> >
> > Someone should really rework this whole thing so that the driver can
> > pass in the link rate+lane count when setting up the link. There's no
> > reason to assume that the source device capabilities match or exceed
> > the MST device caps. It would also allow the driver the dynamically
> > adjust these in response to link failures.
>
> I'm a bit confused, I also think we should pass the link rate+lane count in
> (especially since it'll be very helpful for when we start optimizing PBN
> handling in regards to bw constraints), but I'm not sure what you mean by
> "There's no reason to assume that the source device capabilities match or
> exceed the MST device caps", aren't we doing the opposite here by checking the
> MST device caps?
We assume only the MST device caps matter. Which is fine if the source
device caps are equal or higher. But if the source device isn't as
capable then we're going to calculate the MST things based on link bw
we can not actually achieve. End result is that we potentially try to
push too much data over the link.
I'm not really sure what actually happens if we just miscompute these
things but don't actually oversubscribe the link. Maybe the remote
end gets confused when we tell it some bogus VC parameters? Maybe it
doesn't care? Dunno.
> >
> > > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV
> > > > > and
> > > > > get the slower speed even the mst hub can run in the faster speed.
> > > > >
> > > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > > DP_DP13_DPCD_REV
> > > > > to
> > > > > get the faster capability.
> > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > > >
> > > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > > ---
> > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index 67dd72ea200e..3b84c6801281 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > > dp_link_bw,
> > > > > u8 dp_link_count)
> > > > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr
> > > > > *mgr,
> > > > > bool mst_state)
> > > > > {
> > > > > int ret = 0;
> > > > > + u8 dpcd_ext = 0;
> > > > > + unsigned int dpcd_offset = 0;
> > > > > struct drm_dp_mst_branch *mstb = NULL;
> > > > >
> > > > > mutex_lock(&mgr->payload_lock);
> > > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > > struct drm_dp_payload reset_pay;
> > > > >
> > > > > WARN_ON(mgr->mst_primary);
> > > > > + drm_dp_dpcd_read(mgr->aux,
> > > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > > +
> > > > > + dpcd_offset =
> > > > > + ((dpcd_ext &
> > > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > > DP_DPCD_REV);
> > > > >
> > > > > /* get dpcd info */
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > > >dpcd,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > > >dpcd,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > > goto out_unlock;
> > > > > --
> > > > > 2.25.1
> > > > >
> > > > Add Lyude Paul
> > > >
> > > --
> > > Cheers,
> > > Lyude Paul (she/her)
> > > Software Engineer at Red Hat
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 18:36 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2020-09-09 18:36 UTC (permalink / raw)
To: Lyude Paul
Cc: Thomas Zimmermann, David Airlie, Linux Kernel Mailing List,
Anthony Wong, Koba Ko, dri-devel
On Wed, Sep 09, 2020 at 02:26:11PM -0400, Lyude Paul wrote:
> On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> > On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > > We just added a new helper for DPCD retrieval to drm_dp_helper.c (which
> > > also
> > > handles grabbing the extended receiver caps), we should probably make use
> > > of
> > > it here
> >
> > Someone should really rework this whole thing so that the driver can
> > pass in the link rate+lane count when setting up the link. There's no
> > reason to assume that the source device capabilities match or exceed
> > the MST device caps. It would also allow the driver the dynamically
> > adjust these in response to link failures.
>
> I'm a bit confused, I also think we should pass the link rate+lane count in
> (especially since it'll be very helpful for when we start optimizing PBN
> handling in regards to bw constraints), but I'm not sure what you mean by
> "There's no reason to assume that the source device capabilities match or
> exceed the MST device caps", aren't we doing the opposite here by checking the
> MST device caps?
We assume only the MST device caps matter. Which is fine if the source
device caps are equal or higher. But if the source device isn't as
capable then we're going to calculate the MST things based on link bw
we can not actually achieve. End result is that we potentially try to
push too much data over the link.
I'm not really sure what actually happens if we just miscompute these
things but don't actually oversubscribe the link. Maybe the remote
end gets confused when we tell it some bogus VC parameters? Maybe it
doesn't care? Dunno.
> >
> > > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com> wrote:
> > > > > Currently, DRM get the capability of the mst hub only from DP_DPCD_REV
> > > > > and
> > > > > get the slower speed even the mst hub can run in the faster speed.
> > > > >
> > > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > > DP_DP13_DPCD_REV
> > > > > to
> > > > > get the faster capability.
> > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > > >
> > > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > > ---
> > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index 67dd72ea200e..3b84c6801281 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > > dp_link_bw,
> > > > > u8 dp_link_count)
> > > > > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr
> > > > > *mgr,
> > > > > bool mst_state)
> > > > > {
> > > > > int ret = 0;
> > > > > + u8 dpcd_ext = 0;
> > > > > + unsigned int dpcd_offset = 0;
> > > > > struct drm_dp_mst_branch *mstb = NULL;
> > > > >
> > > > > mutex_lock(&mgr->payload_lock);
> > > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > > struct drm_dp_payload reset_pay;
> > > > >
> > > > > WARN_ON(mgr->mst_primary);
> > > > > + drm_dp_dpcd_read(mgr->aux,
> > > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > > +
> > > > > + dpcd_offset =
> > > > > + ((dpcd_ext &
> > > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > > DP_DPCD_REV);
> > > > >
> > > > > /* get dpcd info */
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > > >dpcd,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > > >dpcd,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > > goto out_unlock;
> > > > > --
> > > > > 2.25.1
> > > > >
> > > > Add Lyude Paul
> > > >
> > > --
> > > Cheers,
> > > Lyude Paul (she/her)
> > > Software Engineer at Red Hat
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
2020-09-09 18:36 ` Ville Syrjälä
@ 2020-09-09 18:58 ` Lyude Paul
-1 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 18:58 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Koba Ko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel,
Linux Kernel Mailing List, Anthony Wong
On Wed, 2020-09-09 at 21:36 +0300, Ville Syrjälä wrote:
> On Wed, Sep 09, 2020 at 02:26:11PM -0400, Lyude Paul wrote:
> > On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> > > On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > > > We just added a new helper for DPCD retrieval to drm_dp_helper.c
> > > > (which
> > > > also
> > > > handles grabbing the extended receiver caps), we should probably make
> > > > use
> > > > of
> > > > it here
> > >
> > > Someone should really rework this whole thing so that the driver can
> > > pass in the link rate+lane count when setting up the link. There's no
> > > reason to assume that the source device capabilities match or exceed
> > > the MST device caps. It would also allow the driver the dynamically
> > > adjust these in response to link failures.
> >
> > I'm a bit confused, I also think we should pass the link rate+lane count
> > in
> > (especially since it'll be very helpful for when we start optimizing PBN
> > handling in regards to bw constraints), but I'm not sure what you mean by
> > "There's no reason to assume that the source device capabilities match or
> > exceed the MST device caps", aren't we doing the opposite here by checking
> > the
> > MST device caps?
>
> We assume only the MST device caps matter. Which is fine if the source
> device caps are equal or higher. But if the source device isn't as
> capable then we're going to calculate the MST things based on link bw
> we can not actually achieve. End result is that we potentially try to
> push too much data over the link.
>
> I'm not really sure what actually happens if we just miscompute these
> things but don't actually oversubscribe the link. Maybe the remote
> end gets confused when we tell it some bogus VC parameters? Maybe it
> doesn't care? Dunno.
Ah-I understand what you mean. From my understanding I think MST devices parse
some of the bandwidth information (since a lot of hubs seem to have a
different full_pbn based on the source caps from what I've seen). But yes-we
probably should also start processing all of the relevant DPCD caps on the
source device's side instead of just trusting MST. I'll add this to my todo
list
>
> > > > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com>
> > > > > wrote:
> > > > > > Currently, DRM get the capability of the mst hub only from
> > > > > > DP_DPCD_REV
> > > > > > and
> > > > > > get the slower speed even the mst hub can run in the faster speed.
> > > > > >
> > > > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > > > DP_DP13_DPCD_REV
> > > > > > to
> > > > > > get the faster capability.
> > > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > > > >
> > > > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > index 67dd72ea200e..3b84c6801281 100644
> > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > > > dp_link_bw,
> > > > > > u8 dp_link_count)
> > > > > > int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > > drm_dp_mst_topology_mgr
> > > > > > *mgr,
> > > > > > bool mst_state)
> > > > > > {
> > > > > > int ret = 0;
> > > > > > + u8 dpcd_ext = 0;
> > > > > > + unsigned int dpcd_offset = 0;
> > > > > > struct drm_dp_mst_branch *mstb = NULL;
> > > > > >
> > > > > > mutex_lock(&mgr->payload_lock);
> > > > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > > > struct drm_dp_payload reset_pay;
> > > > > >
> > > > > > WARN_ON(mgr->mst_primary);
> > > > > > + drm_dp_dpcd_read(mgr->aux,
> > > > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > > > +
> > > > > > + dpcd_offset =
> > > > > > + ((dpcd_ext &
> > > > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > > > DP_DPCD_REV);
> > > > > >
> > > > > > /* get dpcd info */
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > > > > dpcd,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > > > > dpcd,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > > > goto out_unlock;
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > > Add Lyude Paul
> > > > >
> > > > --
> > > > Cheers,
> > > > Lyude Paul (she/her)
> > > > Software Engineer at Red Hat
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set.
@ 2020-09-09 18:58 ` Lyude Paul
0 siblings, 0 replies; 14+ messages in thread
From: Lyude Paul @ 2020-09-09 18:58 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Thomas Zimmermann, David Airlie, Linux Kernel Mailing List,
Anthony Wong, Koba Ko, dri-devel
On Wed, 2020-09-09 at 21:36 +0300, Ville Syrjälä wrote:
> On Wed, Sep 09, 2020 at 02:26:11PM -0400, Lyude Paul wrote:
> > On Wed, 2020-09-09 at 21:20 +0300, Ville Syrjälä wrote:
> > > On Wed, Sep 09, 2020 at 12:33:09PM -0400, Lyude Paul wrote:
> > > > We just added a new helper for DPCD retrieval to drm_dp_helper.c
> > > > (which
> > > > also
> > > > handles grabbing the extended receiver caps), we should probably make
> > > > use
> > > > of
> > > > it here
> > >
> > > Someone should really rework this whole thing so that the driver can
> > > pass in the link rate+lane count when setting up the link. There's no
> > > reason to assume that the source device capabilities match or exceed
> > > the MST device caps. It would also allow the driver the dynamically
> > > adjust these in response to link failures.
> >
> > I'm a bit confused, I also think we should pass the link rate+lane count
> > in
> > (especially since it'll be very helpful for when we start optimizing PBN
> > handling in regards to bw constraints), but I'm not sure what you mean by
> > "There's no reason to assume that the source device capabilities match or
> > exceed the MST device caps", aren't we doing the opposite here by checking
> > the
> > MST device caps?
>
> We assume only the MST device caps matter. Which is fine if the source
> device caps are equal or higher. But if the source device isn't as
> capable then we're going to calculate the MST things based on link bw
> we can not actually achieve. End result is that we potentially try to
> push too much data over the link.
>
> I'm not really sure what actually happens if we just miscompute these
> things but don't actually oversubscribe the link. Maybe the remote
> end gets confused when we tell it some bogus VC parameters? Maybe it
> doesn't care? Dunno.
Ah-I understand what you mean. From my understanding I think MST devices parse
some of the bandwidth information (since a lot of hubs seem to have a
different full_pbn based on the source caps from what I've seen). But yes-we
probably should also start processing all of the relevant DPCD caps on the
source device's side instead of just trusting MST. I'll add this to my todo
list
>
> > > > On Wed, 2020-09-09 at 14:31 +0800, Koba Ko wrote:
> > > > > On Thu, Aug 27, 2020 at 1:30 PM Koba Ko <koba.ko@canonical.com>
> > > > > wrote:
> > > > > > Currently, DRM get the capability of the mst hub only from
> > > > > > DP_DPCD_REV
> > > > > > and
> > > > > > get the slower speed even the mst hub can run in the faster speed.
> > > > > >
> > > > > > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> > > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the
> > > > > > DP_DP13_DPCD_REV
> > > > > > to
> > > > > > get the faster capability.
> > > > > > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> > > > > >
> > > > > > Signed-off-by: Koba Ko <koba.ko@canonical.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +++++++++-
> > > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > index 67dd72ea200e..3b84c6801281 100644
> > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > @@ -3497,6 +3497,8 @@ static int drm_dp_get_vc_payload_bw(u8
> > > > > > dp_link_bw,
> > > > > > u8 dp_link_count)
> > > > > > int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > > drm_dp_mst_topology_mgr
> > > > > > *mgr,
> > > > > > bool mst_state)
> > > > > > {
> > > > > > int ret = 0;
> > > > > > + u8 dpcd_ext = 0;
> > > > > > + unsigned int dpcd_offset = 0;
> > > > > > struct drm_dp_mst_branch *mstb = NULL;
> > > > > >
> > > > > > mutex_lock(&mgr->payload_lock);
> > > > > > @@ -3510,9 +3512,15 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool ms
> > > > > > struct drm_dp_payload reset_pay;
> > > > > >
> > > > > > WARN_ON(mgr->mst_primary);
> > > > > > + drm_dp_dpcd_read(mgr->aux,
> > > > > > + DP_TRAINING_AUX_RD_INTERVAL,
> > > > > > + &dpcd_ext, sizeof(dpcd_ext));
> > > > > > +
> > > > > > + dpcd_offset =
> > > > > > + ((dpcd_ext &
> > > > > > DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) ? DP_DP13_DPCD_REV :
> > > > > > DP_DPCD_REV);
> > > > > >
> > > > > > /* get dpcd info */
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr-
> > > > > > > dpcd,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > + ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr-
> > > > > > > dpcd,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > if (ret != DP_RECEIVER_CAP_SIZE) {
> > > > > > DRM_DEBUG_KMS("failed to read DPCD\n");
> > > > > > goto out_unlock;
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > > Add Lyude Paul
> > > > >
> > > > --
> > > > Cheers,
> > > > Lyude Paul (she/her)
> > > > Software Engineer at Red Hat
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-09-09 18:58 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 5:30 [PATCH] drm/dp: For MST hub, Get max_link_rate&max_lane from extended rx capability field if EXTENDED_RECEIVER_CAPABILITY_FILED_PRESENT is set Koba Ko
2020-08-27 5:30 ` Koba Ko
2020-09-09 6:31 ` Koba Ko
2020-09-09 6:31 ` Koba Ko
2020-09-09 16:33 ` Lyude Paul
2020-09-09 16:33 ` Lyude Paul
2020-09-09 18:20 ` Ville Syrjälä
2020-09-09 18:20 ` Ville Syrjälä
2020-09-09 18:26 ` Lyude Paul
2020-09-09 18:26 ` Lyude Paul
2020-09-09 18:36 ` Ville Syrjälä
2020-09-09 18:36 ` Ville Syrjälä
2020-09-09 18:58 ` Lyude Paul
2020-09-09 18:58 ` Lyude Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.