cosmic-comp icon indicating copy to clipboard operation
cosmic-comp copied to clipboard

Failing to start on Pinebook Pro

Open ids1024 opened this issue 3 years ago • 16 comments

Sway works well, so Cosmic-comp probably should work well enough with the Panfrost driver.

First it seems to error in edid_info. The call to device.get_property_blob returns an ENOENT. Not sure why that is failing, but modifying the function to return Ok allows things to progress further.

Then GbmBufferedSurface::new() fails with an Invalid Argument error. Not sure how to debug that further.

(I had to add prints and unwraps to the code to diagnose the issue this much; exactly where things were failing and the underlying errors weren't being clearly reported. That can be hard, but it would help in the future if this could be improved in some way.)

ids1024 avatar May 10 '22 00:05 ids1024

I don't have a panfrost device for testing, but smithay/cosmic-comp should print some hopefully useful information up until this point. Could you make sure slog/max_level_trace is enabled and capture some logs?

Drakulix avatar May 10 '22 12:05 Drakulix

Testing with gdb, it seems where it's failing is in create_buffer_object_with_modifiers.

ids1024 avatar May 19 '22 19:05 ids1024

Oh, and I guess actually looking at the logs properly:

May 14 03:10:47.680 INFO Initializing drm surface (crtc::Handle(37):plane::Handle(31)) with mode Mode { name: "1920x1080", clock: 148500, size: (1920, 1080), hsync: (1968, 2000, 2200), vsync: (1083, 1088, 1125), hskew: 0, vscan: 0, vrefresh: 60, mode_type: PREFERRED | DRIVER } and connectors [connector::Handle(52)], drm_module: surface, smithay_module: backend_drm_atomic, smithay_module: backend_drm
May 14 03:10:47.682 TRCE Supported scan-out formats for plane (plane::Handle(31)): {DrmFormat { code: DrmFourcc(NV16), modifier: Linear }, DrmFormat { code: DrmFourcc(XR24), modifier: Linear }, DrmFormat { code: DrmFourcc(AR24), modifier: Linear }, DrmFormat { code: DrmFourcc(XB24), modifier: Linear }, DrmFormat { code: DrmFourcc(RG24), modifier: Linear }, DrmFormat { code: DrmFourcc(RG16), modifier: Linear }, DrmFormat { code: DrmFourcc(NV24), modifier: Linear }, DrmFormat { code: DrmFourcc(BG16), modifier: Linear }, DrmFormat { code: DrmFourcc(AB24), modifier: Linear }, DrmFormat { code: DrmFourcc(BG24), modifier: Linear }, DrmFormat { code: DrmFourcc(NV12), modifier: Linear }}, drm_module: surface, smithay_module: backend_drm_atomic, smithay_module: backend_drm
May 14 03:10:47.684 TRCE Plane formats: {DrmFormat { code: DrmFourcc(AR24), modifier: Linear }}, backend: drm_render
May 14 03:10:47.684 TRCE Renderer formats: {DrmFormat { code: DrmFourcc(AR24), modifier: Unrecognized(576460752303423553) }, DrmFormat { code: DrmFourcc(AR24), modifier: Unrecognized(576460752303423569) }, DrmFormat { code: DrmFourcc(AR24), modifier: Unrecognized(580964351930793985) }}, backend: drm_render
May 14 03:10:47.685 DEBG Remaining intersected formats: {}, backend: drm_render
May 14 03:10:47.685 DEBG Testing Formats: [], backend: drm_render
May 14 03:10:47.705 WARN Failed to initialize output: Failed to initialize Gbm surface for eDP-1

I suppose it's expected that "intersected formats" is non-empty? (It doesn't immediately error if it is.) Presumably those renderer formats shouldn't be Unrecognized.

Edit: Looks like the unrecognized modifiers are ARM Frame Buffer Compression (AFBC).

ids1024 avatar May 19 '22 19:05 ids1024

Unrecognized Modifiers are nothing to worry about, but yes intersected formats should not be empty.

I have not yet seen a system where the formats used for scanout are completely disjunct from the formats used for rendering. I am not even sure how one would convert those, given that the renderer has not a single linear format and the byte layout of formats with other modifiers is undefined.

Maybe doing this without any modifiers (Modifier::Implicit) would work, smithay already has to special case some weird combinations of other drivers. Could you try to force that code path and see if it works?

If yes we need to establish that as a fallback when the list of formats turns up empty.

Also could you capture the output of drm_info of the PineBook? Maybe smithay should be selecting another plane on that device.

Drakulix avatar May 20 '22 15:05 Drakulix

I mean, it is fine if it doesn't recognize all the modifiers, but the fact all of them are unrecognized is wrong. Or at least doesn't match how smithay expects it to behave.

drm_info output: pinebook-pro-drm_info.txt

ids1024 avatar May 20 '22 16:05 ids1024

smithay can still utilize Unrecognized modifiers, this is a limitation of drm_fourcc which smithay is using to parse drm values. Its enums are generated from c-headers and the modifiers partially rely heavily on c-macros, which it cannot parse into proper names. But that does not affect functionality. (If you have any idea on how to solve that without maintaining a list by hand, you would be very welcome.)

Drakulix avatar May 20 '22 16:05 Drakulix

drm_info output: pinebook-pro-drm_info.txt

Looking into this output crtc 1 + plane 2 looks like a much better choice, given that plane 2 actually supports the modifiers. cosmic-comp currently keeps matches of previous graphical sessions or picks the first one, that is compatible (without caring about formats).

By the time we initialize the display configuration with smithay crtc-0 is locked in and it assumes it will succeed (because crtc 0 can be used to drive the connector). I have not seen a driver, where this assumption is broken by unsupported formats, but that is technically valid. In fact I am not sure how you would be able to use any of the other planes, when you don't have a renderer to create a linear buffer, presumably only via dumb buffers?

So for the PineBook we need to make the selection logic a bit more complex by comparing formats. (And in the future a potential fallback could also be to use dumb_buffers - maybe even transparently in smithay - although with a significant performance hit. Does the PineBook support external monitors? Because then we need that as well to support one.)

Anyway could you for a test just force display_configuration to return crtc 1 for the connector? It should just work then.

Drakulix avatar May 20 '22 16:05 Drakulix

CRTC 0 on the Pinebook Pro / Pinephone Pro should only support a cursor plane with very limited format support (apart from the primary plane of course). As also visible in the drm_info output, all proper hardware planes are only usable with CRTC 1, which unfortunately is not the default (ChromeOS ships some custom kernel patches that are too hacky for upstream).

I suppose for the format/modifier intersection it would be good to exclude cursor planes?

rmader avatar Nov 11 '22 03:11 rmader

I believe the Pinebook Pro uses 16-bit color, would that be an issue?

jackpot51 avatar Nov 11 '22 17:11 jackpot51

I suppose for the format/modifier intersection it would be good to exclude cursor planes?

Yes we need a heuristic similar to this approach to fix it.

I believe the Pinebook Pro uses 16-bit color, would that be an issue?

The drm_info output from the Pinebook Pro suggests that the hardware would be accepting 8-bit color buffers just fine. Smithay is currently lacking support for 10-bit or even higher colors formats, though support should be relatively easy to add.

Drakulix avatar Nov 11 '22 17:11 Drakulix

I don't mean 16-bit per channel. I mean 16-bit total, 5-bit red, 6-bit green, 5-bit blue. Though, if it can accept 8-bit per channel then that is great.

jackpot51 avatar Nov 11 '22 17:11 jackpot51

I suppose for the format/modifier intersection it would be good to exclude cursor planes?

Yes we need a heuristic similar to this approach to fix it.

Fortunately it's actually advertised - in the drm_info about above it's this line:

"type" (immutable): enum {Overlay, Primary, Cursor} = Cursor

rmader avatar Nov 11 '22 17:11 rmader

I suppose for the format/modifier intersection it would be good to exclude cursor planes?

Yes we need a heuristic similar to this approach to fix it.

Fortunately it's actually advertised - in the drm_info about above it's this line:

"type" (immutable): enum {Overlay, Primary, Cursor} = Cursor

Oh yeah, I am pretty sure though, we never select cursor-planes for anything right now.

I don't mean 16-bit per channel. I mean 16-bit total, 5-bit red, 6-bit green, 5-bit blue. Though, if it can accept 8-bit per channel then that is great.

Ah got it. Also possible, but essentially in the same state as 10-bit color support. Currently smithay is hardcoded to search for either XRGB8888 or ARGB8888.

Drakulix avatar Nov 11 '22 18:11 Drakulix

Currently smithay is hardcoded to search for either XRGB8888 or ARGB8888.

Most compositors require ARGB8888 for cursors AFAIK (Mutter also hardcodes it) and it works fine on the device. Should be the

DrmFormat { code: DrmFourcc(AR24), modifier: Linear }}

above.

I believe the Pinebook Pro uses 16-bit color, would that be an issue?

It supports 8888 and 1010102 formats just fine, here's the output of wayland-info, i.e. formats and modifiers of the primary plane (which is the only non-cursor plane available on CRTC-0):

0x34325241 = 'AR24'; 0x0000000000000000 = LINEAR
0x34325241 = 'AR24'; 0x0800000000000051 = ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE
0x34325241 = 'AR24'; 0x0800000000000041 = ARM_BLOCK_SIZE=16x16,MODE=SPARSE
0x34325241 = 'AR24'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x34325241 = 'AR24'; 0x00ffffffffffffff = INVALID
0x34324241 = 'AB24'; 0x0000000000000000 = LINEAR
0x34324241 = 'AB24'; 0x0800000000000051 = ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE
0x34324241 = 'AB24'; 0x0800000000000041 = ARM_BLOCK_SIZE=16x16,MODE=SPARSE
0x34324241 = 'AB24'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x34324241 = 'AB24'; 0x00ffffffffffffff = INVALID
0x34325258 = 'XR24'; 0x0000000000000000 = LINEAR
0x34325258 = 'XR24'; 0x0800000000000051 = ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE
0x34325258 = 'XR24'; 0x0800000000000041 = ARM_BLOCK_SIZE=16x16,MODE=SPARSE
0x34325258 = 'XR24'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x34325258 = 'XR24'; 0x00ffffffffffffff = INVALID
0x34324258 = 'XB24'; 0x0000000000000000 = LINEAR
0x34324258 = 'XB24'; 0x0800000000000051 = ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE
0x34324258 = 'XB24'; 0x0800000000000041 = ARM_BLOCK_SIZE=16x16,MODE=SPARSE
0x34324258 = 'XB24'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x34324258 = 'XB24'; 0x00ffffffffffffff = INVALID
0x30335241 = 'AR30'; 0x0000000000000000 = LINEAR
0x30335241 = 'AR30'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x30335241 = 'AR30'; 0x00ffffffffffffff = INVALID
0x30334241 = 'AB30'; 0x0000000000000000 = LINEAR
0x30334241 = 'AB30'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x30334241 = 'AB30'; 0x00ffffffffffffff = INVALID
0x30335258 = 'XR30'; 0x0000000000000000 = LINEAR
0x30335258 = 'XR30'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x30335258 = 'XR30'; 0x00ffffffffffffff = INVALID
0x30334258 = 'XB30'; 0x0000000000000000 = LINEAR
0x30334258 = 'XB30'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x30334258 = 'XB30'; 0x00ffffffffffffff = INVALID
0x36314752 = 'RG16'; 0x0000000000000000 = LINEAR
0x36314752 = 'RG16'; 0x0800000000000051 = ARM_BLOCK_SIZE=16x16,MODE=YTR|SPARSE
0x36314752 = 'RG16'; 0x0800000000000041 = ARM_BLOCK_SIZE=16x16,MODE=SPARSE
0x36314752 = 'RG16'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x36314752 = 'RG16'; 0x00ffffffffffffff = INVALID
0x48344241 = 'AB4H'; 0x0000000000000000 = LINEAR
0x48344241 = 'AB4H'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x48344241 = 'AB4H'; 0x00ffffffffffffff = INVALID
0x48344258 = 'XB4H'; 0x0000000000000000 = LINEAR
0x48344258 = 'XB4H'; 0x0810000000000001 = ARM_16X16_BLOCK_U_INTERLEAVED
0x48344258 = 'XB4H'; 0x00ffffffffffffff = INVALID

P.S.: this is missing YUV formats that are not supported by Mutter yet.

rmader avatar Nov 11 '22 18:11 rmader

@ids1024 Is this issue still relevant, or can we close it?

XV-02 avatar Feb 21 '24 20:02 XV-02

Testing with the latest cosmic-comp package, it seems to start fine. Though there are a few warnings:

Feb 22 05:05:09 pop-os cosmic-comp[92909]: ignoring requested context priority, EGL_IMG_context_priority not supported
Feb 22 05:05:10 pop-os cosmic-comp[92909]: Preferred format AB30 not available: NoSupportedPlaneFormat
Feb 22 05:05:10 pop-os cosmic-comp[92909]: Preferred format AR30 not available: NoSupportedPlaneFormat
Feb 22 05:05:10 pop-os cosmic-comp[92909]: Unable to become drm master, assuming unprivileged mode
Feb 22 05:05:10 pop-os cosmic-comp[92909]: Failed to add device /dev/dri/card0: Failed to initialize drm device for: /dev/dri/card0

                                           Caused by:
                                               0: DRM access error: Error loading resource handles on device `Some("/dev/dri/card0")` (Operation not >
                                               1: Operation not supported (os error 95)

Nothing surprising there, or really an issue. We might want to do something to avoid errors like that with kmsro devices.

ids1024 avatar Feb 22 '24 05:02 ids1024