From: Stephen Boyd <swboyd@chromium.org>
To: Jonathan Marek <jonathan@marek.ca>,
Kishon Vijay Abraham I <kishon@ti.com>,
Vinod Koul <vkoul@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
Jeykumar Sankaran <jsanka@codeaurora.org>,
Chandan Uddaraju <chandanu@codeaurora.org>,
Vara Reddy <varar@codeaurora.org>,
Tanmay Shah <tanmay@codeaurora.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Manu Gautam <mgautam@codeaurora.org>,
Sandeep Maheswaram <sanm@codeaurora.org>,
Douglas Anderson <dianders@chromium.org>,
Sean Paul <seanpaul@chromium.org>,
Stephen Boyd <sboyd@kernel.org>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Rob Clark <robdclark@chromium.org>
Subject: Re: [PATCH v2 07/10] phy: qcom-qmp: Add support for DP in USB3+DP combo phy
Date: Thu, 03 Sep 2020 15:41:09 -0700 [thread overview]
Message-ID: <159917286975.334488.16684252260287652678@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <b6f80242-482d-b778-690b-8aefa4e8f23e@marek.ca>
Quoting Jonathan Marek (2020-09-03 13:43:10)
> On 9/2/20 7:02 PM, Stephen Boyd wrote:
> >
> > 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.
Great!
>
> This commit [2] might interest you, so that you can consider what needs
> to change between v3 and v4 PHYs. Note some of the V4 registers have the
> same address as V3, so the diff could be smaller.
Looks like v4 will need to introduce a register indirection table for
the differences. Also need to add a table for the aux initial table
values and the calibration values for aux_cfg1. Seems like it won't be
too bad.
Does DP work with those patches with v4? You should make yourself the
author of commit d3c6da6f87eedb20ea1591aaae1ea4e63d7bd777 ;-)
>
> Do you have any plan for dealing with the SS PHY and DP PHY conflicting
> with each other? For example, PHY_MODE_CTRL needs to be "DP_MODE" for
> 4-lane DP, "DP_MODE | USB3_MODE" for 2-lane DP + USB3, and (AFAIK)
> "USB3_MODE" for superspeedplus usb (and it seems this gates some clocks,
> so you can't read/write dp tx2 registers in 2-lane DP mode for example).
Right. I've seen that behavior as well.
> From your cover letter it sounds like this isn't relevant to your
> hardware, but it looks like both PHYs are writing to the dp_com region
> which is still problematic. (in the branch I linked, I disabled the SS
> PHY to test the DP PHY)
Right. I mentioned in the cover letter that this needs to hook into the
type-c subsystem somehow. I haven't done any of that work because I
don't have a configuration that is as dynamic. As long as the type-c
stuff can express my static configuration it will be fine. If you have
done any work there I'm happy to review the code and test it out on my
configuration.
The driver is setup for DP_MODE | USB3_MODE (i.e. concurrent mode) so it
is already hardcoded for the 2-lane use case that I have. If I didn't
connect two lanes from the phy to a USB hub I could support all the
different combinations but that isn't the case. On phones it is
basically the only case though because the pins from the usb3+dp phy go
straight to the type-c connector.
qcom_qmp_phy_com_init() is the only place I see the driver writing to it
and it is refcounted so basically the first phy to get initialized will
set things up in the common area. I suppose for supporting various use
cases like 4 lanes DP or 2 lanes DP and USB then that refcounting logic
will need to be changed. I'm not sure what is supposed to happen though.
I guess the USB host controller, i.e. dwc3, will have to know to stop
trying to use the phy and then power down and let the DP controller take
over the phy? It's a dance of three or four drivers.
>
> Also some issues I noticed:
> - used QSERDES_COM_RESETSM_CNTRL instead of
> QSERDES_V3_COM_RESETSM_CNTRL2, which has different value
> - in sc7180_dpphy_cfg, .regs is NULL, which results in NULL references
Can you add these as inline review comments? Would help me understand
what you're talking about. Thanks for the review!
next prev parent reply other threads:[~2020-09-03 22:41 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-02 23:02 [PATCH v2 00/10] Support qcom USB3+DP combo phy (or type-c phy) Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 01/10] dt-bindings: phy: qcom,qmp-usb3-dp: Add DP phy information Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 02/10] phy: qcom-qmp: Move phy mode into struct qmp_phy Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 03/10] phy: qcom-qmp: Remove 'initialized' in favor of 'init_count' Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 04/10] phy: qcom-qmp: Move 'serdes' and 'cfg' into 'struct qcom_phy' Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 05/10] phy: qcom-qmp: Get dp_com I/O resource by index Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 06/10] phy: qcom-qmp: Use devm_platform_ioremap_resource() to simplify Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 07/10] phy: qcom-qmp: Add support for DP in USB3+DP combo phy Stephen Boyd
2020-09-03 20:43 ` Jonathan Marek
2020-09-03 22:41 ` Stephen Boyd [this message]
2020-09-03 23:24 ` Jonathan Marek
2020-09-04 12:29 ` Dmitry Baryshkov
2020-09-04 12:44 ` Jonathan Marek
2020-09-04 12:57 ` Dmitry Baryshkov
2020-09-04 13:02 ` Jonathan Marek
2020-09-03 23:26 ` Jonathan Marek
2020-09-08 18:42 ` Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 08/10] phy: qcom-qmp: Add support for sc7180 DP phy Stephen Boyd
2020-09-03 23:29 ` Jonathan Marek
2020-09-08 18:44 ` Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 09/10] clk: qcom: dispcc: Update DP clk ops for phy design Stephen Boyd
2020-09-02 23:02 ` [PATCH v2 10/10] drm/msm/dp: Use qmp phy for DP PLL and PHY Stephen Boyd
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=159917286975.334488.16684252260287652678@swboyd.mtv.corp.google.com \
--to=swboyd@chromium.org \
--cc=bjorn.andersson@linaro.org \
--cc=chandanu@codeaurora.org \
--cc=dianders@chromium.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=jonathan@marek.ca \
--cc=jsanka@codeaurora.org \
--cc=kishon@ti.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mgautam@codeaurora.org \
--cc=robdclark@chromium.org \
--cc=sanm@codeaurora.org \
--cc=sboyd@kernel.org \
--cc=seanpaul@chromium.org \
--cc=tanmay@codeaurora.org \
--cc=varar@codeaurora.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.