[Intel-gfx] [PATCH 11/12] drm/i915: Extend GET_APERTURE ioctl to report available map space

[Tvrtko Ursulin]

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 at intel.com 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?

Regards,

Tvrtko