Re: [konrad.wilk@oracle.com: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.]

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Jun 25, 2013 at 08:29:02AM +0100, Jan Beulich wrote:
> >>> On 24.06.13 at 17:53, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> >  			gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> >  			gfp &= ~(__GFP_IO | __GFP_WAIT);
> >  		}
> > -
> > +#ifdef CONFIG_SWIOTLB
> > +		if (swiotlb_nr_tbl()) {
> > +			st->nents++;
> > +			sg_set_page(sg, page, PAGE_SIZE, 0);
> > +			sg = sg_next(sg);
> > +			continue;
> > +		}
> > +#endif
> >  		if (!i || page_to_pfn(page) != last_pfn + 1) {
> >  			if (i)
> >  				sg = sg_next(sg);
> > @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> >  		}
> >  		last_pfn = page_to_pfn(page);
> >  	}
> > -
> > -	sg_mark_end(sg);
> > +#ifdef CONFIG_SWIOTLB
> > +	if (!swiotlb_nr_tbl())
> > +#endif
> > +		sg_mark_end(sg);
> >  	obj->pages = st;
> >  
> >  	if (i915_gem_object_needs_bit17_swizzle(obj))
> 
> Out of curiosity - while I can see the point of the first hunk, why do
> you need to also suppress the setting of the list terminator?

It was crashing for me as sg was NULL for one-item sg structures. I didn't
dig deep in it, but the combination of 'sg = sg_nex(sg)' and then
sg_mark_end(sg) ended up with sg being NULL.

I could have made the 'sg = sg_next(sg)' be wrapped with a 'if (i < page_count)'
but figured it would be easier to reproduce the original code as faithfully
as possible.

> 
> Jan
>