[Mesa-dev] [PATCH 00/27] Renderbuffer Decompression (and GBM modifiers)

[Daniel Stone]

Hey Ben,
Sorry I didn't get to testing this before now; have been tied up with
all manner of stuff.

On 1 December 2016 at 22:09, Ben Widawsky <benjamin.widawsky at intel.com> wrote:
> The overall strategy is that the buffer/surface is created with a list of
> modifiers. The list of modifiers the hardware is capable of using will come from
> a new kernel API that is aware of the hardware and general constraints. A client
> will request the list of modifiers and pass it directly back in during buffer
> creation (potentially the client can prune the list, but as of now there is no
> reason to.) This new API is being developed by Kristian. I did not get far
> enough to play with that.
>
> For EGL, a similar mechanism would exist whereby when importing a buffer into
> EGL, one would provide a modifier and probably a pointer to the auxiliary data
> upon import. (Import therefore might require multiple dma-buf fds), but for i965
> and Intel, this wouldn't be necessary.

Right, we have EGL_EXT_image_dma_buf_import_modifiers; Varad has a
series on the list already for this which just needs some reviews
(ahem).

> Here is a brief description of the series:
> 1-6 Adds support in GBM for per plane functions where necessary. This is
> required because the kernel expects the auxiliary buffer to be passed along as a
> plane. It has its own offset, and stride, and the client shouldn't need to
> calculate those.

This is missing gbm_bo_get_handle_for_plane(); as you say, a lot of
other hardware tends to use separate buffers rather than
adjacent/offset. So adding that would be nice. Having
gbm_bo_get_plane_count() is really nice though, since it allows us to
have a completely agnostic client (i.e. I don't have to have a map
inside Weston with every exotic format/modifier combination).

> 7-9 Adds support in GBM to understand modifiers. When creating a buffer or
> surface, the client is expected to pass in a list of modifiers that the driver
> will optimally choose from. As a result of this, the GBM APIs need to support
> modifiers.

This bit seems good, and like a reasonable fit for the draft of
GETPLANE2 which is kicking around.

> 10-12 Support Y-tiled modifier. Y-tiling was already a modifier exposed by the
> kernel. With the previous patches in place, it's easy to support this too.

And it works! \o/

> 13-26 Plumbing to support sending CCS buffers to display. Leveraging much of the
> existing code for MCS buffers, these patches creating an MCS for the scanout
> buffer. The trickery here is that a single BO contains both the main surface and
> the auxiliary data. Previously, auxiliary data always lived in its own BO.
>
> 27 Support CCS-modifier. Finally, the code can parse the CCS fb modifier(s) and
> realize the bandwidth savings that come with it.

I've not rebuilt my kernel to test the new CCS bits, so I haven't tested this.

> This was tested using kmscube
> (https://github.com/bwidawsk/kmscube/tree/modifiers). The kmscube implementation
> is missing support for GET_PLANE2 - which is currently being worked on by
> Kristian.

There's also a Weston branch here:
https://git.collabora.com/cgit/user/daniels/weston.git/log/?h=wip/2016-11/gbm-planes-modifiers

This works with Y-tiling for me, but with the same need for
GET_PLANE2; also the branch as-is will provoke a segfault inside
gbm_dri_bo_get_modifier(), which ends up calling intel_query_image()
with image == NULL, when using cursor images. To get it to succeed,
you need to shove an early 'return -1' inside
drm_output_init_cursor_egl() so we fall back to software (well OK, GL)
cursors.

The branch is broken with multihead, but that's the branch it's based
on being broken/WIP, not a result of these patches.

> Upstream plan:
> 1. All of the patches up through 26 should be mergeable today after review.
> 2. After 1-12 land, client support of Y-tiling should be achievable. Modesetting
> driver can probably be updated as can things like Weston. Clients assuming a new
> enough kernel should be able to blindly set the y tiled modifier.
> 3. Once kernel and libdrm support for CCS modifiers, patch 27 can land, however
> CCS isn't yet usable, it is only available as a prototype.
> 4. Kristian's GET_PLANE2 interface needs to be solidified and land.
> 5. Clients will utilize #3 and #4 to use CCS.
> 6. Protocol work, EGL, Wayland, DRIX - etc

Wayland has modifier support already; there are patches out for review
for Weston to support this via the EGL extension above, as well as
inside KMS (part of the atomic branch).

> When Kristian's interface is ready, kmscube can be modified to make use of it.

And I'll modify Weston to use it as well.

Thanks for this, and sorry for the tardy review.

Cheers,
Daniel