* [PATCH 0/2] drm/i915: VLV forcewake fixes
@ 2014-02-24 15:02 ville.syrjala
2014-02-24 15:02 ` [PATCH 1/2] drm/i915: Fix VLV forcewake after reset ville.syrjala
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: ville.syrjala @ 2014-02-24 15:02 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
While looking at the gen8 forcewake stuff I noticed that the VLV code is a bit
different, and also the post-reset forcewake handling is broken. This series
fixes those issues. git am didn't like Mika's forcewake patches for some reason
so I had to base this on nightly. That's going to lead to some conflicts which
sucks a bit, but I was too lazy to hand hold git am.
Ville Syrjälä (2):
drm/i915: Fix VLV forcewake after reset
drm/i915: Drop the forcewake count inc/dec around register read on VLV
drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
--
1.8.3.2
_______________________________________________
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* [PATCH 1/2] drm/i915: Fix VLV forcewake after reset 2014-02-24 15:02 [PATCH 0/2] drm/i915: VLV forcewake fixes ville.syrjala @ 2014-02-24 15:02 ` ville.syrjala [not found] ` <CAOh5HuXvv4Kcp+mCYCPp+hb06M_CxZ-Z+hivB7TOcn72tsaBcQ@mail.gmail.com> 2014-03-05 14:00 ` Daniel Vetter 2014-02-24 15:02 ` [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV ville.syrjala 2014-02-27 20:07 ` [PATCH 3/2] drm/i915: Streamline VLV forcewake handling ville.syrjala 2 siblings, 2 replies; 10+ messages in thread From: ville.syrjala @ 2014-02-24 15:02 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> Use the render/media specific forcewake counts to properly restore the forcewake status after a GPU reset on VLV. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index c628414..09fa555 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -967,10 +967,22 @@ static int gen6_do_reset(struct drm_device *dev) intel_uncore_forcewake_reset(dev); /* If reset with a user forcewake, try to restore, otherwise turn it off */ - if (dev_priv->uncore.forcewake_count) - dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL); - else - dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); + if (IS_VALLEYVIEW(dev)) { + if (dev_priv->uncore.fw_rendercount) + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_RENDER); + else + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_RENDER); + + if (dev_priv->uncore.fw_mediacount) + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_MEDIA); + else + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_MEDIA); + } else { + if (dev_priv->uncore.forcewake_count) + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL); + else + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); + } /* Restore fifo count */ dev_priv->uncore.fifo_count = __raw_i915_read32(dev_priv, GTFIFOCTL) & GT_FIFO_FREE_ENTRIES_MASK; -- 1.8.3.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CAOh5HuXvv4Kcp+mCYCPp+hb06M_CxZ-Z+hivB7TOcn72tsaBcQ@mail.gmail.com>]
* Re: Fwd: [PATCH 1/2] drm/i915: Fix VLV forcewake after reset [not found] ` <CAOh5HuXvv4Kcp+mCYCPp+hb06M_CxZ-Z+hivB7TOcn72tsaBcQ@mail.gmail.com> @ 2014-02-27 14:24 ` S, Deepak 0 siblings, 0 replies; 10+ messages in thread From: S, Deepak @ 2014-02-27 14:24 UTC (permalink / raw) To: Ville Syrjälä, intel-gfx On Wed, Jan 29, 2014 at 9:55 AM, ville syrjala > > > ---------- Forwarded message ---------- > From: ** <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > Date: Mon, Feb 24, 2014 at 8:32 PM > Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Fix VLV forcewake after reset > To: intel-gfx@lists.freedesktop.org <mailto:intel-gfx@lists.freedesktop.org> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > > Use the render/media specific forcewake counts to properly restore the > forcewake status after a GPU reset on VLV. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > --- > drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index c628414..09fa555 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -967,10 +967,22 @@ static int gen6_do_reset(struct drm_device *dev) > intel_uncore_forcewake_reset(dev); > > /* If reset with a user forcewake, try to restore, otherwise > turn it off */ > - if (dev_priv->uncore.forcewake_count) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > FORCEWAKE_ALL); > - else > - dev_priv->uncore.funcs.force_wake_put(dev_priv, > FORCEWAKE_ALL); > + if (IS_VALLEYVIEW(dev)) { > + if (dev_priv->uncore.fw_rendercount) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > FORCEWAKE_RENDER); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > FORCEWAKE_RENDER); > + > + if (dev_priv->uncore.fw_mediacount) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > FORCEWAKE_MEDIA); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > FORCEWAKE_MEDIA); > + } else { > + if (dev_priv->uncore.forcewake_count) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > FORCEWAKE_ALL); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > FORCEWAKE_ALL); > + } > > /* Restore fifo count */ > dev_priv->uncore.fifo_count = __raw_i915_read32(dev_priv, > GTFIFOCTL) & GT_FIFO_FREE_ENTRIES_MASK; > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > Reviewed-by: Deepak S <deepak.s@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: Fix VLV forcewake after reset 2014-02-24 15:02 ` [PATCH 1/2] drm/i915: Fix VLV forcewake after reset ville.syrjala [not found] ` <CAOh5HuXvv4Kcp+mCYCPp+hb06M_CxZ-Z+hivB7TOcn72tsaBcQ@mail.gmail.com> @ 2014-03-05 14:00 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2014-03-05 14:00 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Mon, Feb 24, 2014 at 05:02:08PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Use the render/media specific forcewake counts to properly restore the > forcewake status after a GPU reset on VLV. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index c628414..09fa555 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -967,10 +967,22 @@ static int gen6_do_reset(struct drm_device *dev) > intel_uncore_forcewake_reset(dev); > > /* If reset with a user forcewake, try to restore, otherwise turn it off */ > - if (dev_priv->uncore.forcewake_count) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL); > - else > - dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); > + if (IS_VALLEYVIEW(dev)) { > + if (dev_priv->uncore.fw_rendercount) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_RENDER); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_RENDER); > + > + if (dev_priv->uncore.fw_mediacount) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_MEDIA); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_MEDIA); > + } else { > + if (dev_priv->uncore.forcewake_count) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL); > + else > + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); > + } It starts to feel like we should subsume the forcewake stuff into our power domain handling, and create a bunch of domains for this ... But that's lots of work since we need to switch to a more explicit forcewake power domain handling then I guess. Not sure whether that's worth it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV 2014-02-24 15:02 [PATCH 0/2] drm/i915: VLV forcewake fixes ville.syrjala 2014-02-24 15:02 ` [PATCH 1/2] drm/i915: Fix VLV forcewake after reset ville.syrjala @ 2014-02-24 15:02 ` ville.syrjala [not found] ` <CAOh5HuU0eA0Pb3q=xDP_T_7N8TfWCbKuHZ==dh2G=V_uQfNZww@mail.gmail.com> 2014-02-27 20:07 ` [PATCH 3/2] drm/i915: Streamline VLV forcewake handling ville.syrjala 2 siblings, 1 reply; 10+ messages in thread From: ville.syrjala @ 2014-02-24 15:02 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> VLV is the only platform where we increment/decrement the forcewake count around register access. Drop the inc/dec on VLV to make the forcewake code a bit more unified. The inc/dec are not necessary since we hold the uncore lock around the whole operation. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 09fa555..dacb751 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -502,22 +502,22 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ static u##x \ vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ unsigned fwengine = 0; \ - unsigned *fwcount; \ + unsigned fwcount; \ REG_READ_HEADER(x); \ if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ fwengine = FORCEWAKE_RENDER; \ - fwcount = &dev_priv->uncore.fw_rendercount; \ + fwcount = dev_priv->uncore.fw_rendercount; \ } \ else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ fwengine = FORCEWAKE_MEDIA; \ - fwcount = &dev_priv->uncore.fw_mediacount; \ + fwcount = dev_priv->uncore.fw_mediacount; \ } \ if (fwengine != 0) { \ - if ((*fwcount)++ == 0) \ + if (fwcount == 0) \ (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \ fwengine); \ val = __raw_i915_read##x(dev_priv, reg); \ - if (--(*fwcount) == 0) \ + if (fwcount == 0) \ (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \ fwengine); \ } else { \ -- 1.8.3.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CAOh5HuU0eA0Pb3q=xDP_T_7N8TfWCbKuHZ==dh2G=V_uQfNZww@mail.gmail.com>]
* Re: Fwd: [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV [not found] ` <CAOh5HuU0eA0Pb3q=xDP_T_7N8TfWCbKuHZ==dh2G=V_uQfNZww@mail.gmail.com> @ 2014-02-27 14:26 ` S, Deepak 0 siblings, 0 replies; 10+ messages in thread From: S, Deepak @ 2014-02-27 14:26 UTC (permalink / raw) To: Ville Syrjälä, intel-gfx On Wed, Jan 29, 2014 at 9:55 AM, ville syrjala > > > ---------- Forwarded message ---------- > From: ** <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > Date: Mon, Feb 24, 2014 at 8:32 PM > Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Drop the forcewake count > inc/dec around register read on VLV > To: intel-gfx@lists.freedesktop.org <mailto:intel-gfx@lists.freedesktop.org> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > > VLV is the only platform where we increment/decrement the forcewake > count around register access. Drop the inc/dec on VLV to make the > forcewake code a bit more unified. > > The inc/dec are not necessary since we hold the uncore lock around > the whole operation. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > --- > drivers/gpu/drm/i915/intel_uncore.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 09fa555..dacb751 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -502,22 +502,22 @@ gen6_read##x(struct drm_i915_private *dev_priv, > off_t reg, bool trace) { \ > static u##x \ > vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ > unsigned fwengine = 0; \ > - unsigned *fwcount; \ > + unsigned fwcount; \ > REG_READ_HEADER(x); \ > if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ > fwengine = FORCEWAKE_RENDER; \ > - fwcount = &dev_priv->uncore.fw_rendercount; \ > + fwcount = dev_priv->uncore.fw_rendercount; \ > } \ > else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ > fwengine = FORCEWAKE_MEDIA; \ > - fwcount = &dev_priv->uncore.fw_mediacount; \ > + fwcount = dev_priv->uncore.fw_mediacount; \ > } \ > if (fwengine != 0) { \ > - if ((*fwcount)++ == 0) \ > + if (fwcount == 0) \ > (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \ > > fwengine); \ > val = __raw_i915_read##x(dev_priv, reg); \ > - if (--(*fwcount) == 0) \ > + if (fwcount == 0) \ > (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \ > fwengine); \ > } else { \ > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > Reviewed-by: Deepak S <deepak.s@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/2] drm/i915: Streamline VLV forcewake handling 2014-02-24 15:02 [PATCH 0/2] drm/i915: VLV forcewake fixes ville.syrjala 2014-02-24 15:02 ` [PATCH 1/2] drm/i915: Fix VLV forcewake after reset ville.syrjala 2014-02-24 15:02 ` [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV ville.syrjala @ 2014-02-27 20:07 ` ville.syrjala 2014-02-28 14:26 ` S, Deepak 2 siblings, 1 reply; 10+ messages in thread From: ville.syrjala @ 2014-02-27 20:07 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak S, deepak.s From: Ville Syrjälä <ville.syrjala@linux.intel.com> It occured to me that when we're trying to wake up both render and media wells on VLV, we might end up calling the low level force_wake_get/put two times even though one call would be enough. Make that happen by figuring out which wells really need to be woken up based on the forcewake counts. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 70 +++++++++++++++---------------------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index dacb751..4119ddc 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -251,16 +251,16 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv, unsigned long irqflags; spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - if (FORCEWAKE_RENDER & fw_engine) { - if (dev_priv->uncore.fw_rendercount++ == 0) - dev_priv->uncore.funcs.force_wake_get(dev_priv, - FORCEWAKE_RENDER); - } - if (FORCEWAKE_MEDIA & fw_engine) { - if (dev_priv->uncore.fw_mediacount++ == 0) - dev_priv->uncore.funcs.force_wake_get(dev_priv, - FORCEWAKE_MEDIA); - } + + if (fw_engine & FORCEWAKE_RENDER && + dev_priv->uncore.fw_rendercount++ != 0) + fw_engine &= ~FORCEWAKE_RENDER; + if (fw_engine & FORCEWAKE_MEDIA && + dev_priv->uncore.fw_mediacount++ != 0) + fw_engine &= ~FORCEWAKE_MEDIA; + + if (fw_engine) + dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_engine); spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } @@ -272,19 +272,15 @@ void vlv_force_wake_put(struct drm_i915_private *dev_priv, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - if (FORCEWAKE_RENDER & fw_engine) { - WARN_ON(dev_priv->uncore.fw_rendercount == 0); - if (--dev_priv->uncore.fw_rendercount == 0) - dev_priv->uncore.funcs.force_wake_put(dev_priv, - FORCEWAKE_RENDER); - } + if (fw_engine & FORCEWAKE_RENDER && + --dev_priv->uncore.fw_rendercount != 0) + fw_engine &= ~FORCEWAKE_RENDER; + if (fw_engine & FORCEWAKE_MEDIA && + --dev_priv->uncore.fw_mediacount != 0) + fw_engine &= ~FORCEWAKE_MEDIA; - if (FORCEWAKE_MEDIA & fw_engine) { - WARN_ON(dev_priv->uncore.fw_mediacount == 0); - if (--dev_priv->uncore.fw_mediacount == 0) - dev_priv->uncore.funcs.force_wake_put(dev_priv, - FORCEWAKE_MEDIA); - } + if (fw_engine) + dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine); spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } @@ -502,27 +498,19 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ static u##x \ vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ unsigned fwengine = 0; \ - unsigned fwcount; \ REG_READ_HEADER(x); \ - if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ - fwengine = FORCEWAKE_RENDER; \ - fwcount = dev_priv->uncore.fw_rendercount; \ - } \ - else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ - fwengine = FORCEWAKE_MEDIA; \ - fwcount = dev_priv->uncore.fw_mediacount; \ + if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ + if (dev_priv->uncore.fw_rendercount == 0) \ + fwengine = FORCEWAKE_RENDER; \ + } else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ + if (dev_priv->uncore.fw_mediacount == 0) \ + fwengine = FORCEWAKE_MEDIA; \ } \ - if (fwengine != 0) { \ - if (fwcount == 0) \ - (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \ - fwengine); \ - val = __raw_i915_read##x(dev_priv, reg); \ - if (fwcount == 0) \ - (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \ - fwengine); \ - } else { \ - val = __raw_i915_read##x(dev_priv, reg); \ - } \ + if (fwengine) \ + dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \ + val = __raw_i915_read##x(dev_priv, reg); \ + if (fwengine) \ + dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \ REG_READ_FOOTER; \ } -- 1.8.3.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/2] drm/i915: Streamline VLV forcewake handling 2014-02-27 20:07 ` [PATCH 3/2] drm/i915: Streamline VLV forcewake handling ville.syrjala @ 2014-02-28 14:26 ` S, Deepak 2014-02-28 14:51 ` Ville Syrjälä 0 siblings, 1 reply; 10+ messages in thread From: S, Deepak @ 2014-02-28 14:26 UTC (permalink / raw) To: ville.syrjala, intel-gfx On 2/28/2014 1:37 AM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > It occured to me that when we're trying to wake up both render > and media wells on VLV, we might end up calling the low level > force_wake_get/put two times even though one call would be > enough. Make that happen by figuring out which wells really > need to be woken up based on the forcewake counts. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 70 +++++++++++++++---------------------- > 1 file changed, 29 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index dacb751..4119ddc 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -251,16 +251,16 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv, > unsigned long irqflags; > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > - if (FORCEWAKE_RENDER & fw_engine) { > - if (dev_priv->uncore.fw_rendercount++ == 0) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > - FORCEWAKE_RENDER); > - } > - if (FORCEWAKE_MEDIA & fw_engine) { > - if (dev_priv->uncore.fw_mediacount++ == 0) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > - FORCEWAKE_MEDIA); > - } > + > + if (fw_engine & FORCEWAKE_RENDER && > + dev_priv->uncore.fw_rendercount++ != 0) > + fw_engine &= ~FORCEWAKE_RENDER; > + if (fw_engine & FORCEWAKE_MEDIA && > + dev_priv->uncore.fw_mediacount++ != 0) > + fw_engine &= ~FORCEWAKE_MEDIA; Should we add WARN_ON? I think it will help us if we have forcewake count mismatch? Other than this. Patch looks good. Reviewed-by:Deepak S <deepak.s@intel.com> > + if (fw_engine) > + dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_engine); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > @@ -272,19 +272,15 @@ void vlv_force_wake_put(struct drm_i915_private *dev_priv, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - if (FORCEWAKE_RENDER & fw_engine) { > - WARN_ON(dev_priv->uncore.fw_rendercount == 0); > - if (--dev_priv->uncore.fw_rendercount == 0) > - dev_priv->uncore.funcs.force_wake_put(dev_priv, > - FORCEWAKE_RENDER); > - } > + if (fw_engine & FORCEWAKE_RENDER && > + --dev_priv->uncore.fw_rendercount != 0) > + fw_engine &= ~FORCEWAKE_RENDER; > + if (fw_engine & FORCEWAKE_MEDIA && > + --dev_priv->uncore.fw_mediacount != 0) > + fw_engine &= ~FORCEWAKE_MEDIA; > > - if (FORCEWAKE_MEDIA & fw_engine) { > - WARN_ON(dev_priv->uncore.fw_mediacount == 0); > - if (--dev_priv->uncore.fw_mediacount == 0) > - dev_priv->uncore.funcs.force_wake_put(dev_priv, > - FORCEWAKE_MEDIA); > - } > + if (fw_engine) > + dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > @@ -502,27 +498,19 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ > static u##x \ > vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ > unsigned fwengine = 0; \ > - unsigned fwcount; \ > REG_READ_HEADER(x); \ > - if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ > - fwengine = FORCEWAKE_RENDER; \ > - fwcount = dev_priv->uncore.fw_rendercount; \ > - } \ > - else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ > - fwengine = FORCEWAKE_MEDIA; \ > - fwcount = dev_priv->uncore.fw_mediacount; \ > + if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ > + if (dev_priv->uncore.fw_rendercount == 0) \ > + fwengine = FORCEWAKE_RENDER; \ > + } else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ > + if (dev_priv->uncore.fw_mediacount == 0) \ > + fwengine = FORCEWAKE_MEDIA; \ > } \ > - if (fwengine != 0) { \ > - if (fwcount == 0) \ > - (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \ > - fwengine); \ > - val = __raw_i915_read##x(dev_priv, reg); \ > - if (fwcount == 0) \ > - (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \ > - fwengine); \ > - } else { \ > - val = __raw_i915_read##x(dev_priv, reg); \ > - } \ > + if (fwengine) \ > + dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \ > + val = __raw_i915_read##x(dev_priv, reg); \ > + if (fwengine) \ > + dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \ > REG_READ_FOOTER; \ > } > > _______________________________________________ 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 3/2] drm/i915: Streamline VLV forcewake handling 2014-02-28 14:26 ` S, Deepak @ 2014-02-28 14:51 ` Ville Syrjälä 2014-03-05 14:03 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2014-02-28 14:51 UTC (permalink / raw) To: S, Deepak; +Cc: intel-gfx On Fri, Feb 28, 2014 at 07:56:56PM +0530, S, Deepak wrote: > > > On 2/28/2014 1:37 AM, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > It occured to me that when we're trying to wake up both render > > and media wells on VLV, we might end up calling the low level > > force_wake_get/put two times even though one call would be > > enough. Make that happen by figuring out which wells really > > need to be woken up based on the forcewake counts. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_uncore.c | 70 +++++++++++++++---------------------- > > 1 file changed, 29 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index dacb751..4119ddc 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -251,16 +251,16 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv, > > unsigned long irqflags; > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - if (FORCEWAKE_RENDER & fw_engine) { > > - if (dev_priv->uncore.fw_rendercount++ == 0) > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > > - FORCEWAKE_RENDER); > > - } > > - if (FORCEWAKE_MEDIA & fw_engine) { > > - if (dev_priv->uncore.fw_mediacount++ == 0) > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > > - FORCEWAKE_MEDIA); > > - } > > + > > + if (fw_engine & FORCEWAKE_RENDER && > > + dev_priv->uncore.fw_rendercount++ != 0) > > + fw_engine &= ~FORCEWAKE_RENDER; > > + if (fw_engine & FORCEWAKE_MEDIA && > > + dev_priv->uncore.fw_mediacount++ != 0) > > + fw_engine &= ~FORCEWAKE_MEDIA; > > Should we add WARN_ON? I think it will help us if we have forcewake > count mismatch? > > Other than this. Patch looks good. > Reviewed-by:Deepak S <deepak.s@intel.com> I dropped the WARNs since we didn't have them on other platforms either. But if people think they might help, I'm not opposed to keeping them. > > > + if (fw_engine) > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_engine); > > > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > } > > @@ -272,19 +272,15 @@ void vlv_force_wake_put(struct drm_i915_private *dev_priv, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > - if (FORCEWAKE_RENDER & fw_engine) { > > - WARN_ON(dev_priv->uncore.fw_rendercount == 0); > > - if (--dev_priv->uncore.fw_rendercount == 0) > > - dev_priv->uncore.funcs.force_wake_put(dev_priv, > > - FORCEWAKE_RENDER); > > - } > > + if (fw_engine & FORCEWAKE_RENDER && > > + --dev_priv->uncore.fw_rendercount != 0) > > + fw_engine &= ~FORCEWAKE_RENDER; > > + if (fw_engine & FORCEWAKE_MEDIA && > > + --dev_priv->uncore.fw_mediacount != 0) > > + fw_engine &= ~FORCEWAKE_MEDIA; > > > > - if (FORCEWAKE_MEDIA & fw_engine) { > > - WARN_ON(dev_priv->uncore.fw_mediacount == 0); > > - if (--dev_priv->uncore.fw_mediacount == 0) > > - dev_priv->uncore.funcs.force_wake_put(dev_priv, > > - FORCEWAKE_MEDIA); > > - } > > + if (fw_engine) > > + dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine); > > > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > } > > @@ -502,27 +498,19 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ > > static u##x \ > > vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ > > unsigned fwengine = 0; \ > > - unsigned fwcount; \ > > REG_READ_HEADER(x); \ > > - if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ > > - fwengine = FORCEWAKE_RENDER; \ > > - fwcount = dev_priv->uncore.fw_rendercount; \ > > - } \ > > - else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ > > - fwengine = FORCEWAKE_MEDIA; \ > > - fwcount = dev_priv->uncore.fw_mediacount; \ > > + if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \ > > + if (dev_priv->uncore.fw_rendercount == 0) \ > > + fwengine = FORCEWAKE_RENDER; \ > > + } else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \ > > + if (dev_priv->uncore.fw_mediacount == 0) \ > > + fwengine = FORCEWAKE_MEDIA; \ > > } \ > > - if (fwengine != 0) { \ > > - if (fwcount == 0) \ > > - (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \ > > - fwengine); \ > > - val = __raw_i915_read##x(dev_priv, reg); \ > > - if (fwcount == 0) \ > > - (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \ > > - fwengine); \ > > - } else { \ > > - val = __raw_i915_read##x(dev_priv, reg); \ > > - } \ > > + if (fwengine) \ > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \ > > + val = __raw_i915_read##x(dev_priv, reg); \ > > + if (fwengine) \ > > + dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \ > > REG_READ_FOOTER; \ > > } > > > > -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/2] drm/i915: Streamline VLV forcewake handling 2014-02-28 14:51 ` Ville Syrjälä @ 2014-03-05 14:03 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2014-03-05 14:03 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Fri, Feb 28, 2014 at 04:51:16PM +0200, Ville Syrjälä wrote: > On Fri, Feb 28, 2014 at 07:56:56PM +0530, S, Deepak wrote: > > > > > > On 2/28/2014 1:37 AM, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > It occured to me that when we're trying to wake up both render > > > and media wells on VLV, we might end up calling the low level > > > force_wake_get/put two times even though one call would be > > > enough. Make that happen by figuring out which wells really > > > need to be woken up based on the forcewake counts. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_uncore.c | 70 +++++++++++++++---------------------- > > > 1 file changed, 29 insertions(+), 41 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > > index dacb751..4119ddc 100644 > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > @@ -251,16 +251,16 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv, > > > unsigned long irqflags; > > > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > - if (FORCEWAKE_RENDER & fw_engine) { > > > - if (dev_priv->uncore.fw_rendercount++ == 0) > > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > > > - FORCEWAKE_RENDER); > > > - } > > > - if (FORCEWAKE_MEDIA & fw_engine) { > > > - if (dev_priv->uncore.fw_mediacount++ == 0) > > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, > > > - FORCEWAKE_MEDIA); > > > - } > > > + > > > + if (fw_engine & FORCEWAKE_RENDER && > > > + dev_priv->uncore.fw_rendercount++ != 0) > > > + fw_engine &= ~FORCEWAKE_RENDER; > > > + if (fw_engine & FORCEWAKE_MEDIA && > > > + dev_priv->uncore.fw_mediacount++ != 0) > > > + fw_engine &= ~FORCEWAKE_MEDIA; > > > > Should we add WARN_ON? I think it will help us if we have forcewake > > count mismatch? > > > > Other than this. Patch looks good. > > Reviewed-by:Deepak S <deepak.s@intel.com> > > I dropped the WARNs since we didn't have them on other platforms either. > But if people think they might help, I'm not opposed to keeping them. Especially with the multiple fw domains I think a WARN for a refcount underflow would be useful. btw for vlv fw unification: The delayed put is also still on the table ... Entire series merged, thanks for patches&review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-05 14:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-24 15:02 [PATCH 0/2] drm/i915: VLV forcewake fixes ville.syrjala
2014-02-24 15:02 ` [PATCH 1/2] drm/i915: Fix VLV forcewake after reset ville.syrjala
[not found] ` <CAOh5HuXvv4Kcp+mCYCPp+hb06M_CxZ-Z+hivB7TOcn72tsaBcQ@mail.gmail.com>
2014-02-27 14:24 ` Fwd: " S, Deepak
2014-03-05 14:00 ` Daniel Vetter
2014-02-24 15:02 ` [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV ville.syrjala
[not found] ` <CAOh5HuU0eA0Pb3q=xDP_T_7N8TfWCbKuHZ==dh2G=V_uQfNZww@mail.gmail.com>
2014-02-27 14:26 ` Fwd: " S, Deepak
2014-02-27 20:07 ` [PATCH 3/2] drm/i915: Streamline VLV forcewake handling ville.syrjala
2014-02-28 14:26 ` S, Deepak
2014-02-28 14:51 ` Ville Syrjälä
2014-03-05 14:03 ` Daniel Vetter
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.