Re: [PATCH 3/3] drm/i915: Use insert_page for pwrite_fast

[Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>]

On Thu, 2015-11-05 at 12:34 +0000, Chris Wilson wrote:
> On Thu, Nov 05, 2015 at 05:15:59PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > 
> > In pwrite_fast, map an object page by page if obj_ggtt_pin fails. First,
> > we try a nonblocking pin for the whole object (since that is fastest if
> > reused), then failing that we try to grab one page in the mappable
> > aperture. It also allows us to handle objects larger than the mappable
> > aperture (e.g. if we need to pwrite with vGPU restricting the aperture
> > to a measely 8MiB or something like that).
> > 
> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 92 ++++++++++++++++++++++++++++++-----------
> >  1 file changed, 69 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index bf5ef7a..9132240 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -766,14 +766,26 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> >  			 struct drm_file *file)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_mm_node node;
> >  	ssize_t remain;
> >  	loff_t offset, page_base;
> >  	char __user *user_data;
> > -	int page_offset, page_length, ret;
> > +	int page_offset, page_length, ret, i;
> > +	bool pinned = true;
> >  
> >  	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK);
> > -	if (ret)
> > -		goto out;
> > +	if (ret) {
> > +		pinned = false;
> > +		memset(&node, 0, sizeof(node));
> > +		ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> > +							  &node, 4096, 0,
> > +							  I915_CACHE_NONE, 0,
> > +							  dev_priv->gtt.mappable_end,
> > +							  DRM_MM_SEARCH_DEFAULT,
> > +							  DRM_MM_CREATE_DEFAULT);
> > +		if (ret)
> > +			goto out;
> 
> Prefer to refer to my original patch as to why this wrong.
If you are concerned about pages for the object not getting allocated,
then soon after node insertion we call
i915_gem_object_set_to_gtt_domain() which takes care of page allocations
for the object.
If there is any other concern, please let me know.

Thanks,
Ankit

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