Re: [Intel-gfx] [PATCH 61/70] drm/i915: Make fb_tracking.lock a spinlock
On Tue, Apr 14, 2015 at 03:52:09PM +0100, Tvrtko Ursulin wrote:
/* Delay flushing when rings are still busy.*/
-mutex_lock(dev_priv-fb_tracking.lock);
+spin_lock(dev_priv-fb_tracking.lock);
frontbuffer_bits = ~dev_priv-fb_tracking.busy_bits;
-mutex_unlock(dev_priv-fb_tracking.lock);
+spin_unlock(dev_priv-fb_tracking.lock);
Looks like you could just remove the lock here in process.
...as in we are always protected by struct_mutex? I think Daniel was
planning for a future where that was guaranteed.
Anyway my v2 patch does:
void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
struct intel_engine_cs *ring,
enum fb_op_origin origin);
static inline void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
struct intel_engine_cs *ring,
enum fb_op_origin origin)
{
if (!obj-frontbuffer_bits || !obj-pin_display)
return;
__intel_fb_obj_invalidate(obj, ring, origin);
}
As the function call overhead itself was annoying me in the execbuffer
profiles.
-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
Re: [Intel-gfx] [PATCH 61/70] drm/i915: Make fb_tracking.lock a spinlock
On 04/14/2015 04:05 PM, Chris Wilson wrote: On Tue, Apr 14, 2015 at 03:52:09PM +0100, Tvrtko Ursulin wrote: /* Delay flushing when rings are still busy.*/ - mutex_lock(dev_priv-fb_tracking.lock); + spin_lock(dev_priv-fb_tracking.lock); frontbuffer_bits = ~dev_priv-fb_tracking.busy_bits; - mutex_unlock(dev_priv-fb_tracking.lock); + spin_unlock(dev_priv-fb_tracking.lock); Looks like you could just remove the lock here in process. ...as in we are always protected by struct_mutex? I think Daniel was planning for a future where that was guaranteed. No, it always looks to be updated with a single write - so I don't see why have a lock for this read? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 61/70] drm/i915: Make fb_tracking.lock a spinlock
On 04/07/2015 05:28 PM, Chris Wilson wrote:
We only need a very lightweight mechanism here as the locking is only
used for co-ordinating a bitfield.
Also double check that the object is still pinned to the display plane
before processing the state change.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_frontbuffer.c | 40 +---
3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 97372869097f..eeffefa10612 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1545,7 +1545,7 @@ struct intel_pipe_crc {
};
struct i915_frontbuffer_tracking {
- struct mutex lock;
+ spinlock_t lock;
/*
* Tracking bits for delayed frontbuffer flushing du to gpu activity or
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e9f2d2b102de..43baac2c1e20 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5260,7 +5260,7 @@ i915_gem_load(struct drm_device *dev)
i915_gem_shrinker_init(dev_priv);
- mutex_init(dev_priv-fb_tracking.lock);
+ spin_lock_init(dev_priv-fb_tracking.lock);
}
void i915_gem_release(struct drm_device *dev, struct drm_file *file)
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index a20cffb78c0f..28ce2ab94189 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -139,16 +139,14 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object
*obj,
WARN_ON(!mutex_is_locked(dev-struct_mutex));
- if (!obj-frontbuffer_bits)
+ if (!obj-frontbuffer_bits || !obj-pin_display)
return;
if (ring) {
- mutex_lock(dev_priv-fb_tracking.lock);
- dev_priv-fb_tracking.busy_bits
- |= obj-frontbuffer_bits;
- dev_priv-fb_tracking.flip_bits
- = ~obj-frontbuffer_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
+ dev_priv-fb_tracking.busy_bits |= obj-frontbuffer_bits;
+ dev_priv-fb_tracking.flip_bits = ~obj-frontbuffer_bits;
+ spin_unlock(dev_priv-fb_tracking.lock);
}
intel_mark_fb_busy(dev, obj-frontbuffer_bits, ring);
@@ -175,9 +173,12 @@ void intel_frontbuffer_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev-dev_private;
/* Delay flushing when rings are still busy.*/
- mutex_lock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
frontbuffer_bits = ~dev_priv-fb_tracking.busy_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_unlock(dev_priv-fb_tracking.lock);
Looks like you could just remove the lock here in process.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 61/70] drm/i915: Make fb_tracking.lock a spinlock
We only need a very lightweight mechanism here as the locking is only
used for co-ordinating a bitfield.
Also double check that the object is still pinned to the display plane
before processing the state change.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_frontbuffer.c | 40 +---
3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 97372869097f..eeffefa10612 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1545,7 +1545,7 @@ struct intel_pipe_crc {
};
struct i915_frontbuffer_tracking {
- struct mutex lock;
+ spinlock_t lock;
/*
* Tracking bits for delayed frontbuffer flushing du to gpu activity or
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e9f2d2b102de..43baac2c1e20 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5260,7 +5260,7 @@ i915_gem_load(struct drm_device *dev)
i915_gem_shrinker_init(dev_priv);
- mutex_init(dev_priv-fb_tracking.lock);
+ spin_lock_init(dev_priv-fb_tracking.lock);
}
void i915_gem_release(struct drm_device *dev, struct drm_file *file)
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index a20cffb78c0f..28ce2ab94189 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -139,16 +139,14 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object
*obj,
WARN_ON(!mutex_is_locked(dev-struct_mutex));
- if (!obj-frontbuffer_bits)
+ if (!obj-frontbuffer_bits || !obj-pin_display)
return;
if (ring) {
- mutex_lock(dev_priv-fb_tracking.lock);
- dev_priv-fb_tracking.busy_bits
- |= obj-frontbuffer_bits;
- dev_priv-fb_tracking.flip_bits
- = ~obj-frontbuffer_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
+ dev_priv-fb_tracking.busy_bits |= obj-frontbuffer_bits;
+ dev_priv-fb_tracking.flip_bits = ~obj-frontbuffer_bits;
+ spin_unlock(dev_priv-fb_tracking.lock);
}
intel_mark_fb_busy(dev, obj-frontbuffer_bits, ring);
@@ -175,9 +173,12 @@ void intel_frontbuffer_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev-dev_private;
/* Delay flushing when rings are still busy.*/
- mutex_lock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
frontbuffer_bits = ~dev_priv-fb_tracking.busy_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_unlock(dev_priv-fb_tracking.lock);
+
+ if (frontbuffer_bits == 0)
+ return;
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
@@ -204,21 +205,21 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
WARN_ON(!mutex_is_locked(dev-struct_mutex));
- if (!obj-frontbuffer_bits)
+ if (!obj-frontbuffer_bits || !obj-pin_display)
return;
frontbuffer_bits = obj-frontbuffer_bits;
if (retire) {
- mutex_lock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
/* Filter out new bits since rendering started. */
frontbuffer_bits = dev_priv-fb_tracking.busy_bits;
-
dev_priv-fb_tracking.busy_bits = ~frontbuffer_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_unlock(dev_priv-fb_tracking.lock);
}
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ if (frontbuffer_bits)
+ intel_frontbuffer_flush(dev, frontbuffer_bits);
}
/**
@@ -238,11 +239,11 @@ void intel_frontbuffer_flip_prepare(struct drm_device
*dev,
{
struct drm_i915_private *dev_priv = dev-dev_private;
- mutex_lock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
dev_priv-fb_tracking.flip_bits |= frontbuffer_bits;
/* Remove stale busy bits due to the old buffer. */
dev_priv-fb_tracking.busy_bits = ~frontbuffer_bits;
- mutex_unlock(dev_priv-fb_tracking.lock);
+ spin_unlock(dev_priv-fb_tracking.lock);
}
/**
@@ -260,11 +261,12 @@ void intel_frontbuffer_flip_complete(struct drm_device
*dev,
{
struct drm_i915_private *dev_priv = dev-dev_private;
- mutex_lock(dev_priv-fb_tracking.lock);
+ spin_lock(dev_priv-fb_tracking.lock);
/* Mask any cancelled flips. */
frontbuffer_bits = dev_priv-fb_tracking.flip_bits;
dev_priv-fb_tracking.flip_bits = ~frontbuffer_bits;
-
