Re: [PATCH 4/5] phy: qcom: edp: Introduce support for DisplayPort

[Konrad Dybcio]

On 10/2/23 11:06, Johan Hovold wrote:
> On Tue, Aug 09, 2022 at 09:07:44PM -0700, Bjorn Andersson wrote:
> > The eDP phy can be used to drive either eDP or DP output, with some
> > minor variations in some of the configuration and seemingly a need for
> > implementing swing and pre_emphasis calibration.
> >
> > Introduce a config object, indicating if the phy is operating in eDP or
> > DP mode and swing/pre-emphasis calibration to support this.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> > ---
> >   drivers/phy/qualcomm/phy-qcom-edp.c | 80 +++++++++++++++++++++++++++--
> >   1 file changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> > index 32614fb838b5..301ac422d2fe 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> > @@ -70,8 +70,19 @@
> >   
> >   #define TXn_TRAN_DRVR_EMP_EN                    0x0078
> >   
> > +struct qcom_edp_cfg {
> > +	bool is_dp;
> > +
> > +	/* DP PHY swing and pre_emphasis tables */
> > +	const u8 (*swing_hbr_rbr)[4][4];
> > +	const u8 (*swing_hbr3_hbr2)[4][4];
> > +	const u8 (*pre_emphasis_hbr_rbr)[4][4];
> > +	const u8 (*pre_emphasis_hbr3_hbr2)[4][4];
> > +};
>   
> >   static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
> > @@ -315,7 +381,9 @@ static int qcom_edp_set_vco_div(const struct qcom_edp *edp)
> >   static int qcom_edp_phy_power_on(struct phy *phy)
> >   {
> >   	const struct qcom_edp *edp = phy_get_drvdata(phy);
> > +	const struct qcom_edp_cfg *cfg = edp->cfg;
> >   	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> > +	u8 ldo_config;
> >   	int timeout;
> >   	int ret;
> >   	u32 val;
> > @@ -332,8 +400,11 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> >   	if (timeout)
> >   		return timeout;
> >   
> > -	writel(0x01, edp->tx0 + TXn_LDO_CONFIG);
> > -	writel(0x01, edp->tx1 + TXn_LDO_CONFIG);
> > +
> > +	ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
> > +
> > +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
> > +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>
> When reviewing a patch from Konrad which will start using the eDP
> configuration on the X13s, I noticed that this patch inverted these bits
> for older SoCs (e.g. sc7280 and sc8180xp). They used to be set to 1,
> but after this patch they will be set to 0.
>
> Was that intentional even if it was never mentioned in the commit
> message? Or was it a mistake that should be fixed?
+Abhinav, Jessica

Konrad