From: Deepak S <deepak.s@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/8] drm/i915: Skip uncore lock on earlier gens
Date: Fri, 12 Dec 2014 17:27:19 +0530 [thread overview]
Message-ID: <548AD81F.6070308@linux.intel.com> (raw)
In-Reply-To: <1418063262-32256-3-git-send-email-mika.kuoppala@intel.com>
On Monday 08 December 2014 11:57 PM, Mika Kuoppala wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> With gen < 6 we don't need to take uncore lock as we
> don't have anything to protect from concurrent access.
>
> v2: rebase and account for gen9 changes
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 158 +++++++++++++++++++++---------------
> 1 file changed, 91 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a1ceb92..069fe7a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -707,38 +707,61 @@ hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
> }
> }
>
> -#define REG_READ_HEADER(x) \
> - unsigned long irqflags; \
> +#define GEN2_READ_HEADER(x) \
> u##x val = 0; \
> - assert_device_not_suspended(dev_priv); \
> - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
> + assert_device_not_suspended(dev_priv);
>
> -#define REG_READ_FOOTER \
> - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
> +#define GEN2_READ_FOOTER \
> trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
> return val
>
> -#define __gen4_read(x) \
> +#define __gen2_read(x) \
> static u##x \
> -gen4_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> - REG_READ_HEADER(x); \
> +gen2_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> + GEN2_READ_HEADER(x); \
> val = __raw_i915_read##x(dev_priv, reg); \
> - REG_READ_FOOTER; \
> + GEN2_READ_FOOTER; \
> }
>
> #define __gen5_read(x) \
> static u##x \
> gen5_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> - REG_READ_HEADER(x); \
> + GEN2_READ_HEADER(x); \
> ilk_dummy_write(dev_priv); \
> val = __raw_i915_read##x(dev_priv, reg); \
> - REG_READ_FOOTER; \
> + GEN2_READ_FOOTER; \
> }
>
> +__gen5_read(8)
> +__gen5_read(16)
> +__gen5_read(32)
> +__gen5_read(64)
> +__gen2_read(8)
> +__gen2_read(16)
> +__gen2_read(32)
> +__gen2_read(64)
> +
> +#undef __gen5_read
> +#undef __gen2_read
> +
> +#undef GEN2_READ_FOOTER
> +#undef GEN2_READ_HEADER
> +
> +#define GEN6_READ_HEADER(x) \
> + unsigned long irqflags; \
> + u##x val = 0; \
> + assert_device_not_suspended(dev_priv); \
> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
> +
> +#define GEN6_READ_FOOTER \
> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
> + trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
> + return val
> +
> #define __gen6_read(x) \
> static u##x \
> gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> - REG_READ_HEADER(x); \
> + GEN6_READ_HEADER(x); \
> hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \
> if (dev_priv->uncore.forcewake_count == 0 && \
> NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> @@ -750,14 +773,14 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> } \
> val = __raw_i915_read##x(dev_priv, reg); \
> hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \
> - REG_READ_FOOTER; \
> + GEN6_READ_FOOTER; \
> }
>
> #define __vlv_read(x) \
> static u##x \
> vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> unsigned fwengine = 0; \
> - REG_READ_HEADER(x); \
> + GEN6_READ_HEADER(x); \
> if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \
> if (dev_priv->uncore.fw_rendercount == 0) \
> fwengine = FORCEWAKE_RENDER; \
> @@ -770,14 +793,14 @@ vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> val = __raw_i915_read##x(dev_priv, reg); \
> if (fwengine) \
> dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> - REG_READ_FOOTER; \
> + GEN6_READ_FOOTER; \
> }
>
> #define __chv_read(x) \
> static u##x \
> chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> unsigned fwengine = 0; \
> - REG_READ_HEADER(x); \
> + GEN6_READ_HEADER(x); \
> if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> if (dev_priv->uncore.fw_rendercount == 0) \
> fwengine = FORCEWAKE_RENDER; \
> @@ -795,7 +818,7 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> val = __raw_i915_read##x(dev_priv, reg); \
> if (fwengine) \
> dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> - REG_READ_FOOTER; \
> + GEN6_READ_FOOTER; \
> }
>
> #define SKL_NEEDS_FORCE_WAKE(dev_priv, reg) \
> @@ -804,7 +827,7 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> #define __gen9_read(x) \
> static u##x \
> gen9_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> - REG_READ_HEADER(x); \
> + GEN6_READ_HEADER(x); \
> if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> val = __raw_i915_read##x(dev_priv, reg); \
> } else { \
> @@ -830,7 +853,7 @@ gen9_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> if (fwengine) \
> dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> } \
> - REG_READ_FOOTER; \
> + GEN6_READ_FOOTER; \
> }
>
> __gen9_read(8)
> @@ -849,55 +872,66 @@ __gen6_read(8)
> __gen6_read(16)
> __gen6_read(32)
> __gen6_read(64)
> -__gen5_read(8)
> -__gen5_read(16)
> -__gen5_read(32)
> -__gen5_read(64)
> -__gen4_read(8)
> -__gen4_read(16)
> -__gen4_read(32)
> -__gen4_read(64)
>
> #undef __gen9_read
> #undef __chv_read
> #undef __vlv_read
> #undef __gen6_read
> -#undef __gen5_read
> -#undef __gen4_read
> -#undef REG_READ_FOOTER
> -#undef REG_READ_HEADER
> +#undef GEN6_READ_FOOTER
> +#undef GEN6_READ_HEADER
>
> -#define REG_WRITE_HEADER \
> - unsigned long irqflags; \
> +#define GEN2_WRITE_HEADER \
> trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
> assert_device_not_suspended(dev_priv); \
> - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
>
> -#define REG_WRITE_FOOTER \
> - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags)
> +#define GEN2_WRITE_FOOTER
>
> -#define __gen4_write(x) \
> +#define __gen2_write(x) \
> static void \
> -gen4_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> - REG_WRITE_HEADER; \
> +gen2_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> + GEN2_WRITE_HEADER; \
> __raw_i915_write##x(dev_priv, reg, val); \
> - REG_WRITE_FOOTER; \
> + GEN2_WRITE_FOOTER; \
> }
>
> #define __gen5_write(x) \
> static void \
> gen5_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> - REG_WRITE_HEADER; \
> + GEN2_WRITE_HEADER; \
> ilk_dummy_write(dev_priv); \
> __raw_i915_write##x(dev_priv, reg, val); \
> - REG_WRITE_FOOTER; \
> + GEN2_WRITE_FOOTER; \
> }
>
> +__gen5_write(8)
> +__gen5_write(16)
> +__gen5_write(32)
> +__gen5_write(64)
> +__gen2_write(8)
> +__gen2_write(16)
> +__gen2_write(32)
> +__gen2_write(64)
> +
> +#undef __gen5_write
> +#undef __gen2_write
> +
> +#undef GEN2_WRITE_FOOTER
> +#undef GEN2_WRITE_HEADER
> +
> +#define GEN6_WRITE_HEADER \
> + unsigned long irqflags; \
> + trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
> + assert_device_not_suspended(dev_priv); \
> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
> +
> +#define GEN6_WRITE_FOOTER \
> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags)
> +
> #define __gen6_write(x) \
> static void \
> gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> u32 __fifo_ret = 0; \
> - REG_WRITE_HEADER; \
> + GEN6_WRITE_HEADER; \
> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
> } \
> @@ -905,14 +939,14 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
> if (unlikely(__fifo_ret)) { \
> gen6_gt_check_fifodbg(dev_priv); \
> } \
> - REG_WRITE_FOOTER; \
> + GEN6_WRITE_FOOTER; \
> }
>
> #define __hsw_write(x) \
> static void \
> hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> u32 __fifo_ret = 0; \
> - REG_WRITE_HEADER; \
> + GEN6_WRITE_HEADER; \
> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
> } \
> @@ -923,7 +957,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
> } \
> hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> hsw_unclaimed_reg_detect(dev_priv); \
> - REG_WRITE_FOOTER; \
> + GEN6_WRITE_FOOTER; \
> }
>
> static const u32 gen8_shadowed_regs[] = {
> @@ -950,7 +984,7 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg)
> #define __gen8_write(x) \
> static void \
> gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> - REG_WRITE_HEADER; \
> + GEN6_WRITE_HEADER; \
> hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) { \
> if (dev_priv->uncore.forcewake_count == 0) \
> @@ -965,7 +999,7 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
> } \
> hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> hsw_unclaimed_reg_detect(dev_priv); \
> - REG_WRITE_FOOTER; \
> + GEN6_WRITE_FOOTER; \
> }
>
> #define __chv_write(x) \
> @@ -973,7 +1007,7 @@ static void \
> chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> unsigned fwengine = 0; \
> bool shadowed = is_gen8_shadowed(dev_priv, reg); \
> - REG_WRITE_HEADER; \
> + GEN6_WRITE_HEADER; \
> if (!shadowed) { \
> if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> if (dev_priv->uncore.fw_rendercount == 0) \
> @@ -993,7 +1027,7 @@ chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
> __raw_i915_write##x(dev_priv, reg, val); \
> if (fwengine) \
> dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> - REG_WRITE_FOOTER; \
> + GEN6_WRITE_FOOTER; \
> }
>
> static const u32 gen9_shadowed_regs[] = {
> @@ -1023,7 +1057,7 @@ static bool is_gen9_shadowed(struct drm_i915_private *dev_priv, u32 reg)
> static void \
> gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
> bool trace) { \
> - REG_WRITE_HEADER; \
> + GEN6_WRITE_HEADER; \
> if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)) || \
> is_gen9_shadowed(dev_priv, reg)) { \
> __raw_i915_write##x(dev_priv, reg, val); \
> @@ -1052,7 +1086,7 @@ gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
> dev_priv->uncore.funcs.force_wake_put(dev_priv, \
> fwengine); \
> } \
> - REG_WRITE_FOOTER; \
> + GEN6_WRITE_FOOTER; \
> }
>
> __gen9_write(8)
> @@ -1075,24 +1109,14 @@ __gen6_write(8)
> __gen6_write(16)
> __gen6_write(32)
> __gen6_write(64)
> -__gen5_write(8)
> -__gen5_write(16)
> -__gen5_write(32)
> -__gen5_write(64)
> -__gen4_write(8)
> -__gen4_write(16)
> -__gen4_write(32)
> -__gen4_write(64)
>
> #undef __gen9_write
> #undef __chv_write
> #undef __gen8_write
> #undef __hsw_write
> #undef __gen6_write
> -#undef __gen5_write
> -#undef __gen4_write
> -#undef REG_WRITE_FOOTER
> -#undef REG_WRITE_HEADER
> +#undef GEN6_WRITE_FOOTER
> +#undef GEN6_WRITE_HEADER
>
> #define ASSIGN_WRITE_MMIO_VFUNCS(x) \
> do { \
> @@ -1205,8 +1229,8 @@ void intel_uncore_init(struct drm_device *dev)
> case 4:
> case 3:
> case 2:
> - ASSIGN_WRITE_MMIO_VFUNCS(gen4);
> - ASSIGN_READ_MMIO_VFUNCS(gen4);
> + ASSIGN_WRITE_MMIO_VFUNCS(gen2);
> + ASSIGN_READ_MMIO_VFUNCS(gen2);
> break;
> }
>
changes looks fine to me
Reviewed-by: Deepak S<deepak.s@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2014-12-11 12:00 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-08 18:27 [PATCH 1/8] drm/i915: Rebalance runtime pm vs forcewake Mika Kuoppala
2014-12-08 18:27 ` [PATCH 2/8] drm/i915: Assert that runtime pm is active on user fw access Mika Kuoppala
2014-12-12 11:39 ` Deepak S
2014-12-11 11:53 ` Chris Wilson
2014-12-12 11:59 ` Deepak S
2014-12-08 18:27 ` [PATCH 3/8] drm/i915: Skip uncore lock on earlier gens Mika Kuoppala
2014-12-12 11:57 ` Deepak S [this message]
2014-12-08 18:27 ` [PATCH 4/8] drm/i915: Reduce duplicated forcewake logic Mika Kuoppala
2014-12-12 12:48 ` Deepak S
2014-12-16 15:26 ` Mika Kuoppala
2014-12-08 18:27 ` [PATCH 5/8] drm/i915: Consolidate forcewake code Mika Kuoppala
2014-12-12 13:13 ` Deepak S
2014-12-08 18:27 ` [PATCH 6/8] drm/i915: Make vlv and chv forcewake put generic Mika Kuoppala
2014-12-12 13:16 ` Deepak S
2014-12-08 18:27 ` [PATCH 7/8] drm/i915: Rename the forcewake get/put functions Mika Kuoppala
2014-12-12 13:19 ` Deepak S
2014-12-08 18:27 ` [PATCH 8/8] drm/i915: Follow the forcewake domains type on hw accessors Mika Kuoppala
2014-12-09 11:46 ` [PATCH 8/8] drm/i915: Enum forcewake domains and domain identifiers Mika Kuoppala
2014-12-09 13:32 ` Jani Nikula
2014-12-09 13:36 ` Daniel Vetter
2014-12-09 15:37 ` Mika Kuoppala
2014-12-12 13:21 ` Deepak S
2014-12-09 23:29 ` [PATCH 8/8] drm/i915: Follow the forcewake domains type on hw accessors shuang.he
2014-12-12 13:20 ` Deepak S
2014-12-12 10:00 ` [PATCH 1/8] drm/i915: Rebalance runtime pm vs forcewake Deepak S
2014-12-11 10:15 ` Chris Wilson
2014-12-12 11:24 ` Deepak S
2014-12-15 8:46 ` Daniel Vetter
2014-12-12 16:22 ` Paulo Zanoni
2014-12-12 16:45 ` Chris Wilson
-- strict thread matches above, loose matches on Subject: below --
2014-12-09 12:59 [PATCH 00/10] Prep work patches for GPU scheduler John.C.Harrison
2014-12-09 12:59 ` [PATCH 01/10] drm/i915: Rename 'flags' to 'dispatch_flags' for better code reading John.C.Harrison
2014-12-09 12:59 ` [PATCH 02/10] drm/i915: Add missing trace point to LRC execbuff code path John.C.Harrison
2014-12-09 12:59 ` [PATCH 03/10] drm/i915: Updating assorted register and status page definitions John.C.Harrison
2014-12-10 10:40 ` Daniel Vetter
2014-12-10 16:37 ` Dave Gordon
2014-12-09 12:59 ` [PATCH 04/10] drm/i915: FIFO space query code refactor John.C.Harrison
2014-12-10 10:41 ` Daniel Vetter
2014-12-10 18:12 ` [PATCH v2] " Dave Gordon
2015-02-20 9:34 ` Mika Kuoppala
2015-02-23 15:46 ` Daniel Vetter
2014-12-09 12:59 ` [PATCH 05/10] drm/i915: Disable 'get seqno' workaround for VLV John.C.Harrison
2014-12-10 10:42 ` Daniel Vetter
2014-12-10 17:11 ` Dave Gordon
2014-12-15 9:02 ` Daniel Vetter
2014-12-09 12:59 ` [PATCH 06/10] drm/i915: Add extra add_request calls John.C.Harrison
2014-12-10 10:55 ` Daniel Vetter
2014-12-09 12:59 ` [PATCH 07/10] drm/i915: Early alloc request John.C.Harrison
2014-12-09 12:59 ` [PATCH 08/10] drm/i915: Prelude to splitting i915_gem_do_execbuffer in two John.C.Harrison
2014-12-10 10:58 ` Daniel Vetter
2014-12-10 16:33 ` Dave Gordon
2014-12-10 17:15 ` Daniel Vetter
2014-12-16 14:26 ` Dave Gordon
2014-12-17 20:09 ` Daniel Vetter
2014-12-18 14:06 ` Dave Gordon
2014-12-09 12:59 ` [PATCH 09/10] drm/i915: Split i915_dem_do_execbuffer() in half John.C.Harrison
2014-12-09 12:59 ` [PATCH 10/10] drm/i915: Cache ringbuf pointer in request structure John.C.Harrison
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=548AD81F.6070308@linux.intel.com \
--to=deepak.s@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox