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

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

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux