From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753736AbdKKMr3 (ORCPT ); Sat, 11 Nov 2017 07:47:29 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:46988 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752484AbdKKMr1 (ORCPT ); Sat, 11 Nov 2017 07:47:27 -0500 From: Laurent Pinchart To: Liviu Dudau Cc: Daniel Vetter , Jani Nikula , Sean Paul , David Airlie , Brian Starkey , Mali DP Maintainers , DRI-devel , LKML , intel-gfx@lists.freedesktop.org, Liviu Dudau Subject: Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1 Date: Sat, 11 Nov 2017 14:47:35 +0200 Message-ID: <26071297.cMn4QuKrlS@avalon> Organization: Ideas on Board Oy In-Reply-To: <20171110133310.1225-1-Liviu.Dudau@arm.com> References: <2213365.Z1vsgl9bDt@avalon> <20171110133310.1225-1-Liviu.Dudau@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Liviu, Thank you for the patch. On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote: > drm_gem_cma_prime_import_sg_table() will fail if the number of entries > in the sg_table > 1. However, you can have a device that uses an IOMMU > engine and can map a discontiguous buffer with multiple entries that > have consecutive sg_dma_addresses, effectively making it contiguous. > Allow for that scenario by testing the entries in the sg_table for > contiguous coverage. > > Reviewed-by: Laurent Pinchart > Signed-off-by: Liviu Dudau > --- > > Laurent, > > Thanks for the review! I would like to ask for one more favour: if you > are OK with this version, can you pull this patch through the drm-misc tree? I could, but I'd first need to set dim up, and I'm currently abroad with a bad internet connection and a big deadline for the middle of next week (I know, lots of excuses), so it's not very convenient for me at this time. > drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++-- > include/drm/drm_gem_cma_helper.h | 4 +++- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c > b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d > 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device > *dev, { > struct drm_gem_cma_object *cma_obj; > > - if (sgt->nents != 1) > - return ERR_PTR(-EINVAL); > + if (sgt->nents != 1) { > + /* check if the entries in the sg_table are contiguous */ > + dma_addr_t next_addr = sg_dma_address(sgt->sgl); > + struct scatterlist *s; > + unsigned int i; > + > + for_each_sg(sgt->sgl, s, sgt->nents, i) { > + /* > + * sg_dma_address(s) is only valid for entries > + * that have sg_dma_len(s) != 0 > + */ > + if (!sg_dma_len(s)) > + continue; > + > + if (sg_dma_address(s) != next_addr) > + return ERR_PTR(-EINVAL); > + > + next_addr = sg_dma_address(s) + sg_dma_len(s); > + } > + } > > /* Create a CMA GEM buffer. */ > cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size); > diff --git a/include/drm/drm_gem_cma_helper.h > b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5 > 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -8,7 +8,9 @@ > * struct drm_gem_cma_object - GEM object backed by CMA memory allocations > * @base: base GEM object > * @paddr: physical address of the backing memory > - * @sgt: scatter/gather table for imported PRIME buffers > + * @sgt: scatter/gather table for imported PRIME buffers. The table can > have + * more than one entry but they are guaranteed to have > contiguous + * DMA addresses. > * @vaddr: kernel virtual address of the backing memory > */ > struct drm_gem_cma_object { -- Regards, Laurent Pinchart