Allow dcp interchange to scanout interchange compressed#449
Allow dcp interchange to scanout interchange compressed#449oliverbestmann wants to merge 1 commit intoAsahiLinux:asahifrom
Conversation
2d2ee37 to
d14c2b6
Compare
Conan-Kudo
left a comment
There was a problem hiding this comment.
The UAPI change needs to be separately contributed to Linux directly, not this fork.
See this documentation: https://docs.kernel.org/process/submitting-patches.html
include/uapi/drm/drm_fourcc.h
Outdated
| */ | ||
| #define DRM_FORMAT_MOD_APPLE_GPU_TILED fourcc_mod_code(APPLE, 1) | ||
| #define DRM_FORMAT_MOD_APPLE_GPU_TILED_COMPRESSED fourcc_mod_code(APPLE, 2) | ||
| #define DRM_FORMAT_MOD_APPLE_INTERCHANGE_COMPRESSED fourcc_mod_code(APPLE, 3) |
There was a problem hiding this comment.
For experimenting we should move this into the driver with
#ifndef DRM_FORMAT_MOD_APPLE_INTERCHANGE_COMPRESSED
#define DRM_FORMAT_MOD_APPLE_INTERCHANGE_COMPRESSED fourcc_mod_code(APPLE, 3)
#endif
I'm not sure if we want one or more APPLE_INTERCHANGE_COMPRESSED modifiers to handle YCbCr formats which use 32x32 tiles for Y
There was a problem hiding this comment.
Is this enough of a difference to warrant a new format modifier? If the tile size only changes for the luma channel of Y'CbCr textures, can we not just handle that by checking is_yuv for the framebuffer format info struct? E.g.
tile_w = fb->format->is_yuv ? 32 : 16
0031cb6 to
79a307d
Compare
d14c2b6 to
605fc30
Compare
|
Thank you for the exciting work! I tested this with nixos-apple-silicon 6.10 using your flake, on an M2 Macbook Air. Hyprland fails to launch with the following repeated log message: |
368641c to
cdeaea6
Compare
It was not working with hyprland on my machine too. I've fixed the issue now. Please try again with the latest update to the flake. |
With your latest flake I can run Hyprland (and I've been using it to work for the last few hours without issues). Is there an easy way to check with certainty that compressed scanout is actually being used? |
|
You can use drm_info to see that modifier c00000000000003 is used for the plane. |
yep, I do see that so it seems to be working. |
|
the code misses a check to ensure the computed surface size for interchange framebuffers fits into the framebuffer's reported buffer size |
Whats the expected behavior in that case? assert and kernel panic, or some logging and quietly ignoring the plane? |
check needs to happen in |
|
Tested as mostly-working on G14G, G14S and G13S, however overlays are broken on kwin. May be size/alignment related, as fractional scaling factors and certain cursor images (notably the Breeze I-beam) cause framebuffer creation to fail. This is most obvious with the cursor, but may occur with other oddly-sized overlay framebuffers. |
Thank you. I had no problems running this with kde. I'll try to reproduce the issues as soon as I can. |
Signed-off-by: Oliver Bestmann <oliver.bestmann@googlemail.com>
cdeaea6 to
0ddf41f
Compare
| * The metadata buffer contains 8 bytes per 16x16 compression tile. | ||
| * Addressing is fully twiddled, so both width and height are padded to | ||
| * powers-of-two. | ||
| */ |
There was a problem hiding this comment.
the comment from the drm_fourcc.h description would be more more appropriate:
/*
* ... The metadata section rounds the image dimensions to
* powers-of-two and contains 8 bytes for each 16x16 compression subtile.
* Subtiles are interleaved (Morton order).
*/
It's the most up to date description.
| * Addressing is fully twiddled, so both width and height are padded to | ||
| * powers-of-two. | ||
| */ | ||
| unsigned w_tl = DIV_ROUND_UP(roundup_pow_of_two(width), 16); |
There was a problem hiding this comment.
isn't this the same as as roundup_pow_of_two(tw)?
| static struct apple_interchange_layout | ||
| get_apple_interchange_layout(u32 width, u32 height, u32 bpp) | ||
| { | ||
| const u32 cacheline = 0x80; |
|
|
||
| u32 tw = DIV_ROUND_UP(width, 16); | ||
| u32 th = DIV_ROUND_UP(height, 16); | ||
| u32 tsize_B = 16 * 16 * (bpp / 8); |
There was a problem hiding this comment.
this should use DIV_ROUND_UP(bpp, 8), not that it matters right now.
That would still be for bit-packed formats. Not sure if dcp supports those or if we want to use them, probably not.
Single component formats with 10- or 12-bit per pixel are more common. Possible that drm_format_info_bpp() includes the padding though.
Works together with this mesa MR:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39755