[Intel-gfx] About the wait_for() macro in intel_drv.h

[Wang, Zhi A]

Ping. :)

[code]
#define _wait_for(COND, MS, W) ({ \
        unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1;   \
        int ret__ = 0;                                                  \
        while (!(COND)) {                                               \
                if (time_after(jiffies, timeout__)) {                   \                       <-------- jiffies will only be updated when local CPU interrupt is enabled.
                        if (!(COND))                                    \
                                ret__ = -ETIMEDOUT;                     \
                        break;                                          \
                }                                                       \
                if ((W) && drm_can_sleep()) {                           \
                        usleep_range((W)*1000, (W)*2000);               \
                } else {                                                \
                        cpu_relax();                                    \
                }                                                       \
        }                                                               \
        ret__;                                                          \
})
 
#define wait_for(COND, MS) _wait_for(COND, MS, 1)
#define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
#define wait_for_atomic_us(COND, US) _wait_for((COND),
[/code]

But in below functions:
[code]
#define __gen8_write(x) \
static void \
gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
        GEN6_WRITE_HEADER; \              <------------------------  Disable local CPU interrupt
        hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
        if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
                __force_wake_get(dev_priv, FORCEWAKE_RENDER); \           <------------- wait_for_atomic() in fw_domain_wait_ack_clear()
        __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; \               <----------------------- Re-enable local CPU interrupt.                      
}                                                                                                                                                                                  
[/code]

So if the condition of wait_for_atomic(condition) is not true and wait_for_atomic() is called with local CPU interrupt is disabled like the code piece above, and the system is an UP system, there will be deadlock a we are waitting a interrupt-driven condition with local CPU interrupt is disabled?

Thanks,
Zhi.
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Zhi Wang
Sent: Wednesday, July 15, 2015 10:08 PM
To: intel-gfx at lists.freedesktop.org
Subject: [Intel-gfx] About the wait_for() macro in intel_drv.h

Hi Gurus:
    Recently we found that the wait_for() marco in intel_drv.h looks will cause dead lock. wait_for() macro will try to wait jiffies change in a condition check loop, but when local cpu interrupt is disabled, it seems the jiffies will never change under a uni-processor environment.
And in forcewake_get/put path, the local cpu irq will always be disabled by a spin_lock_irqsave() in genX_read() and some genX_write(), So if the change of HW register (e.g. forcewake ack) turns slow at this time, there should be a dead lock here I think?

Thanks,
Zhi.
_______________________________________________
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx