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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	akash.goel@intel.com
Subject: Re: [PATCH 11/12] drm/i915: Extend GET_APERTURE ioctl to report available map space
Date: Fri, 29 Apr 2016 11:56:28 +0100	[thread overview]
Message-ID: <57233DDC.9030106@linux.intel.com> (raw)
In-Reply-To: <20160429103927.GK30680@nuc-i3427.alporthouse.com>

On 29/04/16 11:39, Chris Wilson wrote:
> On Fri, Apr 29, 2016 at 11:26:45AM +0100, Tvrtko Ursulin wrote:
>>
>> On 29/04/16 11:18, Chris Wilson wrote:
>>> On Fri, Apr 29, 2016 at 11:06:49AM +0100, Tvrtko Ursulin wrote:
>>>> I don't get it - if we are adding something why not add it in a way
>>>> that makes it clear and self-contained - what is the downside of
>>>> what I propose to meet such resistance?
>>>
>>> You're suggesting to add a field I'm not going to use. Is any one?
>>> Until there is an actual use (which would afaict be if one of the
>>> existing values changed meaning, which itself would be an abi break) I'm
>>> not convinced we should be designing for the unknowable. If we did need
>>> to version would we not be better off with a new ioctl?
>>
>> I am suggesting if we are adding aper_total_size, or whatever it is
>> called, to make it usable in an self-contained matter.
>>
>> My impression was you want aper_total_size so I was simply objecting
>> to adding it without being able to directly tell if kernel actually
>> supports that extension.
>
> The two additions that I thought we have reduced it to:
>
> 	u64 mappable_aperture_size
> 	u64 stolen_size;
>
> Of which I agree that the first has some ambiguity over 0. But I don't
> think it makes a difference in practice. I for one would not bother with
> checking a version. I already have a cascade here to deal with not being
> able to use various probes, with an eventual fallback to a safe value.
>
> We know the mappable_aperture_size cannot be zero with the exception
> that the device is disabled - but we have opened the device already.

Since you agree there is ambiguity lets just add a version. You don't 
have to use it, but someone will.

	u32 version; // == 1
  	u64 mappable_aperture_size;

And then it is clear and self-contained.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx