Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 


On Wednesday 21 January 2015 03:31 AM, Rodrigo Vivi wrote:

Hi Sonika, for the login screen my guess is that blinking cursor
waiting for password blocks psr entry.
But I thought it is the psr exit which is taking time which is why it takes time 
to actually show up the characters

For test cases I started the test case enhancement but my psr work is
paused this month for bug maintainance. Any help is welcome.
I was planing also to take a look on new Paulo's fbc tests cases to
see if we could use some pieces on PSR validation. might be worth to
take a look.
Can we test scenarios like delayed psr exits (like the one I mentioned) with test case? 
If yes, I can work with you offline to enhance the igt.

But anyway, for HW tracking I would only consider when I use a KDE for
a while without missing any screen updates and seeing perf counter
increasing. But if you tell this is what happening I trust you and we
can add support for that since it is disabled by default and validate
that carefully also on SKL when trying to enable it by default
everywhere.
Hmm, I have not tested with KDE yet. I have been testing with ubuntu with gnome. 
And it works very smooth there. I will do more experiments with KDE too.


I'm going to review the code more carefully later...

On Tue, Jan 20, 2015 at 3:49 AM, Jindal, Sonika <sonika.jindal@xxxxxxxxx> wrote:

Is somebody working on enhancing the psr testcase?
-Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter
Sent: Tuesday, January 20, 2015 3:20 PM
To: Jindal, Sonika
Cc: Daniel Vetter; intel-gfx; Vivi, Rodrigo
Subject: Re:  [PATCH] drm/i915/skl: Enabling PSR on Skylake

On Mon, Jan 19, 2015 at 05:10:58PM +0530, sonika wrote:

On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:

On Fri, Jan 16, 2015 at 02:07:26PM +0530, Sonika Jindal wrote:

Mainly taking care of some register offsets, otherwise things are
similar to hsw. Also, programming ddi aux to use hardcoded values for psr data select.

v2: introduce  EDP_PSR_AUX_BASE macro (Chris)
v3: Moving to HW tracking for SKL+ platforms, so activating source
psr during psr_enabling and then avoiding psr entries and exits for
each frontbuffer updates.

Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
  drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
  drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
  4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 42c69ca..ee5cd3b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2355,7 +2355,8 @@ struct drm_i915_cmd_table {
  #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
  #define HAS_FPGA_DBG_UNCLAIMED(dev)
(INTEL_INFO(dev)->has_fpga_dbg)
  #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
- IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
+ IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
+ IS_SKYLAKE(dev))
  #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
   IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
  #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) diff --git
a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a828cf5..068c8da 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
  #define   EDP_PSR_TP1_TIME_0us (3<<4)
  #define   EDP_PSR_IDLE_FRAME_SHIFT 0

-#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10) -#define
EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14) -#define
EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18) -#define
EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c) -#define
EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20) -#define
EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
+#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
+ 0x64000 : EDP_PSR_BASE(dev))
+#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10) #define
+EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14) #define
+EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18) #define
+EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c) #define
+EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20) #define
+EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)

  #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
  #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
@@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
  #define   DP_AUX_CH_CTL_PRECHARGE_TEST    (1 << 11)
  #define   DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK    (0x7ff)
  #define   DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT   0
+#define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
+#define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL (1 << 13)
+#define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
+#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
+#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)

  /*
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 79f6d72..010d550 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct
drm_i915_gem_object *obj,

   intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);

- intel_psr_invalidate(dev, obj->frontbuffer_bits);
+
+ if (INTEL_INFO(dev)->gen < 9)
+ intel_psr_invalidate(dev, obj->frontbuffer_bits);
  }

  /**
@@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device
*dev,

   intel_mark_fb_busy(dev, frontbuffer_bits, NULL);

- intel_psr_flush(dev, frontbuffer_bits);
+ if (INTEL_INFO(dev)->gen < 9)
+ intel_psr_flush(dev, frontbuffer_bits);

I'm pretty sure the hw isn't good enough yet to detect everything,
unfortunately the testcase is also not quite ready yet. In any case
these changes should be moved into the inel_psr_* functions and in a
separate patch.

When ubuntu boots up and stops at the login screen, the text I enter
comes slowly, looks like psr exit and entry is not very prompt at that time.
But when I try with SW tracking, I see that PSR doesn't get enabled at
all at that point!
Also, I don't see calls to frontbuffer invalidate and flush at that point.
So, with SW tracking too we will have cases where we don't enter psr.
Any inputs on why frontbuffer calls are not received during login screen?

Please note that after login, it works fine if I type something on the
terminal or do some other things.

I have no idea why or what exactly the ubuntu screen does in it's login function. Just please note that there are _lots_ more desktop environments on linux than just ubunut's flavour of gnome, so we really need to have that psr testcase working (which covers them all) instead of manual testing.
-Daniel


Also someone needs to re-review the psr igt testcase to make sure it
covers everything. Ville could do that too since he's done the fbc
testcase.
-Daniel


   /*
   * FIXME: Unconditional fbc flushing here is a rather gross hack
and diff --git a/drivers/gpu/drm/i915/intel_psr.c
b/drivers/gpu/drm/i915/intel_psr.c
index dd0e6e0..6d2cdb8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
   I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
     intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));

- I915_WRITE(EDP_PSR_AUX_CTL(dev),
+ if (INTEL_INFO(dev)->gen >= 9) {
+ uint32_t val;
+
+ val = I915_READ(EDP_PSR_AUX_CTL(dev)); val &=
+ ~DP_AUX_CH_CTL_TIME_OUT_MASK; val |=
+ DP_AUX_CH_CTL_TIME_OUT_1600us; val &=
+ ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK; val |= (sizeof(aux_msg) <<
+ DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
+ /* Use hardcoded data values for PSR */ val &=
+ ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
+ I915_WRITE(EDP_PSR_AUX_CTL(dev), val); } else {
+ I915_WRITE(EDP_PSR_AUX_CTL(dev),
     DP_AUX_CH_CTL_TIME_OUT_400us |
     (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
     (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
     (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
+ }
  }

  static void vlv_psr_enable_source(struct intel_dp *intel_dp) @@
-355,6 +368,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)

   /* Enable PSR on the panel */
   hsw_psr_enable_sink(intel_dp);
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ intel_psr_activate(intel_dp);
   } else {
   vlv_psr_setup_vsc(intel_dp);

--
1.7.10.4

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

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux