Re: [PATCH 12/12] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region

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

 



On 21/04/16 15:41, Chris Wilson wrote:
On Thu, Apr 21, 2016 at 03:17:06PM +0100, Tvrtko Ursulin wrote:
+void i915_gem_stolen_size_info(struct drm_i915_private *dev_priv,
+			       uint64_t *stolen_free,
+			       uint64_t *stolen_largest)
+{
+	struct drm_mm *mm = &dev_priv->mm.stolen;
+	struct drm_mm_node *head_node = &mm->head_node;
+	struct drm_mm_node *entry;
+	uint64_t hole_size, hole_start, hole_end, largest_hole = 0;
+	uint64_t total_free = 0;
+
+	if (dev_priv->mm.volatile_stolen) {
+		*stolen_free = 0;
+		*stolen_largest = 0;
+		return;
+	}
+
+	if (head_node->hole_follows) {
+		hole_start = drm_mm_hole_node_start(head_node);
+		hole_end = drm_mm_hole_node_end(head_node);
+		hole_size = hole_end - hole_start;
+		total_free += hole_size;
+		if (largest_hole < hole_size)
+			largest_hole = hole_size;
+	}
+

Why does this block need to be separately handled and the loop below
would not cover it? On first iteration entry will be the head node
below as well, no?

Hmm, didn't I/somebody add drm_mm_for_each_hole() ?

I see it in the header, yes.

+	*stolen_free = total_free;
+	*stolen_largest = largest_hole;
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8f38407..424e57e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1012,6 +1012,12 @@ struct drm_i915_gem_get_aperture {
  	 * Total space in the mappable region of the aperture, in bytes
  	 */
  	__u64 map_total_size;
+
+	/**
+	 * Total space in the stolen region, in bytes
+	 */
+	__u64 stolen_total_size;
+

How will the userspace detect existence of the new ioctl fields? Is
it intended that they try to call it with a buffer of certain size
and act on the failure when that fails? Is that good enough or we
need something better like get_param or something?

As we are extending the structure:

1. Old userspace, old kernel: unaffacted

2. Old userspace, new kernel:
Kernel computes the new fields, but the struct is truncated in the copy
back to userspace. userspace only sees the fields it used to, no change.

3. New userspace, old kernel:
Userspace passes in a larger struct, kernel only copies back in the
fields it knows and zero fills the tail of the user's struct. userspace
sees a stolen_total_size of 0 and knows to avoid the new interface.

I suppose it doesn't make any practical difference to the driver between "there is no stolen memory" and "driver does not support stolen memory query / create".

For mappable region it is a bit weirder because it wouldn't be able to tell if there is no mappable or no query support so it would potentially incorrectly avoid mmap_gtt etc.

4. New userspace, new kernel:
Everything explodes^W just works.

In this case, the struct is only an out, so we don't need to do invalid
pad or flag rejection. We just have the ABI rule that anything the
kernel doesn't know about is ignored and zero-filled were applicable.
-Chris


Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://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