Re: [Intel-gfx] [PATCH v3] drm/i915: use hrtimer in wait for vblank

2014-03-25 Thread Murthy, Arun R

On Tuesday 25 March 2014 01:00 PM, Chris Wilson wrote:

On Tue, Mar 25, 2014 at 11:29:02AM +0530, Arun R Murthy wrote:

In wait for vblank use usleep_range, which will use hrtimers instead of
msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
Using usleep_range uses hrtimers and hence are precise, worst case will
trigger an interrupt at the higher/max timeout.

Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have
noticed the time consumed by wait for vblank is ~4ms to ~17ms.

Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae
Signed-off-by: Arun R Murthy 

No. I feel strongly that we do not want more wait_for_X() with strange
semantics.
http://sweng.the-davies.net/Home/rustys-api-design-manifesto


Will revert this additional wait_for_X.
Will update the existing _wait_for as per the kernel documentation for 
timers.


Thanks and Regards,
Arun R Murthy
--
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3] drm/i915: use hrtimer in wait for vblank

2014-03-25 Thread Chris Wilson
On Tue, Mar 25, 2014 at 11:29:02AM +0530, Arun R Murthy wrote:
> In wait for vblank use usleep_range, which will use hrtimers instead of
> msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
> Using usleep_range uses hrtimers and hence are precise, worst case will
> trigger an interrupt at the higher/max timeout.
> 
> Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have
> noticed the time consumed by wait for vblank is ~4ms to ~17ms.
> 
> Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae
> Signed-off-by: Arun R Murthy 

No. I feel strongly that we do not want more wait_for_X() with strange
semantics.
http://sweng.the-davies.net/Home/rustys-api-design-manifesto
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3] drm/i915: use hrtimer in wait for vblank

2014-03-24 Thread Arun R Murthy
In wait for vblank use usleep_range, which will use hrtimers instead of
msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
Using usleep_range uses hrtimers and hence are precise, worst case will
trigger an interrupt at the higher/max timeout.

Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have
noticed the time consumed by wait for vblank is ~4ms to ~17ms.

Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae
Signed-off-by: Arun R Murthy 
---
 drivers/gpu/drm/i915/intel_display.c |2 +-
 drivers/gpu/drm/i915/intel_dp.c  |4 ++--
 drivers/gpu/drm/i915/intel_drv.h |   19 ---
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..9de2678 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -761,7 +761,7 @@ static void g4x_wait_for_vblank(struct drm_device *dev, int 
pipe)
 
frame = I915_READ(frame_reg);
 
-   if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
+   if (wait_for_us(I915_READ_NOTRACE(frame_reg) != frame, 50, 1000))
DRM_DEBUG_KMS("vblank wait timed out\n");
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1ef3d4..14927e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1074,7 +1074,7 @@ static void wait_panel_status(struct intel_dp *intel_dp,
I915_READ(pp_stat_reg),
I915_READ(pp_ctrl_reg));
 
-   if (_wait_for((I915_READ(pp_stat_reg) & mask) == value, 5000, 10)) {
+   if (wait_for_ms((I915_READ(pp_stat_reg) & mask) == value, 5000, 10)) {
DRM_ERROR("Panel status timeout: status %08x control %08x\n",
I915_READ(pp_stat_reg),
I915_READ(pp_ctrl_reg));
@@ -1808,7 +1808,7 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
   I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE);
 
/* Wait till PSR is idle */
-   if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
+   if (wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
   EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
DRM_ERROR("Timed out waiting for PSR Idle State\n");
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..bbda97e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -42,8 +42,8 @@
  * having timed out, since the timeout could be due to preemption or similar 
and
  * we've never had a chance to check the condition before the timeout.
  */
-#define _wait_for(COND, MS, W) ({ \
-   unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1;   \
+#define _wait_for(COND, TIMEOUT, MS, US) ({ \
+   unsigned long timeout__ = jiffies + msecs_to_jiffies(TIMEOUT) + 1;\
int ret__ = 0;  \
while (!(COND)) {   \
if (time_after(jiffies, timeout__)) {   \
@@ -51,8 +51,11 @@
ret__ = -ETIMEDOUT; \
break;  \
}   \
-   if (W && drm_can_sleep())  {\
-   msleep(W);  \
+   if ((MS | US) && drm_can_sleep())  {\
+   if (MS) \
+   msleep(MS); \
+   else\
+   usleep_range(US, US * 2);   \
} else {\
cpu_relax();\
}   \
@@ -60,10 +63,12 @@
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(COND, TIMEOUT) _wait_for(COND, TIMEOUT, 1, 0)
+#define wait_for_ms(COND, TIMEOUT, MS) _wait_for(COND, TIMEOUT, MS, 0)
+#define wait_for_us(COND, TIMEOUT, US) _wait_for(COND, TIMEOUT, 0, US)
+#define wait_for_atomic(COND, TIMEOUT) _wait_for(COND, TIMEOUT, 0, 0)
 #define wait_for_atomic_us(COND, US) _wait_for((COND), \
-  DIV_ROUND_UP((US), 1000), 0)
+  DIV_ROUND_UP((US), 1000), 0, 0)
 
 #define KHz(x) (1000 * (x))
 #define MHz(x) KHz(1000 * (x))
-- 
1.7.9.5