[Intel-gfx] [PATCH] drm/i915: Release power ref after updating wm

[Matt Roper]

On Wed, Mar 16, 2016 at 05:15:31PM +0200, Imre Deak wrote:
> Matt has a similar fix already on the list, Matt/Ville what's the plan
> with that? Could we have a stop-gap solution?
> 
> --Imre

Yeah, I wrote two separate patches that would both be fixes for that
issue.  One of them moved the PM reference drop like the patch below;
the other wrapped the actual watermark programming in a PM
get_if_enabled() call to make sure the reference was held throughout the
watermark programming, or skipped the watermark programming entirely if
we were already fully powered down.

Neither patch got a reviewed-by and I got wrapped up in something else
and forgot to poke anyone.  I don't have a strong preference either way
on which fix is best (they should both be fine for now I think).  I
think longer-term we'll eventually need the second approach above once
we allow the watermark programming to happen asynchronously via a
workqueue task.


Matt

> 
> On Wed, 2016-03-16 at 17:11 +0200, Gabriel Feceoru wrote:
> > On HSW/BDW there are random dmesg-warn errors while running kms
> > tests, like:
> > 
> > [90587.867077] Call Trace:
> > [90587.867085]  [<ffffffff81403dae>] dump_stack+0x67/0x99
> > [90587.867094]  [<ffffffff810790f1>] warn_slowpath_common+0x81/0xc0
> > [90587.867099]  [<ffffffff81079177>] warn_slowpath_fmt+0x47/0x50
> > [90587.867150]  [<ffffffffa036e280>] hsw_write32+0x280/0x290 [i915]
> > [90587.867186]  [<ffffffffa0315cc8>] _ilk_disable_lp_wm+0x98/0xd0
> > [i915]
> > [90587.867222]  [<ffffffffa031b1f2>]
> > ilk_program_watermarks+0x4a2/0x940 [i915]
> > [90587.867260]  [<ffffffffa031b6e0>]
> > ilk_optimize_watermarks+0x50/0x70 [i915]
> > [90587.867311]  [<ffffffffa03894e2>] intel_atomic_commit+0x742/0x2170
> > [i915]
> > [90587.867319]  [<ffffffff810c06b0>] ? wait_woken+0xa0/0xa0
> > [90587.867326]  [<ffffffff81535062>] drm_atomic_commit+0x32/0x50
> > [90587.867333]  [<ffffffff8150fd89>]
> > drm_atomic_helper_connector_dpms+0xe9/0x1a0
> > [90587.867340]  [<ffffffff81529acd>]
> > drm_mode_obj_set_property_ioctl+0x21d/0x230
> > [90587.867346]  [<ffffffff81529b0b>]
> > drm_mode_connector_property_set_ioctl+0x2b/0x30
> > [90587.867352]  [<ffffffff8151a7a6>] drm_ioctl+0x236/0x4f0
> > [90587.867358]  [<ffffffff81529ae0>] ?
> > drm_mode_obj_set_property_ioctl+0x230/0x230
> > [90587.867366]  [<ffffffff811e90de>] do_vfs_ioctl+0x8e/0x680
> > [90587.867372]  [<ffffffff811f4f5a>] ? __fget_light+0x6a/0x90
> > [90587.867378]  [<ffffffff811e970c>] SyS_ioctl+0x3c/0x70
> > [90587.867386]  [<ffffffff817cbf9b>]
> > entry_SYSCALL_64_fastpath+0x16/0x73
> > 
> > Since writing the watermarks requires MMIO access, this should be
> > done before
> > releasing the power reference.
> > 
> > This clears the flip-flops in kms tests on HSW/BDW.
> > 
> > Signed-off-by: Gabriel Feceoru <gabriel.feceoru at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index ce55f0b..b141e52 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13631,9 +13631,6 @@ static int intel_atomic_commit(struct
> > drm_device *dev,
> >  			modeset_put_power_domains(dev_priv,
> > put_domains[i]);
> >  	}
> >  
> > -	if (intel_state->modeset)
> > -		intel_display_power_put(dev_priv,
> > POWER_DOMAIN_MODESET);
> > -
> >  	/*
> >  	 * Now that the vblank has passed, we can go ahead and
> > program the
> >  	 * optimal watermarks on platforms that need two-step
> > watermark
> > @@ -13648,6 +13645,9 @@ static int intel_atomic_commit(struct
> > drm_device *dev,
> >  			dev_priv-
> > >display.optimize_watermarks(intel_cstate);
> >  	}
> >  
> > +	if (intel_state->modeset)
> > +		intel_display_power_put(dev_priv,
> > POWER_DOMAIN_MODESET);
> > +
> >  	mutex_lock(&dev->struct_mutex);
> >  	drm_atomic_helper_cleanup_planes(dev, state);
> >  	mutex_unlock(&dev->struct_mutex);

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795