[Intel-gfx] [PATCH] drm/i915: Rebalance runtime pm vs forcewake

[Paulo Zanoni]

2014-03-14 15:43 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> On Fri, Mar 14, 2014 at 5:13 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> On Fri, Mar 14, 2014 at 04:51:16PM +0100, Daniel Vetter wrote:
>>> On Fri, Mar 14, 2014 at 08:37:15AM +0000, Chris Wilson wrote:
>>> > ---
>>> >  drivers/gpu/drm/i915/i915_drv.c     | 2 +-
>>> >  drivers/gpu/drm/i915/intel_uncore.c | 9 ++-------
>>> >  2 files changed, 3 insertions(+), 8 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>>> > index 5a0d34c47885..3fbf8aa8d119 100644
>>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> > @@ -845,11 +845,11 @@ static int i915_runtime_suspend(struct device *device)
>>> >     struct drm_i915_private *dev_priv = dev->dev_private;
>>> >
>>> >     WARN_ON(!HAS_RUNTIME_PM(dev));
>>> > -   assert_force_wake_inactive(dev_priv);
>>>
>>> Why is this necessary? Also I've already pushed a pile of other patches on
>>> top of all this, so I think a full commit is better. Also gives us an
>>> excuse to document our flailing here a bit better in a neat commit message
>>> ... Imo we should also mention that the forcewake_put here isn't really
>>> perf critical any more (if this is really the case).
>>
>> I was continuing the conversation with example code... This is, I think,
>> the simplest method for removing the pm_put from the forcewake timer,
>> and just wanted to make sure that we were in agreement before writing a
>> paragraph to explain the problem.
>
> Ah, with closer reading of your patch I've noticed that the
> uncore_fini is after the above assert, so this indeed has to go.
>
> I'm also ok with the overall patch if that doesn't cause another round
> back to reinstate the delayed forcewake put here ;-)

Hi

Last week Chris sent me a rebased version of this patch on pastebin. I
tested it, and when I run the "rte" subtest from pm_pc8, I get many
instances of the "WARN_ON(dev->irq_enabled)" that happens inside
intel_disable_gt_powersave().

I also tried to apply "drm/i915: Fix runtime PM inbalance due to reg
I/O forcewake dance", but the patch does not apply cleanly.

That leaves us with the original "drm/i915: don't schedule
force_wake_timer at gen6_read".

I'd really like to get this bug fixed ASAP as it completely prevents
runtime PM from working at all, and we already have fixes for it since
weeks ago. Daniel? Chris?

Thanks,
Paulo

> -Daniel
> --
> 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



-- 
Paulo Zanoni