Re: [RFC] drm/i915: Scratch page optimization for blanking buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2014-05-05 at 16:07 +0200, Daniel Vetter wrote:
> On Mon, May 05, 2014 at 05:13:18PM +0530, akash.goel@xxxxxxxxx wrote:
> > From: Akash Goel <akash.goel@xxxxxxxxx>
> > 
> > There is a use case, when user space (display compositor) tries
> > to directly flip a fb (without any prior rendering) on primary
> > plane. So the backing pages of the object are allocated at page
> > flip time only, which takes time. Since, this buffer is supposed to
> > serve as a blanking buffer (black colored), we can setup all the GTT entries
> > of that blanking buffer with scratch page (which is already zeroed out).
> > This saves the time in allocation of real backing physical space for the
> > blanking buffer and flushing of CPU cache.
> > 
> > Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
> 
> That sounds very much like a special case of the fallocate ioctl where we
> simply allocat 0 real backing storage pages.
> -Daniel
> 

Hi Daniel,

Yes no real backing storage is needed, but still the object should be
allowed to get mapped into the GGTT, using the scratch page (already
zeroed out).

Is there a patch for a new drm/i915 ioctl similar to fallocate, which
could be used here?

Best regards
Akash

> > ---
> >  drivers/gpu/drm/i915/i915_gem.c      | 18 ++++++++-
> >  drivers/gpu/drm/i915/i915_gem_gtt.c  |  8 ++++
> >  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 97 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index b19ccb8..7c3963c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1781,6 +1781,17 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> >  	 * lists early. */
> >  	list_del(&obj->global_list);
> >  
> > +	/*
> > +	 * If so far the object was backed up by a scratch page, then remove
> > +	 * that association & make it reusable as a normal Gem object
> > +	 */
> > +	if ((unsigned long)obj->pages == (unsigned long)(obj)) {
> > +		obj->pages = NULL;
> > +		obj->base.read_domains = obj->base.write_domain =
> > +						I915_GEM_DOMAIN_CPU;
> > +		return 0;
> > +	}
> > +
> >  	ops->put_pages(obj);
> >  	obj->pages = NULL;
> >  
> > @@ -3772,7 +3783,12 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> >  	if (ret)
> >  		goto err_unpin_display;
> >  
> > -	i915_gem_object_flush_cpu_write_domain(obj, true);
> > +	/*
> > +	 * Check if object is backed up by a scratch page, in that case CPU
> > +	 * cache flush is not required, thus skip it.
> > +	 */
> > +	if ((unsigned long)(obj->pages) != (unsigned long)obj)
> > +		i915_gem_object_flush_cpu_write_domain(obj, true);
> >  
> >  	old_write_domain = obj->base.write_domain;
> >  	old_read_domains = obj->base.read_domains;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f6354e0..fb3193a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1566,6 +1566,14 @@ static void ggtt_bind_vma(struct i915_vma *vma,
> >  	if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) {
> >  		if (!obj->has_global_gtt_mapping ||
> >  		    (cache_level != obj->cache_level)) {
> > +			if ((unsigned long)(obj->pages) == (unsigned long)obj) {
> > +				/* Leave the scratch page mapped into the GTT
> > +				 * entries of the object, as it is actually
> > +				 * supposed to be backed up by scratch page
> > +				 * only */
> > +				obj->has_global_gtt_mapping = 1;
> > +				return;
> > +			}
> >  			vma->vm->insert_entries(vma->vm, obj->pages,
> >  						vma->node.start,
> >  						cache_level);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 59303213..dff85e4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -40,6 +40,7 @@
> >  #include <drm/drm_dp_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <linux/dma_remapping.h>
> > +#include <linux/shmem_fs.h>
> >  
> >  static void intel_increase_pllclock(struct drm_crtc *crtc);
> >  static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
> > @@ -8469,6 +8470,75 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
> >  	kfree(intel_crtc);
> >  }
> >  
> > +static inline void
> > +intel_use_srcatch_page_for_fb(struct drm_i915_gem_object *obj)
> > +{
> > +	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > +	int ret;
> > +
> > +	/* A fb being flipped without having any allocated backing physical
> > +	 * space (without any prior rendering), is most probably going to be
> > +	 * used as a blanking buffer (black colored). So instead of allocating
> > +	 * the real backing physical space for this buffer, we can try to
> > +	 * currently back this object by a scratch page, which is already
> > +	 * allocated. So we check if no shmem data pages have been allocated to
> > +	 * the fb we can back it by a scratch page and thus save time by avoid-
> > +	 * ing allocation of backing physicl space & subsequent CPU cache flush
> > +	 */
> > +	if (obj->base.filp) {
> > +		struct inode *inode = file_inode(obj->base.filp);
> > +		struct shmem_inode_info *info = SHMEM_I(inode);
> > +		spin_lock(&info->lock);
> > +		ret = info->alloced;
> > +		spin_unlock(&info->lock);
> > +		if ((ret == 0) && (obj->pages == NULL)) {
> > +			/*
> > +			 * Set the 'pages' field with the object pointer
> > +			 * itself, this will avoid the need of a new field in
> > +			 * obj structure to identify the object backed up by a
> > +			 * scratch page and will also avoid the call to
> > +			 * 'get_pages', thus also saving on the time required
> > +			 * for allocation of 'scatterlist' structure.
> > +			 */
> > +			obj->pages = (struct sg_table *)(obj);
> > +
> > +			/*
> > +			 * To avoid calls to gtt prepare & finish, as those
> > +			 * will dereference the 'pages' field
> > +			 */
> > +			obj->has_dma_mapping = 1;
> > +			list_add_tail(&obj->global_list,
> > +					&dev_priv->mm.unbound_list);
> > +
> > +			trace_printk("Using Scratch page for obj %p\n", obj);
> > +		}
> > +	}
> > +}
> > +
> > +static inline void
> > +intel_drop_srcatch_page_for_fb(struct drm_i915_gem_object *obj)
> > +{
> > +	int ret;
> > +	/*
> > +	 * Unmap the object backed up by scratch page, as it is no
> > +	 * longer being scanned out and thus it can be now allowed
> > +	 * to be used as a normal object.
> > +	 * Assumption: The User space will ensure that only when the
> > +	 * object is no longer being scanned out, it will be reused
> > +	 * for rendering. This is a valid assumption as there is no
> > +	 * such handling in driver for other regular fb objects also.
> > +	 */
> > +	if ((unsigned long)obj->pages ==
> > +				(unsigned long)obj) {
> > +		ret = i915_gem_object_ggtt_unbind(obj);
> > +		/* EBUSY is ok: this means that pin count is still not zero */
> > +		if (ret && ret != -EBUSY)
> > +			DRM_ERROR("unbind error %d\n", ret);
> > +		i915_gem_object_put_pages(obj);
> > +		obj->has_dma_mapping = 0;
> > +	}
> > +}
> > +
> >  static void intel_unpin_work_fn(struct work_struct *__work)
> >  {
> >  	struct intel_unpin_work *work =
> > @@ -8477,6 +8547,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
> >  
> >  	mutex_lock(&dev->struct_mutex);
> >  	intel_unpin_fb_obj(work->old_fb_obj);
> > +	intel_drop_srcatch_page_for_fb(work->old_fb_obj);
> >  	drm_gem_object_unreference(&work->pending_flip_obj->base);
> >  	drm_gem_object_unreference(&work->old_fb_obj->base);
> >  
> > @@ -8770,6 +8841,7 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> >  	if (IS_VALLEYVIEW(dev) || ring == NULL || ring->id != RCS)
> >  		ring = &dev_priv->ring[BCS];
> >  
> > +	intel_use_srcatch_page_for_fb(obj);
> >  	ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> >  	if (ret)
> >  		goto err;
> > -- 
> > 1.9.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux