>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Saturday, February 28, 2015 6:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Runyan, Arthur J; Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 2/7] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT
>logic
>
>Since the begining there is a missunderstanding on the meaning of this
>dpcd bit.
>This bit should'n indicate wheter to use link standby or not, but just
Shouldn't , whether
>be used to configure TP1, TP2 and TP3 times and tell hw aux should be skiped
>since HW is the responsible one.
>
>Even with help of frontbuffer tracking to do exit HW tracking is still
>fully responsible for this exit logic with DP training or not.
I think it will be better to rephrase this as something like below:
"Even with help of frontbuffer tracking, HW is still fully responsible for
PSR exit logic with/without DP training"
>
>Cc: Arthur Runyan <arthur.j.run...@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
>---
> drivers/gpu/drm/i915/intel_psr.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_psr.c
>b/drivers/gpu/drm/i915/intel_psr.c
>index 2e6831d..6c8e9e0 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -242,8 +242,10 @@ static void hsw_psr_enable_source(struct intel_dp
>*intel_dp)
> uint32_t val = 0x0;
> const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
>- if (dev_priv->psr.link_standby) {
>+ if (dev_priv->psr.link_standby)
> val |= EDP_PSR_LINK_STANDBY;
>+
>+ if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
Shouldn't this 'if' check also include if (psr.link_standby) ?
Otherwise, even for link_disable cases we are configuring to 'skip
TPs'. Will this work ?
I see that in the next patch, you are removing this whole link_standby
thing. So, would leave it up to you to either add this check or not.
Thanks,
Durga
> val |= EDP_PSR_TP2_TP3_TIME_0us;
> val |= EDP_PSR_TP1_TIME_0us;
> val |= EDP_PSR_SKIP_AUX_EXIT;
>@@ -354,8 +356,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> /* First we check VBT, but we must respect sink and source
> * known restrictions */
> dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
>- if ((intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) ||
>- (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A))
>+ if (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A)
> dev_priv->psr.link_standby = true;
>
> dev_priv->psr.busy_frontbuffer_bits = 0;
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
