This function will be needed by the next patch where the driver calculates the BW based on driver specific parameters, so export it.
At the same time sanitize the function params, passing the more natural link rate instead of the encoding of the same rate.
Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 24 ++++++++++++++++++------ include/drm/drm_dp_mst_helper.h | 1 + 2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 475939138b21..dc96cbf78cc6 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3629,14 +3629,26 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, return 0; }
-static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count) +/** + * drm_dp_get_vc_payload_bw - get the VC payload BW for an MST link + * @rate: link rate in 10kbits/s units + * @lane_count: lane count + * + * Calculate the toal bandwidth of a MultiStream Transport link. The returned + * value is in units of PBNs/(timeslots/1 MTP). This value can be used to + * convert the number of PBNs required for a given stream to the number of + * timeslots this stream requires in each MTP. + */ +int drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count) { - if (dp_link_bw == 0 || dp_link_count == 0) - DRM_DEBUG_KMS("invalid link bandwidth in DPCD: %x (link count: %d)\n", - dp_link_bw, dp_link_count); + if (link_rate == 0 || link_lane_count == 0) + DRM_DEBUG_KMS("invalid link rate/lane count: (%d / %d)\n", + link_rate, link_lane_count);
- return dp_link_bw * dp_link_count / 2; + /* See DP v2.0 2.6.4.2, VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ + return link_rate * link_lane_count / 54000; } +EXPORT_SYMBOL(drm_dp_get_vc_payload_bw);
/** * drm_dp_read_mst_cap() - check whether or not a sink supports MST @@ -3692,7 +3704,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms goto out_unlock; }
- mgr->pbn_div = drm_dp_get_vc_payload_bw(mgr->dpcd[1], + mgr->pbn_div = drm_dp_get_vc_payload_bw(drm_dp_bw_code_to_link_rate(mgr->dpcd[1]), mgr->dpcd[2] & DP_MAX_LANE_COUNT_MASK); if (mgr->pbn_div == 0) { ret = -EINVAL; diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index f5e92fe9151c..bd1c39907b92 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -783,6 +783,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
+int drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count);
int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port, - crtc_state->pbn, 0); + crtc_state->pbn, + drm_dp_get_vc_payload_bw(crtc_state->port_clock, + crtc_state->lane_count)); if (slots == -EDEADLK) return slots; if (slots >= 0)
On Mon, 2021-01-25 at 19:36 +0200, Imre Deak wrote:
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
mst_mgr,
connector->port, - crtc_state->pbn, 0); + crtc_state->pbn, + drm_dp_get_vc_payload_bw(crtc_state->port_clock, +
This patch looks fine, however you should take care to also update the documentation for drm_dp_atomic_find_vcpi_slots() so that it mentiones that pbn_div should be DSC aware but also is not exclusive to systems supporting DSC over MST (see the docs for the @pbn_div parameter)
Thank you for doing this! I've been meaning to fix the WD19 issues for a while now but have been too bogged down by other stuff to spend any time on MST recently.
crtc_state->lane_count)); if (slots == -EDEADLK) return slots; if (slots >= 0)
On Mon, Jan 25, 2021 at 02:24:58PM -0500, Lyude Paul wrote:
On Mon, 2021-01-25 at 19:36 +0200, Imre Deak wrote:
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
mst_mgr,
connector->port, - crtc_state->pbn, 0); + crtc_state->pbn, + drm_dp_get_vc_payload_bw(crtc_state->port_clock, +
This patch looks fine, however you should take care to also update the documentation for drm_dp_atomic_find_vcpi_slots() so that it mentiones that pbn_div should be DSC aware but also is not exclusive to systems supporting DSC over MST (see the docs for the @pbn_div parameter)
I thought (as a follow-up work) that drm_dp_atomic_find_vcpi_slots() and drm_dp_mst_allocate_vcpi() could be made more generic, requiring the drivers to always pass in pbn_div. By that we could remove mst_mgr::pbn_div, keeping only one copy of this value (the one passed to the above functions).
Thank you for doing this! I've been meaning to fix the WD19 issues for a while now but have been too bogged down by other stuff to spend any time on MST recently.
crtc_state->lane_count)); if (slots == -EDEADLK) return slots; if (slots >= 0)
-- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!
On Mon, 2021-01-25 at 23:04 +0200, Imre Deak wrote:
On Mon, Jan 25, 2021 at 02:24:58PM -0500, Lyude Paul wrote:
On Mon, 2021-01-25 at 19:36 +0200, Imre Deak wrote:
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
mst_mgr,
connector->port, - crtc_state->pbn, 0); + crtc_state->pbn, + drm_dp_get_vc_payload_bw(crtc_state->port_clock, +
This patch looks fine, however you should take care to also update the documentation for drm_dp_atomic_find_vcpi_slots() so that it mentiones that pbn_div should be DSC aware but also is not exclusive to systems supporting DSC over MST (see the docs for the @pbn_div parameter)
I thought (as a follow-up work) that drm_dp_atomic_find_vcpi_slots() and drm_dp_mst_allocate_vcpi() could be made more generic, requiring the drivers to always pass in pbn_div. By that we could remove mst_mgr::pbn_div, keeping only one copy of this value (the one passed to the above functions).
I'm fine with that! The only thing I ask is (even though it's taken forever) we are eventually planning on making it so that we'll have MST helpers that can suggest changing the PBN divisor in order to implement link fallback retraining. As long as we're still able to make that work in the future, I'm totally fine with this.
Thank you for doing this! I've been meaning to fix the WD19 issues for a while now but have been too bogged down by other stuff to spend any time on MST recently.
crtc_state->lane_count)); if (slots == -EDEADLK) return slots; if (slots >= 0)
-- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!
On Mon, Jan 25, 2021 at 05:55:03PM -0500, Lyude Paul wrote:
On Mon, 2021-01-25 at 23:04 +0200, Imre Deak wrote:
On Mon, Jan 25, 2021 at 02:24:58PM -0500, Lyude Paul wrote:
On Mon, 2021-01-25 at 19:36 +0200, Imre Deak wrote:
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
mst_mgr,
connector->port, - crtc_state->pbn, 0); + crtc_state->pbn, + drm_dp_get_vc_payload_bw(crtc_state->port_clock, +
This patch looks fine, however you should take care to also update the documentation for drm_dp_atomic_find_vcpi_slots() so that it mentiones that pbn_div should be DSC aware but also is not exclusive to systems supporting DSC over MST (see the docs for the @pbn_div parameter)
I thought (as a follow-up work) that drm_dp_atomic_find_vcpi_slots() and drm_dp_mst_allocate_vcpi() could be made more generic, requiring the drivers to always pass in pbn_div. By that we could remove mst_mgr::pbn_div, keeping only one copy of this value (the one passed to the above functions).
I'm fine with that! The only thing I ask is (even though it's taken forever) we are eventually planning on making it so that we'll have MST helpers that can suggest changing the PBN divisor in order to implement link fallback retraining. As long as we're still able to make that work in the future, I'm totally fine with this.
I don't see a problem wrt. link retraining, pbn_div passed to the above functions should just reflect the actual rate and lane count the link was trained with.
Thank you for doing this! I've been meaning to fix the WD19 issues for a while now but have been too bogged down by other stuff to spend any time on MST recently.
crtc_state->lane_count)); if (slots == -EDEADLK) return slots; if (slots >= 0)
-- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!
-- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!
On Mon, Jan 25, 2021 at 07:36:36PM +0200, Imre Deak wrote:
Atm the driver will calculate a wrong MST timeslots/MTP (aka time unit) value for MST streams if the link parameters (link rate or lane count) are limited in a way independent of the sink capabilities (reported by DPCD).
One example of such a limitation is when a MUX between the sink and source connects only a limited number of lanes to the display and connects the rest of the lanes to other peripherals (USB).
Another issue is that atm MST core calculates the divider based on the backwards compatible DPCD (at address 0x0000) vs. the extended capability info (at address 0x2200). This can result in leaving some part of the MST BW unused (For instance in case of the WD19TB dock).
Fix the above two issues by calculating the PBN divider value based on the rate and lane count link parameters that the driver uses for all other computation.
Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/2977 Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Signed-off-by: Imre Deak imre.deak@intel.com
Looks all right to fix some of the immediate problems.
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index d6a1b961a0e8..b4621ed0127e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -68,7 +68,9 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port,
crtc_state->pbn, 0);
crtc_state->pbn,drm_dp_get_vc_payload_bw(crtc_state->port_clock,crtc_state->lane_count));-- 2.25.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2021-01-25 at 19:36 +0200, Imre Deak wrote:
This function will be needed by the next patch where the driver calculates the BW based on driver specific parameters, so export it.
At the same time sanitize the function params, passing the more natural link rate instead of the encoding of the same rate.
Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjala ville.syrjala@intel.com Cc: stable@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/drm_dp_mst_topology.c | 24 ++++++++++++++++++------ include/drm/drm_dp_mst_helper.h | 1 + 2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 475939138b21..dc96cbf78cc6 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3629,14 +3629,26 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, return 0; } -static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count) +/**
- drm_dp_get_vc_payload_bw - get the VC payload BW for an MST link
- @rate: link rate in 10kbits/s units
- @lane_count: lane count
- Calculate the toal bandwidth of a MultiStream Transport link. The returned
s/toal/total/
With that fixed, this patch is:
Reviewed-by: Lyude Paul lyude@redhat.com
- value is in units of PBNs/(timeslots/1 MTP). This value can be used to
- convert the number of PBNs required for a given stream to the number of
- timeslots this stream requires in each MTP.
- */
+int drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count) { - if (dp_link_bw == 0 || dp_link_count == 0) - DRM_DEBUG_KMS("invalid link bandwidth in DPCD: %x (link count: %d)\n", - dp_link_bw, dp_link_count); + if (link_rate == 0 || link_lane_count == 0) + DRM_DEBUG_KMS("invalid link rate/lane count: (%d / %d)\n", + link_rate, link_lane_count); - return dp_link_bw * dp_link_count / 2; + /* See DP v2.0 2.6.4.2, VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ + return link_rate * link_lane_count / 54000; } +EXPORT_SYMBOL(drm_dp_get_vc_payload_bw); /** * drm_dp_read_mst_cap() - check whether or not a sink supports MST @@ -3692,7 +3704,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms goto out_unlock; } - mgr->pbn_div = drm_dp_get_vc_payload_bw(mgr->dpcd[1], + mgr->pbn_div = drm_dp_get_vc_payload_bw(drm_dp_bw_code_to_link_rate(mgr->dpcd[1]), mgr->dpcd[2] & DP_MAX_LANE_COUNT_MASK); if (mgr->pbn_div == 0) { ret = -EINVAL; diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index f5e92fe9151c..bd1c39907b92 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -783,6 +783,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector, struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); +int drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count); int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
linux-stable-mirror@lists.linaro.org
-
Imre Deak -
Lyude Paul -
Ville Syrjälä