[Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
sonika
sonika.jindal at intel.com
Tue Jan 20 20:53:26 PST 2015
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 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
>
>
More information about the Intel-gfx
mailing list