From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [konrad.wilk@oracle.com: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.] Date: Tue, 25 Jun 2013 10:13:25 -0400 Message-ID: <20130625141325.GD28113@phenom.dumpdata.com> References: <20130624155351.GA23702@phenom.dumpdata.com> <51C962DE02000078000E0349@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <51C962DE02000078000E0349@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: kwilk_org_ww@oracle.com, xen-devel List-Id: xen-devel@lists.xenproject.org On Tue, Jun 25, 2013 at 08:29:02AM +0100, Jan Beulich wrote: > >>> On 24.06.13 at 17:53, Konrad Rzeszutek Wilk 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 >