Re: [PATCH v4 5/8] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround

[Cristian Ciocaltea]

On 7/20/26 5:19 PM, Dmitry Baryshkov wrote:
> On Fri, Jun 12, 2026 at 02:45:24AM +0300, Cristian Ciocaltea wrote:
>> Since commit ba9c2fe18c17 ("drm/rockchip: dw_hdmi_qp: Switch to
>> phy_configure()") the TMDS rate setup doesn't rely anymore on the
>> unconventional usage of the bus width, instead it is managed exclusively
>> through the HDMI PHY configuration API.
>>
>> Drop the now obsolete workaround to retrieve the TMDS character rate via
>> phy_get_bus_width() during power_on().
>>
>> While at it, get rid of the extra call to rk_hdptx_phy_consumer_put() by
>> moving the statement at the end of the function.
>>
>> Tested-by: Thomas Niederprüm <dubito@xxxxxxxxx>
>> Tested-by: Simon Wright <simon@xxxxxxxxx>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
>> ---
>>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 27 +++++------------------
>>  1 file changed, 6 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index 25bd821cd039..35997087d61c 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -1660,22 +1660,6 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>>  	enum phy_mode mode = phy_get_mode(phy);
>>  	int ret, lane;
>>  
>> -	if (mode != PHY_MODE_DP) {
>> -		if (!hdptx->hdmi_cfg.rate && hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL) {
>> -			/*
>> -			 * FIXME: Temporary workaround to setup TMDS char rate
>> -			 * from the RK DW HDMI QP bridge driver.
>> -			 * Will be removed as soon the switch to the HDMI PHY
>> -			 * configuration API has been completed on both ends.
>> -			 */
>> -			hdptx->hdmi_cfg.rate = phy_get_bus_width(hdptx->phy) & 0xfffffff;
>> -			hdptx->hdmi_cfg.rate *= 100;
>> -		}
>> -
>> -		dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
>> -			hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
>> -	}
>> -
>>  	ret = rk_hdptx_phy_consumer_get(hdptx);
>>  	if (ret)
>>  		return ret;
>> @@ -1701,9 +1685,10 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>>  		rk_hdptx_dp_pll_init(hdptx);
>>  
>>  		ret = rk_hdptx_dp_aux_init(hdptx);
>> -		if (ret)
>> -			rk_hdptx_phy_consumer_put(hdptx, true);
>>  	} else {
>> +		dev_dbg(hdptx->dev, "%s rate=%llu bpc=%u\n", __func__,
>> +			hdptx->hdmi_cfg.rate, hdptx->hdmi_cfg.bpc);
>> +
>>  		if (hdptx->pll_config_dirty)
>>  			ret = rk_hdptx_pll_cmn_config(hdptx);
>>  
>> @@ -1716,11 +1701,11 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>>  			else
>>  				ret = rk_hdptx_tmds_ropll_mode_config(hdptx);
>>  		}
>> -
>> -		if (ret)
>> -			rk_hdptx_phy_consumer_put(hdptx, true);
>>  	}
>>  
>> +	if (ret)
>> +		rk_hdptx_phy_consumer_put(hdptx, true);
> 
> This looks likes an unrelated change. Could you please split it to a
> separate commit?

Ack.

Thanks,
Cristian