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

[Konrad Dybcio <konrad.dybcio@linaro.org>]

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@linaro.org>
>> ---
>>   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