| Message ID | 1458141104-18137-1-git-send-email-gabriel.feceoru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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@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);
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@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);
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);
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@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)