Re: [PATCH 12/41] drm/bridge: analogix_dp: add fast link train for eDP
On 21.03.2017 21:37, Sean Paul wrote:
> On Thu, Mar 16, 2017 at 03:14:28PM +0100, Andrzej Hajda wrote:
>> On 10.03.2017 05:32, Sean Paul wrote:
>>> From: zain wang
>>>
>>> We would meet a short black screen when exit PSR with the full link
>>> training, In this case, we should use fast link train instead of full
>>> link training.
>>>
>>> Signed-off-by: zain wang
>>> Signed-off-by: Sean Paul
>>> ---
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142
>>> -
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
>>> 2 files changed, 114 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 64d94a34874d..5bc151b0995b 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -10,17 +10,18 @@
>>> * option) any later version.
>>> */
>>>
>>> -#include
>>> -#include
>>> -#include
>>> #include
>>> -#include
>>> +#include
>>> +#include
>>> +#include
>>> #include
>>> +#include
>>> +#include
>>> +#include
>>> #include
>>> #include
>>> -#include
>>> -#include
>>> #include
>>> +#include
>>>
>>> #include
>>> #include
>>> @@ -35,6 +36,8 @@
>>>
>>> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>>>
>>> +static const bool verify_fast_training;
>>> +
>> Quite odd debug sentinel, I am not sure if it is good practice to use
>> such construct.
> IIRC, they originally had a #define and then used #ifdef below to gate the
> verification. My concern with that was the code would rot if it wasn't
> compiled.
It just looks unusual, I am not strongly against current version.
>
>
>
>>> @@ -853,10 +933,10 @@ static void analogix_dp_commit(struct
>>> analogix_dp_device *dp)
>>> DRM_ERROR("failed to disable the panel\n");
>>> }
>>>
>>> - ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
>>> -dp->video_info.max_link_rate);
>>> + ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
>>> +DP_TIMEOUT_TRAINING_US * 5);
>>> if (ret) {
>>> - dev_err(dp->dev, "unable to do link train\n");
>>> + dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
>> And here we have ", ret=%d\n". Maybe it would be good to make it
>> consistent across whole driver.
> Meh. I don't think it's worth bikeshedding over commas in error messages. I'm
> just happy there are error checks/messages.
But if there will be next iteration it is not big deal to fix it then :)
Regards
Andrzej
>
>
>> Another issue with consistency in DRM_DEV_ERROR vs dev_err.
> Yeah, that's annoying.
>
> Sean
>
>> Regards
>> Andrzej
>>
>>> return;
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> index e135a42cb19e..920607d7eb3e 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> @@ -20,6 +20,8 @@
>>> #define MAX_CR_LOOP 5
>>> #define MAX_EQ_LOOP 5
>>>
>>> +/* Training takes 22ms if AUX channel comm fails. Use this as retry
>>> interval */
>>> +#define DP_TIMEOUT_TRAINING_US 22000
>>> #define DP_TIMEOUT_PSR_LOOP_MS 300
>>>
>>> /* DP_MAX_LANE_COUNT */
>>> @@ -171,6 +173,7 @@ struct analogix_dp_device {
>>> int hpd_gpio;
>>> boolforce_hpd;
>>> boolpsr_enable;
>>> + boolfast_train_support;
>>>
>>> struct mutexpanel_lock;
>>> boolpanel_is_modeset;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/41] drm/bridge: analogix_dp: add fast link train for eDP
On Thu, Mar 16, 2017 at 03:14:28PM +0100, Andrzej Hajda wrote:
> On 10.03.2017 05:32, Sean Paul wrote:
> > From: zain wang
> >
> > We would meet a short black screen when exit PSR with the full link
> > training, In this case, we should use fast link train instead of full
> > link training.
> >
> > Signed-off-by: zain wang
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142
> > -
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
> > 2 files changed, 114 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index 64d94a34874d..5bc151b0995b 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -10,17 +10,18 @@
> > * option) any later version.
> > */
> >
> > -#include
> > -#include
> > -#include
> > #include
> > -#include
> > +#include
> > +#include
> > +#include
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include
> > -#include
> > -#include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -35,6 +36,8 @@
> >
> > #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
> >
> > +static const bool verify_fast_training;
> > +
>
> Quite odd debug sentinel, I am not sure if it is good practice to use
> such construct.
IIRC, they originally had a #define and then used #ifdef below to gate the
verification. My concern with that was the code would rot if it wasn't compiled.
> > @@ -853,10 +933,10 @@ static void analogix_dp_commit(struct
> > analogix_dp_device *dp)
> > DRM_ERROR("failed to disable the panel\n");
> > }
> >
> > - ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
> > -dp->video_info.max_link_rate);
> > + ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
> > +DP_TIMEOUT_TRAINING_US * 5);
> > if (ret) {
> > - dev_err(dp->dev, "unable to do link train\n");
> > + dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
>
> And here we have ", ret=%d\n". Maybe it would be good to make it
> consistent across whole driver.
Meh. I don't think it's worth bikeshedding over commas in error messages. I'm
just happy there are error checks/messages.
> Another issue with consistency in DRM_DEV_ERROR vs dev_err.
Yeah, that's annoying.
Sean
>
> Regards
> Andrzej
>
> > return;
> > }
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> > index e135a42cb19e..920607d7eb3e 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> > @@ -20,6 +20,8 @@
> > #define MAX_CR_LOOP 5
> > #define MAX_EQ_LOOP 5
> >
> > +/* Training takes 22ms if AUX channel comm fails. Use this as retry
> > interval */
> > +#define DP_TIMEOUT_TRAINING_US 22000
> > #define DP_TIMEOUT_PSR_LOOP_MS 300
> >
> > /* DP_MAX_LANE_COUNT */
> > @@ -171,6 +173,7 @@ struct analogix_dp_device {
> > int hpd_gpio;
> > boolforce_hpd;
> > boolpsr_enable;
> > + boolfast_train_support;
> >
> > struct mutexpanel_lock;
> > boolpanel_is_modeset;
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/41] drm/bridge: analogix_dp: add fast link train for eDP
On 10.03.2017 05:32, Sean Paul wrote:
> From: zain wang
>
> We would meet a short black screen when exit PSR with the full link
> training, In this case, we should use fast link train instead of full
> link training.
>
> Signed-off-by: zain wang
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142
> -
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
> 2 files changed, 114 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 64d94a34874d..5bc151b0995b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -10,17 +10,18 @@
> * option) any later version.
> */
>
> -#include
> -#include
> -#include
> #include
> -#include
> +#include
> +#include
> +#include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> -#include
> -#include
> #include
> +#include
>
> #include
> #include
> @@ -35,6 +36,8 @@
>
> #define to_dp(nm)container_of(nm, struct analogix_dp_device, nm)
>
> +static const bool verify_fast_training;
> +
Quite odd debug sentinel, I am not sure if it is good practice to use
such construct.
> struct bridge_init {
> struct i2c_client *client;
> struct device_node *node;
> @@ -531,7 +534,7 @@ static int analogix_dp_process_equalizer_training(struct
> analogix_dp_device *dp)
> {
> int lane, lane_count, retval;
> u32 reg;
> - u8 link_align, link_status[2], adjust_request[2];
> + u8 link_align, link_status[2], adjust_request[2], spread;
>
> usleep_range(400, 401);
>
> @@ -574,6 +577,20 @@ static int analogix_dp_process_equalizer_training(struct
> analogix_dp_device *dp)
> dev_dbg(dp->dev, "final lane count = %.2x\n",
> dp->link_train.lane_count);
>
> + retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD,
> +&spread);
> + if (retval != 1) {
> + dev_err(dp->dev, "failed to read downspread %d\n",
comma before %d ?
> + retval);
> + dp->fast_train_support = false;
> + } else {
> + dp->fast_train_support =
> + (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?
> + true : false;
dp->fast_train_support = !!(spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
or even
dp->fast_train_support = spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
if dp->fast_train_support is bool
> + }
> + dev_dbg(dp->dev, "fast link training %s\n",
> + dp->fast_train_support ? "supported" : "unsupported");
> +
> /* set enhanced mode if available */
> analogix_dp_set_enhanced_mode(dp);
> dp->link_train.lt_state = FINISHED;
> @@ -630,10 +647,12 @@ static void analogix_dp_get_max_rx_lane_count(struct
> analogix_dp_device *dp,
> *lane_count = DPCD_MAX_LANE_COUNT(data);
> }
>
> -static void analogix_dp_init_training(struct analogix_dp_device *dp,
> - enum link_lane_count_type max_lane,
> - int max_rate)
> +static int analogix_dp_full_link_train(struct analogix_dp_device *dp,
> +u32 max_lanes, u32 max_rate)
> {
> + int retval = 0;
> + bool training_finished = false;
> +
> /*
>* MACRO_RST must be applied after the PLL_LOCK to avoid
>* the DP inter pair skew issue for at least 10 us
> @@ -659,18 +678,13 @@ static void analogix_dp_init_training(struct
> analogix_dp_device *dp,
> }
>
> /* Setup TX lane count & rate */
> - if (dp->link_train.lane_count > max_lane)
> - dp->link_train.lane_count = max_lane;
> + if (dp->link_train.lane_count > max_lanes)
> + dp->link_train.lane_count = max_lanes;
> if (dp->link_train.link_rate > max_rate)
> dp->link_train.link_rate = max_rate;
>
> /* All DP analog module power up */
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
> -}
> -
> -static int analogix_dp_sw_link_training(struct analogix_dp_device *dp)
> -{
> - int retval = 0, training_finished = 0;
>
> dp->link_train.lt_state = START;
>
> @@ -705,22 +719,88 @@ static int analogix_dp_sw_link_training(struct
> analogix_dp_device *dp)
> return retval;
> }
>
> -static int analogix_dp_set_link_train(struct analogix_dp_device *dp,
> - u32 count, u32 bwtype)
> +static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
> {
> - int i;
> - int retval;
> + int i, ret;
> + u8 link_align, link_status[2];
> + enum pll_status status;
>
> - for (i = 0; i
[PATCH 12/41] drm/bridge: analogix_dp: add fast link train for eDP
From: zain wang
We would meet a short black screen when exit PSR with the full link
training, In this case, we should use fast link train instead of full
link training.
Signed-off-by: zain wang
Signed-off-by: Sean Paul
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142 -
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
2 files changed, 114 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 64d94a34874d..5bc151b0995b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -10,17 +10,18 @@
* option) any later version.
*/
-#include
-#include
-#include
#include
-#include
+#include
+#include
+#include
#include
+#include
+#include
+#include
#include
#include
-#include
-#include
#include
+#include
#include
#include
@@ -35,6 +36,8 @@
#define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
+static const bool verify_fast_training;
+
struct bridge_init {
struct i2c_client *client;
struct device_node *node;
@@ -531,7 +534,7 @@ static int analogix_dp_process_equalizer_training(struct
analogix_dp_device *dp)
{
int lane, lane_count, retval;
u32 reg;
- u8 link_align, link_status[2], adjust_request[2];
+ u8 link_align, link_status[2], adjust_request[2], spread;
usleep_range(400, 401);
@@ -574,6 +577,20 @@ static int analogix_dp_process_equalizer_training(struct
analogix_dp_device *dp)
dev_dbg(dp->dev, "final lane count = %.2x\n",
dp->link_train.lane_count);
+ retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD,
+ &spread);
+ if (retval != 1) {
+ dev_err(dp->dev, "failed to read downspread %d\n",
+ retval);
+ dp->fast_train_support = false;
+ } else {
+ dp->fast_train_support =
+ (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?
+ true : false;
+ }
+ dev_dbg(dp->dev, "fast link training %s\n",
+ dp->fast_train_support ? "supported" : "unsupported");
+
/* set enhanced mode if available */
analogix_dp_set_enhanced_mode(dp);
dp->link_train.lt_state = FINISHED;
@@ -630,10 +647,12 @@ static void analogix_dp_get_max_rx_lane_count(struct
analogix_dp_device *dp,
*lane_count = DPCD_MAX_LANE_COUNT(data);
}
-static void analogix_dp_init_training(struct analogix_dp_device *dp,
- enum link_lane_count_type max_lane,
- int max_rate)
+static int analogix_dp_full_link_train(struct analogix_dp_device *dp,
+ u32 max_lanes, u32 max_rate)
{
+ int retval = 0;
+ bool training_finished = false;
+
/*
* MACRO_RST must be applied after the PLL_LOCK to avoid
* the DP inter pair skew issue for at least 10 us
@@ -659,18 +678,13 @@ static void analogix_dp_init_training(struct
analogix_dp_device *dp,
}
/* Setup TX lane count & rate */
- if (dp->link_train.lane_count > max_lane)
- dp->link_train.lane_count = max_lane;
+ if (dp->link_train.lane_count > max_lanes)
+ dp->link_train.lane_count = max_lanes;
if (dp->link_train.link_rate > max_rate)
dp->link_train.link_rate = max_rate;
/* All DP analog module power up */
analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
-}
-
-static int analogix_dp_sw_link_training(struct analogix_dp_device *dp)
-{
- int retval = 0, training_finished = 0;
dp->link_train.lt_state = START;
@@ -705,22 +719,88 @@ static int analogix_dp_sw_link_training(struct
analogix_dp_device *dp)
return retval;
}
-static int analogix_dp_set_link_train(struct analogix_dp_device *dp,
- u32 count, u32 bwtype)
+static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
{
- int i;
- int retval;
+ int i, ret;
+ u8 link_align, link_status[2];
+ enum pll_status status;
- for (i = 0; i < DP_TIMEOUT_LOOP_COUNT; i++) {
- analogix_dp_init_training(dp, count, bwtype);
- retval = analogix_dp_sw_link_training(dp);
- if (retval == 0)
- break;
+ analogix_dp_reset_macro(dp);
- usleep_range(100, 110);
+ analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
+ analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
+
+ for (i = 0; i < dp->link_train.lane_count; i++) {
+
