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

[Dmitry Baryshkov]

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?

> +
>  	return ret;
>  }
>  
> 
> -- 
> 2.54.0
> 

-- 
With best wishes
Dmitry