Re: [PATCH RESEND v3 09/15] drm/msm/dp: move phy_configure_opts to dp_ctrl

2024-01-26 Thread Kuogee Hsieh



On 1/26/2024 10:26 AM, Dmitry Baryshkov wrote:

There is little point in sharing phy configuration structure between
several modules. Move it to dp_ctrl, which becomes the only submodule
re-configuring the PHY.

Signed-off-by: Dmitry Baryshkov 

Tested-by: Kuogee Hsieh 
Reviewed-by: Kuogee Hsieh 

---
  drivers/gpu/drm/msm/dp/dp_catalog.c | 19 -
  drivers/gpu/drm/msm/dp/dp_catalog.h |  2 --
  drivers/gpu/drm/msm/dp/dp_ctrl.c| 41 -
  drivers/gpu/drm/msm/dp/dp_parser.h  |  3 ---
  4 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5142aeb705a4..e07651768805 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -765,25 +765,6 @@ void dp_catalog_ctrl_phy_reset(struct dp_catalog 
*dp_catalog)
dp_write_ahb(catalog, REG_DP_PHY_CTRL, 0x0);
  }
  
-int dp_catalog_ctrl_update_vx_px(struct dp_catalog *dp_catalog,

-   u8 v_level, u8 p_level)
-{
-   struct dp_catalog_private *catalog = container_of(dp_catalog,
-   struct dp_catalog_private, dp_catalog);
-   struct dp_io *dp_io = catalog->io;
-   struct phy *phy = dp_io->phy;
-   struct phy_configure_opts_dp *opts_dp = &dp_io->phy_opts.dp;
-
-   /* TODO: Update for all lanes instead of just first one */
-   opts_dp->voltage[0] = v_level;
-   opts_dp->pre[0] = p_level;
-   opts_dp->set_voltages = 1;
-   phy_configure(phy, &dp_io->phy_opts);
-   opts_dp->set_voltages = 0;
-
-   return 0;
-}
-
  void dp_catalog_ctrl_send_phy_pattern(struct dp_catalog *dp_catalog,
u32 pattern)
  {
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 38786e855b51..ba7c62ba7ca3 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -111,8 +111,6 @@ void dp_catalog_ctrl_set_psr(struct dp_catalog *dp_catalog, 
bool enter);
  u32 dp_catalog_link_is_connected(struct dp_catalog *dp_catalog);
  u32 dp_catalog_hpd_get_intr_status(struct dp_catalog *dp_catalog);
  void dp_catalog_ctrl_phy_reset(struct dp_catalog *dp_catalog);
-int dp_catalog_ctrl_update_vx_px(struct dp_catalog *dp_catalog, u8 v_level,
-   u8 p_level);
  int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog);
  u32 dp_catalog_ctrl_read_psr_interrupt_status(struct dp_catalog *dp_catalog);
  void dp_catalog_ctrl_update_transfer_unit(struct dp_catalog *dp_catalog,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index e367eb8e5bea..4aea72a2b8e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -87,6 +87,8 @@ struct dp_ctrl_private {
  
  	struct clk *pixel_clk;
  
+	union phy_configure_opts phy_opts;

+
struct completion idle_comp;
struct completion psr_op_comp;
struct completion video_comp;
@@ -1017,6 +1019,21 @@ static int dp_ctrl_wait4video_ready(struct 
dp_ctrl_private *ctrl)
return ret;
  }
  
+static int dp_ctrl_set_vx_px(struct dp_ctrl_private *ctrl,

+u8 v_level, u8 p_level)
+{
+   union phy_configure_opts *phy_opts = &ctrl->phy_opts;
+
+   /* TODO: Update for all lanes instead of just first one */
+   phy_opts->dp.voltage[0] = v_level;
+   phy_opts->dp.pre[0] = p_level;
+   phy_opts->dp.set_voltages = 1;
+   phy_configure(ctrl->parser->io.phy, phy_opts);
+   phy_opts->dp.set_voltages = 0;
+
+   return 0;
+}
+
  static int dp_ctrl_update_vx_px(struct dp_ctrl_private *ctrl)
  {
struct dp_link *link = ctrl->link;
@@ -1029,7 +1046,7 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private 
*ctrl)
drm_dbg_dp(ctrl->drm_dev,
"voltage level: %d emphasis level: %d\n",
voltage_swing_level, pre_emphasis_level);
-   ret = dp_catalog_ctrl_update_vx_px(ctrl->catalog,
+   ret = dp_ctrl_set_vx_px(ctrl,
voltage_swing_level, pre_emphasis_level);
  
  	if (ret)

@@ -1425,16 +1442,14 @@ static void dp_ctrl_link_clk_disable(struct dp_ctrl 
*dp_ctrl)
  static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl)
  {
int ret = 0;
-   struct dp_io *dp_io = &ctrl->parser->io;
-   struct phy *phy = dp_io->phy;
-   struct phy_configure_opts_dp *opts_dp = &dp_io->phy_opts.dp;
+   struct phy *phy = ctrl->parser->io.phy;
const u8 *dpcd = ctrl->panel->dpcd;
  
-	opts_dp->lanes = ctrl->link->link_params.num_lanes;

-   opts_dp->link_rate = ctrl->link->link_params.rate / 100;
-   opts_dp->ssc = drm_dp_max_downspread(dpcd);
+   ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
+   ctrl->phy_opts.dp.link_rate = ctrl->link->link_params.rate / 100;
+   ctrl->phy_opts.dp.ssc = drm_dp_max_downspread(dpcd);
  
-	phy_configure(phy, &dp_i

[PATCH RESEND v3 09/15] drm/msm/dp: move phy_configure_opts to dp_ctrl

2024-01-26 Thread Dmitry Baryshkov
There is little point in sharing phy configuration structure between
several modules. Move it to dp_ctrl, which becomes the only submodule
re-configuring the PHY.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/dp/dp_catalog.c | 19 -
 drivers/gpu/drm/msm/dp/dp_catalog.h |  2 --
 drivers/gpu/drm/msm/dp/dp_ctrl.c| 41 -
 drivers/gpu/drm/msm/dp/dp_parser.h  |  3 ---
 4 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5142aeb705a4..e07651768805 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -765,25 +765,6 @@ void dp_catalog_ctrl_phy_reset(struct dp_catalog 
*dp_catalog)
dp_write_ahb(catalog, REG_DP_PHY_CTRL, 0x0);
 }
 
-int dp_catalog_ctrl_update_vx_px(struct dp_catalog *dp_catalog,
-   u8 v_level, u8 p_level)
-{
-   struct dp_catalog_private *catalog = container_of(dp_catalog,
-   struct dp_catalog_private, dp_catalog);
-   struct dp_io *dp_io = catalog->io;
-   struct phy *phy = dp_io->phy;
-   struct phy_configure_opts_dp *opts_dp = &dp_io->phy_opts.dp;
-
-   /* TODO: Update for all lanes instead of just first one */
-   opts_dp->voltage[0] = v_level;
-   opts_dp->pre[0] = p_level;
-   opts_dp->set_voltages = 1;
-   phy_configure(phy, &dp_io->phy_opts);
-   opts_dp->set_voltages = 0;
-
-   return 0;
-}
-
 void dp_catalog_ctrl_send_phy_pattern(struct dp_catalog *dp_catalog,
u32 pattern)
 {
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 38786e855b51..ba7c62ba7ca3 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -111,8 +111,6 @@ void dp_catalog_ctrl_set_psr(struct dp_catalog *dp_catalog, 
bool enter);
 u32 dp_catalog_link_is_connected(struct dp_catalog *dp_catalog);
 u32 dp_catalog_hpd_get_intr_status(struct dp_catalog *dp_catalog);
 void dp_catalog_ctrl_phy_reset(struct dp_catalog *dp_catalog);
-int dp_catalog_ctrl_update_vx_px(struct dp_catalog *dp_catalog, u8 v_level,
-   u8 p_level);
 int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog);
 u32 dp_catalog_ctrl_read_psr_interrupt_status(struct dp_catalog *dp_catalog);
 void dp_catalog_ctrl_update_transfer_unit(struct dp_catalog *dp_catalog,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index e367eb8e5bea..4aea72a2b8e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -87,6 +87,8 @@ struct dp_ctrl_private {
 
struct clk *pixel_clk;
 
+   union phy_configure_opts phy_opts;
+
struct completion idle_comp;
struct completion psr_op_comp;
struct completion video_comp;
@@ -1017,6 +1019,21 @@ static int dp_ctrl_wait4video_ready(struct 
dp_ctrl_private *ctrl)
return ret;
 }
 
+static int dp_ctrl_set_vx_px(struct dp_ctrl_private *ctrl,
+u8 v_level, u8 p_level)
+{
+   union phy_configure_opts *phy_opts = &ctrl->phy_opts;
+
+   /* TODO: Update for all lanes instead of just first one */
+   phy_opts->dp.voltage[0] = v_level;
+   phy_opts->dp.pre[0] = p_level;
+   phy_opts->dp.set_voltages = 1;
+   phy_configure(ctrl->parser->io.phy, phy_opts);
+   phy_opts->dp.set_voltages = 0;
+
+   return 0;
+}
+
 static int dp_ctrl_update_vx_px(struct dp_ctrl_private *ctrl)
 {
struct dp_link *link = ctrl->link;
@@ -1029,7 +1046,7 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private 
*ctrl)
drm_dbg_dp(ctrl->drm_dev,
"voltage level: %d emphasis level: %d\n",
voltage_swing_level, pre_emphasis_level);
-   ret = dp_catalog_ctrl_update_vx_px(ctrl->catalog,
+   ret = dp_ctrl_set_vx_px(ctrl,
voltage_swing_level, pre_emphasis_level);
 
if (ret)
@@ -1425,16 +1442,14 @@ static void dp_ctrl_link_clk_disable(struct dp_ctrl 
*dp_ctrl)
 static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl)
 {
int ret = 0;
-   struct dp_io *dp_io = &ctrl->parser->io;
-   struct phy *phy = dp_io->phy;
-   struct phy_configure_opts_dp *opts_dp = &dp_io->phy_opts.dp;
+   struct phy *phy = ctrl->parser->io.phy;
const u8 *dpcd = ctrl->panel->dpcd;
 
-   opts_dp->lanes = ctrl->link->link_params.num_lanes;
-   opts_dp->link_rate = ctrl->link->link_params.rate / 100;
-   opts_dp->ssc = drm_dp_max_downspread(dpcd);
+   ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
+   ctrl->phy_opts.dp.link_rate = ctrl->link->link_params.rate / 100;
+   ctrl->phy_opts.dp.ssc = drm_dp_max_downspread(dpcd);
 
-   phy_configure(phy, &dp_io->phy_opts);
+   phy_configure(phy, &ctrl->phy_opts);
phy_power_on(phy);
 
dev_pm_opp_s