Bob Wang <zhe1.wang@xxxxxxxxx> writes: > On 09/22/2014 04:11 PM, Mika Kuoppala wrote: >> Damien Lespiau <damien.lespiau@xxxxxxxxx> writes: >> >>> From: Zhe Wang <zhe1.wang@xxxxxxxxx> >>> >>> Enable multi-engine forcewake for Gen9. >>> >>> v2: Rebase on top of nightly >>> Move the register range definitions to intel_uncore.c >>> Whitespace fixes >>> (Damien) >>> >>> Signed-off-by: Zhe Wang <zhe1.wang@xxxxxxxxx> >>> Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/i915/intel_uncore.c | 120 ++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 120 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c >>> index 7b7fc9e..f289f4f 100644 >>> --- a/drivers/gpu/drm/i915/intel_uncore.c >>> +++ b/drivers/gpu/drm/i915/intel_uncore.c >>> @@ -674,6 +674,34 @@ void assert_force_wake_inactive(struct drm_i915_private *dev_priv) >>> REG_RANGE((reg), 0x14000, 0x14400) || \ >>> REG_RANGE((reg), 0x22000, 0x24000)) >>> >>> +#define FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) \ >>> + ((reg) >= 0xC00 && (reg) < 0x2000) >>> + >>> +#define FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) \ >>> + (((reg) >= 0x2000 && (reg) < 0x4000) || \ >>> + ((reg) >= 0x5200 && (reg) < 0x8000) || \ >>> + ((reg) >= 0x8300 && (reg) < 0x8500) || \ >>> + ((reg) >= 0x8C00 && (reg) < 0x8D00) || \ >>> + ((reg) >= 0xB000 && (reg) < 0xB480) || \ >>> + ((reg) >= 0xE000 && (reg) < 0xE800)) >>> + >>> +#define FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) \ >>> + (((reg) >= 0x8800 && (reg) < 0x8A00) || \ >>> + ((reg) >= 0xD000 && (reg) < 0xD800) || \ >>> + ((reg) >= 0x12000 && (reg) < 0x14000) || \ >>> + ((reg) >= 0x1A000 && (reg) < 0x1EA00) || \ >>> + ((reg) >= 0x30000 && (reg) < 0x40000)) >>> + >>> +#define FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg) \ >>> + ((reg) >= 0x9400 && (reg) < 0x9800) >> Please use REG_RANGE for all above. > ok, we can change this for these macros. >>> +#define FORCEWAKE_GEN9_BLITTER_RANGE_OFFSET(reg) \ >>> + ((reg) < 0x40000 &&\ >>> + !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) && \ >>> + !FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) && \ >>> + !FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) && \ >>> + !FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) >>> + >>> static void >>> ilk_dummy_write(struct drm_i915_private *dev_priv) >>> { >>> @@ -804,6 +832,43 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ >>> REG_READ_FOOTER; \ >>> } >>> >>> +#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); \ >>> + if (!NEEDS_FORCE_WAKE((dev_priv), (reg)) || \ >> As gen9 doesn't have FORCEWAKE register, you can save one >> comparison here if you discard the macro and opencode. > Hi Mika, > Excuse me, I didn't really get this. can you explain more on it? > > Gen9 has 3 separate forcewake registers. > we still have to check for 2 ranges to ensure forcewake is not required > 1. >= 0x40000 > 2. 0xC00-0x1FFF (this range is new and always accessible) I was a little bit vague on what macro I was talking about. The macro in question is: /* We give fast paths for the really cool registers */ #define NEEDS_FORCE_WAKE(dev_priv, reg) \ ((reg) < 0x40000 && (reg) != FORCEWAKE) and as we dont have generic FORCEWAKE (0xA18C) on gen9, we end up doing one needless != for each register access. -Mika > Thanks, > Bob >>> + FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg)) { \ >>> + val = __raw_i915_read##x(dev_priv, reg); \ >>> + } else { \ >>> + unsigned fwengine = 0; \ >>> + if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_rendercount == 0) \ >>> + fwengine = FORCEWAKE_RENDER; \ >>> + } else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_mediacount == 0) \ >>> + fwengine = FORCEWAKE_MEDIA; \ >>> + } else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_rendercount == 0) \ >>> + fwengine |= FORCEWAKE_RENDER; \ >>> + if (dev_priv->uncore.fw_mediacount == 0) \ >>> + fwengine |= FORCEWAKE_MEDIA; \ >>> + } else { \ >>> + if (dev_priv->uncore.fw_blittercount == 0) \ >>> + fwengine = FORCEWAKE_BLITTER; \ >>> + } \ >>> + 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; \ >>> +} >>> + >>> +__gen9_read(8) >>> +__gen9_read(16) >>> +__gen9_read(32) >>> +__gen9_read(64) >>> __chv_read(8) >>> __chv_read(16) >>> __chv_read(32) >>> @@ -825,6 +890,7 @@ __gen4_read(16) >>> __gen4_read(32) >>> __gen4_read(64) >>> >>> +#undef __gen9_read >>> #undef __chv_read >>> #undef __vlv_read >>> #undef __gen6_read >>> @@ -962,6 +1028,46 @@ chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) >>> REG_WRITE_FOOTER; \ >>> } >>> >>> +#define __gen9_write(x) \ >>> +static void \ >>> +gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \ >>> + bool trace) { \ >>> + REG_WRITE_HEADER; \ >>> + if (!NEEDS_FORCE_WAKE((dev_priv), (reg)) || \ >> ditto >> >> -Mika >> >>> + FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg)) { \ >>> + __raw_i915_write##x(dev_priv, reg, val); \ >>> + } else { \ >>> + unsigned fwengine = 0; \ >>> + if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_rendercount == 0) \ >>> + fwengine = FORCEWAKE_RENDER; \ >>> + } else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_mediacount == 0) \ >>> + fwengine = FORCEWAKE_MEDIA; \ >>> + } else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) { \ >>> + if (dev_priv->uncore.fw_rendercount == 0) \ >>> + fwengine |= FORCEWAKE_RENDER; \ >>> + if (dev_priv->uncore.fw_mediacount == 0) \ >>> + fwengine |= FORCEWAKE_MEDIA; \ >>> + } else { \ >>> + if (dev_priv->uncore.fw_blittercount == 0) \ >>> + fwengine = FORCEWAKE_BLITTER; \ >>> + } \ >>> + if (fwengine) \ >>> + dev_priv->uncore.funcs.force_wake_get(dev_priv, \ >>> + fwengine); \ >>> + __raw_i915_write##x(dev_priv, reg, val); \ >>> + if (fwengine) \ >>> + dev_priv->uncore.funcs.force_wake_put(dev_priv, \ >>> + fwengine); \ >>> + } \ >>> + REG_WRITE_FOOTER; \ >>> +} >>> + >>> +__gen9_write(8) >>> +__gen9_write(16) >>> +__gen9_write(32) >>> +__gen9_write(64) >>> __chv_write(8) >>> __chv_write(16) >>> __chv_write(32) >>> @@ -987,6 +1093,7 @@ __gen4_write(16) >>> __gen4_write(32) >>> __gen4_write(64) >>> >>> +#undef __gen9_write >>> #undef __chv_write >>> #undef __gen8_write >>> #undef __hsw_write >>> @@ -1054,6 +1161,19 @@ void intel_uncore_init(struct drm_device *dev) >>> >>> switch (INTEL_INFO(dev)->gen) { >>> default: >>> + WARN_ON(1); >>> + return; >>> + case 9: >>> + dev_priv->uncore.funcs.mmio_writeb = gen9_write8; >>> + dev_priv->uncore.funcs.mmio_writew = gen9_write16; >>> + dev_priv->uncore.funcs.mmio_writel = gen9_write32; >>> + dev_priv->uncore.funcs.mmio_writeq = gen9_write64; >>> + dev_priv->uncore.funcs.mmio_readb = gen9_read8; >>> + dev_priv->uncore.funcs.mmio_readw = gen9_read16; >>> + dev_priv->uncore.funcs.mmio_readl = gen9_read32; >>> + dev_priv->uncore.funcs.mmio_readq = gen9_read64; >>> + break; >>> + case 8: >>> if (IS_CHERRYVIEW(dev)) { >>> dev_priv->uncore.funcs.mmio_writeb = chv_write8; >>> dev_priv->uncore.funcs.mmio_writew = chv_write16; >>> -- >>> 1.8.3.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx