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
