On Thu 21 Oct 10:40 PDT 2021, Vinod Koul wrote: > On 16-10-21, 16:21, Bjorn Andersson wrote: > > Many recent Qualcomm platforms comes with native DP and eDP support. > > This consists of a controller in the MDSS and a QMP-like PHY. > > > > While similar to the well known QMP block, the eDP PHY only has TX lanes > > and the programming sequences are slightly different. Rather than > > continuing the trend of parameterize the QMP driver to pieces, this > > introduces the support as a new driver. > > > > The registration of link and pixel clocks are borrowed from the QMP > > driver. The non-DP link frequencies are omitted for now. > > > > The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only > > the prior is supported for now. > > since this is QMP phy, pls add an explanation why common QMP driver > is not used here? > Looked at this again, doesn't the second paragraph answer that? > > +static int qcom_edp_phy_init(struct phy *phy) > > +{ [..] > > + writel(0x00, edp->edp + DP_PHY_AUX_CFG0); > > + writel(0x13, edp->edp + DP_PHY_AUX_CFG1); > > + writel(0x24, edp->edp + DP_PHY_AUX_CFG2); > > + writel(0x00, edp->edp + DP_PHY_AUX_CFG3); > > + writel(0x0a, edp->edp + DP_PHY_AUX_CFG4); > > + writel(0x26, edp->edp + DP_PHY_AUX_CFG5); > > + writel(0x0a, edp->edp + DP_PHY_AUX_CFG6); > > + writel(0x03, edp->edp + DP_PHY_AUX_CFG7); > > + writel(0x37, edp->edp + DP_PHY_AUX_CFG8); > > + writel(0x03, edp->edp + DP_PHY_AUX_CFG9); > > In qmp phy we use a table for this, that looks very elegant and I am > sure next rev will have different magic numbers, so should we go the > table approach here on as well..? > Comparing the v3 and v4 USB/DP combo phy and this, the only number that differs is CFG_AUX2 and CFG_AUX8. CFG_AUX8 is 0x37 for eDP and 0xb7 for DP and AUX_CFG2 seems better to mask together, but I don't fully understand the content yet. I did check two other platforms and they have the same sequence, except one additional bit in AUX_CFG2. There also seem to be a few additional permutations of this value, so I don't think tables are the solution. So I think it's better if we leave this as proposed and then parameterize the two individual entries as needed when we go forward - or determine that I missed something. Regards, Bjorn