Re: [PATCH 01/11] drm/i915: Add support for mapping an object page by page

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

 




On 14/01/16 13:34, Chris Wilson wrote:
On Thu, Jan 14, 2016 at 10:32:11AM +0000, Tvrtko Ursulin wrote:
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad8..5f86596 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -317,6 +317,11 @@ struct i915_address_space {
  			    uint64_t start,
  			    uint64_t length,
  			    bool use_scratch);
+	void (*insert_page)(struct i915_address_space *vm,
+			    dma_addr_t addr,
+			    uint64_t offset,
+			    enum i915_cache_level cache_level,
+			    u32 flags);
  	void (*insert_entries)(struct i915_address_space *vm,
  			       struct sg_table *st,
  			       uint64_t start,

Why not extend the current API to support start page offset and
number of pages? Could default to full object like today if zero.
Eg:

  void (*insert_entries)(struct i915_address_space *vm,
			struct sg_table *st,
+			unsigned page_offset,
+			unsigned num_pages,

Ouch. That would be quite slow for the insert_page() use case of
page-by-page looping.

It could use the same last page caching trick so why would be be so slow?

			uint64_t start,
			enum i915_cache_level cache_level,
			u32 flags);

That way we would not have two functions for effectively the same
thing operating on different type of input parameters.

If extending insert_entries is not preferable, then still we could
add another compatible one, like insert_entries_range or something,
and then both could share the same underlying implementation for
less code.

Like this, this patch already does not match current codebase - see
assert_rpm_atomic_begin in insert_entries.

Also if API between the two was compatible there would be no need
for i915_gem_object_get_dma_address() and i915_gem_object_get_page()
could be used instead.

The point was to write a lowlevel analog to provide a complementary API
to insert_entries that could be used for everywhere the we wanted to peek
through the GTT without even touching an object, i.e. for cases where we
might allocate a scratch page and temporarily put it into the GTT.

Well maybe it is not that bad on the API front, but code duplication would still not be my first choice, as demonstrated by the divergence which already happened. Need to think about it some more.

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