Re: [Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

2015-07-27 Thread Vivi, Rodrigo 
On Mon, 2015-07-27 at 10:31 +0200, Daniel Vetter wrote:
 On Fri, Jul 24, 2015 at 04:38:56PM -0700, Rodrigo Vivi wrote:
  Since active function on VLV immediately activate PSR let's give more
  time for idleness. Different from core platforms where we have idle_frames
  count.
  
  Also kms_psr_sink_crc now is automated and always get this:
  
  [drm:intel_enable_pipe] enabling pipe A
  [drm:intel_edp_backlight_on]
  [drm:intel_panel_enable_backlight] pipe
  [drm:intel_panel_enable_backlight] pipe A
  [drm:intel_panel_actually_set_backlight] set backlight PWM = 7812
  
  PSR gets enabled somewhere here after backlight.
  
  [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0
  [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
  [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
  
  PSR gets flushed around here by intel_atomic_commit
  
  [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
  [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
  [drm:intel_set_memory_cxsr] memory self-refresh is enabled
  [drm:intel_connector_check_state] [CONNECTOR:39:eDP-1]
  [drm:check_encoder_state] [ENCODER:30:DAC-30]
  [drm:check_encoder_state] [ENCODER:31:TMDS-31]
  [drm:check_encoder_state] [ENCODER:36:TMDS-36]
  [drm:check_encoder_state] [ENCODER:38:TMDS-38]
  [drm:check_crtc_state] [CRTC:21]
  [drm:check_crtc_state] [CRTC:26]
  [drm:intel_psr_activate [i915]] *ERROR* PSR Active
  [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x
  [drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun
  [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO
  Underrun.
  
  It is true that in a product we won't keep disabling and enabling planes so
  frequently, but for safeness let's stay conservative.
  
  It is also true that 500ms is an etternity. But PSR is anyway a power saving
  feature for idle scenario. So if it is idle feature stays on and 500ms to 
  get
  it reanabled is not that insane.
 
 I'm a bit confused, does this patch fix the above underruns?2
yes.

  Any idea why
 underruns happen at all?
No.

  Do they only happen right after a full modeset?
yes, and only after the full modeset. if test continue to run I will get
all screen uppdates normally. Blank with underrun just happen on the
first screen update right after the full modeset.

350ms would be enough to get ride of this blanks, exept when it happens
after dpms off/on where 350 didn't help and 500ms was needed.



 There's some more details in the changelog below but it's not entirely
 clear to me really. 

Below was just the history on this patch, that I just remember about it
when I run test cases myself on VLV or BSW.

 Please clarify (preferrably with a paragraph I can add to
 the commit message).
 -Daniel
 
  
  v2: Rebase over intel_psr.c and fix typo.
  v3: Revival: Manual tests indicated that this is needed. With a short delay
  there is a huge risk of getting blank screens when planes are being 
  enabled.
  v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but
  actually time for link training what we aren't doing, but with only 100 
  sec
  in some cases kms_psr_sink_crc manual was showing blank screen,
  so let's use this for now. Also changed comment by a FIXME.
  v5: Rebase after a long time, remove FIXME and update comment above.
  
  Reviewed-by: Durgadoss R durgados...@intel.com (v4)
  Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
  ---
   drivers/gpu/drm/i915/intel_psr.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_psr.c 
  b/drivers/gpu/drm/i915/intel_psr.c
  index acd8ec8..0f446b7 100644
  --- a/drivers/gpu/drm/i915/intel_psr.c
  +++ b/drivers/gpu/drm/i915/intel_psr.c
  @@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev,
  struct drm_i915_private *dev_priv = dev-dev_private;
  struct drm_crtc *crtc;
  enum pipe pipe;
  +   int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500);
   
  mutex_lock(dev_priv-psr.lock);
  if (!dev_priv-psr.enabled) {
  @@ -733,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev,
   
  if (!dev_priv-psr.active  !dev_priv-psr.busy_frontbuffer_bits)
  schedule_delayed_work(dev_priv-psr.work,
  - msecs_to_jiffies(100));
  + msecs_to_jiffies(delay));
  mutex_unlock(dev_priv-psr.lock);
   }
   
  -- 
  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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

2015-07-27 Thread Daniel Vetter 
On Fri, Jul 24, 2015 at 04:38:56PM -0700, Rodrigo Vivi wrote:
 Since active function on VLV immediately activate PSR let's give more
 time for idleness. Different from core platforms where we have idle_frames
 count.
 
 Also kms_psr_sink_crc now is automated and always get this:
 
 [drm:intel_enable_pipe] enabling pipe A
 [drm:intel_edp_backlight_on]
 [drm:intel_panel_enable_backlight] pipe
 [drm:intel_panel_enable_backlight] pipe A
 [drm:intel_panel_actually_set_backlight] set backlight PWM = 7812
 
 PSR gets enabled somewhere here after backlight.
 
 [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0
 [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
 
 PSR gets flushed around here by intel_atomic_commit
 
 [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
 [drm:intel_set_memory_cxsr] memory self-refresh is enabled
 [drm:intel_connector_check_state] [CONNECTOR:39:eDP-1]
 [drm:check_encoder_state] [ENCODER:30:DAC-30]
 [drm:check_encoder_state] [ENCODER:31:TMDS-31]
 [drm:check_encoder_state] [ENCODER:36:TMDS-36]
 [drm:check_encoder_state] [ENCODER:38:TMDS-38]
 [drm:check_crtc_state] [CRTC:21]
 [drm:check_crtc_state] [CRTC:26]
 [drm:intel_psr_activate [i915]] *ERROR* PSR Active
 [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x
 [drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun
 [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO
 Underrun.
 
 It is true that in a product we won't keep disabling and enabling planes so
 frequently, but for safeness let's stay conservative.
 
 It is also true that 500ms is an etternity. But PSR is anyway a power saving
 feature for idle scenario. So if it is idle feature stays on and 500ms to get
 it reanabled is not that insane.

I'm a bit confused, does this patch fix the above underruns? Any idea why
underruns happen at all? Do they only happen right after a full modeset?
There's some more details in the changelog below but it's not entirely
clear to me really. Please clarify (preferrably with a paragraph I can add to
the commit message).
-Daniel

 
 v2: Rebase over intel_psr.c and fix typo.
 v3: Revival: Manual tests indicated that this is needed. With a short delay
 there is a huge risk of getting blank screens when planes are being 
 enabled.
 v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but
 actually time for link training what we aren't doing, but with only 100 
 sec
 in some cases kms_psr_sink_crc manual was showing blank screen,
 so let's use this for now. Also changed comment by a FIXME.
 v5: Rebase after a long time, remove FIXME and update comment above.
 
 Reviewed-by: Durgadoss R durgados...@intel.com (v4)
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_psr.c 
 b/drivers/gpu/drm/i915/intel_psr.c
 index acd8ec8..0f446b7 100644
 --- a/drivers/gpu/drm/i915/intel_psr.c
 +++ b/drivers/gpu/drm/i915/intel_psr.c
 @@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev,
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;
   enum pipe pipe;
 + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500);
  
   mutex_lock(dev_priv-psr.lock);
   if (!dev_priv-psr.enabled) {
 @@ -733,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev,
  
   if (!dev_priv-psr.active  !dev_priv-psr.busy_frontbuffer_bits)
   schedule_delayed_work(dev_priv-psr.work,
 -   msecs_to_jiffies(100));
 +   msecs_to_jiffies(delay));
   mutex_unlock(dev_priv-psr.lock);
  }
  
 -- 
 1.9.3
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

2015-07-24 Thread Rodrigo Vivi 
Since active function on VLV immediately activate PSR let's give more
time for idleness. Different from core platforms where we have idle_frames
count.

Also kms_psr_sink_crc now is automated and always get this:

[drm:intel_enable_pipe] enabling pipe A
[drm:intel_edp_backlight_on]
[drm:intel_panel_enable_backlight] pipe
[drm:intel_panel_enable_backlight] pipe A
[drm:intel_panel_actually_set_backlight] set backlight PWM = 7812

PSR gets enabled somewhere here after backlight.

[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0
[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp

PSR gets flushed around here by intel_atomic_commit

[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
[drm:intel_set_memory_cxsr] memory self-refresh is enabled
[drm:intel_connector_check_state] [CONNECTOR:39:eDP-1]
[drm:check_encoder_state] [ENCODER:30:DAC-30]
[drm:check_encoder_state] [ENCODER:31:TMDS-31]
[drm:check_encoder_state] [ENCODER:36:TMDS-36]
[drm:check_encoder_state] [ENCODER:38:TMDS-38]
[drm:check_crtc_state] [CRTC:21]
[drm:check_crtc_state] [CRTC:26]
[drm:intel_psr_activate [i915]] *ERROR* PSR Active
[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x
[drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun
[drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO
Underrun.

It is true that in a product we won't keep disabling and enabling planes so
frequently, but for safeness let's stay conservative.

It is also true that 500ms is an etternity. But PSR is anyway a power saving
feature for idle scenario. So if it is idle feature stays on and 500ms to get
it reanabled is not that insane.

v2: Rebase over intel_psr.c and fix typo.
v3: Revival: Manual tests indicated that this is needed. With a short delay
there is a huge risk of getting blank screens when planes are being enabled.
v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but
actually time for link training what we aren't doing, but with only 100 sec
in some cases kms_psr_sink_crc manual was showing blank screen,
so let's use this for now. Also changed comment by a FIXME.
v5: Rebase after a long time, remove FIXME and update comment above.

Reviewed-by: Durgadoss R durgados...@intel.com (v4)
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_psr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index acd8ec8..0f446b7 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
+   int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500);
 
mutex_lock(dev_priv-psr.lock);
if (!dev_priv-psr.enabled) {
@@ -733,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev,
 
if (!dev_priv-psr.active  !dev_priv-psr.busy_frontbuffer_bits)
schedule_delayed_work(dev_priv-psr.work,
- msecs_to_jiffies(100));
+ msecs_to_jiffies(delay));
mutex_unlock(dev_priv-psr.lock);
 }
 
-- 
1.9.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx