Hi Dmitry,
On 2024/12/30 20:45, Dmitry Baryshkov wrote:
On Thu, Dec 26, 2024 at 02:33:03PM +0800, Damon Ding wrote:
Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
pre-emphasis configurations of each link rate have been verified according
to the eDP 1.3 requirements.
Well... Please describe what's happening here. That the HDMI PHY on your
platform also provides support for DP / eDP. Please document any design
decisions that you had to make.
Yes, I will add more relevant descriptions in the next version.
Signed-off-by: Damon Ding <damon.ding@xxxxxxxxxxxxxx>
---
Changes in v2:
- Add the module author
Changes in v3:
- Split this patch into two, one for correction and the other for
extension
Changes in v4:
- Add link_rate and lanes parameters in struct rk_hdptx_phy to store the
phy_configure() result for &phy_configure_opts.dp.link_rate and
&phy_configure_opts.dp.lanes
---
.../phy/rockchip/phy-rockchip-samsung-hdptx.c | 896 +++++++++++++++++-
1 file changed, 889 insertions(+), 7 deletions(-)
@@ -933,9 +1484,339 @@ static int rk_hdptx_phy_power_off(struct phy *phy)
return rk_hdptx_phy_consumer_put(hdptx, false);
}
+static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ return 0;
+}
No need for the stub, please drop it. The host controller driver can
still call phy_set_mode() / _ext(), the call will return 0.
Without the &phy_ops.set_mode(), the phy driver can not get phy_mode to
distinguish between HDMI and DP mode via the phy_get_mode(), even if the
host driver calls phy_set_mode() / _ext(). Additionally, the previous
discussion [0] also mentioned future considerations for dynamic
switching. Indeed, I should add a related comment before the 'return 0;'
to enhance understandability.
+
+static int rk_hdptx_phy_verify_config(struct rk_hdptx_phy *hdptx,
+ struct phy_configure_opts_dp *dp)
+{
+ int i;
+
+ if (dp->set_rate) {
+ switch (dp->link_rate) {
+ case 1620:
+ case 2700:
+ case 5400:
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ if (dp->set_lanes) {
+ switch (dp->lanes) {
+ case 0:
Is it really a valid config to have 0 lanes?
The case of 0 lanes is indeed unnecessary.
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ if (dp->set_voltages) {
+ for (i = 0; i < hdptx->lanes; i++) {
+ if (dp->voltage[i] > 3 || dp->pre[i] > 3)
+ return -EINVAL;
+
+ if (dp->voltage[i] + dp->pre[i] > 3)
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
[..]
+
+static int rk_hdptx_phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ struct rk_hdptx_phy *hdptx = phy_get_drvdata(phy);
+ enum phy_mode mode = phy_get_mode(phy);
+ int ret;
+
+ if (mode != PHY_MODE_DP)
+ return -EINVAL;
I'd say, return 0;
Yes, it is really not an error.
+
+ ret = rk_hdptx_phy_verify_config(hdptx, &opts->dp);
+ if (ret) {
+ dev_err(hdptx->dev, "invalid params for phy configure\n");
+ return ret;
+ }
+
+ if (opts->dp.set_rate) {
+ ret = rk_hdptx_phy_set_rate(hdptx, &opts->dp);
+ if (ret) {
+ dev_err(hdptx->dev, "failed to set rate: %d\n", ret);
+ return ret;
+ }
+ }
+
+ if (opts->dp.set_lanes) {
+ ret = rk_hdptx_phy_set_lanes(hdptx, &opts->dp);
+ if (ret) {
+ dev_err(hdptx->dev, "failed to set lanes: %d\n", ret);
+ return ret;
+ }
+ }
+
+ if (opts->dp.set_voltages) {
+ ret = rk_hdptx_phy_set_voltages(hdptx, &opts->dp);
+ if (ret) {
+ dev_err(hdptx->dev, "failed to set voltages: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static const struct phy_ops rk_hdptx_phy_ops = {
.power_on = rk_hdptx_phy_power_on,
.power_off = rk_hdptx_phy_power_off,
+ .set_mode = rk_hdptx_phy_set_mode,
+ .configure = rk_hdptx_phy_configure,
.owner = THIS_MODULE,
};
@@ -1149,5 +2030,6 @@ module_platform_driver(rk_hdptx_phy_driver);
MODULE_AUTHOR("Algea Cao <algea.cao@xxxxxxxxxxxxxx>");
MODULE_AUTHOR("Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>");
+MODULE_AUTHOR("Damon Ding <damon.ding@xxxxxxxxxxxxxx>");
MODULE_DESCRIPTION("Samsung HDMI/eDP Transmitter Combo PHY Driver");
MODULE_LICENSE("GPL");
--
2.34.1
Best regards
Damon
[0]https://patchwork.kernel.org/project/linux-rockchip/patch/20241127075157.856029-5-damon.ding@xxxxxxxxxxxxxx/
