[Intel-gfx] [PATCH] drm/i915: Reduce duplicated forcewake logic
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Wed Oct 1 17:53:07 CEST 2014
On 09/10/2014 07:34 PM, Chris Wilson wrote:
> Introduce a structure to track the individual forcewake domains and use
> that to eliminated duplicate logic.
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 41 +++---
> drivers/gpu/drm/i915/i915_drv.h | 32 +++--
> drivers/gpu/drm/i915/intel_uncore.c | 265 ++++++++++++++++--------------------
> 3 files changed, 157 insertions(+), 181 deletions(-)
As said yesterday I like the idea, some comments and questions inline.
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a72d8b8..c3176f1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1317,7 +1317,6 @@ static int vlv_drpc_info(struct seq_file *m)
> struct drm_device *dev = node->minor->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 rpmodectl1, rcctl1;
> - unsigned fw_rendercount = 0, fw_mediacount = 0;
>
> intel_runtime_pm_get(dev_priv);
>
> @@ -1350,22 +1349,12 @@ static int vlv_drpc_info(struct seq_file *m)
> seq_printf(m, "Media RC6 residency since boot: %u\n",
> I915_READ(VLV_GT_MEDIA_RC6));
>
> - spin_lock_irq(&dev_priv->uncore.lock);
> - fw_rendercount = dev_priv->uncore.fw_rendercount;
> - fw_mediacount = dev_priv->uncore.fw_mediacount;
> - spin_unlock_irq(&dev_priv->uncore.lock);
> -
> - seq_printf(m, "Forcewake Render Count = %u\n", fw_rendercount);
> - seq_printf(m, "Forcewake Media Count = %u\n", fw_mediacount);
> -
> -
> - return 0;
> + return i915_gen6_forcewake_count_info(m, data);
> }
>
>
> static int gen6_drpc_info(struct seq_file *m)
> {
> -
> struct drm_info_node *node = m->private;
> struct drm_device *dev = node->minor->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1379,7 +1368,7 @@ static int gen6_drpc_info(struct seq_file *m)
> intel_runtime_pm_get(dev_priv);
>
> spin_lock_irq(&dev_priv->uncore.lock);
> - forcewake_count = dev_priv->uncore.forcewake_count;
> + forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_RENDER].wake_count;
> spin_unlock_irq(&dev_priv->uncore.lock);
>
> if (forcewake_count) {
> @@ -1976,21 +1965,23 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
> struct drm_info_node *node = m->private;
> struct drm_device *dev = node->minor->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - unsigned forcewake_count = 0, fw_rendercount = 0, fw_mediacount = 0;
> + const char *domain_names[] = {
> + "render",
> + "media",
> + };
> + int i;
>
> spin_lock_irq(&dev_priv->uncore.lock);
> - if (IS_VALLEYVIEW(dev)) {
> - fw_rendercount = dev_priv->uncore.fw_rendercount;
> - fw_mediacount = dev_priv->uncore.fw_mediacount;
> - } else
> - forcewake_count = dev_priv->uncore.forcewake_count;
> - spin_unlock_irq(&dev_priv->uncore.lock);
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + if ((dev_priv->uncore.fw_domains & (1 << i)) == 0)
> + continue;
What do you think about something like for_each_forcewake_domain (and
perhaps for_each_possible_forcewake_domain) for readability since there
is a fair number of these loops?
> - if (IS_VALLEYVIEW(dev)) {
> - seq_printf(m, "fw_rendercount = %u\n", fw_rendercount);
> - seq_printf(m, "fw_mediacount = %u\n", fw_mediacount);
> - } else
> - seq_printf(m, "forcewake count = %u\n", forcewake_count);
> + seq_printf(m, "%s.wake_count = %u\n",
> + domain_names[i],
> + dev_priv->uncore.fw_domain[i].wake_count);
> + }
> +
> + spin_unlock_irq(&dev_priv->uncore.lock);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5d0b38c..6424191 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -528,18 +528,30 @@ struct intel_uncore_funcs {
> uint64_t val, bool trace);
> };
>
> +enum {
> + FW_DOMAIN_RENDER = 0,
> + FW_DOMAIN_MEDIA,
> +
> + FW_DOMAIN_COUNT
> +};
Would there be any need to attempt hiding these somehow so people don't
try to pass these instead of FORCEWAKE_... ones into relevant functions?
No idea how though. Maybe gcc will complain if passing in enum into int?
> +
> struct intel_uncore {
> spinlock_t lock; /** lock is also taken in irq contexts. */
>
> struct intel_uncore_funcs funcs;
>
> unsigned fifo_count;
> - unsigned forcewake_count;
> -
> - unsigned fw_rendercount;
> - unsigned fw_mediacount;
> + unsigned fw_domains;
>
> - struct timer_list force_wake_timer;
> + struct intel_uncore_forcewake_domain {
> + struct drm_i915_private *i915;
Should this be named dev_priv for consistency?
> + int id;
> + unsigned wake_count;
> + struct timer_list timer;
> + } fw_domain[FW_DOMAIN_COUNT];
> +#define FORCEWAKE_RENDER (1 << FW_DOMAIN_RENDER)
> +#define FORCEWAKE_MEDIA (1 << FW_DOMAIN_MEDIA)
> +#define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
> };
>
> #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> @@ -2948,8 +2960,10 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
> * 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, int fw_engine);
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> + unsigned fw_domains);
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> + unsigned fw_domains);
> void assert_force_wake_inactive(struct drm_i915_private *dev_priv);
>
> int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
> @@ -2981,10 +2995,6 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
> int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
> int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
>
> -#define FORCEWAKE_RENDER (1 << 0)
> -#define FORCEWAKE_MEDIA (1 << 1)
> -#define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
> -
>
> #define I915_READ8(reg) dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
> #define I915_WRITE8(reg, val) dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 3b3d3e0..641950b 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -73,7 +73,7 @@ static void __gen6_gt_force_wake_reset(struct drm_i915_private *dev_priv)
> }
>
> static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
> if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1) == 0,
> FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -99,7 +99,7 @@ static void __gen7_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
> }
>
> static void __gen7_gt_force_wake_mt_get(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
> u32 forcewake_ack;
>
> @@ -136,7 +136,7 @@ static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
> }
>
> static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
> __raw_i915_write32(dev_priv, FORCEWAKE, 0);
> /* something from same cacheline, but !FORCEWAKE */
> @@ -145,7 +145,7 @@ static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> }
>
> static void __gen7_gt_force_wake_mt_put(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
> __raw_i915_write32(dev_priv, FORCEWAKE_MT,
> _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> @@ -194,7 +194,7 @@ static void vlv_force_wake_reset(struct drm_i915_private *dev_priv)
> }
>
> static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
> /* Check for Render Engine */
> if (FORCEWAKE_RENDER & fw_engine) {
> @@ -238,9 +238,8 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> }
>
> static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
> - int fw_engine)
> + int fw_engine)
> {
What's up with the above series of indentation changes?
> -
> /* Check for Render Engine */
> if (FORCEWAKE_RENDER & fw_engine)
> __raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> @@ -258,59 +257,35 @@ static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
> gen6_gt_check_fifodbg(dev_priv);
> }
>
> -static void vlv_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> - 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);
> -}
> -
> -static void vlv_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> - if (fw_engine & FORCEWAKE_RENDER) {
> - WARN_ON(!dev_priv->uncore.fw_rendercount);
> - if (--dev_priv->uncore.fw_rendercount != 0)
> - fw_engine &= ~FORCEWAKE_RENDER;
> - }
> -
> - if (fw_engine & FORCEWAKE_MEDIA) {
> - WARN_ON(!dev_priv->uncore.fw_mediacount);
> - if (--dev_priv->uncore.fw_mediacount != 0)
> - fw_engine &= ~FORCEWAKE_MEDIA;
> - }
> -
> - if (fw_engine)
> - dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine);
> -}
> -
> static void gen6_force_wake_timer(unsigned long arg)
> {
> - struct drm_i915_private *dev_priv = (void *)arg;
> + struct intel_uncore_forcewake_domain *domain = (void *)arg;
> unsigned long irqflags;
>
> - assert_device_not_suspended(dev_priv);
> + assert_device_not_suspended(domain->i915);
>
> - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> - WARN_ON(!dev_priv->uncore.forcewake_count);
> + spin_lock_irqsave(&domain->i915->uncore.lock, irqflags);
> + WARN_ON(!domain->wake_count);
>
> - if (--dev_priv->uncore.forcewake_count == 0)
> - dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
> - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> + if (--domain->wake_count == 0)
> + domain->i915->uncore.funcs.force_wake_put(domain->i915,
> + 1 << domain->id);
> + spin_unlock_irqrestore(&domain->i915->uncore.lock, irqflags);
> }
>
> void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long irqflags;
> + int i;
>
> - if (del_timer_sync(&dev_priv->uncore.force_wake_timer))
> - gen6_force_wake_timer((unsigned long)dev_priv);
> + for (i = 0; i < FW_DOMAIN_COUNT; i++ ) {
> + if ((dev_priv->uncore.fw_domains & (1 << i)) == 0)
> + continue;
> +
> + if (del_timer_sync(&dev_priv->uncore.fw_domain[i].timer))
> + gen6_force_wake_timer((unsigned long)&dev_priv->uncore.fw_domain[i]);
> + }
>
> /* Hold uncore.lock across reset to prevent any register access
> * with forcewake not set correctly
> @@ -327,18 +302,12 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
>
> if (restore) { /* If reset with a user forcewake, try to restore */
> unsigned fw = 0;
> + int i;
>
> - if (IS_VALLEYVIEW(dev)) {
> - if (dev_priv->uncore.fw_rendercount)
> - fw |= FORCEWAKE_RENDER;
> -
> - if (dev_priv->uncore.fw_mediacount)
> - fw |= FORCEWAKE_MEDIA;
> - } else {
> - if (dev_priv->uncore.forcewake_count)
> - fw = FORCEWAKE_ALL;
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + if (dev_priv->uncore.fw_domain[i].wake_count)
> + fw |= 1 << i;
> }
> -
> if (fw)
> dev_priv->uncore.funcs.force_wake_get(dev_priv, fw);
>
> @@ -389,50 +358,60 @@ void intel_uncore_sanitize(struct drm_device *dev)
> * 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 fw_engine)
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> + unsigned fw_domains)
Is gen6_ still the best prefix for these two?
Should you also use the opportunity to add kerneldoc now that Daniel is
making a bit push in that direction?
> {
> unsigned long irqflags;
> + int i;
Not sure if it makes any difference nowadays to use unsigned for loops
like this.
>
> if (!dev_priv->uncore.funcs.force_wake_get)
> return;
>
> WARN_ON(!pm_runtime_active(&dev_priv->dev->pdev->dev));
>
> + fw_domains &= dev_priv->uncore.fw_domains;
> +
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - /* Redirect to VLV specific routine */
> - if (IS_VALLEYVIEW(dev_priv->dev)) {
> - vlv_force_wake_get(dev_priv, fw_engine);
> - } else {
> - if (dev_priv->uncore.forcewake_count++ == 0)
> - dev_priv->uncore.funcs.force_wake_get(dev_priv,
> - FORCEWAKE_ALL);
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + if ((fw_domains & (1 << i)) == 0)
> + continue;
> +
> + if (dev_priv->uncore.fw_domain[i].wake_count++)
> + fw_domains &= ~(1 << i);
> }
> +
> + if (fw_domains)
> + dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +
> spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> }
>
> /*
> * see gen6_gt_force_wake_get()
> */
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> + unsigned fw_domains)
> {
> unsigned long irqflags;
> + int i;
>
> if (!dev_priv->uncore.funcs.force_wake_put)
> return;
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - /* Redirect to VLV specific routine */
> - if (IS_VALLEYVIEW(dev_priv->dev)) {
> - vlv_force_wake_put(dev_priv, fw_engine);
> - } else {
> - WARN_ON(!dev_priv->uncore.forcewake_count);
> - if (--dev_priv->uncore.forcewake_count == 0) {
> - dev_priv->uncore.forcewake_count++;
> - mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
> - jiffies + 1);
> - }
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + if ((fw_domains & (1 << i)) == 0)
> + continue;
> +
> + WARN_ON(!dev_priv->uncore.fw_domain[i].wake_count);
> + if (--dev_priv->uncore.fw_domain[i].wake_count)
> + continue;
> +
> + dev_priv->uncore.fw_domain[i].wake_count++;
I would put a comment here about why we are incrementing after decrementing.
Possibly also what is the purpose of the timer if not already documented
somewhere.
> + mod_timer_pinned(&dev_priv->uncore.fw_domain[i].timer,
> + jiffies + 1);
> }
>
> spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> @@ -441,10 +420,13 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>
> void assert_force_wake_inactive(struct drm_i915_private *dev_priv)
> {
> + int i;
> +
> if (!dev_priv->uncore.funcs.force_wake_get)
> return;
>
> - WARN_ON(dev_priv->uncore.forcewake_count > 0);
> + for (i = 0; i < FW_DOMAIN_COUNT; i++)
> + WARN_ON(dev_priv->uncore.fw_domain[i].wake_count > 0);
> }
>
> /* We give fast paths for the really cool registers */
> @@ -578,19 +560,38 @@ __gen2_read(64)
> trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
> return val
>
> +static inline void __force_wake_get(struct drm_i915_private *dev_priv,
> + unsigned fw_domains)
This function is to be used by what type of callers compared to
gen6_gt_force_wake_get?
> +{
> + int i;
> +
> + /* Ideally GCC would be constant-fold and eliminate this loop */
> +
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + if ((fw_domains & (1 << i)) == 0)
> + continue;
> +
> + if (dev_priv->uncore.fw_domain[i].wake_count) {
> + fw_domains &= ~(1 << i);
> + continue;
> + }
> +
> + dev_priv->uncore.fw_domain[i].wake_count++;
> + mod_timer_pinned(&dev_priv->uncore.fw_domain[i].timer,
> + jiffies + 1);
> + }
> +
> + if (fw_domains)
> + dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +}
> +
> #define __gen6_read(x) \
> static u##x \
> gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> 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))) { \
> - dev_priv->uncore.funcs.force_wake_get(dev_priv, \
> - FORCEWAKE_ALL); \
> - dev_priv->uncore.forcewake_count++; \
> - mod_timer_pinned(&dev_priv->uncore.force_wake_timer, \
> - jiffies + 1); \
> - } \
> + if (NEEDS_FORCE_WAKE((dev_priv), (reg))) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> val = __raw_i915_read##x(dev_priv, reg); \
> hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \
> GEN6_READ_FOOTER; \
> @@ -599,45 +600,27 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> #define __vlv_read(x) \
> static u##x \
> vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> - unsigned fwengine = 0; \
> GEN6_READ_HEADER(x); \
> - 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) \
> - dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
> + if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> + else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> val = __raw_i915_read##x(dev_priv, reg); \
> - if (fwengine) \
> - dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
This changes from immediate to delayed put - would it make sense to move
it into a follow up patch?
> 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; \
> GEN6_READ_HEADER(x); \
> - if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_rendercount == 0) \
> - fwengine = FORCEWAKE_RENDER; \
> - } else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_mediacount == 0) \
> - fwengine = FORCEWAKE_MEDIA; \
> - } else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_rendercount == 0) \
> - fwengine |= FORCEWAKE_RENDER; \
> - if (dev_priv->uncore.fw_mediacount == 0) \
> - fwengine |= FORCEWAKE_MEDIA; \
> - } \
> - if (fwengine) \
> - dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
> + if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> + else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> + else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, \
> + FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
> val = __raw_i915_read##x(dev_priv, reg); \
> - if (fwengine) \
> - dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> GEN6_READ_FOOTER; \
> }
>
> @@ -766,17 +749,9 @@ static void \
> gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> 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) \
> - dev_priv->uncore.funcs.force_wake_get(dev_priv, \
> - FORCEWAKE_ALL); \
> - __raw_i915_write##x(dev_priv, reg, val); \
> - if (dev_priv->uncore.forcewake_count == 0) \
> - dev_priv->uncore.funcs.force_wake_put(dev_priv, \
> - FORCEWAKE_ALL); \
> - } else { \
> - __raw_i915_write##x(dev_priv, reg, val); \
> - } \
> + if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> + __raw_i915_write##x(dev_priv, reg, val); \
> hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> hsw_unclaimed_reg_detect(dev_priv); \
> GEN6_WRITE_FOOTER; \
> @@ -785,28 +760,17 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
> #define __chv_write(x) \
> 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); \
> GEN6_WRITE_HEADER; \
> if (!shadowed) { \
> - if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_rendercount == 0) \
> - fwengine = FORCEWAKE_RENDER; \
> - } else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_mediacount == 0) \
> - fwengine = FORCEWAKE_MEDIA; \
> - } else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> - if (dev_priv->uncore.fw_rendercount == 0) \
> - fwengine |= FORCEWAKE_RENDER; \
> - if (dev_priv->uncore.fw_mediacount == 0) \
> - fwengine |= FORCEWAKE_MEDIA; \
> - } \
> + if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> + else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> + else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> + __force_wake_get(dev_priv, FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
> } \
> - 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); \
> GEN6_WRITE_FOOTER; \
> }
>
> @@ -837,18 +801,18 @@ __gen6_write(64)
> void intel_uncore_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - setup_timer(&dev_priv->uncore.force_wake_timer,
> - gen6_force_wake_timer, (unsigned long)dev_priv);
> + int i;
>
> intel_uncore_early_sanitize(dev, false);
>
> if (IS_VALLEYVIEW(dev)) {
> dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
> dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
> + dev_priv->uncore.fw_domains = FORCEWAKE_RENDER | FORCEWAKE_MEDIA;
> } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> dev_priv->uncore.funcs.force_wake_get = __gen7_gt_force_wake_mt_get;
> dev_priv->uncore.funcs.force_wake_put = __gen7_gt_force_wake_mt_put;
> + dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
> } else if (IS_IVYBRIDGE(dev)) {
> u32 ecobus;
>
> @@ -880,11 +844,22 @@ void intel_uncore_init(struct drm_device *dev)
> dev_priv->uncore.funcs.force_wake_put =
> __gen6_gt_force_wake_put;
> }
> + dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
> } else if (IS_GEN6(dev)) {
> dev_priv->uncore.funcs.force_wake_get =
> __gen6_gt_force_wake_get;
> dev_priv->uncore.funcs.force_wake_put =
> __gen6_gt_force_wake_put;
> + dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
> + }
> +
> + for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> + dev_priv->uncore.fw_domain[i].i915 = dev_priv;
> + dev_priv->uncore.fw_domain[i].id = i;
> +
> + setup_timer(&dev_priv->uncore.fw_domain[i].timer,
> + gen6_force_wake_timer,
> + (unsigned long)&dev_priv->uncore.fw_domain[i]);
> }
>
> switch (INTEL_INFO(dev)->gen) {
>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list