On 02/11/2022 18:47, Doug Anderson wrote:
Hi,
On Tue, Nov 1, 2022 at 7:37 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
Hi,
On Mon, Oct 31, 2022 at 5:15 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
On 01/11/2022 03:08, Doug Anderson wrote:
Hi,
On Mon, Oct 31, 2022 at 2:11 PM Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> wrote:
Hi Dmitry,
Link rate is advertised by sink, but adjusted (reduced the link rate)
by host during link training.
Therefore should be fine if host did not support HBR3 rate.
It will reduce to lower link rate during link training procedures.
kuogee
On 10/31/2022 11:46 AM, Dmitry Baryshkov wrote:
On 31/10/2022 20:27, Kuogee Hsieh wrote:
An HBR3-capable device shall also support TPS4. Since TPS4 feature
had been implemented already, it is not necessary to limit link
rate at HBR2 (5.4G). This patch remove this limitation to support
HBR3 (8.1G) link rate.
The DP driver supports several platforms including sdm845 and can
support, if I'm not mistaken, platforms up to msm8998/sdm630/660.
Could you please confirm that all these SoCs have support for HBR3?
With that fact being confirmed:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_panel.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149ceb..3344f5a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel
*dp_panel)
if (link_info->num_lanes > dp_panel->max_dp_lanes)
link_info->num_lanes = dp_panel->max_dp_lanes;
- /* Limit support upto HBR2 until HBR3 support is added */
- if (link_info->rate >=
(drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
- link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
-
drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
drm_dbg_dp(panel->drm_dev, "lane_count=%d\n",
link_info->num_lanes);
Stephen might remember better, but I could have sworn that the problem
was that there might be something in the middle that couldn't support
the higher link rate. In other words, I think we have:
SoC <--> TypeC Port Controller <--> Display
The SoC might support HBR3 and the display might support HBR3, but the
TCPC (Type C Port Controller) might not. I think that the TCPC is a
silent/passive component so it can't really let anyone know about its
limitations.
In theory I guess you could rely on link training to just happen to
fail if you drive the link too fast for the TCPC to handle. Does this
actually work reliably?
I think the other option that was discussed in the past was to add
something in the device tree for this. Either you could somehow model
the TCPC in DRM and thus know that a given model of TCPC limits the
link rate or you could hack in a property in the DP controller to
limit it.
Latest pmic_glink proposal from Bjorn include adding the drm_bridge for
the TCPC. Such bridge can in theory limit supported modes and rates.
Excellent! Even so, I think this isn't totally a solved problem,
right? Even though a bridge seems like a good place for this, last I
remember checking the bridge API wasn't expressive enough to solve
this problem. A bridge could limit pixel clocks just fine, but here we
need to take into account other considerations to know if a given
pixel clock can work at 5.4 GHz or not. For instance, if we're at 4
lanes we could maybe make a given pixel clock at 5.4 GHz but not if we
only have 2 lanes. I don't think that the DP controller passes the
number of lanes to other parts of the bridge chain, though maybe
there's some trick for it?
...I guess the other problem is that all existing users aren't
currently modeling their TCPC in this way. What happens to them?
FWIW, I did more research on the "let's rely on link training to
detect TCPC's that only support HBR2". I haven't tested it myself, but
from looking at a 1.5 year old internal bug where we discussed this
before, both others at Qualcomm and others at Google were skeptical
about this. Both parties had past experience where link training would
succeed but the display wouldn't be reliable at the higher link rate.
I guess that leaves us with 3 possible approaches:
1. Someone figures out how to model this with the bridge chain and
then we only allow HBR3 if we detect we've got a TCPC that supports
it. This seems like the cleanest / best but feels like a long pole.
Not only have we been trying to get the TCPC-modeled-as-a-bridge stuff
landed for a long time but even when we do it we still don't have a
solution for how to communicate the number of lanes and other stuff
between the TCPC and the DP controller so we have to enrich the bridge
interface.
I think we'd need some OOB interface. For example for DSI interfaces we
have mipi_dsi_device struct to communicate such OOB data.
Also take a note regarding data-lanes from my previous email.
2. We add in a DT property to the display controller node that says
the max link rate for use on this board. This feels like a hack, but
maybe it's not too bad. Certainly it would be incredibly simple to
implement. Actually... ...one could argue that even if we later model
the TCPC as a bridge that this property would still be valid / useful!
You could certainly imagine that the SoC supports HBR3 and the TCPC
supports HBR3 but that the board routing between the SoC and the TCPC
is bad and only supports HBR2. In this case the only way out is
essentially a "board constraint" AKA a DT property in the DP
controller.
We have been discussing similar topics with Abhinav. Krzysztof suggested
using link-frequencies property to provide max and min values.
3. We could do some hack based on the SoC. We could assume that newer
SoCs will have a TCPC that was tested with HBR3. This doesn't require
any DT changes and would work, but feels like it won't stand the test
of time.
I'd vote for #2 but I'm interested in what others say.
-Doug
--
With best wishes
Dmitry
