[Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
Rodrigo Vivi
rodrigo.vivi at gmail.com
Tue Jan 20 14:01:25 PST 2015
Hi Sonika, for the login screen my guess is that blinking cursor
waiting for password blocks psr entry.
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.
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.
I'm going to review the code more carefully later...
On Tue, Jan 20, 2015 at 3:49 AM, Jindal, Sonika <sonika.jindal at intel.com> wrote:
> Is somebody working on enhancing the psr testcase?
> -Sonika
>
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] 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: [Intel-gfx] [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 at intel.com>
>> >>---
>> >> 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 at lists.freedesktop.org
>> >>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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
More information about the Intel-gfx
mailing list