[Intel-gfx] [RFC] drm/i915: Render decompression support for Gen9 and above

[Daniel Stone]

Hi,

On 25 January 2016 at 19:04, Smith, Gary K <gary.k.smith at intel.com> wrote:
> It’s a few hundred lines of extra code and additional complexity in open-source userspace to track the additional fb's in flight in order to save the i915 code from performing a couple of extra if's during a flip. It's not a good tradeoff at all. We already have the experience of requiring to change the format of fb's dynamically between opaque and transparent as an example of what has to happen. Its also much harder to debug as there is now no fixed mapping between the buffer and the fb.

I don't see how it really ends up being a few hundred extra lines, but
OK. It's also really - really - not about extra if's during the flip,
but about the design of _userspace_ components which are not able to
use property-based interfaces because of the separation of concerns
between rendering and display. It also goes against what other drivers
have been working with.

>> The property-based design precludes optimal use of current open-source userspace.
> I don’t see how it even effects open-source userspace. It can just ignore it entirely.

i.e. this property is useless for open-source userspace, which can
never hint that render compression has been resolved. So fine by me if
you want to merge it I guess, and we'll just ignore it.

Cheers,
Daniel

> Thanks
> Gary
>
>
> -----Original Message-----
> From: Daniel Stone [mailto:daniel at fooishbar.org]
> Sent: Monday, January 25, 2016 6:37 PM
> To: Smith, Gary K <gary.k.smith at intel.com>
> Cc: Jesse Barnes <jbarnes at virtuousgeek.org>; Daniel Vetter <daniel at ffwll.ch>; intel-gfx at lists.freedesktop.org; Thierry Reding <thierry.reding at avionic-desi.gn.de>
> Subject: Re: [Intel-gfx] [RFC] drm/i915: Render decompression support for Gen9 and above
>
> Hi,
>
> On 25 January 2016 at 18:15, Smith, Gary K <gary.k.smith at intel.com> wrote:
>> I really do not understand what the issue is here. It’s a hint provided by the user to indicate that the current fb (which was allocated with the appropriate aux buffer/compression fb modifiers) has been entirely resolved (uncompressed) and hence can be used anywhere an uncompressed fb would normally be required.
>>
>> Creating a fb that says that there is no aux buffer is entirely wrong in this case as there is an aux buffer, it just happens to be in a particular known state for this particular flip.
>>
>> If GBM doesn’t want to optimize its power usage when(if) it knows the buffer is uncompressed, then it doesn’t have to set the property at all.
>
> GBM cannot set the property, because GBM is not the element in charge of presenting the buffers. The property-based design precludes optimal use of current open-source userspace.
>
> The flipside here is a minor inconvenience (having to create multiple
> framebuffers) to closed userspace; last time this came up, when it was asked if this was actually a serious memory concern or not, there was no response.
>
> Cheers,
> Daniel
>
>> Thanks
>> Gary
>>
>>
>>
>>
>> -----Original Message-----
>> From: Jesse Barnes [mailto:jbarnes at virtuousgeek.org]
>> Sent: Monday, January 25, 2016 5:39 PM
>> To: Daniel Stone <daniel at fooishbar.org>; Daniel Vetter
>> <daniel at ffwll.ch>
>> Cc: intel-gfx at lists.freedesktop.org; Thierry Reding
>> <thierry.reding at avionic-desi.gn.de>; Smith, Gary K
>> <gary.k.smith at intel.com>
>> Subject: Re: [Intel-gfx] [RFC] drm/i915: Render decompression support
>> for Gen9 and above
>>
>> On 01/19/2016 02:28 AM, Daniel Stone wrote:
>>>>>> >> > We aren't just talking about a few fbs here, we already see
>>>>>> >> > more than
>>>>>> >> > 100 fbs active during complex situations. Potentially
>>>>>> >> > doubling this number is surely a significant increase in
>>>>>> >> > memory usage, both from the management side in userspace and the kernel side.
>>>> >
>>>> > 8kb kernel memory for the additional 2 copies of drm_framebuffer
>>>> > structs for 100 buffers. That's about as much as the minimal
>>>> > overhead for just 1 underlying gem object (counting the sg table,
>>>> > vma, gtt pte tracking, gem object and shmem backing node and pagecache entries). 2 integers in userspace.
>>>> >
>>>> > Do you have some data to show that overhead?
>>> I agree with this view as well, and it does seem to be the way chosen
>>> for generic userspace on other drivers.
>>>
>>> For context, the way ChromeOS and Wayland compositors (Weston,
>>> Mutter,
>>> Enlightenment) work is that a userspace library called GBM is
>>> distributed as part of EGL, which is the native EGL platform/winsys
>>> for rendering on KMS. The major difference with GBM, however, is that
>>> it does _not_ do presentation: presentation is explicitly controlled
>>> by the compositor itself.
>>>
>>> In order to use this new property, we would have to add API to
>>> EGL/GBM to extract a list of property names to set, which wouldn't
>>> really make for great API. It'd be much cleaner for these users to
>>> stick with FB modifiers, especially as they destroy and recreate the
>>> FB objects (something we've not seen have any performance impact) for
>>> every flip anyway. From my side, I'd be much happier using
>>> generically-applicable FB modifiers, than continuing along the property explosion.
>>>
>>> The other sticking point is that if I go from flipping GPU buffers
>>> with render compression enabled to software buffers, from userspace
>>> that means I then need to explicitly go unset the render
>>> decompression flag before I can display software buffers, else the
>>> flips just get rejected; something which isn't the case with FB
>>> modifiers. One more thing to go wrong ...
>>
>> Just for background, we ended up with a property for this attribute due to a request from the only userland folks we had at the time (our hwcomposer team).  They felt it would be simpler to use a property in this specific case, though they already do have a number of fb objects to deal with.  Really I can make an argument either way for how well each matches hardware behavior, so figured we'd just go with a property due to someone expressing a preference.
>>
>> This has probably already been changed in an updated patch (still catching up on mail), but I thought I'd at least chime in on the thinking on this from way back (around a year ago now I think).
>>
>> Cc'ing Gary in case he has further comment.
>>
>> Jesse
>> ---------------------------------------------------------------------
>> Intel Corporation (UK) Limited
>> Registered No. 1134945 (England)
>> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.