All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
Date: Mon, 8 Dec 2014 17:53:56 +0100	[thread overview]
Message-ID: <20141208165356.GF27182@phenom.ffwll.local> (raw)
In-Reply-To: <1418054960-1403-7-git-send-email-przanoni@gmail.com>

On Mon, Dec 08, 2014 at 02:09:15PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We want to port FBC to the frontbuffer tracking infrastructure, but
> for that we need to know what caused the object invalidation/flush so
> we can react accordingly: CPU mmaps need manual, GTT mmaps and
> flips don't need handling and ring rendering needs nukes.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |  7 +++++++
>  drivers/gpu/drm/i915/i915_gem.c            | 10 +++++-----
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h           | 11 +++++++----
>  drivers/gpu/drm/i915/intel_frontbuffer.c   | 15 ++++++++++-----
>  5 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9d694f1..ea3cc81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -685,6 +685,13 @@ struct intel_context {
>  	struct list_head link;
>  };
>  
> +enum fb_op_origin {
> +	ORIGIN_GTT,
> +	ORIGIN_CPU,
> +	ORIGIN_RENDER,

Maybe GPU instead of RENDER since it includes the blitter? Render
typically only means the render ring in gem code. And maybe add an I915_
prefix or so at least to the enum.

Anyway that's it from me with bikesheds, lgtm overall.
-Daniel

> +	ORIGIN_FLIP,
> +};
> +
>  struct i915_fbc {
>  	unsigned long size;
>  	unsigned threshold;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index de241eb..7ef12e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2321,7 +2321,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
>  			list_move_tail(&vma->mm_list, &vm->inactive_list);
>  	}
>  
> -	intel_fb_obj_flush(obj, true);
> +	intel_fb_obj_flush(obj, true, ORIGIN_RENDER);
>  
>  	list_del_init(&obj->ring_list);
>  
> @@ -3665,7 +3665,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
>  	old_write_domain = obj->base.write_domain;
>  	obj->base.write_domain = 0;
>  
> -	intel_fb_obj_flush(obj, false);
> +	intel_fb_obj_flush(obj, false, ORIGIN_GTT);
>  
>  	trace_i915_gem_object_change_domain(obj,
>  					    obj->base.read_domains,
> @@ -3688,7 +3688,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
>  	old_write_domain = obj->base.write_domain;
>  	obj->base.write_domain = 0;
>  
> -	intel_fb_obj_flush(obj, false);
> +	intel_fb_obj_flush(obj, false, ORIGIN_CPU);
>  
>  	trace_i915_gem_object_change_domain(obj,
>  					    obj->base.read_domains,
> @@ -3745,7 +3745,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  	}
>  
>  	if (write)
> -		intel_fb_obj_invalidate(obj, NULL);
> +		intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
>  
>  	trace_i915_gem_object_change_domain(obj,
>  					    old_read_domains,
> @@ -4072,7 +4072,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
>  	}
>  
>  	if (write)
> -		intel_fb_obj_invalidate(obj, NULL);
> +		intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
>  
>  	trace_i915_gem_object_change_domain(obj,
>  					    old_read_domains,
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 0c25f62..af290e6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -965,7 +965,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>  			obj->dirty = 1;
>  			i915_gem_request_assign(&obj->last_write_req, req);
>  
> -			intel_fb_obj_invalidate(obj, ring);
> +			intel_fb_obj_invalidate(obj, ring, ORIGIN_RENDER);
>  
>  			/* update for the implicit flush after a batch */
>  			obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 588b618..633fb9a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>  
>  /* intel_frontbuffer.c */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> -			     struct intel_engine_cs *ring);
> +			     struct intel_engine_cs *ring,
> +			     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,
>  				     unsigned frontbuffer_bits);
>  void intel_frontbuffer_flush(struct drm_device *dev,
> -			     unsigned frontbuffer_bits);
> +			     unsigned frontbuffer_bits,
> +			     enum fb_op_origin origin);
>  /**
>   * intel_frontbuffer_flip - synchronous frontbuffer flip
>   * @dev: DRM device
> @@ -863,10 +865,11 @@ static inline
>  void intel_frontbuffer_flip(struct drm_device *dev,
>  			    unsigned frontbuffer_bits)
>  {
> -	intel_frontbuffer_flush(dev, frontbuffer_bits);
> +	intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
>  }
>  
> -void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
> +void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
> +			enum fb_op_origin origin);
>  
>  
>  /* intel_audio.c */
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 79f6d72..7bdac69 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -127,6 +127,7 @@ static void intel_mark_fb_busy(struct drm_device *dev,
>   * intel_fb_obj_invalidate - invalidate frontbuffer object
>   * @obj: GEM object to invalidate
>   * @ring: set for asynchronous rendering
> + * @origin: which operation caused the invalidation
>   *
>   * This function gets called every time rendering on the given object starts and
>   * frontbuffer caching (fbc, low refresh rate for DRRS, panel self refresh) must
> @@ -135,7 +136,8 @@ static void intel_mark_fb_busy(struct drm_device *dev,
>   * scheduled.
>   */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> -			     struct intel_engine_cs *ring)
> +			     struct intel_engine_cs *ring,
> +			     enum fb_op_origin origin)
>  {
>  	struct drm_device *dev = obj->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -163,6 +165,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>   * intel_frontbuffer_flush - flush frontbuffer
>   * @dev: DRM device
>   * @frontbuffer_bits: frontbuffer plane tracking bits
> + * @origin: which operation caused the invalidation
>   *
>   * This function gets called every time rendering on the given planes has
>   * completed and frontbuffer caching can be started again. Flushes will get
> @@ -171,7 +174,8 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>   * Can be called without any locks held.
>   */
>  void intel_frontbuffer_flush(struct drm_device *dev,
> -			     unsigned frontbuffer_bits)
> +			     unsigned frontbuffer_bits,
> +			     enum fb_op_origin origin)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> @@ -199,13 +203,14 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>   * intel_fb_obj_flush - flush frontbuffer object
>   * @obj: GEM object to flush
>   * @retire: set when retiring asynchronous rendering
> + * @origin: which operation caused the invalidation
>   *
>   * This function gets called every time rendering on the given object has
>   * 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)
> +			bool retire, enum fb_op_origin origin)
>  {
>  	struct drm_device *dev = obj->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -227,7 +232,7 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
>  		mutex_unlock(&dev_priv->fb_tracking.lock);
>  	}
>  
> -	intel_frontbuffer_flush(dev, frontbuffer_bits);
> +	intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
>  }
>  
>  /**
> @@ -275,5 +280,5 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev,
>  	dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
>  	mutex_unlock(&dev_priv->fb_tracking.lock);
>  
> -	intel_frontbuffer_flush(dev, frontbuffer_bits);
> +	intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
>  }
> -- 
> 2.1.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-12-08 16:53 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
2014-12-08 16:09 ` [PATCH 01/11] drm/i915: Move FBC stuff to intel_fbc.c Paulo Zanoni
2014-12-08 16:09 ` [PATCH 02/11] drm/i915: Introduce FBC DocBook Paulo Zanoni
2014-12-08 16:49   ` Daniel Vetter
2014-12-08 14:46     ` [PATCH] " Rodrigo Vivi
2014-12-08 21:48     ` [PATCH 02/11] " Rodrigo Vivi
2014-12-09  9:52       ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled Paulo Zanoni
2014-12-13  0:53   ` Rodrigo Vivi
2014-12-15  8:35     ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED Paulo Zanoni
2014-12-13  0:55   ` Rodrigo Vivi
2014-12-15  8:37     ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc Paulo Zanoni
2014-12-13  0:58   ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking Paulo Zanoni
2014-12-08 16:53   ` Daniel Vetter [this message]
2014-12-13  1:05     ` Rodrigo Vivi
2014-12-15  8:43       ` Daniel Vetter
2014-12-15 13:58         ` Paulo Zanoni
2014-12-08 16:09 ` [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites Paulo Zanoni
2014-12-08 16:55   ` Daniel Vetter
2014-12-13  1:10     ` Rodrigo Vivi
2014-12-15  8:39       ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC Paulo Zanoni
2014-12-13  1:12   ` [PATCH 6/9] drm/i915: add frontbuffer " Rodrigo Vivi
2014-12-13  1:16   ` [PATCH 08/11] drm/i915: add fronbuffer " Rodrigo Vivi
2014-12-15 14:00     ` [PATCH 8/11] drm/i915: add frontbuffer " Paulo Zanoni
2014-12-08 16:09 ` [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc() Paulo Zanoni
2014-12-13  1:20   ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A Paulo Zanoni
2014-12-13  1:23   ` Rodrigo Vivi
2014-12-15  8:41     ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes Paulo Zanoni
2014-12-09 16:08   ` shuang.he
2014-12-13  1:25   ` Rodrigo Vivi
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
2014-12-08 16:12   ` [PATCH igt 2/4] tests/kms_fb_crc: call gem_sync() instead of gem_bo_busy() Paulo Zanoni
2014-12-08 16:12   ` [PATCH igt 3/4] tests/kms_fbc_crc: add wait_for_fbc_enabled() Paulo Zanoni
2014-12-08 16:12   ` [PATCH igt 4/4] tests/kms_fbc_crc: also gem_sync() on exec_nop() Paulo Zanoni
2014-12-08 16:40   ` [PATCH igt 1/4] lib: add igt_wait() Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141208165356.GF27182@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    --cc=przanoni@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.