* [PATCH 0/2] forcewake warning fixes @ 2011-06-22 16:54 Ben Widawsky 2011-06-22 16:55 ` [PATCH 1/2] drm/i915: more forcewake lock error info Ben Widawsky 2011-06-22 16:55 ` [PATCH 2/2] drm/i915: save/resume forcewake lock fixes Ben Widawsky 0 siblings, 2 replies; 10+ messages in thread From: Ben Widawsky @ 2011-06-22 16:54 UTC (permalink / raw) To: intel-gfx; +Cc: Alexander Zhaunerchyk I'm having some trouble testing these currently. Hopefully someone else can verify them. Ben Widawsky (2): drm/i915: more forcewake lock error info drm/i915: save/resume forcewake lock fixes drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++---- drivers/gpu/drm/i915/i915_drv.h | 8 +++++--- drivers/gpu/drm/i915/i915_suspend.c | 5 +++++ 3 files changed, 18 insertions(+), 7 deletions(-) -- 1.7.5.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/i915: more forcewake lock error info 2011-06-22 16:54 [PATCH 0/2] forcewake warning fixes Ben Widawsky @ 2011-06-22 16:55 ` Ben Widawsky 2011-06-22 17:00 ` Chris Wilson 2011-06-22 16:55 ` [PATCH 2/2] drm/i915: save/resume forcewake lock fixes Ben Widawsky 1 sibling, 1 reply; 10+ messages in thread From: Ben Widawsky @ 2011-06-22 16:55 UTC (permalink / raw) To: intel-gfx; +Cc: Alexander Zhaunerchyk Get some more useful info for debugging backtraces on forcewake problems. Appropriate debug flags will be required for the drm module. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++---- drivers/gpu/drm/i915/i915_drv.h | 8 +++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 0defd42..af59811 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -317,13 +317,15 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) * be called at the beginning of the sequence followed by a call to * gen6_gt_force_wake_put() at the end of the sequence. */ -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) +int gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) { - WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); + int ret = WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); /* Forcewake is atomic in case we get in here without the lock */ if (atomic_add_return(1, &dev_priv->forcewake_count) == 1) __gen6_gt_force_wake_get(dev_priv); + + return ret; } static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) @@ -335,12 +337,14 @@ static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) /* * see gen6_gt_force_wake_get() */ -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) +int gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) { - WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); + int ret = WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); if (atomic_dec_and_test(&dev_priv->forcewake_count)) __gen6_gt_force_wake_put(dev_priv); + + return ret; } void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8a9fd91..7421eb9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1362,8 +1362,8 @@ extern void intel_display_print_error_state(struct seq_file *m, * must be set to prevent GT core from power down and stale values being * returned. */ -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv); -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv); +int gen6_gt_force_wake_get(struct drm_i915_private *dev_priv); +int gen6_gt_force_wake_put(struct drm_i915_private *dev_priv); void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv); /* We give fast paths for the really cool registers */ @@ -1376,7 +1376,9 @@ void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv); static inline u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ u##x val = 0; \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ - gen6_gt_force_wake_get(dev_priv); \ + if (gen6_gt_force_wake_get(dev_priv)) { \ + DRM_DEBUG_DRIVER("unlocked forcewake for 0x%08x\n", reg); \ + } \ val = read##y(dev_priv->regs + reg); \ gen6_gt_force_wake_put(dev_priv); \ } else { \ -- 1.7.5.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: more forcewake lock error info 2011-06-22 16:55 ` [PATCH 1/2] drm/i915: more forcewake lock error info Ben Widawsky @ 2011-06-22 17:00 ` Chris Wilson 2011-06-22 17:07 ` Ben Widawsky 0 siblings, 1 reply; 10+ messages in thread From: Chris Wilson @ 2011-06-22 17:00 UTC (permalink / raw) To: Ben Widawsky, intel-gfx; +Cc: Alexander Zhaunerchyk On Wed, 22 Jun 2011 09:55:00 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > Get some more useful info for debugging backtraces on forcewake > problems. Appropriate debug flags will be required for the drm module. Wouldn't it be neater to pass the reg down to gen6_gt_force_wake_get() so that we only have a single debug message and not an additional one for every inlined read/write? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: more forcewake lock error info 2011-06-22 17:00 ` Chris Wilson @ 2011-06-22 17:07 ` Ben Widawsky 2011-06-22 17:15 ` Chris Wilson 0 siblings, 1 reply; 10+ messages in thread From: Ben Widawsky @ 2011-06-22 17:07 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Alexander Zhaunerchyk [-- Attachment #1.1: Type: text/plain, Size: 987 bytes --] On Wed, Jun 22, 2011 at 10:00 AM, Chris Wilson <chris@chris-wilson.co.uk>wrote: > On Wed, 22 Jun 2011 09:55:00 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > > Get some more useful info for debugging backtraces on forcewake > > problems. Appropriate debug flags will be required for the drm module. > > Wouldn't it be neater to pass the reg down to > gen6_gt_force_wake_get() so that we only have a single debug message and > not an additional one for every inlined read/write? > -Chris > Urg... having some trouble with my email right now, so you have to deal with my gmail/webmail... 1. Yes, but I was lazy. 2. People call force_wake_get() without actually specifying a register, so I didn't want to enforce people entering bogus info on those calls. > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > [-- Attachment #1.2: Type: text/html, Size: 1749 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: more forcewake lock error info 2011-06-22 17:07 ` Ben Widawsky @ 2011-06-22 17:15 ` Chris Wilson 2011-06-22 17:26 ` Ben Widawsky 0 siblings, 1 reply; 10+ messages in thread From: Chris Wilson @ 2011-06-22 17:15 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Alexander Zhaunerchyk On Wed, 22 Jun 2011 10:07:50 -0700, Ben Widawsky <bwidawsk@gmail.com> wrote: > On Wed, Jun 22, 2011 at 10:00 AM, Chris Wilson <chris@chris-wilson.co.uk>wrote: > > > On Wed, 22 Jun 2011 09:55:00 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > > > Get some more useful info for debugging backtraces on forcewake > > > problems. Appropriate debug flags will be required for the drm module. > > > > Wouldn't it be neater to pass the reg down to > > gen6_gt_force_wake_get() so that we only have a single debug message and > > not an additional one for every inlined read/write? > > -Chris > > > > Urg... having some trouble with my email right now, so you have to deal with > my gmail/webmail... > > 1. Yes, but I was lazy. > 2. People call force_wake_get() without actually specifying a register, so I > didn't want to enforce people entering bogus info on those calls. We could always go for the traditional file:line automagic macros... But we already have that from the stacktrace if people are good enough to provide symbols. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: more forcewake lock error info 2011-06-22 17:15 ` Chris Wilson @ 2011-06-22 17:26 ` Ben Widawsky 0 siblings, 0 replies; 10+ messages in thread From: Ben Widawsky @ 2011-06-22 17:26 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Alexander Zhaunerchyk On Wed, Jun 22, 2011 at 06:15:40PM +0100, Chris Wilson wrote: > On Wed, 22 Jun 2011 10:07:50 -0700, Ben Widawsky <bwidawsk@gmail.com> wrote: > > On Wed, Jun 22, 2011 at 10:00 AM, Chris Wilson <chris@chris-wilson.co.uk>wrote: > > > > > On Wed, 22 Jun 2011 09:55:00 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > > > > Get some more useful info for debugging backtraces on forcewake > > > > problems. Appropriate debug flags will be required for the drm module. > > > > > > Wouldn't it be neater to pass the reg down to > > > gen6_gt_force_wake_get() so that we only have a single debug message and > > > not an additional one for every inlined read/write? > > > -Chris > > > > > > > Urg... having some trouble with my email right now, so you have to deal with > > my gmail/webmail... > > > > 1. Yes, but I was lazy. > > 2. People call force_wake_get() without actually specifying a register, so I > > didn't want to enforce people entering bogus info on those calls. > > We could always go for the traditional file:line automagic macros... > But we already have that from the stacktrace if people are good enough to > provide symbols. Yeah, I figured this would be easiest as it somewhat removes variance between commits, I can just search for the unprotected register access on the branch du jour (especially with you guys refactoring the code all time time... JBARNES). I'm fine if the consensus is to throw this one out. Seemed helpful at the time, so I thought I'd post it. > -Chris Ben ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/i915: save/resume forcewake lock fixes 2011-06-22 16:54 [PATCH 0/2] forcewake warning fixes Ben Widawsky 2011-06-22 16:55 ` [PATCH 1/2] drm/i915: more forcewake lock error info Ben Widawsky @ 2011-06-22 16:55 ` Ben Widawsky 2011-06-22 17:04 ` Chris Wilson 2011-06-22 17:50 ` Andrey Rahmatullin 1 sibling, 2 replies; 10+ messages in thread From: Ben Widawsky @ 2011-06-22 16:55 UTC (permalink / raw) To: intel-gfx; +Cc: Alexander Zhaunerchyk The lock must be held for the saving and restoring of VGA state. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> CC: Alexander Zhaunerchyk <alex.vizor@gmail.com> CC: Andrey Rahmatullin <wrar@wrar.name> --- drivers/gpu/drm/i915/i915_suspend.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index 60a94d2..e8152d2 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -678,6 +678,7 @@ void i915_save_display(struct drm_device *dev) } /* VGA state */ + mutex_lock(&dev->struct_mutex); dev_priv->saveVGA0 = I915_READ(VGA0); dev_priv->saveVGA1 = I915_READ(VGA1); dev_priv->saveVGA_PD = I915_READ(VGA_PD); @@ -687,6 +688,7 @@ void i915_save_display(struct drm_device *dev) dev_priv->saveVGACNTRL = I915_READ(VGACNTRL); i915_save_vga(dev); + mutex_unlock(&dev->struct_mutex); } void i915_restore_display(struct drm_device *dev) @@ -780,6 +782,8 @@ void i915_restore_display(struct drm_device *dev) I915_WRITE(CPU_VGACNTRL, dev_priv->saveVGACNTRL); else I915_WRITE(VGACNTRL, dev_priv->saveVGACNTRL); + + mutex_lock(&dev->struct_mutex); I915_WRITE(VGA0, dev_priv->saveVGA0); I915_WRITE(VGA1, dev_priv->saveVGA1); I915_WRITE(VGA_PD, dev_priv->saveVGA_PD); @@ -787,6 +791,7 @@ void i915_restore_display(struct drm_device *dev) udelay(150); i915_restore_vga(dev); + mutex_unlock(&dev->struct_mutex); } int i915_save_state(struct drm_device *dev) -- 1.7.5.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: save/resume forcewake lock fixes 2011-06-22 16:55 ` [PATCH 2/2] drm/i915: save/resume forcewake lock fixes Ben Widawsky @ 2011-06-22 17:04 ` Chris Wilson 2011-06-22 17:29 ` Keith Packard 2011-06-22 17:50 ` Andrey Rahmatullin 1 sibling, 1 reply; 10+ messages in thread From: Chris Wilson @ 2011-06-22 17:04 UTC (permalink / raw) To: Ben Widawsky, intel-gfx; +Cc: Alexander Zhaunerchyk On Wed, 22 Jun 2011 09:55:01 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > The lock must be held for the saving and restoring of VGA state. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > CC: Alexander Zhaunerchyk <alex.vizor@gmail.com> > CC: Andrey Rahmatullin <wrar@wrar.name> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: save/resume forcewake lock fixes 2011-06-22 17:04 ` Chris Wilson @ 2011-06-22 17:29 ` Keith Packard 0 siblings, 0 replies; 10+ messages in thread From: Keith Packard @ 2011-06-22 17:29 UTC (permalink / raw) To: Chris Wilson, Ben Widawsky, intel-gfx; +Cc: Alexander Zhaunerchyk [-- Attachment #1.1: Type: text/plain, Size: 212 bytes --] On Wed, 22 Jun 2011 18:04:46 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> I've added this to drm-intel-fixes -- keith.packard@intel.com [-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: save/resume forcewake lock fixes 2011-06-22 16:55 ` [PATCH 2/2] drm/i915: save/resume forcewake lock fixes Ben Widawsky 2011-06-22 17:04 ` Chris Wilson @ 2011-06-22 17:50 ` Andrey Rahmatullin 1 sibling, 0 replies; 10+ messages in thread From: Andrey Rahmatullin @ 2011-06-22 17:50 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Alexander Zhaunerchyk [-- Attachment #1.1: Type: text/plain, Size: 66 bytes --] Tested-by: Andrey Rahmatullin <wrar@wrar.name> -- WBR, wRAR [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-06-22 17:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-22 16:54 [PATCH 0/2] forcewake warning fixes Ben Widawsky 2011-06-22 16:55 ` [PATCH 1/2] drm/i915: more forcewake lock error info Ben Widawsky 2011-06-22 17:00 ` Chris Wilson 2011-06-22 17:07 ` Ben Widawsky 2011-06-22 17:15 ` Chris Wilson 2011-06-22 17:26 ` Ben Widawsky 2011-06-22 16:55 ` [PATCH 2/2] drm/i915: save/resume forcewake lock fixes Ben Widawsky 2011-06-22 17:04 ` Chris Wilson 2011-06-22 17:29 ` Keith Packard 2011-06-22 17:50 ` Andrey Rahmatullin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.