On Wed, Jul 27, 2016 at 12:14:54PM +0100, Chris Wilson wrote:
> We only need a very lightweight mechanism here as the locking is only
> used for co-ordinating a bitfield.
>
> v2: Move the cheap unlikely tests into the caller
>
> Signed-off-by: Chris Wilson <[email protected]>
I think the code shuffling in here badly breaks the kerneldoc. Best fix
would be to extract a small header for frontbuffer tracking and pull that
into the kernel doc. Much less preferred is to explicitly pull in the
kerneldoc function-by-function (but that tends to be rather fragile when
someone adds something new).
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 29 ++++++++++++++---
> drivers/gpu/drm/i915/intel_frontbuffer.c | 54
> ++++++++++++++------------------
> 4 files changed, 51 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3a68e604ad10..a24d31e3e014 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1669,7 +1669,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 1fb958dcc749..7db0808f6961 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4447,7 +4447,7 @@ i915_gem_load_init(struct drm_device *dev)
>
> dev_priv->mm.interruptible = true;
>
> - mutex_init(&dev_priv->fb_tracking.lock);
> + spin_lock_init(&dev_priv->fb_tracking.lock);
> }
>
> void i915_gem_load_cleanup(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e74d851868c5..01056ce8d461 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1135,8 +1135,6 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc
> *crtc, bool state);
> uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>
> /* intel_frontbuffer.c */
> -void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> - enum fb_op_origin origin);
> void intel_frontbuffer_flip_prepare(struct drm_device *dev,
> unsigned frontbuffer_bits);
> void intel_frontbuffer_flip_complete(struct drm_device *dev,
> @@ -1147,8 +1145,31 @@ unsigned int intel_fb_align_height(struct drm_device
> *dev,
> unsigned int height,
> uint32_t pixel_format,
> uint64_t fb_format_modifier);
> -void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
> - enum fb_op_origin origin);
> +
> +void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> + enum fb_op_origin origin);
> +static inline void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> + enum fb_op_origin origin)
> +{
> + if (!obj->frontbuffer_bits)
> + return;
> +
> + __intel_fb_obj_invalidate(obj, origin);
> +}
> +
> +void __intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> + bool retire,
> + enum fb_op_origin origin);
> +static inline void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> + bool retire,
> + enum fb_op_origin origin)
> +{
> + if (!obj->frontbuffer_bits)
> + return;
> +
> + __intel_fb_obj_flush(obj, retire, origin);
> +}
> +
> u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> uint64_t fb_modifier, uint32_t pixel_format);
>
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index ac85357010b4..a38ccfe4894a 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -76,24 +76,19 @@
> * until the rendering completes or a flip on this frontbuffer plane is
> * scheduled.
> */
> -void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> - enum fb_op_origin origin)
> +void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> + enum fb_op_origin origin)
> {
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (!obj->frontbuffer_bits)
> - return;
> -
> if (origin == ORIGIN_CS) {
> - 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_psr_invalidate(dev, obj->frontbuffer_bits);
> @@ -120,11 +115,11 @@ static void intel_frontbuffer_flush(struct drm_device
> *dev,
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> /* 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)
> + if (frontbuffer_bits == 0)
> return;
>
> intel_edp_drrs_flush(dev, frontbuffer_bits);
> @@ -142,8 +137,9 @@ static void intel_frontbuffer_flush(struct drm_device
> *dev,
> * completed and frontbuffer caching can be started again. If @retire is true
> * then any delayed flushes will be unblocked.
> */
> -void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> - bool retire, enum fb_op_origin origin)
> +void __intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> + bool retire,
> + enum fb_op_origin origin)
> {
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -151,21 +147,18 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (!obj->frontbuffer_bits)
> - 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, origin);
> + if (frontbuffer_bits)
> + intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
> }
>
> /**
> @@ -185,11 +178,11 @@ void intel_frontbuffer_flip_prepare(struct drm_device
> *dev,
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - 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);
>
> intel_psr_single_frame_update(dev, frontbuffer_bits);
> }
> @@ -209,13 +202,14 @@ void intel_frontbuffer_flip_complete(struct drm_device
> *dev,
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - 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;
> - mutex_unlock(&dev_priv->fb_tracking.lock);
> + spin_unlock(&dev_priv->fb_tracking.lock);
>
> - intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
> + if (frontbuffer_bits)
> + intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
> }
>
> /**
> @@ -234,10 +228,10 @@ void intel_frontbuffer_flip(struct drm_device *dev,
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - mutex_lock(&dev_priv->fb_tracking.lock);
> + spin_lock(&dev_priv->fb_tracking.lock);
> /* 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);
>
> intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
> }
> --
> 2.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
