Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
-Original Message- From: Vivi, Rodrigo Sent: Friday, April 10, 2015 11:40 PM To: intel-gfx@lists.freedesktop.org Cc: Vivi, Rodrigo; R, Durgadoss; Runyan, Arthur J Subject: [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic Since the beginning there is a missunderstanding on the meaning of this dpcd bit. This bit shouldn't indicate whether to use link standby or not, but just 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, HW is still fully responsible for PSR exit logic with/without DP training. DP_PSR_NO_TRAIN_ON_EXIT means the source doesn't need to do the training, but it doesn't tell to avoid TP patterns, so we will send minimal TP1 and avoid TP2. It also means that sink itself can take up to 5 idle frames for training. 6 in our case since we might be off by 1. So we also increment idle_frames by 4 here. v2: Fix and improve commit message (Durga). v3: Use minimal TP1 time avoiding TP2 and increase idle frame. Cc: Durgadoss R durgados...@intel.com Cc: Arthur Runyan arthur.j.run...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Durgadoss R durgados...@intel.com Thanks, Durga --- drivers/gpu/drm/i915/intel_psr.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index db95b39..0e3b652 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -264,11 +264,17 @@ 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) { + /* It doesn't mean we shouldn't send TPS patters, so let's + send the minimal TP1 possible and skip TP2. */ + val |= EDP_PSR_TP1_TIME_100us; val |= EDP_PSR_TP2_TP3_TIME_0us; - val |= EDP_PSR_TP1_TIME_0us; val |= EDP_PSR_SKIP_AUX_EXIT; + /* Sink should be able to train with the 5 or 6 idle patterns */ + idle_frames += 4; } I915_WRITE(EDP_PSR_CTL(dev), val | @@ -381,8 +387,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; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6179 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 276/276 274/276 ILK 301/301 301/301 SNB -1 316/316 315/316 IVB 328/328 328/328 BYT 285/285 285/285 HSW 394/394 394/394 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt@gen3_render_mixed_blits FAIL(3)PASS(4) FAIL(2) PNV igt@gen3_render_tiledx_blits FAIL(4)PASS(4) FAIL(2) *SNB igt@kms_flip@dpms-vs-vblank-race PASS(5) DMESG_WARN(1)PASS(1) (dmesg patch applied)drm:intel_dp_start_link_train[i915]]*ERROR*too_many_voltage_retries,give_up@too many voltage .* give up drm:intel_dp_complete_link_train[i915]]*ERROR*failed_to_train_DP,aborting@failed to train .* aborting Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
Hi Durga, are you ok with this one? Hi Art, do you believe there is any risk of avoiding TP when panel tells DP_PSR_NO_TRAIN_ON_EXIT and link is disabled? I believe on the other discussion we conluded that DP_PSR_NO_TRAIN_ON_EXIT was only responsible for TP configurations regardless links standby or not, right? Thanks, Rodrigo. On Mon, Mar 16, 2015 at 10:35 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote: Since the begining there is a missunderstanding on the meaning of this dpcd bit. This bit shouldn't indicate whether to use link standby or not, but just 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, HW is still fully responsible for PSR exit logic with/without DP training. v2: Fix and improve commit message (Durga). Cc: Durgadoss R durgados...@intel.com 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) { 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; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
I think there may be a restriction that we cannot set 0us to all the training patterns. I'll check on that. -Original Message- From: Rodrigo Vivi [mailto:rodrigo.v...@gmail.com] Sent: Tuesday, March 24, 2015 8:30 AM To: Vivi, Rodrigo Cc: intel-gfx; Runyan, Arthur J Subject: Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic Hi Durga, are you ok with this one? Hi Art, do you believe there is any risk of avoiding TP when panel tells DP_PSR_NO_TRAIN_ON_EXIT and link is disabled? I believe on the other discussion we conluded that DP_PSR_NO_TRAIN_ON_EXIT was only responsible for TP configurations regardless links standby or not, right? Thanks, Rodrigo. On Mon, Mar 16, 2015 at 10:35 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote: Since the begining there is a missunderstanding on the meaning of this dpcd bit. This bit shouldn't indicate whether to use link standby or not, but just 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, HW is still fully responsible for PSR exit logic with/without DP training. v2: Fix and improve commit message (Durga). Cc: Durgadoss R durgados...@intel.com 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) { 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; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] 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 shouldn't indicate whether to use link standby or not, but just 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, HW is still fully responsible for PSR exit logic with/without DP training. v2: Fix and improve commit message (Durga). Cc: Durgadoss R durgados...@intel.com 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) { 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; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5962 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 275/275 275/275 ILK 303/303 303/303 SNB -1 279/279 278/279 IVB 343/343 343/343 BYT 287/287 287/287 HSW 361/361 361/361 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied *SNB igt_gem_persistent_relocs_forked-interruptible-thrash-inactive PASS(3) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx