Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1478766 > unrolled thread
| Started by | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| First post | 2016-09-08 05:50 +0200 |
| Last post | 2016-09-20 04:20 +0200 |
| Articles | 9 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Yakir Yang <ykk@rock-chips.com> - 2016-09-08 05:50 +0200
Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Sean Paul <seanpaul@chromium.org> - 2016-09-08 16:20 +0200
Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Yakir Yang <ykk@rock-chips.com> - 2016-09-09 11:20 +0200
[PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Yakir Yang <ykk@rock-chips.com> - 2016-09-09 11:50 +0200
Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Sean Paul <seanpaul@chromium.org> - 2016-09-12 16:00 +0200
Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR Yakir Yang <ykk@rock-chips.com> - 2016-09-20 04:30 +0200
[PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop Yakir Yang <ykk@rock-chips.com> - 2016-09-09 11:50 +0200
Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop Sean Paul <seanpaul@chromium.org> - 2016-09-12 16:00 +0200
Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop Yakir Yang <ykk@rock-chips.com> - 2016-09-20 04:20 +0200
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-08 05:50 +0200 |
| Subject | [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <seYvD-xM-5@gated-at.bofh.it> |
Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
function, or printing the error Sink PSR state if we failed to effect
the request PSR setting.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v2:
- A bunch of good fixes from Sean
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 4 ++--
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
psr_vsc.DB0 = 0;
psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
+ return analogix_dp_send_psr_spd(dp, &psr_vsc);
}
EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
psr_vsc.DB0 = 0;
psr_vsc.DB1 = 0;
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
+ return analogix_dp_send_psr_spd(dp, &psr_vsc);
}
EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..6c07a50 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
- struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+ struct edp_vsc_psr *vsc);
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
struct drm_dp_aux_msg *msg);
#endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..09d703b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
}
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
- struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+ struct edp_vsc_psr *vsc)
{
+ unsigned long timeout;
unsigned int val;
+ u8 sink;
/* don't send info frame */
val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
val |= IF_EN;
writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+ timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
+ while (time_before(jiffies, timeout)) {
+ val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+ if (val != 1) {
+ dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+ return val;
+ }
+
+ if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
+ !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
+ break;
+
+ usleep_range(1000, 1500);
+ }
+
+ dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
+
+ return -ETIMEDOUT;
}
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
--
1.9.1
[toc] | [next] | [standalone]
| From | Sean Paul <seanpaul@chromium.org> |
|---|---|
| Date | 2016-09-08 16:20 +0200 |
| Subject | Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <sf8lj-6Sl-11@gated-at.bofh.it> |
| In reply to | #1478766 |
On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
> function, or printing the error Sink PSR state if we failed to effect
> the request PSR setting.
>
Let's change to:
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR
setting.
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v2:
> - A bunch of good fixes from Sean
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 4 ++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
> 3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = 0;
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..6c07a50 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc);
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> struct drm_dp_aux_msg *msg);
> #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..09d703b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
> }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc)
> {
> + unsigned long timeout;
> unsigned int val;
> + u8 sink;
>
> /* don't send info frame */
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> val |= IF_EN;
> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> + timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
retries, whereas you're using it as number of ms. Fortunately, the
retry number is so high that this works out :)
In a separate patch preceding this one, can you change
DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
timeout loops to use time_before() like this one instead of blindly
looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.
> + while (time_before(jiffies, timeout)) {
> + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> + if (val != 1) {
> + dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
> + return val;
Ok, since this is my snippet this comment is my fault, and I apologize
for that :). However, this could return 0. If drm_dp_dpcd_readb
returns 0, you probably want to retry (same as -EBUSY).
> + }
> +
> + if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
> + !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
> + break;
> +
> + usleep_range(1000, 1500);
> + }
> +
> + dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
Nit: I think you want to say "PSR failed to take effect" or "Failed to
apply PSR"
Sean
> +
> + return -ETIMEDOUT;
> }
>
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-09 11:20 +0200 |
| Subject | Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <sfq8y-Za-29@gated-at.bofh.it> |
| In reply to | #1479265 |
On 09/08/2016 10:12 PM, Sean Paul wrote:
> On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
>> function, or printing the error Sink PSR state if we failed to effect
>> the request PSR setting.
>>
>
> Let's change to:
>
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR
> setting.
Done,
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v2:
>> - A bunch of good fixes from Sean
>>
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 4 ++--
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
>> 3 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 5fe3982..c0ce16a 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>> psr_vsc.DB0 = 0;
>> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>>
>> - analogix_dp_send_psr_spd(dp, &psr_vsc);
>> - return 0;
>> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
>> }
>> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>
>> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>> psr_vsc.DB0 = 0;
>> psr_vsc.DB1 = 0;
>>
>> - analogix_dp_send_psr_spd(dp, &psr_vsc);
>> - return 0;
>> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
>> }
>> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..6c07a50 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> - struct edp_vsc_psr *vsc);
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> + struct edp_vsc_psr *vsc);
>> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> struct drm_dp_aux_msg *msg);
>> #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..09d703b 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>> }
>>
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> - struct edp_vsc_psr *vsc)
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> + struct edp_vsc_psr *vsc)
>> {
>> + unsigned long timeout;
>> unsigned int val;
>> + u8 sink;
>>
>> /* don't send info frame */
>> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> val |= IF_EN;
>> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> + timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
> Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
> retries, whereas you're using it as number of ms. Fortunately, the
> retry number is so high that this works out :)
>
> In a separate patch preceding this one, can you change
> DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
> timeout loops to use time_before() like this one instead of blindly
> looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.
Done, and after do some experiments, I found we need to set the timeout
to 300ms. Cause in some case we would take about 290ms here to get the
right psr state.
>> + while (time_before(jiffies, timeout)) {
>> + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
>> + if (val != 1) {
>> + dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
>> + return val;
> Ok, since this is my snippet this comment is my fault, and I apologize
> for that :). However, this could return 0. If drm_dp_dpcd_readb
> returns 0, you probably want to retry (same as -EBUSY).
done, just return -EBUSY
>
>> + }
>> +
>> + if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
>> + !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
>> + break;
>> +
>> + usleep_range(1000, 1500);
>> + }
>> +
>> + dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
> Nit: I think you want to say "PSR failed to take effect" or "Failed to
> apply PSR"
Done
- Yakir
> Sean
>
>> +
>> + return -ETIMEDOUT;
>> }
>>
>> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-09 11:50 +0200 |
| Subject | [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <sfqBz-18C-1@gated-at.bofh.it> |
| In reply to | #1478766 |
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR setting.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Update commit message
- Add DP_TIMEOUT_PSR_LOOP_MS marcos
- Correct the return values of analogix_dp_send_psr_spd()
Changes in v2:
- A bunch of good fixes from Sean
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 5 +++--
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
psr_vsc.DB0 = 0;
psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
+ return analogix_dp_send_psr_spd(dp, &psr_vsc);
}
EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
psr_vsc.DB0 = 0;
psr_vsc.DB1 = 0;
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
+ return analogix_dp_send_psr_spd(dp, &psr_vsc);
}
EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index d564e90..a27f1e3 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,6 +20,7 @@
#define MAX_EQ_LOOP 5
#define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
+#define DP_TIMEOUT_PSR_LOOP_MS msecs_to_jiffies(300)
/* DP_MAX_LANE_COUNT */
#define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
@@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
- struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+ struct edp_vsc_psr *vsc);
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
struct drm_dp_aux_msg *msg);
#endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 15a4cf0..7fd4ed0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
}
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
- struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+ struct edp_vsc_psr *vsc)
{
+ unsigned long timeout;
unsigned int val;
+ u8 sink;
/* don't send info frame */
val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
val |= IF_EN;
writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+ timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
+ while (time_before(jiffies, timeout)) {
+ val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+ if (val != 1) {
+ dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+ return -EBUSY;
+ }
+
+ if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
+ (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
+ return 0;
+
+ usleep_range(1000, 1500);
+ }
+
+ dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
+
+ return -ETIMEDOUT;
}
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Sean Paul <seanpaul@chromium.org> |
|---|---|
| Date | 2016-09-12 16:00 +0200 |
| Subject | Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <sgzW9-41n-1@gated-at.bofh.it> |
| In reply to | #1479801 |
On Fri, Sep 9, 2016 at 5:45 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR setting.
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Update commit message
> - Add DP_TIMEOUT_PSR_LOOP_MS marcos
> - Correct the return values of analogix_dp_send_psr_spd()
>
> Changes in v2:
> - A bunch of good fixes from Sean
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 5 +++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
> 3 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = 0;
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index d564e90..a27f1e3 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,6 +20,7 @@
> #define MAX_EQ_LOOP 5
>
> #define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
> +#define DP_TIMEOUT_PSR_LOOP_MS msecs_to_jiffies(300)
Same comment here re: units.
300ms seems like a really long time. Why does it take this long?
Sean
>
> /* DP_MAX_LANE_COUNT */
> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
> @@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc);
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> struct drm_dp_aux_msg *msg);
> #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 15a4cf0..7fd4ed0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
> }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc)
> {
> + unsigned long timeout;
> unsigned int val;
> + u8 sink;
>
> /* don't send info frame */
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> val |= IF_EN;
> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> + timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
> + while (time_before(jiffies, timeout)) {
> + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> + if (val != 1) {
> + dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
> + return -EBUSY;
> + }
> +
> + if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
> + (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
> + return 0;
> +
> + usleep_range(1000, 1500);
> + }
> +
> + dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
> +
> + return -ETIMEDOUT;
> }
>
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-20 04:30 +0200 |
| Subject | Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR |
| Message-ID | <sjiYN-5sN-5@gated-at.bofh.it> |
| In reply to | #1481301 |
Hi Sean,
On 09/12/2016 09:52 PM, Sean Paul wrote:
> On Fri, Sep 9, 2016 at 5:45 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
>> function, or print the sink PSR error state if we failed to apply the
>> requested PSR setting.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v3:
>> - Update commit message
>> - Add DP_TIMEOUT_PSR_LOOP_MS marcos
>> - Correct the return values of analogix_dp_send_psr_spd()
>>
>> Changes in v2:
>> - A bunch of good fixes from Sean
>>
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 5 +++--
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 ++++++++++++++++++++--
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 5fe3982..c0ce16a 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>> psr_vsc.DB0 = 0;
>> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>>
>> - analogix_dp_send_psr_spd(dp, &psr_vsc);
>> - return 0;
>> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
>> }
>> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>
>> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>> psr_vsc.DB0 = 0;
>> psr_vsc.DB1 = 0;
>>
>> - analogix_dp_send_psr_spd(dp, &psr_vsc);
>> - return 0;
>> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
>> }
>> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index d564e90..a27f1e3 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -20,6 +20,7 @@
>> #define MAX_EQ_LOOP 5
>>
>> #define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
>> +#define DP_TIMEOUT_PSR_LOOP_MS msecs_to_jiffies(300)
> Same comment here re: units.
>
> 300ms seems like a really long time. Why does it take this long?
This magic number '300ms' just come from my test, I haven't found the
description in eDP 1.4a Spec about what exact time should Sink take to
entry PSR.
- Yakir
> Sean
>
>
>> /* DP_MAX_LANE_COUNT */
>> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
>> @@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> - struct edp_vsc_psr *vsc);
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> + struct edp_vsc_psr *vsc);
>> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> struct drm_dp_aux_msg *msg);
>> #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index 15a4cf0..7fd4ed0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>> }
>>
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> - struct edp_vsc_psr *vsc)
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> + struct edp_vsc_psr *vsc)
>> {
>> + unsigned long timeout;
>> unsigned int val;
>> + u8 sink;
>>
>> /* don't send info frame */
>> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> val |= IF_EN;
>> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> + timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
>> + while (time_before(jiffies, timeout)) {
>> + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
>> + if (val != 1) {
>> + dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
>> + return -EBUSY;
>> + }
>> +
>> + if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
>> + (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
>> + return 0;
>> +
>> + usleep_range(1000, 1500);
>> + }
>> +
>> + dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
>> +
>> + return -ETIMEDOUT;
>> }
>>
>> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-09 11:50 +0200 |
| Subject | [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop |
| Message-ID | <sfqBz-18C-3@gated-at.bofh.it> |
| In reply to | #1478766 |
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Suggested by Sean
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 ++-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 +++++++++---------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..d564e90 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -16,10 +16,11 @@
#include <drm/drm_crtc.h>
#include <drm/drm_dp_helper.h>
-#define DP_TIMEOUT_LOOP_COUNT 100
#define MAX_CR_LOOP 5
#define MAX_EQ_LOOP 5
+#define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
+
/* DP_MAX_LANE_COUNT */
#define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
#define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..15a4cf0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
{
u32 reg;
- int timeout_loop = 0;
+ unsigned long timeout;
analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
@@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
analogix_dp_set_pll_power_down(dp, 0);
+ timeout = jiffies + DP_TIMEOUT_LOOP_MS;
while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
- timeout_loop++;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (time_after(jiffies, timeout)) {
dev_err(dp->dev, "failed to get pll lock status\n");
return;
}
@@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
{
int reg;
int retval = 0;
- int timeout_loop = 0;
+ unsigned long timeout;
/* Enable AUX CH operation */
reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
@@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
/* Is AUX CH command reply received? */
+ timeout = jiffies + DP_TIMEOUT_LOOP_MS;
reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
while (!(reg & RPLY_RECEIV)) {
- timeout_loop++;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (time_after(jiffies, timeout)) {
dev_err(dp->dev, "AUX CH command reply failed!\n");
return -ETIMEDOUT;
}
@@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
{
u32 reg;
u8 *buffer = msg->buffer;
- int timeout_loop = 0;
+ unsigned long timeout;
unsigned int i;
int num_transferred = 0;
@@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
/* Is AUX CH command reply received? */
/* TODO: Wait for an interrupt instead of looping? */
+ timeout = jiffies + DP_TIMEOUT_LOOP_MS;
reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
while (!(reg & RPLY_RECEIV)) {
- timeout_loop++;
- if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
+ if (time_after(jiffies, timeout)) {
dev_err(dp->dev, "AUX CH command reply failed!\n");
return -ETIMEDOUT;
}
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Sean Paul <seanpaul@chromium.org> |
|---|---|
| Date | 2016-09-12 16:00 +0200 |
| Subject | Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop |
| Message-ID | <sgzW9-41n-3@gated-at.bofh.it> |
| In reply to | #1479802 |
On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Suggested by Sean
>
> Changes in v2: None
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 ++-
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 +++++++++---------
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..d564e90 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -16,10 +16,11 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_dp_helper.h>
>
> -#define DP_TIMEOUT_LOOP_COUNT 100
> #define MAX_CR_LOOP 5
> #define MAX_EQ_LOOP 5
>
> +#define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
The name suggests the units here are ms, but you're storing jiffies.
Do the msecs_to_jiffies conversion down below.
> +
> /* DP_MAX_LANE_COUNT */
> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
> #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..15a4cf0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
> void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> {
> u32 reg;
> - int timeout_loop = 0;
> + unsigned long timeout;
>
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>
> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> analogix_dp_set_pll_power_down(dp, 0);
>
> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);
> while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - timeout_loop++;
> - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> + if (time_after(jiffies, timeout)) {
> dev_err(dp->dev, "failed to get pll lock status\n");
> return;
> }
> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
> {
> int reg;
> int retval = 0;
> - int timeout_loop = 0;
> + unsigned long timeout;
>
> /* Enable AUX CH operation */
> reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
> writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>
> /* Is AUX CH command reply received? */
> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
> reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> while (!(reg & RPLY_RECEIV)) {
> - timeout_loop++;
> - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> + if (time_after(jiffies, timeout)) {
> dev_err(dp->dev, "AUX CH command reply failed!\n");
> return -ETIMEDOUT;
> }
> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> {
> u32 reg;
> u8 *buffer = msg->buffer;
> - int timeout_loop = 0;
> + unsigned long timeout;
> unsigned int i;
> int num_transferred = 0;
>
> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>
> /* Is AUX CH command reply received? */
> /* TODO: Wait for an interrupt instead of looping? */
> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
> reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> while (!(reg & RPLY_RECEIV)) {
> - timeout_loop++;
> - if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> + if (time_after(jiffies, timeout)) {
> dev_err(dp->dev, "AUX CH command reply failed!\n");
> return -ETIMEDOUT;
> }
> --
> 1.9.1
>
>
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-09-20 04:20 +0200 |
| Subject | Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop |
| Message-ID | <sjiP7-5pr-1@gated-at.bofh.it> |
| In reply to | #1481302 |
Hi Sean,
On 09/12/2016 09:51 PM, Sean Paul wrote:
> On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v3:
>> - Suggested by Sean
>>
>> Changes in v2: None
>>
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 ++-
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 +++++++++---------
>> 2 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..d564e90 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -16,10 +16,11 @@
>> #include <drm/drm_crtc.h>
>> #include <drm/drm_dp_helper.h>
>>
>> -#define DP_TIMEOUT_LOOP_COUNT 100
>> #define MAX_CR_LOOP 5
>> #define MAX_EQ_LOOP 5
>>
>> +#define DP_TIMEOUT_LOOP_MS msecs_to_jiffies(1)
> The name suggests the units here are ms, but you're storing jiffies.
> Do the msecs_to_jiffies conversion down below.
I suddenly realized that 'analogix_dp_core.c' also used the
'DP_TIMEOUT_LOOP_COUNT' macros, and 'analogix_dp_core.c' have four kinds
of timeout,
- DP_TIMEOUT_LOOP_COUNT * 1us
- DP_TIMEOUT_LOOP_COUNT * 10us
- DP_TIMEOUT_LOOP_COUNT * 100us
- DP_TIMEOUT_LOOP_COUNT * 1000us
I may guess it's not necessary to replace the 'DP_TIMEOUT_LOOP_COUNT'
now :-)
- Yakir
>
>> +
>> /* DP_MAX_LANE_COUNT */
>> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
>> #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..15a4cf0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>> void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>> {
>> u32 reg;
>> - int timeout_loop = 0;
>> + unsigned long timeout;
>>
>> analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>>
>> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>> if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>> analogix_dp_set_pll_power_down(dp, 0);
>>
>> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
> timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);
>
>> while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>> - timeout_loop++;
>> - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> + if (time_after(jiffies, timeout)) {
>> dev_err(dp->dev, "failed to get pll lock status\n");
>> return;
>> }
>> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>> {
>> int reg;
>> int retval = 0;
>> - int timeout_loop = 0;
>> + unsigned long timeout;
>>
>> /* Enable AUX CH operation */
>> reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>> writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>>
>> /* Is AUX CH command reply received? */
>> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>> reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>> while (!(reg & RPLY_RECEIV)) {
>> - timeout_loop++;
>> - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> + if (time_after(jiffies, timeout)) {
>> dev_err(dp->dev, "AUX CH command reply failed!\n");
>> return -ETIMEDOUT;
>> }
>> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> {
>> u32 reg;
>> u8 *buffer = msg->buffer;
>> - int timeout_loop = 0;
>> + unsigned long timeout;
>> unsigned int i;
>> int num_transferred = 0;
>>
>> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>
>> /* Is AUX CH command reply received? */
>> /* TODO: Wait for an interrupt instead of looping? */
>> + timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>> reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>> while (!(reg & RPLY_RECEIV)) {
>> - timeout_loop++;
>> - if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
>> + if (time_after(jiffies, timeout)) {
>> dev_err(dp->dev, "AUX CH command reply failed!\n");
>> return -ETIMEDOUT;
>> }
>> --
>> 1.9.1
>>
>>
>
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web