[PATCH v2 1/5] drm/gem: replace misleading comment

[PATCH v2 1/5] drm/gem: replace misleading comment

[David Herrmann]

shmem supports page-relocations during swapin since quite some time. It
was implemented in:

    commit bde05d1ccd512696b09db9dd2e5f33ad19152605
    Author: Hugh Dickins <hughd@google.com>
    Date:   Tue May 29 15:06:38 2012 -0700

        shmem: replace page if mapping excludes its zone

The gem-comment about wrongly placed DMA32 pages is no longer valid.
Replace it with a proper comment but keep the BUG_ON() to verify correct
shmem behavior.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_gem.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 9909bef..f7d7119 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -474,21 +474,10 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask)
 			goto fail;
 		pages[i] = p;
 
-		/* There is a hypothetical issue w/ drivers that require
-		 * buffer memory in the low 4GB.. if the pages are un-
-		 * pinned, and swapped out, they can end up swapped back
-		 * in above 4GB.  If pages are already in memory, then
-		 * shmem_read_mapping_page_gfp will ignore the gfpmask,
-		 * even if the already in-memory page disobeys the mask.
-		 *
-		 * It is only a theoretical issue today, because none of
-		 * the devices with this limitation can be populated with
-		 * enough memory to trigger the issue.  But this BUG_ON()
-		 * is here as a reminder in case the problem with
-		 * shmem_read_mapping_page_gfp() isn't solved by the time
-		 * it does become a real issue.
-		 *
-		 * See this thread: http://lkml.org/lkml/2011/7/11/238
+		/* Make sure shmem keeps __GFP_DMA32 allocated pages in the
+		 * correct region during swapin. Note that this requires
+		 * __GFP_DMA32 to be set in mapping_gfp_mask(inode->i_mapping)
+		 * so shmem can relocate pages during swapin if required.
 		 */
 		BUG_ON((gfpmask & __GFP_DMA32) &&
 				(page_to_pfn(p) >= 0x00100000UL));
-- 
1.9.3

[PATCH 2/5] drm/armada: use shmem helpers if possible

[David Herrmann]

shmem_read_mapping_page() uses mapping_gfp_mask(mapping) as default gfp
mask. No reason to use shmem_read_mapping_page_gfp() directly if we want
the default behavior.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/armada/armada_gem.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index 887816f..bb9b642 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -433,7 +433,6 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
 
 	if (dobj->obj.filp) {
 		struct address_space *mapping;
-		gfp_t gfp;
 		int count;
 
 		count = dobj->obj.size / PAGE_SIZE;
@@ -441,12 +440,11 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
 			goto free_sgt;
 
 		mapping = file_inode(dobj->obj.filp)->i_mapping;
-		gfp = mapping_gfp_mask(mapping);
 
 		for_each_sg(sgt->sgl, sg, count, i) {
 			struct page *page;
 
-			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
+			page = shmem_read_mapping_page(mapping, i);
 			if (IS_ERR(page)) {
 				num = i;
 				goto release;
-- 
1.9.3

[PATCH 3/5] drm/i915: use shmem helpers if possible

[David Herrmann]

Instead of shuffling gfp-masks all the time, use the
shmem_read_mapping_page() helper. Note that __GFP_IO and __GFP_WAIT are
set in mapping_gfp_mask() for i915, so the behavior is still the same.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e1fa919..1a72aad 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1925,16 +1925,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 			 * our own buffer, now let the real VM do its job and
 			 * go down in flames if truly OOM.
 			 */
-			gfp &= ~(__GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD);
-			gfp |= __GFP_IO | __GFP_WAIT;
-
 			i915_gem_shrink_all(dev_priv);
-			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
+			page = shmem_read_mapping_page(mapping, i);
 			if (IS_ERR(page))
 				goto err_pages;
-
-			gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
-			gfp &= ~(__GFP_IO | __GFP_WAIT);
 		}
 #ifdef CONFIG_SWIOTLB
 		if (swiotlb_nr_tbl()) {
-- 
1.9.3

[PATCH 4/5] drm/omap: use __GFP_DMA32 for shmem-backed gem

[David Herrmann]

OMAP requires bo-pages to be in the DMA32 zone. Explicitly request this by
setting __GFP_DMA32 as mapping-gfp-mask during shmem initialization. This
drops HIGHMEM from the gfp-mask and uses DMA32 instead. shmem-core takes
care to relocate pages during swap-in in case they have been loaded into
the wrong zone.

It is _not_ possible to pass __GFP_DMA32 to shmem_read_mapping_page_gfp()
as the page might have already been swapped-in at that time. The zone-mask
must be set during initialization and be kept constant for now.

Remove the now superfluous TODO in omap_gem.c.

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 95dbce2..1331fd5 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -233,10 +233,6 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 
 	WARN_ON(omap_obj->pages);
 
-	/* TODO: __GFP_DMA32 .. but somehow GFP_HIGHMEM is coming from the
-	 * mapping_gfp_mask(mapping) which conflicts w/ GFP_DMA32.. probably
-	 * we actually want CMA memory for it all anyways..
-	 */
 	pages = drm_gem_get_pages(obj, GFP_KERNEL);
 	if (IS_ERR(pages)) {
 		dev_err(obj->dev->dev, "could not get pages: %ld\n", PTR_ERR(pages));
@@ -1347,6 +1343,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_gem_object *omap_obj;
 	struct drm_gem_object *obj = NULL;
+	struct address_space *mapping;
 	size_t size;
 	int ret;
 
@@ -1404,14 +1401,16 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
 		omap_obj->height = gsize.tiled.height;
 	}
 
-	ret = 0;
-	if (flags & (OMAP_BO_DMA|OMAP_BO_EXT_MEM))
+	if (flags & (OMAP_BO_DMA|OMAP_BO_EXT_MEM)) {
 		drm_gem_private_object_init(dev, obj, size);
-	else
+	} else {
 		ret = drm_gem_object_init(dev, obj, size);
+		if (ret)
+			goto fail;
 
-	if (ret)
-		goto fail;
+		mapping = file_inode(obj->filp)->i_mapping;
+		mapping_set_gfp_mask(mapping, GFP_USER | __GFP_DMA32);
+	}
 
 	return obj;
 
-- 
1.9.3

[PATCH 5/5] drm/gem: remove misleading gfp parameter to get_pages()

[David Herrmann]

drm_gem_get_pages() currently allows passing a 'gfp' parameter that is
passed to shmem combined with mapping_gfp_mask(). Given that the default
mapping_gfp_mask() is GFP_HIGHUSER, it is _very_ unlikely that anyone will
ever make use of that parameter. In fact, all drivers currently pass
redundant flags or 0.

This patch removes the 'gfp' parameter. The only reason to keep it is to
remove flags like __GFP_WAIT. But in its current form, it can only be used
to add flags. So to remove __GFP_WAIT, you'd have to drop it from the
mapping_gfp_mask, which again is stupid as this mask is used by shmem-core
for other allocations, too.

If any driver ever requires that parameter, we can introduce a new helper
that takes the raw 'gfp' parameter. The caller'd be responsible to combine
it with mapping_gfp_mask() in a suitable way. The current
drm_gem_get_pages() helper would then simply use mapping_gfp_mask() and
call the new helper. This is what shmem_read_mapping_pages{_gfp,} does
right now.

Moreover, the gfp-zone flag-usage is not obvious: If you pass a modified
zone, shmem core will WARN() or even BUG(). In other words, the following
must be true for 'gfp' passed to shmem_read_mapping_pages_gfp():
    gfp_zone(mapping_gfp_mask(mapping)) == gfp_zone(gfp)
Add a comment to drm_gem_read_pages() explaining that constraint.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_gem.c          | 29 ++++++++++++++++++++---------
 drivers/gpu/drm/gma500/gtt.c       |  2 +-
 drivers/gpu/drm/msm/msm_gem.c      |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c |  2 +-
 drivers/gpu/drm/udl/udl_gem.c      |  8 ++++----
 include/drm/drmP.h                 |  2 +-
 6 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index f7d7119..6adee4c 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -441,18 +441,31 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset);
  * drm_gem_get_pages - helper to allocate backing pages for a GEM object
  * from shmem
  * @obj: obj in question
- * @gfpmask: gfp mask of requested pages
+ *
+ * This reads the page-array of the shmem-backing storage of the given gem
+ * object. An array of pages is returned. If a page is not allocated or
+ * swapped-out, this will allocate/swap-in the required pages. Note that the
+ * whole object is covered by the page-array and pinned in memory.
+ *
+ * Use drm_gem_put_pages() to release the array and unpin all pages.
+ *
+ * This uses the GFP-mask set on the shmem-mapping (see mapping_set_gfp_mask()).
+ * If you require other GFP-masks, you have to do those allocations yourself.
+ *
+ * Note that you are not allowed to change gfp-zones during runtime. That is,
+ * shmem_read_mapping_page_gfp() must be called with the same gfp_zone(gfp) as
+ * set during initialization. If you have special zone constraints, set them
+ * after drm_gem_init_object() via mapping_set_gfp_mask(). shmem-core takes care
+ * to keep pages in the required zone during swap-in.
  */
-struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask)
+struct page **drm_gem_get_pages(struct drm_gem_object *obj)
 {
-	struct inode *inode;
 	struct address_space *mapping;
 	struct page *p, **pages;
 	int i, npages;
 
 	/* This is the shared memory object that backs the GEM resource */
-	inode = file_inode(obj->filp);
-	mapping = inode->i_mapping;
+	mapping = file_inode(obj->filp)->i_mapping;
 
 	/* We already BUG_ON() for non-page-aligned sizes in
 	 * drm_gem_object_init(), so we should never hit this unless
@@ -466,10 +479,8 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask)
 	if (pages == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	gfpmask |= mapping_gfp_mask(mapping);
-
 	for (i = 0; i < npages; i++) {
-		p = shmem_read_mapping_page_gfp(mapping, i, gfpmask);
+		p = shmem_read_mapping_page(mapping, i);
 		if (IS_ERR(p))
 			goto fail;
 		pages[i] = p;
@@ -479,7 +490,7 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask)
 		 * __GFP_DMA32 to be set in mapping_gfp_mask(inode->i_mapping)
 		 * so shmem can relocate pages during swapin if required.
 		 */
-		BUG_ON((gfpmask & __GFP_DMA32) &&
+		BUG_ON((mapping_gfp_mask(mapping) & __GFP_DMA32) &&
 				(page_to_pfn(p) >= 0x00100000UL));
 	}
 
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 592d205..ce015db 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -206,7 +206,7 @@ static int psb_gtt_attach_pages(struct gtt_range *gt)
 
 	WARN_ON(gt->pages);
 
-	pages = drm_gem_get_pages(&gt->gem, 0);
+	pages = drm_gem_get_pages(&gt->gem);
 	if (IS_ERR(pages))
 		return PTR_ERR(pages);
 
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index bb8026d..6866879 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -73,7 +73,7 @@ static struct page **get_pages(struct drm_gem_object *obj)
 		int npages = obj->size >> PAGE_SHIFT;
 
 		if (iommu_present(&platform_bus_type))
-			p = drm_gem_get_pages(obj, 0);
+			p = drm_gem_get_pages(obj);
 		else
 			p = get_pages_vram(obj, npages);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 1331fd5..7c68c8a 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -233,7 +233,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 
 	WARN_ON(omap_obj->pages);
 
-	pages = drm_gem_get_pages(obj, GFP_KERNEL);
+	pages = drm_gem_get_pages(obj);
 	if (IS_ERR(pages)) {
 		dev_err(obj->dev->dev, "could not get pages: %ld\n", PTR_ERR(pages));
 		return PTR_ERR(pages);
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index c041cd7..8044f5f 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -107,14 +107,14 @@ int udl_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	}
 }
 
-static int udl_gem_get_pages(struct udl_gem_object *obj, gfp_t gfpmask)
+static int udl_gem_get_pages(struct udl_gem_object *obj)
 {
 	struct page **pages;
 
 	if (obj->pages)
 		return 0;
 
-	pages = drm_gem_get_pages(&obj->base, gfpmask);
+	pages = drm_gem_get_pages(&obj->base);
 	if (IS_ERR(pages))
 		return PTR_ERR(pages);
 
@@ -147,7 +147,7 @@ int udl_gem_vmap(struct udl_gem_object *obj)
 		return 0;
 	}
 		
-	ret = udl_gem_get_pages(obj, GFP_KERNEL);
+	ret = udl_gem_get_pages(obj);
 	if (ret)
 		return ret;
 
@@ -205,7 +205,7 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
 	}
 	gobj = to_udl_bo(obj);
 
-	ret = udl_gem_get_pages(gobj, GFP_KERNEL);
+	ret = udl_gem_get_pages(gobj);
 	if (ret)
 		goto out;
 	ret = drm_gem_create_mmap_offset(obj);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 12f10bc..40b6e2f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1572,7 +1572,7 @@ void drm_gem_free_mmap_offset(struct drm_gem_object *obj);
 int drm_gem_create_mmap_offset(struct drm_gem_object *obj);
 int drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size);
 
-struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask);
+struct page **drm_gem_get_pages(struct drm_gem_object *obj);
 void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages,
 		bool dirty, bool accessed);
 
-- 
1.9.3

Re: [PATCH v2 1/5] drm/gem: replace misleading comment

[Daniel Vetter]

On Sun, May 25, 2014 at 2:34 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> shmem supports page-relocations during swapin since quite some time. It
> was implemented in:
>
>     commit bde05d1ccd512696b09db9dd2e5f33ad19152605
>     Author: Hugh Dickins <hughd@google.com>
>     Date:   Tue May 29 15:06:38 2012 -0700
>
>         shmem: replace page if mapping excludes its zone
>
> The gem-comment about wrongly placed DMA32 pages is no longer valid.
> Replace it with a proper comment but keep the BUG_ON() to verify correct
> shmem behavior.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/5] drm/armada: use shmem helpers if possible

[Russell King - ARM Linux]

On Sun, May 25, 2014 at 02:34:09PM +0200, David Herrmann wrote:
> shmem_read_mapping_page() uses mapping_gfp_mask(mapping) as default gfp
> mask. No reason to use shmem_read_mapping_page_gfp() directly if we want
> the default behavior.
> 
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Nothing obviously wrong and looks sane enough.

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

Thanks.

Airlied, can you take this one as well please?  Thanks.

> ---
>  drivers/gpu/drm/armada/armada_gem.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 887816f..bb9b642 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -433,7 +433,6 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
>  
>  	if (dobj->obj.filp) {
>  		struct address_space *mapping;
> -		gfp_t gfp;
>  		int count;
>  
>  		count = dobj->obj.size / PAGE_SIZE;
> @@ -441,12 +440,11 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
>  			goto free_sgt;
>  
>  		mapping = file_inode(dobj->obj.filp)->i_mapping;
> -		gfp = mapping_gfp_mask(mapping);
>  
>  		for_each_sg(sgt->sgl, sg, count, i) {
>  			struct page *page;
>  
> -			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
> +			page = shmem_read_mapping_page(mapping, i);
>  			if (IS_ERR(page)) {
>  				num = i;
>  				goto release;
> -- 
> 1.9.3
> 

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 4/5] drm/omap: use __GFP_DMA32 for shmem-backed gem

[David Herrmann]

Hi Tomi

Any chance you could give this a spin on an omap device? It shouldn't
affect any 32bit devices, so this is mostly a cosmetic change.
However, I'd really like to get rid of that 'TODO' item. So a
"Tested-by:" would be really nice.

Thanks
David

On Sun, May 25, 2014 at 2:34 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> OMAP requires bo-pages to be in the DMA32 zone. Explicitly request this by
> setting __GFP_DMA32 as mapping-gfp-mask during shmem initialization. This
> drops HIGHMEM from the gfp-mask and uses DMA32 instead. shmem-core takes
> care to relocate pages during swap-in in case they have been loaded into
> the wrong zone.
>
> It is _not_ possible to pass __GFP_DMA32 to shmem_read_mapping_page_gfp()
> as the page might have already been swapped-in at that time. The zone-mask
> must be set during initialization and be kept constant for now.
>
> Remove the now superfluous TODO in omap_gem.c.
>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_gem.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index 95dbce2..1331fd5 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -233,10 +233,6 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
>
>         WARN_ON(omap_obj->pages);
>
> -       /* TODO: __GFP_DMA32 .. but somehow GFP_HIGHMEM is coming from the
> -        * mapping_gfp_mask(mapping) which conflicts w/ GFP_DMA32.. probably
> -        * we actually want CMA memory for it all anyways..
> -        */
>         pages = drm_gem_get_pages(obj, GFP_KERNEL);
>         if (IS_ERR(pages)) {
>                 dev_err(obj->dev->dev, "could not get pages: %ld\n", PTR_ERR(pages));
> @@ -1347,6 +1343,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>         struct omap_drm_private *priv = dev->dev_private;
>         struct omap_gem_object *omap_obj;
>         struct drm_gem_object *obj = NULL;
> +       struct address_space *mapping;
>         size_t size;
>         int ret;
>
> @@ -1404,14 +1401,16 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev,
>                 omap_obj->height = gsize.tiled.height;
>         }
>
> -       ret = 0;
> -       if (flags & (OMAP_BO_DMA|OMAP_BO_EXT_MEM))
> +       if (flags & (OMAP_BO_DMA|OMAP_BO_EXT_MEM)) {
>                 drm_gem_private_object_init(dev, obj, size);
> -       else
> +       } else {
>                 ret = drm_gem_object_init(dev, obj, size);
> +               if (ret)
> +                       goto fail;
>
> -       if (ret)
> -               goto fail;
> +               mapping = file_inode(obj->filp)->i_mapping;
> +               mapping_set_gfp_mask(mapping, GFP_USER | __GFP_DMA32);
> +       }
>
>         return obj;
>
> --
> 1.9.3
>

Re: [PATCH 3/5] drm/i915: use shmem helpers if possible

[Daniel Vetter]

On Sun, May 25, 2014 at 02:34:10PM +0200, David Herrmann wrote:
> Instead of shuffling gfp-masks all the time, use the
> shmem_read_mapping_page() helper. Note that __GFP_IO and __GFP_WAIT are
> set in mapping_gfp_mask() for i915, so the behavior is still the same.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e1fa919..1a72aad 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1925,16 +1925,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>  			 * our own buffer, now let the real VM do its job and
>  			 * go down in flames if truly OOM.
>  			 */
> -			gfp &= ~(__GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD);
> -			gfp |= __GFP_IO | __GFP_WAIT;
> -
>  			i915_gem_shrink_all(dev_priv);
> -			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
> +			page = shmem_read_mapping_page(mapping, i);
>  			if (IS_ERR(page))
>  				goto err_pages;
> -
> -			gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> -			gfp &= ~(__GFP_IO | __GFP_WAIT);
>  		}
>  #ifdef CONFIG_SWIOTLB
>  		if (swiotlb_nr_tbl()) {
> -- 
> 1.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/5] drm/omap: use __GFP_DMA32 for shmem-backed gem

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]

Hi,

On 02/06/14 16:03, David Herrmann wrote:
> Hi Tomi
> 
> Any chance you could give this a spin on an omap device? It shouldn't
> affect any 32bit devices, so this is mostly a cosmetic change.
> However, I'd really like to get rid of that 'TODO' item. So a
> "Tested-by:" would be really nice.

I made a quick test on omap4. I verified that the changed code is ran,
and I can get a picture on my monitor with omapdrm.

So tested by me, but I'm not familiar with shmem so I can't really say
anything about the change itself without studying this further.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/omap: use __GFP_DMA32 for shmem-backed gem

[Rob Clark]

On Thu, Jun 5, 2014 at 9:52 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On 02/06/14 16:03, David Herrmann wrote:
>> Hi Tomi
>>
>> Any chance you could give this a spin on an omap device? It shouldn't
>> affect any 32bit devices, so this is mostly a cosmetic change.
>> However, I'd really like to get rid of that 'TODO' item. So a
>> "Tested-by:" would be really nice.
>
> I made a quick test on omap4. I verified that the changed code is ran,
> and I can get a picture on my monitor with omapdrm.
>
> So tested by me, but I'm not familiar with shmem so I can't really say
> anything about the change itself without studying this further.

Thanks for testing it.  The change looks ok, I just had a nagging
doubt about it because I had problems w/ GFP32 before (but was
probably just missing mapping_set_gfp_mask()), so was hoping that
someone could confirm that it actually did work as intended.

Anyways, with that:

Reviewed-by: Rob Clark <robdclark@gmail.com>


BR,
-R

Re: [PATCH 4/5] drm/omap: use __GFP_DMA32 for shmem-backed gem

[David Herrmann]

Hi

On Thu, Jun 5, 2014 at 4:04 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Jun 5, 2014 at 9:52 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> Hi,
>>
>> On 02/06/14 16:03, David Herrmann wrote:
>>> Hi Tomi
>>>
>>> Any chance you could give this a spin on an omap device? It shouldn't
>>> affect any 32bit devices, so this is mostly a cosmetic change.
>>> However, I'd really like to get rid of that 'TODO' item. So a
>>> "Tested-by:" would be really nice.
>>
>> I made a quick test on omap4. I verified that the changed code is ran,
>> and I can get a picture on my monitor with omapdrm.
>>
>> So tested by me, but I'm not familiar with shmem so I can't really say
>> anything about the change itself without studying this further.
>
> Thanks for testing it.  The change looks ok, I just had a nagging
> doubt about it because I had problems w/ GFP32 before (but was
> probably just missing mapping_set_gfp_mask()), so was hoping that
> someone could confirm that it actually did work as intended.
>
> Anyways, with that:
>
> Reviewed-by: Rob Clark <robdclark@gmail.com>

Thanks Tomi and Rob! I think to avoid conflicts with PATCH 5/5, Dave
should take this through drm-next? In case there're no objections to
5/5..

Thanks
David