Messages in this thread | | | Date | Wed, 2 Nov 2022 20:23:17 +0300 | Subject | Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3 | From | Dmitry Baryshkov <> |
| |
On 02/11/2022 18:47, Doug Anderson wrote: > Hi, > > On Tue, Nov 1, 2022 at 7:37 AM Doug Anderson <dianders@chromium.org> wrote: >> >> Hi, >> >> On Mon, Oct 31, 2022 at 5:15 PM Dmitry Baryshkov >> <dmitry.baryshkov@linaro.org> wrote: >>> >>> On 01/11/2022 03:08, Doug Anderson wrote: >>>> Hi, >>>> >>>> On Mon, Oct 31, 2022 at 2:11 PM Kuogee Hsieh <quic_khsieh@quicinc.com> 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@linaro.org> >>>>>> >>>>>> >>>>>>> >>>>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >>>>>>> --- >>>>>>> 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
| |