drm/i915: Avoid vblank counter for gen9+

Commit Message

Rodrigo Vivi Feb. 17, 2016, 11:14 p.m. UTC

On Tue, Feb 16, 2016 at 7:50 AM, Daniel Vetter <[e-mail]> wrote:
> On Thu, Feb 11, 2016 at 09:00:47AM -0800, Rodrigo Vivi wrote:
>> Framecounter register is read-only so DMC cannot restore it
>> after exiting DC5 and DC6.
>>
>> Easiest way to go is to avoid the counter and use vblank
>> interruptions for this platform and for all the following
>> ones since DMC came to stay. At least while we can't change
>> this register to read-write.
>>
>> Signed-off-by: Rodrigo Vivi <[e-mail]>
>
> Now my comments also in public:
> - Do we still get reasonable dc5 residency with this - it means we'll keep
>   vblank irq running forever.
>
> - I'm a bit unclear on what exactly this fixes - have you tested that
>   long-lasting vblank waits are still accurate? Just want to make sure we
>   don't just paper over the issue and desktops can still get stuck waiting
>   for a vblank.

apparently no... so please just ignore this patch for now... after a
while with that patch I was seeing the issue again...

>
> Just a bit suprised that the only problem is the framecounter, and not
> that vblanks stop happening too.
>
> We need to also know these details for the proper fix, which will involve
> grabbing power well references (might need a new one for vblank
> interrupts) to make sure.

Yeap, I liked this idea... so combining a power domain reference with
a vblank count restore once we know the dc off is blocked we could
workaround this case... something like:


@@ -2796,6 +2799,8 @@ static void gen8_disable_vblank(struct
drm_device *dev, unsigned int pipe)
        spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
        bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
        spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+       intel_display_power_put(dev_priv, POWER_DOMAIN_VBLANK);
 }

where POWER_DOMAIN_VBLANK is part of:
#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (              \
        BIT(POWER_DOMAIN_VBLANK) |                      \


However I have my dmesg flooded by:


[   69.025562] BUG: sleeping function called from invalid context at
drivers/base/power/runtime.c:955
[   69.025576] in_atomic(): 1, irqs_disabled(): 1, pid: 995, name: Xorg
[   69.025582] Preemption disabled at:[<ffffffff815a1fee>]
drm_vblank_get+0x4e/0xd0

[   69.025619] CPU: 3 PID: 995 Comm: Xorg Tainted: G     U  W
4.5.0-rc4+ #11
[   69.025628] Hardware name: Intel Corporation Kabylake Client
platform/Skylake U DDR3L RVP7, BIOS KBLSE2R1.R00.X019.B01.1512230743
12/23/2015
[   69.025637]  0000000000000000 ffff88003f0bfbb0 ffffffff8148e983
0000000000000000
[   69.025653]  ffff880085b04200 ffff88003f0bfbd0 ffffffff81133ece
ffffffff81d77f23
[   69.025667]  00000000000003bb ffff88003f0bfbf8 ffffffff81133f89
ffff88016913a098
[   69.025680] Call Trace:
[   69.025697]  [<ffffffff8148e983>] dump_stack+0x65/0x92
[   69.025711]  [<ffffffff81133ece>] ___might_sleep+0x10e/0x180
[   69.025722]  [<ffffffff81133f89>] __might_sleep+0x49/0x80
[   69.025739]  [<ffffffff815d21d9>] __pm_runtime_resume+0x79/0x80
[   69.025841]  [<ffffffffa005ebc8>] intel_runtime_pm_get+0x28/0x90 [i915]
[   69.025924]  [<ffffffffa005ec49>] intel_display_power_get+0x19/0x50 [i915]
[   69.025995]  [<ffffffffa004aea4>] gen8_enable_vblank+0x34/0xc0 [i915]
[   69.026016]  [<ffffffff815a1f46>] drm_vblank_enable+0x76/0xd0




Another thing that I search in the spec was for an Interrupt to know
when we came back from DC5 or DC6 or got power well re-enabled, so we
would be able to restore the drm last counter... but I couldn't find
any...


Any other idea?


>
> Cheers, Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 25a8937..c294a4b 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -4556,7 +4556,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>>
>>       pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
>>
>> -     if (IS_GEN2(dev_priv)) {
>> +     if (INTEL_INFO(dev_priv)->gen >= 9) {
>> +             dev->max_vblank_count = 0;
>> +             dev->driver->get_vblank_counter = g4x_get_vblank_counter;
>> +     } else if (IS_GEN2(dev_priv)) {
>>               dev->max_vblank_count = 0;
>>               dev->driver->get_vblank_counter = i8xx_get_vblank_counter;
>>       } else if (IS_G4X(dev_priv) || INTEL_INFO(dev_priv)->gen >= 5) {
>> @@ -4572,7 +4575,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>>        * Gen2 doesn't have a hardware frame counter and so depends on
>>        * vblank interrupts to produce sane vblank seuquence numbers.
>>        */
>> -     if (!IS_GEN2(dev_priv))
>> +     if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9)
>>               dev->vblank_disable_immediate = true;
>>
>>       dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
>> --
>> 2.4.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> [e-mail]
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 25a8937..2b18778 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2743,7 +2743,10 @@  static int gen8_enable_vblank(struct drm_device
*dev, unsigned int pipe)
        struct drm_i915_private *dev_priv = dev->dev_private;
        unsigned long irqflags;

+       intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK);
+
        spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+       dev->vblank[pipe].last = g4x_get_vblank_counter(dev, pipe);
        bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
        spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);