Re: [PATCH 4/5] drm/i915: Add a partial GGTT view type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

On 04/24/2015 01:09 PM, Joonas Lahtinen wrote:

Partial view type allows manipulating parts of huge BOs through the GGTT,
which was not previously possible due to constraint that whole object had
to be mapped for any access to it through GGTT.

Signed-off-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem_gtt.c |   46 +++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/i915_gem_gtt.h |   15 ++++++++++--
  2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5babbd3..5937d3d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2764,6 +2764,47 @@ err_st_alloc:
  	return ERR_PTR(ret);
  }

+static struct sg_table *
+intel_partial_pages(const struct i915_ggtt_view *view,
+		    struct drm_i915_gem_object *obj)
+{
+	struct sg_table *st;
+	struct scatterlist *sg;
+	struct sg_page_iter obj_sg_iter;
+	int ret;
+
+	st = kmalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		goto err_st_alloc;
+
+	ret = sg_alloc_table(st, view->params.partial.size, GFP_KERNEL);
+	if (ret)
+		goto err_sg_alloc;
+
+	sg = st->sgl;
+	st->nents = 0;

sg_alloc_table configures the sg_table so not needed I think. Although I do see I am also doing it. :)

+	for_each_sg_page(obj->pages->sgl, &obj_sg_iter, obj->pages->nents,
+		view->params.partial.offset)
+	{
+		if (st->nents >= view->params.partial.size)
+			break;
+
+		sg_set_page(sg, NULL, PAGE_SIZE, 0);
+		sg_dma_address(sg) = sg_page_iter_dma_address(&obj_sg_iter);
+		sg_dma_len(sg) = PAGE_SIZE;
+
+		sg = sg_next(sg);
+		st->nents++;
+	}

I suppose in this case (as opposed to rotated view) using sg_alloc_table_from_pages() could produce a more compact table. With the caveat of that it doesn't always work (see i915_gem_userptr.c/st_set_pages).

So maybe promote to driver public st_set_pages and call in on an array of pages?

+
+	return st;
+
+err_sg_alloc:
+	kfree(st);

Here you lose ret from sg_alloc_table.

+err_st_alloc:
+	return ERR_PTR(-ENOMEM);
+}
+
  static int
  i915_get_ggtt_vma_pages(struct i915_vma *vma)
  {
@@ -2777,6 +2818,9 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
  	else if (vma->ggtt_view.type == I915_GGTT_VIEW_ROTATED)
  		vma->ggtt_view.pages =
  			intel_rotate_fb_obj_pages(&vma->ggtt_view, vma->obj);
+	else if (vma->ggtt_view.type == I915_GGTT_VIEW_PARTIAL)
+		vma->ggtt_view.pages =
+			intel_partial_pages(&vma->ggtt_view, vma->obj);
  	else
  		WARN_ONCE(1, "GGTT view %u not implemented!\n",
  			  vma->ggtt_view.type);
@@ -2859,6 +2903,8 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
  	if (view->type == I915_GGTT_VIEW_NORMAL ||
  	    view->type == I915_GGTT_VIEW_ROTATED) {
  		return obj->base.size;
+	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
+		return view->params.partial.size << PAGE_SHIFT;
  	} else {
  		WARN_ONCE(1, "GGTT view %u not implemented!\n", view->type);
  		return obj->base.size;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 34b7cca..ab1ad8a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -117,7 +117,8 @@ typedef uint64_t gen8_pde_t;

  enum i915_ggtt_view_type {
  	I915_GGTT_VIEW_NORMAL = 0,
-	I915_GGTT_VIEW_ROTATED
+	I915_GGTT_VIEW_ROTATED,
+	I915_GGTT_VIEW_PARTIAL,
  };

  struct intel_rotation_info {
@@ -130,6 +131,13 @@ struct intel_rotation_info {
  struct i915_ggtt_view {
  	enum i915_ggtt_view_type type;

+	union {
+		struct {
+			pgoff_t offset;
+			size_t size;

Size is in pages right? Maybe it would be more self-documenting to use some basic type like unsigned int or long since size_t, to me at least, suggests bytes.

+		} partial;
+	} params;
+
  	struct sg_table *pages;

  	union {
@@ -495,7 +503,10 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a,
  	if (WARN_ON(!a || !b))
  		return false;

-	return a->type == b->type;
+	if (a->type != b->type)
+		return false;
+
+	return !memcmp(&a->params, &b->params, sizeof(a->params));

So for rotated views it would still do memcmp. OK structure is zeroed on alloc, but it is pointless to do so.

Regards,

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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux