* [PATCH v5 0/3] cleared DP_DOWNSPREAD_CTRL register
@ 2022-09-12 16:23 Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training Kuogee Hsieh
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 16:23 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, bjorn.andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_sbillaka, freedreno,
linux-arm-msm, linux-kernel
cleared DP_DOWNSPREAD_CTRL register before start link training
Kuogee Hsieh (3):
drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link
training
drm/msm/dp: replace variable err with len at dp_aux_link_power_up()
drm/msm/dp: retry 3 times if set sink to D0 poweer state failed
drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
drivers/gpu/drm/msm/dp/dp_link.c | 21 ++++++++++++---------
2 files changed, 17 insertions(+), 17 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
2022-09-12 16:23 [PATCH v5 0/3] cleared DP_DOWNSPREAD_CTRL register Kuogee Hsieh
@ 2022-09-12 16:23 ` Kuogee Hsieh
2022-09-12 18:39 ` Dmitry Baryshkov
2022-09-12 16:23 ` [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up() Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed Kuogee Hsieh
2 siblings, 1 reply; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 16:23 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, bjorn.andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_sbillaka, freedreno,
linux-arm-msm, linux-kernel
DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
upstream device disconnect. This patch will enforce this rule by always
cleared DOWNSPREAD_CTRL register to 0 before start link training. At rare
case that DP MSA timing parameters may be mis-interpreted by the sink
which causes audio sampling rate be calculated wrongly and cause audio
did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
Changes in v2:
1) fix spelling at commit text
2) merge ssc variable into encoding[0]
Changes in v3:
-- correct spelling of DOWNSPREAD_CTRL
-- replace err with len of ssize_t
Changes in v4:
-- split into 2 patches
Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index ab6aa13..2c74c59 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl,
{
int ret = 0;
const u8 *dpcd = ctrl->panel->dpcd;
- u8 encoding = DP_SET_ANSI_8B10B;
- u8 ssc;
+ u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
u8 assr;
struct dp_link_info link_info = {0};
@@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl,
dp_aux_link_configure(ctrl->aux, &link_info);
- if (drm_dp_max_downspread(dpcd)) {
- ssc = DP_SPREAD_AMP_0_5;
- drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
- }
+ if (drm_dp_max_downspread(dpcd))
+ encoding[0] |= DP_SPREAD_AMP_0_5;
- drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
- &encoding, 1);
+ /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */
+ drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2);
if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up()
2022-09-12 16:23 [PATCH v5 0/3] cleared DP_DOWNSPREAD_CTRL register Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training Kuogee Hsieh
@ 2022-09-12 16:23 ` Kuogee Hsieh
2022-09-12 18:34 ` Dmitry Baryshkov
2022-09-12 16:23 ` [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed Kuogee Hsieh
2 siblings, 1 reply; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 16:23 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, bjorn.andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_sbillaka, freedreno,
linux-arm-msm, linux-kernel
drm_dp_dpcd_readb() will return 1 to indicate one byte had been read
successfully. This patch replace variable "err" with "len" have more
correct meaning.
changes in v5:
-- split into 3 patches
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
drivers/gpu/drm/msm/dp/dp_link.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 36f0af0..9d5381d 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -49,21 +49,21 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
struct dp_link_info *link)
{
u8 value;
- int err;
+ ssize_t len;
if (link->revision < 0x11)
return 0;
- err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
- if (err < 0)
- return err;
+ len = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
+ if (len < 0)
+ return len;
value &= ~DP_SET_POWER_MASK;
value |= DP_SET_POWER_D0;
- err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
- if (err < 0)
- return err;
+ len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
+ if (len < 0)
+ return len;
usleep_range(1000, 2000);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed
2022-09-12 16:23 [PATCH v5 0/3] cleared DP_DOWNSPREAD_CTRL register Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up() Kuogee Hsieh
@ 2022-09-12 16:23 ` Kuogee Hsieh
2022-09-12 18:37 ` Dmitry Baryshkov
2 siblings, 1 reply; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 16:23 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, bjorn.andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_sbillaka, freedreno,
linux-arm-msm, linux-kernel
Bring sink out of D3 (power down) mode into D0 (normal operation) mode
by setting DP_SET_POWER_D0 bit to DP_SET_POWER dpcd register. This
patch will retry 3 times if written to DP_SET_POWER register failed.
Changes in v5:
-- split into two patches
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
drivers/gpu/drm/msm/dp/dp_link.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 9d5381d..4360728 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -50,6 +50,7 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
{
u8 value;
ssize_t len;
+ int i;
if (link->revision < 0x11)
return 0;
@@ -61,11 +62,13 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
value &= ~DP_SET_POWER_MASK;
value |= DP_SET_POWER_D0;
- len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
- if (len < 0)
- return len;
-
- usleep_range(1000, 2000);
+ /* retry for 1ms to give the sink time to wake up */
+ for (i = 0; i < 3; i++) {
+ len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
+ usleep_range(1000, 2000);
+ if (len == 1)
+ break;
+ }
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up()
2022-09-12 16:23 ` [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up() Kuogee Hsieh
@ 2022-09-12 18:34 ` Dmitry Baryshkov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-09-12 18:34 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 12/09/2022 19:23, Kuogee Hsieh wrote:
> drm_dp_dpcd_readb() will return 1 to indicate one byte had been read
> successfully. This patch replace variable "err" with "len" have more
> correct meaning.
>
> changes in v5:
> -- split into 3 patches
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/dp/dp_link.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
> index 36f0af0..9d5381d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_link.c
> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
> @@ -49,21 +49,21 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
> struct dp_link_info *link)
> {
> u8 value;
> - int err;
> + ssize_t len;
>
> if (link->revision < 0x11)
> return 0;
>
> - err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
> - if (err < 0)
> - return err;
> + len = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
> + if (len < 0)
> + return len;
>
> value &= ~DP_SET_POWER_MASK;
> value |= DP_SET_POWER_D0;
>
> - err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
> - if (err < 0)
> - return err;
> + len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
> + if (len < 0)
> + return len;
>
> usleep_range(1000, 2000);
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed
2022-09-12 16:23 ` [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed Kuogee Hsieh
@ 2022-09-12 18:37 ` Dmitry Baryshkov
2022-09-12 19:26 ` Kuogee Hsieh
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-09-12 18:37 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 12/09/2022 19:23, Kuogee Hsieh wrote:
> Bring sink out of D3 (power down) mode into D0 (normal operation) mode
> by setting DP_SET_POWER_D0 bit to DP_SET_POWER dpcd register. This
> patch will retry 3 times if written to DP_SET_POWER register failed.
Could you please elaborate this change? Can the sink succeed in reading
the DP_SET_POWER, but fail writing DP_SET_POWER?
>
> Changes in v5:
> -- split into two patches
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_link.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
> index 9d5381d..4360728 100644
> --- a/drivers/gpu/drm/msm/dp/dp_link.c
> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
> @@ -50,6 +50,7 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
> {
> u8 value;
> ssize_t len;
> + int i;
>
> if (link->revision < 0x11)
> return 0;
> @@ -61,11 +62,13 @@ static int dp_aux_link_power_up(struct drm_dp_aux *aux,
> value &= ~DP_SET_POWER_MASK;
> value |= DP_SET_POWER_D0;
>
> - len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
> - if (len < 0)
> - return len;
> -
> - usleep_range(1000, 2000);
> + /* retry for 1ms to give the sink time to wake up */
> + for (i = 0; i < 3; i++) {
> + len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
> + usleep_range(1000, 2000);
> + if (len == 1)
> + break;
> + }
>
> return 0;
> }
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
2022-09-12 16:23 ` [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training Kuogee Hsieh
@ 2022-09-12 18:39 ` Dmitry Baryshkov
2022-09-12 19:21 ` Kuogee Hsieh
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-09-12 18:39 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 12/09/2022 19:23, Kuogee Hsieh wrote:
> DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
> upstream device disconnect. This patch will enforce this rule by always
> cleared DOWNSPREAD_CTRL register to 0 before start link training. At rare
> case that DP MSA timing parameters may be mis-interpreted by the sink
> which causes audio sampling rate be calculated wrongly and cause audio
> did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
>
> Changes in v2:
> 1) fix spelling at commit text
> 2) merge ssc variable into encoding[0]
>
> Changes in v3:
> -- correct spelling of DOWNSPREAD_CTRL
> -- replace err with len of ssize_t
>
> Changes in v4:
> -- split into 2 patches
>
> Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index ab6aa13..2c74c59 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl,
> {
> int ret = 0;
> const u8 *dpcd = ctrl->panel->dpcd;
> - u8 encoding = DP_SET_ANSI_8B10B;
> - u8 ssc;
> + u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
> u8 assr;
> struct dp_link_info link_info = {0};
>
> @@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl,
>
> dp_aux_link_configure(ctrl->aux, &link_info);
>
> - if (drm_dp_max_downspread(dpcd)) {
> - ssc = DP_SPREAD_AMP_0_5;
> - drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
> - }
> + if (drm_dp_max_downspread(dpcd))
> + encoding[0] |= DP_SPREAD_AMP_0_5;
It would be simpler to call drm_dp_dpcd_write(ssc, DP_DOWNSPREAD_CTRL,
1) unconditionally here. You won't have to change the
encoding/DP_MAIN_LINK_CHANNEL_CODING_SET/etc.
>
> - drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
> - &encoding, 1);
> + /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */
> + drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2);
>
> if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
> assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
2022-09-12 18:39 ` Dmitry Baryshkov
@ 2022-09-12 19:21 ` Kuogee Hsieh
2022-09-12 19:25 ` Dmitry Baryshkov
0 siblings, 1 reply; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 19:21 UTC (permalink / raw)
To: Dmitry Baryshkov, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 9/12/2022 11:39 AM, Dmitry Baryshkov wrote:
> On 12/09/2022 19:23, Kuogee Hsieh wrote:
>> DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
>> upstream device disconnect. This patch will enforce this rule by always
>> cleared DOWNSPREAD_CTRL register to 0 before start link training. At
>> rare
>> case that DP MSA timing parameters may be mis-interpreted by the sink
>> which causes audio sampling rate be calculated wrongly and cause audio
>> did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
>>
>> Changes in v2:
>> 1) fix spelling at commit text
>> 2) merge ssc variable into encoding[0]
>>
>> Changes in v3:
>> -- correct spelling of DOWNSPREAD_CTRL
>> -- replace err with len of ssize_t
>>
>> Changes in v4:
>> -- split into 2 patches
>>
>> Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support")
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>> drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index ab6aa13..2c74c59 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct
>> dp_ctrl_private *ctrl,
>> {
>> int ret = 0;
>> const u8 *dpcd = ctrl->panel->dpcd;
>> - u8 encoding = DP_SET_ANSI_8B10B;
>> - u8 ssc;
>> + u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
>> u8 assr;
>> struct dp_link_info link_info = {0};
>> @@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct
>> dp_ctrl_private *ctrl,
>> dp_aux_link_configure(ctrl->aux, &link_info);
>> - if (drm_dp_max_downspread(dpcd)) {
>> - ssc = DP_SPREAD_AMP_0_5;
>> - drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
>> - }
>> + if (drm_dp_max_downspread(dpcd))
>> + encoding[0] |= DP_SPREAD_AMP_0_5;
>
> It would be simpler to call drm_dp_dpcd_write(ssc, DP_DOWNSPREAD_CTRL,
> 1) unconditionally here. You won't have to change the
> encoding/DP_MAIN_LINK_CHANNEL_CODING_SET/etc.
The difference is one write with 2 bytes against two writes with one
byte each.
I think it is more efficient to combine two bytes into one write since
these two bytes are consecutive address.
>
>> - drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
>> - &encoding, 1);
>> + /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */
>> + drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2);
>> if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
>> assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
2022-09-12 19:21 ` Kuogee Hsieh
@ 2022-09-12 19:25 ` Dmitry Baryshkov
2022-09-30 1:36 ` Abhinav Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-09-12 19:25 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 12/09/2022 22:21, Kuogee Hsieh wrote:
>
> On 9/12/2022 11:39 AM, Dmitry Baryshkov wrote:
>> On 12/09/2022 19:23, Kuogee Hsieh wrote:
>>> DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
>>> upstream device disconnect. This patch will enforce this rule by always
>>> cleared DOWNSPREAD_CTRL register to 0 before start link training. At
>>> rare
>>> case that DP MSA timing parameters may be mis-interpreted by the sink
>>> which causes audio sampling rate be calculated wrongly and cause audio
>>> did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
>>>
>>> Changes in v2:
>>> 1) fix spelling at commit text
>>> 2) merge ssc variable into encoding[0]
>>>
>>> Changes in v3:
>>> -- correct spelling of DOWNSPREAD_CTRL
>>> -- replace err with len of ssize_t
>>>
>>> Changes in v4:
>>> -- split into 2 patches
>>>
>>> Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support")
>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>> ---
>>> drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
>>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> index ab6aa13..2c74c59 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> @@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct
>>> dp_ctrl_private *ctrl,
>>> {
>>> int ret = 0;
>>> const u8 *dpcd = ctrl->panel->dpcd;
>>> - u8 encoding = DP_SET_ANSI_8B10B;
>>> - u8 ssc;
>>> + u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
>>> u8 assr;
>>> struct dp_link_info link_info = {0};
>>> @@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct
>>> dp_ctrl_private *ctrl,
>>> dp_aux_link_configure(ctrl->aux, &link_info);
>>> - if (drm_dp_max_downspread(dpcd)) {
>>> - ssc = DP_SPREAD_AMP_0_5;
>>> - drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
>>> - }
>>> + if (drm_dp_max_downspread(dpcd))
>>> + encoding[0] |= DP_SPREAD_AMP_0_5;
>>
>> It would be simpler to call drm_dp_dpcd_write(ssc, DP_DOWNSPREAD_CTRL,
>> 1) unconditionally here. You won't have to change the
>> encoding/DP_MAIN_LINK_CHANNEL_CODING_SET/etc.
>
> The difference is one write with 2 bytes against two writes with one
> byte each.
>
> I think it is more efficient to combine two bytes into one write since
> these two bytes are consecutive address.
I probably wouldn't do so, nevertheless:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>>
>>> - drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
>>> - &encoding, 1);
>>> + /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */
>>> + drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2);
>>> if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
>>> assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed
2022-09-12 18:37 ` Dmitry Baryshkov
@ 2022-09-12 19:26 ` Kuogee Hsieh
2022-11-02 15:51 ` Dmitry Baryshkov
0 siblings, 1 reply; 12+ messages in thread
From: Kuogee Hsieh @ 2022-09-12 19:26 UTC (permalink / raw)
To: Dmitry Baryshkov, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 9/12/2022 11:37 AM, Dmitry Baryshkov wrote:
> On 12/09/2022 19:23, Kuogee Hsieh wrote:
>> Bring sink out of D3 (power down) mode into D0 (normal operation) mode
>> by setting DP_SET_POWER_D0 bit to DP_SET_POWER dpcd register. This
>> patch will retry 3 times if written to DP_SET_POWER register failed.
>
> Could you please elaborate this change? Can the sink succeed in
> reading the DP_SET_POWER, but fail writing DP_SET_POWER?
yes, there is possible since it is not only set local sink device but
also all downstream sink devices to D0 state.
>
>>
>> Changes in v5:
>> -- split into two patches
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>> drivers/gpu/drm/msm/dp/dp_link.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c
>> b/drivers/gpu/drm/msm/dp/dp_link.c
>> index 9d5381d..4360728 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_link.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
>> @@ -50,6 +50,7 @@ static int dp_aux_link_power_up(struct drm_dp_aux
>> *aux,
>> {
>> u8 value;
>> ssize_t len;
>> + int i;
>> if (link->revision < 0x11)
>> return 0;
>> @@ -61,11 +62,13 @@ static int dp_aux_link_power_up(struct drm_dp_aux
>> *aux,
>> value &= ~DP_SET_POWER_MASK;
>> value |= DP_SET_POWER_D0;
>> - len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> - if (len < 0)
>> - return len;
>> -
>> - usleep_range(1000, 2000);
>> + /* retry for 1ms to give the sink time to wake up */
>> + for (i = 0; i < 3; i++) {
>> + len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> + usleep_range(1000, 2000);
>> + if (len == 1)
>> + break;
>> + }
>> return 0;
>> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
2022-09-12 19:25 ` Dmitry Baryshkov
@ 2022-09-30 1:36 ` Abhinav Kumar
0 siblings, 0 replies; 12+ messages in thread
From: Abhinav Kumar @ 2022-09-30 1:36 UTC (permalink / raw)
To: Dmitry Baryshkov, Kuogee Hsieh, dri-devel, robdclark, sean,
swboyd, dianders, vkoul, daniel, airlied, agross,
bjorn.andersson
Cc: quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 9/12/2022 12:25 PM, Dmitry Baryshkov wrote:
> On 12/09/2022 22:21, Kuogee Hsieh wrote:
>>
>> On 9/12/2022 11:39 AM, Dmitry Baryshkov wrote:
>>> On 12/09/2022 19:23, Kuogee Hsieh wrote:
>>>> DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
>>>> upstream device disconnect. This patch will enforce this rule by always
>>>> cleared DOWNSPREAD_CTRL register to 0 before start link training. At
>>>> rare
>>>> case that DP MSA timing parameters may be mis-interpreted by the sink
>>>> which causes audio sampling rate be calculated wrongly and cause audio
>>>> did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
>>>>
>>>> Changes in v2:
>>>> 1) fix spelling at commit text
>>>> 2) merge ssc variable into encoding[0]
>>>>
>>>> Changes in v3:
>>>> -- correct spelling of DOWNSPREAD_CTRL
>>>> -- replace err with len of ssize_t
>>>>
>>>> Changes in v4:
>>>> -- split into 2 patches
>>>>
>>>> Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support")
Fixes tag is wrong here. It should be:
Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 +++++--------
>>>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> index ab6aa13..2c74c59 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> @@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct
>>>> dp_ctrl_private *ctrl,
>>>> {
>>>> int ret = 0;
>>>> const u8 *dpcd = ctrl->panel->dpcd;
>>>> - u8 encoding = DP_SET_ANSI_8B10B;
>>>> - u8 ssc;
>>>> + u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
>>>> u8 assr;
>>>> struct dp_link_info link_info = {0};
>>>> @@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct
>>>> dp_ctrl_private *ctrl,
>>>> dp_aux_link_configure(ctrl->aux, &link_info);
>>>> - if (drm_dp_max_downspread(dpcd)) {
>>>> - ssc = DP_SPREAD_AMP_0_5;
>>>> - drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
>>>> - }
>>>> + if (drm_dp_max_downspread(dpcd))
>>>> + encoding[0] |= DP_SPREAD_AMP_0_5;
>>>
>>> It would be simpler to call drm_dp_dpcd_write(ssc,
>>> DP_DOWNSPREAD_CTRL, 1) unconditionally here. You won't have to change
>>> the encoding/DP_MAIN_LINK_CHANNEL_CODING_SET/etc.
>>
>> The difference is one write with 2 bytes against two writes with one
>> byte each.
>>
>> I think it is more efficient to combine two bytes into one write since
>> these two bytes are consecutive address.
>
> I probably wouldn't do so, nevertheless:
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>>
>>>
>>>> - drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
>>>> - &encoding, 1);
>>>> + /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */
>>>> + drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2);
>>>> if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
>>>> assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed
2022-09-12 19:26 ` Kuogee Hsieh
@ 2022-11-02 15:51 ` Dmitry Baryshkov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-11-02 15:51 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, bjorn.andersson
Cc: quic_abhinavk, quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 12/09/2022 22:26, Kuogee Hsieh wrote:
>
> On 9/12/2022 11:37 AM, Dmitry Baryshkov wrote:
>> On 12/09/2022 19:23, Kuogee Hsieh wrote:
>>> Bring sink out of D3 (power down) mode into D0 (normal operation) mode
>>> by setting DP_SET_POWER_D0 bit to DP_SET_POWER dpcd register. This
>>> patch will retry 3 times if written to DP_SET_POWER register failed.
>>
>> Could you please elaborate this change? Can the sink succeed in
>> reading the DP_SET_POWER, but fail writing DP_SET_POWER?
>
> yes, there is possible since it is not only set local sink device but
> also all downstream sink devices to D0 state.
Ack. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>>
>>>
>>> Changes in v5:
>>> -- split into two patches
>>>
>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>> ---
>>> drivers/gpu/drm/msm/dp/dp_link.c | 13 ++++++++-----
>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c
>>> b/drivers/gpu/drm/msm/dp/dp_link.c
>>> index 9d5381d..4360728 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_link.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
>>> @@ -50,6 +50,7 @@ static int dp_aux_link_power_up(struct drm_dp_aux
>>> *aux,
>>> {
>>> u8 value;
>>> ssize_t len;
>>> + int i;
>>> if (link->revision < 0x11)
>>> return 0;
>>> @@ -61,11 +62,13 @@ static int dp_aux_link_power_up(struct drm_dp_aux
>>> *aux,
>>> value &= ~DP_SET_POWER_MASK;
>>> value |= DP_SET_POWER_D0;
>>> - len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>>> - if (len < 0)
>>> - return len;
>>> -
>>> - usleep_range(1000, 2000);
>>> + /* retry for 1ms to give the sink time to wake up */
>>> + for (i = 0; i < 3; i++) {
>>> + len = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>>> + usleep_range(1000, 2000);
>>> + if (len == 1)
>>> + break;
>>> + }
>>> return 0;
>>> }
>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-11-02 15:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 16:23 [PATCH v5 0/3] cleared DP_DOWNSPREAD_CTRL register Kuogee Hsieh
2022-09-12 16:23 ` [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training Kuogee Hsieh
2022-09-12 18:39 ` Dmitry Baryshkov
2022-09-12 19:21 ` Kuogee Hsieh
2022-09-12 19:25 ` Dmitry Baryshkov
2022-09-30 1:36 ` Abhinav Kumar
2022-09-12 16:23 ` [PATCH v5 2/3] drm/msm/dp: replace variable err with len at dp_aux_link_power_up() Kuogee Hsieh
2022-09-12 18:34 ` Dmitry Baryshkov
2022-09-12 16:23 ` [PATCH v5 3/3] drm/msm/dp: retry 3 times if set sink to D0 poweer state failed Kuogee Hsieh
2022-09-12 18:37 ` Dmitry Baryshkov
2022-09-12 19:26 ` Kuogee Hsieh
2022-11-02 15:51 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).