Re: [Intel-gfx] [PATCH 6/7] drm/i915: Save latest known sink CRC to compensate delayed counter reset.
On Thu, Jul 23, 2015 at 04:35:49PM -0700, Rodrigo Vivi wrote:
> By Vesa DP 1.2 Spec TEST_CRC_COUNT should be
> "reset to 0 when TEST_SINK bit 0 = 0."
>
> However for some strange reason when PSR is enabled in
> certain platforms this is not true. At least not immediatelly.
>
> So we face cases like this:
>
> first get_sink_crc operation:
>count: 0, crc:
>count: 1, crc: c101c101c101
> returned expected crc: c101c101c101
>
> secont get_sink_crc operation:
>count: 1, crc: c101c101c101
>count: 0, crc:
>count: 1, crc: c101
> should return expected crc: c101
>
> But also the reset to 0 should be faster resulting into:
>
> get_sink_crc operation:
>count: 1, crc: c101c101c101
>count: 1, crc: c101
> should return expected crc: c101
>
> So in order to know that the second one is valid one
> we need to compare the pair (count, crc) with latest (count, crc).
>
> If the pair changed you have your valid CRC.
>
> Signed-off-by: Rodrigo Vivi
We discussed this before, unfortunately I don't see any other way to do
this, since there is no way to know that the crc count really restarted,
became 0 and then 1 again. So I think this workaround is needed.
Reviewed-by: Rafael Antognolli
> ---
> drivers/gpu/drm/i915/intel_dp.c | 42
> +---
> drivers/gpu/drm/i915/intel_drv.h | 8 +++-
> 2 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3ba031d..c7372a1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3978,7 +3978,7 @@ static int intel_dp_sink_crc_stop(struct intel_dp
> *intel_dp)
> goto out;
> }
>
> - intel_dp->sink_crc_started = false;
> + intel_dp->sink_crc.started = false;
> out:
> hsw_enable_ips(intel_crtc);
> return ret;
> @@ -3991,7 +3991,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
> *intel_dp)
> u8 buf;
> int ret;
>
> - if (intel_dp->sink_crc_started) {
> + if (intel_dp->sink_crc.started) {
> ret = intel_dp_sink_crc_stop(intel_dp);
> if (ret)
> return ret;
> @@ -4003,6 +4003,8 @@ static int intel_dp_sink_crc_start(struct intel_dp
> *intel_dp)
> if (!(buf & DP_TEST_CRC_SUPPORTED))
> return -ENOTTY;
>
> + intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
> +
> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
> return -EIO;
>
> @@ -4014,7 +4016,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
> *intel_dp)
> return -EIO;
> }
>
> - intel_dp->sink_crc_started = true;
> + intel_dp->sink_crc.started = true;
> return 0;
> }
>
> @@ -4024,29 +4026,39 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
> *crc)
> struct drm_device *dev = dig_port->base.base.dev;
> struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
> u8 buf;
> - int test_crc_count;
> + int count, ret;
> int attempts = 6;
> - int ret;
>
> ret = intel_dp_sink_crc_start(intel_dp);
> if (ret)
> return ret;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0) {
> - ret = -EIO;
> - goto stop;
> - }
> -
> - test_crc_count = buf & DP_TEST_COUNT_MASK;
> -
> do {
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_TEST_SINK_MISC, &buf) < 0) {
> ret = -EIO;
> goto stop;
> }
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> - } while (--attempts && (buf & DP_TEST_COUNT_MASK) == test_crc_count);
> + count = buf & DP_TEST_COUNT_MASK;
> + /*
> + * Count might be reset during the loop. In this case
> + * last known count needs to be reset as well.
> + */
> + if (count == 0)
> + intel_dp->sink_crc.last_count = 0;
> +
> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6)
> < 0) {
> + ret = -EIO;
> + goto stop;
> + }
> + } while (--attempts && (count == 0 || (count ==
> intel_dp->sink_crc.last_count &&
> +
> !memcmp(intel_dp->sink_crc.last_crc, crc,
> +6 * sizeof(u8);
> +
> + intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
> + memcpy(intel_dp->sink_crc.last_crc, crc, 6 * sizeof(u8));
>
> if (attempts == 0) {
> DRM_DEBUG_KMS("Panel is unable to calculate CRC after 6
> vbla
[Intel-gfx] [PATCH 6/7] drm/i915: Save latest known sink CRC to compensate delayed counter reset.
By Vesa DP 1.2 Spec TEST_CRC_COUNT should be
"reset to 0 when TEST_SINK bit 0 = 0."
However for some strange reason when PSR is enabled in
certain platforms this is not true. At least not immediatelly.
So we face cases like this:
first get_sink_crc operation:
count: 0, crc:
count: 1, crc: c101c101c101
returned expected crc: c101c101c101
secont get_sink_crc operation:
count: 1, crc: c101c101c101
count: 0, crc:
count: 1, crc: c101
should return expected crc: c101
But also the reset to 0 should be faster resulting into:
get_sink_crc operation:
count: 1, crc: c101c101c101
count: 1, crc: c101
should return expected crc: c101
So in order to know that the second one is valid one
we need to compare the pair (count, crc) with latest (count, crc).
If the pair changed you have your valid CRC.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 42 +---
drivers/gpu/drm/i915/intel_drv.h | 8 +++-
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ba031d..c7372a1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3978,7 +3978,7 @@ static int intel_dp_sink_crc_stop(struct intel_dp
*intel_dp)
goto out;
}
- intel_dp->sink_crc_started = false;
+ intel_dp->sink_crc.started = false;
out:
hsw_enable_ips(intel_crtc);
return ret;
@@ -3991,7 +3991,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
u8 buf;
int ret;
- if (intel_dp->sink_crc_started) {
+ if (intel_dp->sink_crc.started) {
ret = intel_dp_sink_crc_stop(intel_dp);
if (ret)
return ret;
@@ -4003,6 +4003,8 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
if (!(buf & DP_TEST_CRC_SUPPORTED))
return -ENOTTY;
+ intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
+
if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
return -EIO;
@@ -4014,7 +4016,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
return -EIO;
}
- intel_dp->sink_crc_started = true;
+ intel_dp->sink_crc.started = true;
return 0;
}
@@ -4024,29 +4026,39 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
*crc)
struct drm_device *dev = dig_port->base.base.dev;
struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
u8 buf;
- int test_crc_count;
+ int count, ret;
int attempts = 6;
- int ret;
ret = intel_dp_sink_crc_start(intel_dp);
if (ret)
return ret;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0) {
- ret = -EIO;
- goto stop;
- }
-
- test_crc_count = buf & DP_TEST_COUNT_MASK;
-
do {
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+
if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_TEST_SINK_MISC, &buf) < 0) {
ret = -EIO;
goto stop;
}
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- } while (--attempts && (buf & DP_TEST_COUNT_MASK) == test_crc_count);
+ count = buf & DP_TEST_COUNT_MASK;
+ /*
+* Count might be reset during the loop. In this case
+* last known count needs to be reset as well.
+*/
+ if (count == 0)
+ intel_dp->sink_crc.last_count = 0;
+
+ if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6)
< 0) {
+ ret = -EIO;
+ goto stop;
+ }
+ } while (--attempts && (count == 0 || (count ==
intel_dp->sink_crc.last_count &&
+
!memcmp(intel_dp->sink_crc.last_crc, crc,
+ 6 * sizeof(u8);
+
+ intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
+ memcpy(intel_dp->sink_crc.last_crc, crc, 6 * sizeof(u8));
if (attempts == 0) {
DRM_DEBUG_KMS("Panel is unable to calculate CRC after 6
vblanks\n");
@@ -4054,8 +4066,6 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
goto stop;
}
- if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
- ret = -EIO;
stop:
intel_dp_sink_crc_stop(intel_dp);
return ret;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc74400..c072820 100644
--- a/drivers/gpu/drm/i915/i
