Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-04-01 Thread Dmitry Baryshkov 
On Mon, 1 Apr 2024 at 23:15, Kuogee Hsieh  wrote:
>
>
> On 4/1/2024 9:25 AM, Kuogee Hsieh wrote:
> >
> > On 2/3/2024 5:47 AM, Dmitry Baryshkov wrote:
> >> Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
> >> pre-emphasis to 2, while the real maximum value for the sum of the
> >> voltage swing and pre-emphasis is 3. Fix the DP code to remove this
> >> limitation.
> >>
> >> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> >> Signed-off-by: Dmitry Baryshkov 
> > Reviewed-by: Kuogee Hsieh 
> Tested-by: Kuogee Hsieh 

Have you tested this on any device that actually reaches swing or pre_emph of 3?

-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-04-01 Thread Kuogee Hsieh 



On 4/1/2024 9:25 AM, Kuogee Hsieh wrote:


On 2/3/2024 5:47 AM, Dmitry Baryshkov wrote:

Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
pre-emphasis to 2, while the real maximum value for the sum of the
voltage swing and pre-emphasis is 3. Fix the DP code to remove this
limitation.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Dmitry Baryshkov 

Reviewed-by: Kuogee Hsieh 

Tested-by: Kuogee Hsieh 

---
  drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
  drivers/gpu/drm/msm/dp/dp_link.c | 22 +++---
  drivers/gpu/drm/msm/dp/dp_link.h | 14 +-
  3 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
b/drivers/gpu/drm/msm/dp/dp_ctrl.c

index 77a8d9366ed7..26241970019f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1024,14 +1024,14 @@ static int dp_ctrl_update_vx_px(struct 
dp_ctrl_private *ctrl)

  if (ret)
  return ret;
  -    if (voltage_swing_level >= DP_TRAIN_VOLTAGE_SWING_MAX) {
+    if (voltage_swing_level >= DP_TRAIN_LEVEL_MAX) {
  drm_dbg_dp(ctrl->drm_dev,
  "max. voltage swing level reached %d\n",
  voltage_swing_level);
  max_level_reached |= DP_TRAIN_MAX_SWING_REACHED;
  }
  -    if (pre_emphasis_level >= DP_TRAIN_PRE_EMPHASIS_MAX) {
+    if (pre_emphasis_level >= DP_TRAIN_LEVEL_MAX) {
  drm_dbg_dp(ctrl->drm_dev,
  "max. pre-emphasis level reached %d\n",
  pre_emphasis_level);
@@ -1122,7 +1122,7 @@ static int dp_ctrl_link_train_1(struct 
dp_ctrl_private *ctrl,

  }
    if (ctrl->link->phy_params.v_level >=
-    DP_TRAIN_VOLTAGE_SWING_MAX) {
+    DP_TRAIN_LEVEL_MAX) {
  DRM_ERROR_RATELIMITED("max v_level reached\n");
  return -EAGAIN;
  }
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c 
b/drivers/gpu/drm/msm/dp/dp_link.c

index 98427d45e9a7..e7da0571ecff 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1107,6 +1107,7 @@ int dp_link_get_colorimetry_config(struct 
dp_link *dp_link)

  int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
  {
  int i;
+    u8 max_p_level;
  int v_max = 0, p_max = 0;
  struct dp_link_private *link;
  @@ -1138,30 +1139,29 @@ int dp_link_adjust_levels(struct dp_link 
*dp_link, u8 *link_status)
   * Adjust the voltage swing and pre-emphasis level combination 
to within

   * the allowable range.
   */
-    if (dp_link->phy_params.v_level > DP_TRAIN_VOLTAGE_SWING_MAX) {
+    if (dp_link->phy_params.v_level > DP_TRAIN_LEVEL_MAX) {
  drm_dbg_dp(link->drm_dev,
  "Requested vSwingLevel=%d, change to %d\n",
  dp_link->phy_params.v_level,
-    DP_TRAIN_VOLTAGE_SWING_MAX);
-    dp_link->phy_params.v_level = DP_TRAIN_VOLTAGE_SWING_MAX;
+    DP_TRAIN_LEVEL_MAX);
+    dp_link->phy_params.v_level = DP_TRAIN_LEVEL_MAX;
  }
  -    if (dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_MAX) {
+    if (dp_link->phy_params.p_level > DP_TRAIN_LEVEL_MAX) {
  drm_dbg_dp(link->drm_dev,
  "Requested preEmphasisLevel=%d, change to %d\n",
  dp_link->phy_params.p_level,
-    DP_TRAIN_PRE_EMPHASIS_MAX);
-    dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_MAX;
+    DP_TRAIN_LEVEL_MAX);
+    dp_link->phy_params.p_level = DP_TRAIN_LEVEL_MAX;
  }
  -    if ((dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_LVL_1)
-    && (dp_link->phy_params.v_level ==
-    DP_TRAIN_VOLTAGE_SWING_LVL_2)) {
+    max_p_level = DP_TRAIN_LEVEL_MAX - dp_link->phy_params.v_level;
+    if (dp_link->phy_params.p_level > max_p_level) {
  drm_dbg_dp(link->drm_dev,
  "Requested preEmphasisLevel=%d, change to %d\n",
  dp_link->phy_params.p_level,
-    DP_TRAIN_PRE_EMPHASIS_LVL_1);
-    dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_LVL_1;
+    max_p_level);
+    dp_link->phy_params.p_level = max_p_level;
  }
    drm_dbg_dp(link->drm_dev, "adjusted: v_level=%d, p_level=%d\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h 
b/drivers/gpu/drm/msm/dp/dp_link.h

index 9dd4dd926530..79c3a02b8dac 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -19,19 +19,7 @@ struct dp_link_info {
  unsigned long capabilities;
  };
  -enum dp_link_voltage_level {
-    DP_TRAIN_VOLTAGE_SWING_LVL_0    = 0,
-    DP_TRAIN_VOLTAGE_SWING_LVL_1    = 1,
-    DP_TRAIN_VOLTAGE_SWING_LVL_2    = 2,
-    DP_TRAIN_VOLTAGE_SWING_MAX    = DP_TRAIN_VOLTAGE_SWING_LVL_2,
-};
-
-enum dp_link_preemaphasis_level {
-    DP_TRAIN_PRE_EMPHASIS_LVL_0    = 0,
-    DP_TRAIN_PRE_EMPHASIS_LVL_1    = 1,
-    DP_TRAIN_PRE_EMPHASIS_LVL_2    = 2,
-    DP_TRAIN_PRE_EMPHASIS_MAX    = DP_TRAIN_PRE_EMPHASIS_LVL_2,
-};
+#define

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-04-01 Thread Kuogee Hsieh 



On 2/3/2024 5:47 AM, Dmitry Baryshkov wrote:

Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
pre-emphasis to 2, while the real maximum value for the sum of the
voltage swing and pre-emphasis is 3. Fix the DP code to remove this
limitation.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Dmitry Baryshkov 

Reviewed-by: Kuogee Hsieh 

---
  drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
  drivers/gpu/drm/msm/dp/dp_link.c | 22 +++---
  drivers/gpu/drm/msm/dp/dp_link.h | 14 +-
  3 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 77a8d9366ed7..26241970019f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1024,14 +1024,14 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private 
*ctrl)
if (ret)
return ret;
  
-	if (voltage_swing_level >= DP_TRAIN_VOLTAGE_SWING_MAX) {

+   if (voltage_swing_level >= DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(ctrl->drm_dev,
"max. voltage swing level reached %d\n",
voltage_swing_level);
max_level_reached |= DP_TRAIN_MAX_SWING_REACHED;
}
  
-	if (pre_emphasis_level >= DP_TRAIN_PRE_EMPHASIS_MAX) {

+   if (pre_emphasis_level >= DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(ctrl->drm_dev,
"max. pre-emphasis level reached %d\n",
pre_emphasis_level);
@@ -1122,7 +1122,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private 
*ctrl,
}
  
  		if (ctrl->link->phy_params.v_level >=

-   DP_TRAIN_VOLTAGE_SWING_MAX) {
+   DP_TRAIN_LEVEL_MAX) {
DRM_ERROR_RATELIMITED("max v_level reached\n");
return -EAGAIN;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 98427d45e9a7..e7da0571ecff 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1107,6 +1107,7 @@ int dp_link_get_colorimetry_config(struct dp_link 
*dp_link)
  int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
  {
int i;
+   u8 max_p_level;
int v_max = 0, p_max = 0;
struct dp_link_private *link;
  
@@ -1138,30 +1139,29 @@ int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)

 * Adjust the voltage swing and pre-emphasis level combination to within
 * the allowable range.
 */
-   if (dp_link->phy_params.v_level > DP_TRAIN_VOLTAGE_SWING_MAX) {
+   if (dp_link->phy_params.v_level > DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(link->drm_dev,
"Requested vSwingLevel=%d, change to %d\n",
dp_link->phy_params.v_level,
-   DP_TRAIN_VOLTAGE_SWING_MAX);
-   dp_link->phy_params.v_level = DP_TRAIN_VOLTAGE_SWING_MAX;
+   DP_TRAIN_LEVEL_MAX);
+   dp_link->phy_params.v_level = DP_TRAIN_LEVEL_MAX;
}
  
-	if (dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_MAX) {

+   if (dp_link->phy_params.p_level > DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(link->drm_dev,
"Requested preEmphasisLevel=%d, change to %d\n",
dp_link->phy_params.p_level,
-   DP_TRAIN_PRE_EMPHASIS_MAX);
-   dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_MAX;
+   DP_TRAIN_LEVEL_MAX);
+   dp_link->phy_params.p_level = DP_TRAIN_LEVEL_MAX;
}
  
-	if ((dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_LVL_1)

-   && (dp_link->phy_params.v_level ==
-   DP_TRAIN_VOLTAGE_SWING_LVL_2)) {
+   max_p_level = DP_TRAIN_LEVEL_MAX - dp_link->phy_params.v_level;
+   if (dp_link->phy_params.p_level > max_p_level) {
drm_dbg_dp(link->drm_dev,
"Requested preEmphasisLevel=%d, change to %d\n",
dp_link->phy_params.p_level,
-   DP_TRAIN_PRE_EMPHASIS_LVL_1);
-   dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_LVL_1;
+   max_p_level);
+   dp_link->phy_params.p_level = max_p_level;
}
  
  	drm_dbg_dp(link->drm_dev, "adjusted: v_level=%d, p_level=%d\n",

diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index 9dd4dd926530..79c3a02b8dac 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -19,19 +19,7 @@ struct dp_link_info {
unsigned long capabilities;
  };
  
-enum dp_link_voltage_level {

-   DP_TRAIN_VOLTAGE_SWING_LVL_0= 0,
-   DP_TRAIN_VOLTAGE_SWING_LVL_1= 1,
-   DP_TRAIN_VOLTAGE_SWING_LVL_2= 2,
-

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-03-29 Thread Abhinav Kumar 

Hi Doug

On 3/29/2024 4:07 PM, Doug Anderson wrote:

Hi,

On Sat, Feb 3, 2024 at 5:47 AM Dmitry Baryshkov
 wrote:


Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
pre-emphasis to 2, while the real maximum value for the sum of the
voltage swing and pre-emphasis is 3. Fix the DP code to remove this
limitation.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
  drivers/gpu/drm/msm/dp/dp_link.c | 22 +++---
  drivers/gpu/drm/msm/dp/dp_link.h | 14 +-
  3 files changed, 15 insertions(+), 27 deletions(-)


What ever happened with this patch? It seemed important so I've been
trying to check back on it, but it seems to still be in limbo. I was
assuming that (maybe?) Abhinav would check things against the hardware
documentation and give it a Reviewed-by and then it would land...

-Doug


The issue for which this patch was originally made (DP link training 
issue on x1e80100) was not getting fixed by this patch.


That one turned out as actually a PLL locking issue. So this kind of 
went off the radar as it was not immediately needed to fix anything.


I will wait for Kuogee's response on this patch. He was OOO entire Feb 
so this got missed.

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-03-29 Thread Doug Anderson 
Hi,

On Sat, Feb 3, 2024 at 5:47 AM Dmitry Baryshkov
 wrote:
>
> Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
> pre-emphasis to 2, while the real maximum value for the sum of the
> voltage swing and pre-emphasis is 3. Fix the DP code to remove this
> limitation.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
>  drivers/gpu/drm/msm/dp/dp_link.c | 22 +++---
>  drivers/gpu/drm/msm/dp/dp_link.h | 14 +-
>  3 files changed, 15 insertions(+), 27 deletions(-)

What ever happened with this patch? It seemed important so I've been
trying to check back on it, but it seems to still be in limbo. I was
assuming that (maybe?) Abhinav would check things against the hardware
documentation and give it a Reviewed-by and then it would land...

-Doug

[PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

2024-02-03 Thread Dmitry Baryshkov 
Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
pre-emphasis to 2, while the real maximum value for the sum of the
voltage swing and pre-emphasis is 3. Fix the DP code to remove this
limitation.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
 drivers/gpu/drm/msm/dp/dp_link.c | 22 +++---
 drivers/gpu/drm/msm/dp/dp_link.h | 14 +-
 3 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 77a8d9366ed7..26241970019f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1024,14 +1024,14 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private 
*ctrl)
if (ret)
return ret;
 
-   if (voltage_swing_level >= DP_TRAIN_VOLTAGE_SWING_MAX) {
+   if (voltage_swing_level >= DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(ctrl->drm_dev,
"max. voltage swing level reached %d\n",
voltage_swing_level);
max_level_reached |= DP_TRAIN_MAX_SWING_REACHED;
}
 
-   if (pre_emphasis_level >= DP_TRAIN_PRE_EMPHASIS_MAX) {
+   if (pre_emphasis_level >= DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(ctrl->drm_dev,
"max. pre-emphasis level reached %d\n",
pre_emphasis_level);
@@ -1122,7 +1122,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private 
*ctrl,
}
 
if (ctrl->link->phy_params.v_level >=
-   DP_TRAIN_VOLTAGE_SWING_MAX) {
+   DP_TRAIN_LEVEL_MAX) {
DRM_ERROR_RATELIMITED("max v_level reached\n");
return -EAGAIN;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 98427d45e9a7..e7da0571ecff 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1107,6 +1107,7 @@ int dp_link_get_colorimetry_config(struct dp_link 
*dp_link)
 int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
 {
int i;
+   u8 max_p_level;
int v_max = 0, p_max = 0;
struct dp_link_private *link;
 
@@ -1138,30 +1139,29 @@ int dp_link_adjust_levels(struct dp_link *dp_link, u8 
*link_status)
 * Adjust the voltage swing and pre-emphasis level combination to within
 * the allowable range.
 */
-   if (dp_link->phy_params.v_level > DP_TRAIN_VOLTAGE_SWING_MAX) {
+   if (dp_link->phy_params.v_level > DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(link->drm_dev,
"Requested vSwingLevel=%d, change to %d\n",
dp_link->phy_params.v_level,
-   DP_TRAIN_VOLTAGE_SWING_MAX);
-   dp_link->phy_params.v_level = DP_TRAIN_VOLTAGE_SWING_MAX;
+   DP_TRAIN_LEVEL_MAX);
+   dp_link->phy_params.v_level = DP_TRAIN_LEVEL_MAX;
}
 
-   if (dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_MAX) {
+   if (dp_link->phy_params.p_level > DP_TRAIN_LEVEL_MAX) {
drm_dbg_dp(link->drm_dev,
"Requested preEmphasisLevel=%d, change to %d\n",
dp_link->phy_params.p_level,
-   DP_TRAIN_PRE_EMPHASIS_MAX);
-   dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_MAX;
+   DP_TRAIN_LEVEL_MAX);
+   dp_link->phy_params.p_level = DP_TRAIN_LEVEL_MAX;
}
 
-   if ((dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_LVL_1)
-   && (dp_link->phy_params.v_level ==
-   DP_TRAIN_VOLTAGE_SWING_LVL_2)) {
+   max_p_level = DP_TRAIN_LEVEL_MAX - dp_link->phy_params.v_level;
+   if (dp_link->phy_params.p_level > max_p_level) {
drm_dbg_dp(link->drm_dev,
"Requested preEmphasisLevel=%d, change to %d\n",
dp_link->phy_params.p_level,
-   DP_TRAIN_PRE_EMPHASIS_LVL_1);
-   dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_LVL_1;
+   max_p_level);
+   dp_link->phy_params.p_level = max_p_level;
}
 
drm_dbg_dp(link->drm_dev, "adjusted: v_level=%d, p_level=%d\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index 9dd4dd926530..79c3a02b8dac 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -19,19 +19,7 @@ struct dp_link_info {
unsigned long capabilities;
 };
 
-enum dp_link_voltage_level {
-   DP_TRAIN_VOLTAGE_SWING_LVL_0= 0,
-   DP_TRAIN_VOLTAGE_SWING_LVL_1= 1,
-   DP_TRAIN_VOLTAGE_SWING_LVL_2= 2,
-   DP_TRAIN_VOLTAGE_SWING_MAX  =