On 26/04/16 10:44, Chris Wilson wrote:
On Mon, Apr 25, 2016 at 03:51:09PM +0100, Tvrtko Ursulin wrote:
On 25/04/16 11:35, Ankitprasad Sharma wrote:
On Thu, 2016-04-21 at 15:59 +0100, Tvrtko Ursulin wrote:
On 21/04/16 15:46, Chris Wilson wrote:
On Thu, Apr 21, 2016 at 03:04:52PM +0100, Tvrtko Ursulin wrote:
Hi,
On 20/04/16 12:17, ankitprasad.r.sharma@xxxxxxxxx wrote:
+ mutex_unlock(&dev->struct_mutex);
+
+ seq_printf(m, "Total size of the GTT: %llu bytes\n",
+ arg.aper_size);
+ seq_printf(m, "Available space in the GTT: %llu bytes\n",
+ arg.aper_available_size);
+ seq_printf(m, "Total space in the mappable aperture: %llu bytes\n",
+ arg.map_total_size);
+ seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
+ map_space);
+ seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
+ map_largest);
+ seq_printf(m, "Available space for fences: %llu bytes\n",
+ fence_space);
+ seq_printf(m, "Single largest fence available: %llu bytes\n",
+ fence_largest);
+
+ return 0;
+}
+
In general I find this a lot of code for a feature of questionable
utility. As such I would prefer someone really stated the need for
this and explained how it really is useful - even though whetever
number they get from this may be completely irrelevant by the time
it is acted upon.
Yes, with the exception of the size of the mappable aperture, this is
really is debug info. It will get automatically dumped by userspace
when it sees an ENOSPC, and that may prove enough to solve the riddle of
why it failed. However, this information is terrible outdated and now
longer of such relevance.
As for the mappable aperture size, there has been a request many years
ago! could we provide it without resorting to a privilege operation. I
guess by know that request has died out - but there is still the issue
with libpciassess that make it unsuitable for use inside a library where
one may want to avoid it and use a simple ioctl on the device you
already have open.
Yes, it is meh.
Aperture size in the ioctl is fine I think, just that detection caveat
what I asked in the other reply.
Here I wanted to suggest dropping all the non-trivial debugfs stuff and
just leave the info queried via i915_gem_get_aperture ioctl. So
effectively dropping the list traversal and vma sorting bits.
I think, debug info regarding the mappable space is good to have for
debugging purpose as Chris mentioned.
Also, the list traversal and the vma sorting stuff will only be called
for debugging purposes, not slowing anything down or so.
I am pretty indifferent on the topic of debugfs edition.
But for the ioctl extension, how about adding a version field as the
first one in the extended area?
A version number only makes sense when you are changing the meaning of
an existing field. Adding one implies that we are planning to do so, are
we?
In the scenarios, I've run through I haven't found one where a caller
would behave any differently faced with "0 - ioctl version not
supported" and "0 - no available space (mappable/stolen)". Adding a
version doesn't help using the new fields afaict. The argument is the
same as whether a flags field is forward thinking or unthinkingly
forward.
I was thinking that if 0 = no aperture or ioctl not supported userspace
has to try one mmap_gtt to find out which is true, will it be ENODEV or
ENOSPC (assuming, haven't checked). If we put a version in there then it
can avoid doing that. Sounds like a better interface to me and I don't
see any downsides to it.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
