On Thu, Apr 28, 2016 at 10:30:32AM +0100, Tvrtko Ursulin wrote: > > 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. I was thinking either userspace already cares and has a method for finding the size of the PCI memory region or it doesn't. If it doesn't and the ioctl reports 0 and its older second method fails with EPERM/EACCESS then it is no worse off than before. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
