Message-ID: <6017ca10-e4c8-4104-df2d-03984da2a655@marek.ca>
Date:   Fri, 4 Sep 2020 09:02:36 -0400
From:   Jonathan Marek <jonathan@...ek.ca>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Jeykumar Sankaran <jsanka@...eaurora.org>,
        Chandan Uddaraju <chandanu@...eaurora.org>,
        Vara Reddy <varar@...eaurora.org>,
        Tanmay Shah <tanmay@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Manu Gautam <mgautam@...eaurora.org>,
        Sandeep Maheswaram <sanm@...eaurora.org>,
        Douglas Anderson <dianders@...omium.org>,
        Sean Paul <seanpaul@...omium.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Clark <robdclark@...omium.org>
Subject: Re: [PATCH v2 07/10] phy: qcom-qmp: Add support for DP in USB3+DP
 combo phy

On 9/4/20 8:57 AM, Dmitry Baryshkov wrote:
> On 04/09/2020 15:44, Jonathan Marek wrote:
>> On 9/4/20 8:29 AM, Dmitry Baryshkov wrote:
>>> On 03/09/2020 23:43, Jonathan Marek wrote:
>>>> On 9/2/20 7:02 PM, Stephen Boyd wrote:
>>>>> Add support for the USB3 + DisplayPort (DP) "combo" phy to the qmp phy
>>>>> driver. We already have support for the USB3 part of the combo phy, so
>>>>> most additions are for the DP phy.
>>>>>
>>>>> Split up the qcom_qmp_phy{enable,disable}() functions into the phy 
>>>>> init,
>>>>> power on, power off, and exit functions that the common phy framework
>>>>> expects so that the DP phy can add even more phy ops like
>>>>> phy_calibrate() and phy_configure(). This allows us to initialize 
>>>>> the DP
>>>>> PHY and configure the AUX channel before powering on the PHY at the 
>>>>> link
>>>>> rate that was negotiated during link training.
>>>>>
>>>>> The general design is as follows:
>>>>>
>>>>>    1) DP controller calls phy_init() to initialize the PHY and 
>>>>> configure
>>>>>    the dp_com register region.
>>>>>
>>>>>    2) DP controller calls phy_configure() to tune the link rate and
>>>>>    voltage swing and pre-emphasis settings.
>>>>>
>>>>>    3) DP controller calls phy_power_on() to enable the PLL and 
>>>>> power on
>>>>>    the phy.
>>>>>
>>>>>    4) DP controller calls phy_configure() again to tune the voltage 
>>>>> swing
>>>>>    and pre-emphasis settings determind during link training.
>>>>>
>>>>>    5) DP controller calls phy_calibrate() some number of times to 
>>>>> change
>>>>>    the aux settings if the aux channel times out during link training.
>>>>>
>>>>>    6) DP controller calls phy_power_off() if the link rate is to be
>>>>>    changed and goes back to step 2 to try again at a different link 
>>>>> rate.
>>>>>
>>>>>    5) DP controller calls phy_power_off() and then phy_exit() to power
>>>>>    down the PHY when it is done.
>>>>>
>>>>> The DP PHY contains a PLL that is different from the one used for the
>>>>> USB3 PHY. Instead of a pipe clk there is a link clk and a pixel clk
>>>>> output from the DP PLL after going through various dividers. Introduce
>>>>> clk ops for these two clks that just tell the child clks what the
>>>>> frequency of the pixel and link are. When the phy link rate is
>>>>> configured we call clk_set_rate() to update the child clks in the
>>>>> display clk controller on what rate is in use. The clk frequencies
>>>>> always differ based on the link rate (i.e. 1.6Gb/s 2.7Gb/s, 
>>>>> 5.4Gb/s, or
>>>>> 8.1Gb/s corresponding to various transmission modes like HBR1, HBR2 or
>>>>> HBR3) so we simply store the link rate and use that to calculate 
>>>>> the clk
>>>>> frequencies.
>>>>>
>>>>> The PLL enable sequence is a little different from other QMP phy 
>>>>> PLLs so
>>>>> we power on the PLL in qcom_qmp_phy_configure_dp_phy() that gets 
>>>>> called
>>>>> from phy_power_on(). This should probably be split out better so that
>>>>> each phy has a way to run the final PLL/PHY enable sequence.
>>>>>
>>>>> This code is based on a submission of this phy and PLL in the drm
>>>>> subsystem.
>>>>
>>>> I updated my upstream-based sm8150/sm8250 displayport stack [1] to 
>>>> use these patches.
>>>
>>> I have tried your branch on my RB5 with two different dongles. Both 
>>> dongles provide the same behaviour:
>>>   - on first plug I see VDM Tx errors,
>>>   - after I unplug and replug the dongle, PD phy seems to be stuck on 
>>> sending capabilities.
>>>
>>> See attached logs.
>>>
>>> Also I had to add typec_unregister_port(port->typec_port); to 
>>> IS_ERR(alt) in your tcpm.c hack.
>>>
>>> I'm currently finishing the driver for the mux/redriver, will retry 
>>> testing afterwards.
>>>
>>
>> As I mentioned the TCPM driver has a lot of issues. The "hard reset" 
>> isn't implemented correctly so going into that mode gets it stuck in a 
>> bad state. Note I am using this dongle [1], and it only works 
>> correctly in sink mode (with the dongle providing power), in source 
>> mode it does negotiate the alt mode, but never gets the HPD event that 
>> DP driver is waiting for.
>>
>> https://www.amazon.ca/Cable-Matters-Multiport-DisplayPort-Ethernet/dp/B06Y5N3YCD 
> 
> 
> I'll take a look for dongles that work in source mode (with RB5 being 
> the sink). Reset being not fully implemented would answer on questions 
> about replug. Any idea about VDM Tx errors?
> 
> 

Unfortunately I don't have a good idea. If you want to compare, here is 
my tcpm log [1] in source mode (HDK providing the power, it doesn't get 
the HPD event)

[1] https://gist.github.com/flto/62f352dbff3601abe05013bfebe7c0ab

Powered by blists - more mailing lists