modesetting: Fix hw cursor v2 #734

stefan11111 · 2025-08-20T17:53:12Z

This adds my changes on top of #691

Closes: #549
Closes: #675

stefan11111 · 2025-08-20T17:54:19Z

@notbabaisyou Ping, as I couldn't add you as a reviewer through the github interface.

notbabaisyou

I'm really not a fan of combining both approaches here, this just makes one massive mess and I'd rather have one or the other. Of course I'm biased and prefer my one since it's a more widely used solution (i.e. Wayland) but I guess someone like @metux would probably be suited to picking one of the approaches here.

There's some smaller nits that I've left out and some formatting nitpicks too.

hw/xfree86/drivers/video/modesetting/drmmode_display.c

notbabaisyou · 2025-08-20T18:11:57Z

hw/xfree86/drivers/video/modesetting/drmmode_display.c

+}
+
+static void
+drmmode_paint_cursor(CARD32 * restrict cursor, int cursor_pitch, int cursor_width, int cursor_height,


I'm not a fan of this, takes too many arguments and I think the memcpy's will break on a cursor size change.

What this does is:

If this is the first time we paint the cursor, paint the entire cursor.

Otherwise, let width_todo be max(new_glyph_width, old_glyph_width),
and let height_todo max(new_glyph_height, old_glyph_height).

And only paint the top width_todo * height_todo corner of the cursor.

hw/xfree86/drivers/video/modesetting/driver.c

hw/xfree86/drivers/video/modesetting/drmmode_display.c

stefan11111 · 2025-08-20T20:27:14Z

I'm really not a fan of combining both approaches here, this just makes one massive mess and I'd rather have one or the other.

We could merge your pr first, and I can then rebase, if you prefer.

Of course I'm biased and prefer my one since it's a more widely used solution (i.e. Wayland) but I guess someone like @metux would probably be suited to picking one of the approaches here.

This pr is still using the SIZE_HINTS from your pr.
What I've changed from your pr is the helper for getting the sizes, and the fact that I'm using the smallest possible size instead of a larger one.

The rest are additions.

stefan11111 · 2025-08-20T20:29:00Z

@metux @callmetango @notbabaisyou @cepelinas9000
Not related to this pr, but to the modesetting driver.

Do you think it's a good idea for sometime in the future to switch from using dumb bo's for the cursor and instead use gbm bo's?

hw/xfree86/drivers/video/modesetting/drmmode_display.c

metux · 2025-08-21T09:20:03Z

@metux @callmetango @notbabaisyou @cepelinas9000 Not related to this pr, but to the modesetting driver.

Do you think it's a good idea for sometime in the future to switch from using dumb bo's for the cursor and instead use gbm bo's?

Not sure whether it's actually worth it. The buffers are pretty small and rarely updated, and HW scanout/overlays usually already optimized for them.

Here's what chatgpt has to say about this:
https://chatgpt.com/share/68a6e4b0-4db4-8005-92aa-02aa79507e13

stefan11111 · 2025-08-21T11:42:54Z

@metux @callmetango @notbabaisyou @cepelinas9000 Not related to this pr, but to the modesetting driver.
Do you think it's a good idea for sometime in the future to switch from using dumb bo's for the cursor and instead use gbm bo's?

Not sure whether it's actually worth it. The buffers are pretty small and rarely updated, and HW scanout/overlays usually already optimized for them.

Wdym overlays here?

I meant that we could create a cursor buffer with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE, and use that instead of the dumb buffers we use now.

I don't think we want scanout here, I think we only want that for the screen buffer.

The cursor buffer does get updated frequently, every time the mouse hovers over a text window, for example.

stefan11111 · 2025-08-21T11:52:05Z

@metux @callmetango @cepelinas9000 @notbabaisyou

I also added an option for setting the hw cursor fallback size via xorg.conf.

Many cards don't support SIZE_HINTS, and for those, we always use the fallback.

The way this patch is now, for those cards, we either use the 64x64 fallback, or what the users puts in xorg.conf.

Is this really a good idea, or should we keep the old probe for checking the minimum size on these cards?
I'm not sure that it's a good idea to put it on the user to tell us what size hw cursor they should use, I think we should probe the cursor size in the old way on these cards.

What are your thoughts?

metux · 2025-08-21T13:30:57Z

Wdym overlays here?

Hardware overlays - in C64 we called them sprites, X11 calls them "hardware cursors".
A separate tiny framebuffer that the scanout unit directly blends over the main one at configurable position.
That's how moving (or changing) the cursor costs almost nothing, compared to always repainting the affected screen sections.

I meant that we could create a cursor buffer with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE, and use that instead of the dumb buffers we use now.

Yes, but then we're depending on gbm (IMHO we currently depending on it when glamor enabled).

I don't think we want scanout here, I think we only want that for the screen buffer.

Isn't the whole idea that the scanout automatically paints (blends in) the cursor on its own ?

The cursor buffer does get updated frequently, every time the mouse hovers over a text window, for example.

That would be soft-cursors, eg. on a really dumb framebuffer.

callmetango · 2025-08-21T13:51:43Z

@stefan11111

I also added an option for setting the hw cursor fallback size via xorg.conf.

Many cards don't support SIZE_HINTS, and for those, we always use the fallback.

The way this patch is now, for those cards, we either use the 64x64 fallback, or what the users puts in xorg.conf.

Is this really a good idea, or should we keep the old probe for checking the minimum size on these cards? I'm not sure that it's a good idea to put it on the user to tell us what size hw cursor they should use, I think we should probe the cursor size in the old way on these cards.

What are your thoughts?

I think we should:

Make great efforts to determine the cursor size automatically.
If this is impossible then we should fallback on a sensible default.
If this default does not work everywhere then we should give the user the option to set it to whatever works for him.

stefan11111 · 2025-08-21T14:38:07Z

Wdym overlays here?

Hardware overlays - in C64 we called them sprites, X11 calls them "hardware cursors". A separate tiny framebuffer that the scanout unit directly blends over the main one at configurable position. That's how moving (or changing) the cursor costs almost nothing, compared to always repainting the affected screen sections.

So you mean the cursor plane, right?

As I understand it, there are 3 types of drm planes, primary, cursor and overlay.

The primary plane presents the main part of the image.
The cursor plane, which gets rendered by the gpu on top of all the planes, at the current cursor position.
And overlay planes, which are rendered between the primary and cursor planes, and are used for hardware alpha blending.

I meant that we could create a cursor buffer with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE, and use that instead of the dumb buffers we use now.

Yes, but then we're depending on gbm (IMHO we currently depending on it when glamor enabled).

As you said, we already use libgbm.

I don't think we want scanout here, I think we only want that for the screen buffer.

Isn't the whole idea that the scanout automatically paints (blends in) the cursor on its own ?

Yes, but I don't think we need to explicitly mark the buffer as scanout capable.
From what I saw in wayland code, cursor buffers are allocated without GBM_BO_USE_SCANOUT, and only with
GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE

The cursor buffer does get updated frequently, every time the mouse hovers over a text window, for example.

That would be soft-cursors, eg. on a really dumb framebuffer.

The cursor buffer has to be painted whenever the glyph changes.
Otherwise, how would the gpu know that we want the cursor image to change?

cepelinas9000 · 2025-08-21T18:27:45Z

@metux @callmetango @cepelinas9000 @notbabaisyou

I also added an option for setting the hw cursor fallback size via xorg.conf.

Many cards don't support SIZE_HINTS, and for those, we always use the fallback.

The way this patch is now, for those cards, we either use the 64x64 fallback, or what the users puts in xorg.conf.

Is this really a good idea, or should we keep the old probe for checking the minimum size on these cards? I'm not sure that it's a good idea to put it on the user to tell us what size hw cursor they should use, I think we should probe the cursor size in the old way on these cards.

What are your thoughts?

My thoughts, (I doubt worth the effort) I would instead FallbackCursor, I add ForceCursorSizes where user explicitly lists which cursor sizes supported, for example:

ForceCursorSizes "32x32@64,64x64,128x128@512,256x256,512x512"

The semantics, cursor sizes always from smallest to largest (in case square - by area). @ mean cursor sprite pitch (in case width is bigger than use width value), this parameter is stricy - once used values is changed on next one @, it is equivalent to:

ForceCursorSizes "32x32@64,64x64@64,128x128@512,256x256@512,512x512@512"

stefan11111 · 2025-08-21T19:35:16Z

@metux @callmetango @cepelinas9000 @notbabaisyou
I also added an option for setting the hw cursor fallback size via xorg.conf.
Many cards don't support SIZE_HINTS, and for those, we always use the fallback.
The way this patch is now, for those cards, we either use the 64x64 fallback, or what the users puts in xorg.conf.
Is this really a good idea, or should we keep the old probe for checking the minimum size on these cards? I'm not sure that it's a good idea to put it on the user to tell us what size hw cursor they should use, I think we should probe the cursor size in the old way on these cards.
What are your thoughts?

My thoughts, (I doubt worth the effort) I would instead FallbackCursor, I add ForceCursorSizes where user explicitly lists which cursor sizes supported, for example:
ForceCursorSizes "32x32@64,64x64,128x128@512,256x256,512x512"
The semantics, cursor sizes always from smallest to largest (in case square - by area). @ mean cursor sprite pitch (in case width is bigger than use width value), this parameter is stricy - once used values is changed on next one @, it is equivalent to:
ForceCursorSizes "32x32@64,64x64@64,128x128@512,256x256@512,512x512@512"

For now, I added the option to force a particular width and height, and pitch is determined automatically.

stefan11111 · 2025-08-21T19:38:08Z

@stefan11111

I also added an option for setting the hw cursor fallback size via xorg.conf.
Many cards don't support SIZE_HINTS, and for those, we always use the fallback.
The way this patch is now, for those cards, we either use the 64x64 fallback, or what the users puts in xorg.conf.
Is this really a good idea, or should we keep the old probe for checking the minimum size on these cards? I'm not sure that it's a good idea to put it on the user to tell us what size hw cursor they should use, I think we should probe the cursor size in the old way on these cards.
What are your thoughts?

I think we should:
1. Make great efforts to determine the cursor size automatically.

2. If this is impossible then we should fallback on a sensible default.

3. If this default does not work everywhere then we should give the user the option to set it to whatever works for him.

I added back the old probe.
Now, we do all we can to automatically find the cursor size we should use, and we also allow the user to force a particular size if they need or want.

hw/xfree86/drivers/video/modesetting/modesetting.man

metux · 2025-08-22T11:36:56Z

So you mean the cursor plane, right?

Yes, that would be a more high level term ;-)

From Xserver PoW, there are HW cursors, which the HW automatically paints - Xserver only needs to update the buffer when the cursor image actually changes. Traditionally that's done in the scanout unit, but that's really HW specific.

As I understand it, there are 3 types of drm planes, primary, cursor and overlay.

Yes. in DRM terminology, the overlays are usually for things like HW colorspace transformation (eg. additional YUV images from a video). How much they're different from the other types is really HW specific, theoretically they could even be all the same on some HW.

The cursor plane, which gets rendered by the gpu on top of all the planes, at the current cursor position.

On modern GPUs, yes. Older HW does that inside the scanout unit, much like the old 6560 did (interleaving two bit streams right before the encoder). That's why the distinction between the plane types, and Xserver terminology "HW cursor" still matters.

And overlay planes, which are rendered between the primary and cursor planes, and are used for hardware alpha blending.

Note that X11, overlays theoretically could also go to different outputs. That's one of the things Xv is for. Strange historical stuff like a video output going through separate connector, which then finally might become mixed into the main image somewhere in the physical screen setup (analog). No idea whether KMS ever supported such stuff.

Maybe we should make some dictionary, to prevent any babylonian confusion here ;-)

I meant that we could create a cursor buffer with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE, and use that instead of the dumb buffers we use now.

Yes, but then we're depending on gbm (IMHO we currently depending on it when glamor enabled).

As you said, we already use libgbm.

Haven't checked yet whether that's also the case w/o glamor. If it is, then I have no objections.
I don't like the idea of making modesetting hard-depend on GL and gbm if it doesn't already.

Yes, but I don't think we need to explicitly mark the buffer as scanout capable. From what I saw in wayland code, cursor buffers are allocated without GBM_BO_USE_SCANOUT, and only with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE

Careful with taking Wayland as reference - they're depending on modern GPUs. Older HW which might do the HW cursors in the scanout (instead of GPU/render) isn't practically supported by Wayland at all (those could only operate on dumb framebuffer, w/o any HW acceleration at all).

The cursor buffer does get updated frequently, every time the mouse hovers over a text window, for example.

That would be soft-cursors, eg. on a really dumb framebuffer.

The cursor buffer has to be painted whenever the glyph changes. Otherwise, how would the gpu know that we want the cursor image to change?

Yes, the cursor buffer needs to be updated (or switched to another one), when the cursor image changes (for whatever reason). But it doesn't need to be repainted (blitted onto) the main image, eg. when it moves - didn't count that as "frequently" ;-)
(maybe i've just got your last sentence wrong).

stefan11111 · 2025-08-22T12:15:08Z

Haven't checked yet whether that's also the case w/o glamor. If it is, then I have no objections. I don't like the idea of making modesetting hard-depend on GL and gbm if it doesn't already.

In that case, we could try to support both dumb bo's and gbm bo's, and use gbm bo's if libgbm is available.

Yes, but I don't think we need to explicitly mark the buffer as scanout capable. From what I saw in wayland code, cursor buffers are allocated without GBM_BO_USE_SCANOUT, and only with GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE

Careful with taking Wayland as reference - they're depending on modern GPUs. Older HW which might do the HW cursors in the scanout (instead of GPU/render) isn't practically supported by Wayland at all (those could only operate on dumb framebuffer, w/o any HW acceleration at all).

What do you mean by 'in the scanout'?

The cursor buffer does get updated frequently, every time the mouse hovers over a text window, for example.

That would be soft-cursors, eg. on a really dumb framebuffer.

The cursor buffer has to be painted whenever the glyph changes. Otherwise, how would the gpu know that we want the cursor image to change?

Yes, the cursor buffer needs to be updated (or switched to another one), when the cursor image changes (for whatever reason).

Yes, that's what I meant by painting the cursor, updating the cursor buffer, not rendering it at the correct position.

For gbm vs dumb, I saw some discussion here:
https://docs.nvidia.com/jetson/archives/r36.4/DeveloperGuide/SD/WindowingSystems/WestonWayland.html
https://xeechou.net/posts/drm-backend-ii/
https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2520

metux · 2025-08-22T14:55:36Z

What do you mean by 'in the scanout'?

The piece of logic that's periodically reading out the framebuffer(s) and generating the data stream for the encoder. Then the encoder will make the actually output signal for the display (eg. VGA, PAL, HDMI, DP, ...). Back in the old times (when display signals been analog), this had been called "ramdac" (you'll still find that name in the Xserver codebase).
That's also the piece

Sometimes those terms become smeared a bit (eg. some people calling the whole graphics cards the "GPU"), because there're tons of different practical configurations and often only several pieces of it are CPU-accessible. For completeness: we haven't even talked about PHYs - but except for obscure embedded systems you probably won't ever get in touch with them (and even then, it's probably deeply buried in the kernel drivers).

Yeah, graphics HW isn't entirely trivial :o

(gladly, today, we don't need to sync CPU with display clocks anymore, like back on C64 when we did "impossible" things like painting over the screen border ;-))

Yes, that's what I meant by painting the cursor, updating the cursor buffer, not rendering it at the correct position.

Ah, okay, we've been talking about different things.

For gbm vs dumb, I saw some discussion here:
https://docs.nvidia.com/jetson/archives/r36.4/DeveloperGuide/SD/WindowingSystems/WestonWayland.html

This is assuming the kernel driver is recent enough to support gbm (and that this one is working on all cards we wanna support).
I really have no idea, how the situation is on non-Linux platforms (eg. Illumos, *BSD).

https://xeechou.net/posts/drm-backend-ii/

No mention of cursors at all. Probably just from application/client's PoV, who rarely deal with cursors at all.

https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2520

Unfortunately, not mentioned which of the various Xilinx designs they're actually talking about, and how it's working under the hood. Gutt feeling: they've got a simple multi-buffer scanout (depending on current pixel position, the cursor pixel is either coming from translated vram cell or default to all-transparent, and then alpha-blended (eg. XOR) with the main framebuffer's current pixel) ... just like HW cursors were done in the old times. And depending on the actual machine's physical memory/bus layout, that sprite buffer might or might not be in main RAM, or possibly constraint to certain regions, etc.
eg. IIRC, the old 6559 had a fixed aperture (fewer than 16 address lines poulated) and sprites could only be mapped in certain rasters - saving a lot unncessary gates this way. those concepts are still frequently found in today's embedded designs.

Bottom line: we need to be careful with our assumptions, treat hw cursors as special entities, let the underlying drivers find out how exactly to deal with them.

metux · 2025-08-22T15:01:20Z

Since the queue is a bit bigger and we already accumulated a lot of discussion here, shall we perhaps split it into several ones ? Haven't read each patch one by one yet, but feels like several are disjunct and could land separately.

stefan11111 · 2025-08-22T16:06:03Z

Since the queue is a bit bigger and we already accumulated a lot of discussion here, shall we perhaps split it into several ones ? Haven't read each patch one by one yet, but feels like several are disjunct and could land separately.

Could you merge @notbabaisyou 's pr, and then I'll rebase this on top of it, and go from there?

stefan11111 · 2025-08-22T17:42:39Z

I added support for gbm cursor bo's.
It's a poc right now, but it should work.
The driver now first tries gbm buffers, and if that doesn't work it falls back to dumb buffers.

…rted by all CRTCs. Signed-off-by: stefan11111 <[email protected]>

On many cards, SIZE_HINTS isn't implemented, so the hardware cursor used is the fallback one. With this, we allow the user to specify what size the hardware cursor should have, instead of forcing 64x64. Signed-off-by: stefan11111 <[email protected]>

On most cards, SIZE_HINTS isn't available. Without this, most users would have to set the fallback cursor size themselves, or rely on the 64x64 default. Signed-off-by: stefan11111 <[email protected]>

Now that we probe for the cursor size automatically, even if SIZE_HINTS isn't available, it isn't very useful to set the size of the fallback cursor. However, it might still be useful to force a particular cursor size via xorg.conf. For example, if on a system the probes fail, or if the user wants a particular cursor size, that is higher that the minimum size. Signed-off-by: stefan11111 <[email protected]>

We first try to allocate a gbm buffer for the hw cursor. If that fails, we fall back to dumb buffers. Signed-off-by: stefan11111 <[email protected]>

Signed-off-by: stefan11111 <[email protected]>

metux · 2025-09-26T15:18:49Z

@stefan11111 what's the status on this one ?

stefan11111 · 2025-09-26T15:28:18Z

I'm pulling commits from this one into separate pr's, and improving them as I do that.
I'll probably close this one eventually.

github-actions · 2025-10-21T08:51:19Z

Merge Conflict found

stefan11111 requested review from algrid, b-aaz, callmetango, cepelinas9000 and metux August 20, 2025 17:53

notbabaisyou reviewed Aug 20, 2025

View reviewed changes

callmetango added hardware-cursor Hardware cursor related bug Something isn't working labels Aug 20, 2025

cepelinas9000 reviewed Aug 20, 2025

View reviewed changes

hw/xfree86/drivers/video/modesetting/drmmode_display.c Outdated Show resolved Hide resolved

stefan11111 force-pushed the fix-hw-cursor-v2 branch from 5203b6c to f28eaa4 Compare August 21, 2025 12:07

cepelinas9000 reviewed Aug 21, 2025

View reviewed changes

hw/xfree86/drivers/video/modesetting/modesetting.man Show resolved Hide resolved

stefan11111 force-pushed the fix-hw-cursor-v2 branch from a74f39e to 8bcd1dc Compare August 21, 2025 19:52

X11Libre deleted a comment from github-actions bot Sep 11, 2025

stefan11111 force-pushed the fix-hw-cursor-v2 branch 2 times, most recently from 41c4636 to 5fdc765 Compare September 15, 2025 10:52

X11Libre deleted a comment from github-actions bot Sep 15, 2025

stefan11111 added 6 commits September 20, 2025 15:59

modesetting: Initialize the cursor image with the smallest size suppo…

1213690

…rted by all CRTCs. Signed-off-by: stefan11111 <[email protected]>

modesetting: probe using the old method is SIZE_HINTS isn't available

71ae1a4

On most cards, SIZE_HINTS isn't available. Without this, most users would have to set the fallback cursor size themselves, or rely on the 64x64 default. Signed-off-by: stefan11111 <[email protected]>

modesetting: add support for gbm cursor buffers

6f0f4e1

We first try to allocate a gbm buffer for the hw cursor. If that fails, we fall back to dumb buffers. Signed-off-by: stefan11111 <[email protected]>

modesetting: always try to map gbm bo's when requested

ba22b40

Signed-off-by: stefan11111 <[email protected]>

stefan11111 force-pushed the fix-hw-cursor-v2 branch from 5fdc765 to ba22b40 Compare September 20, 2025 12:59

Uh oh!

modesetting: Fix hw cursor v2 #734

Are you sure you want to change the base?

modesetting: Fix hw cursor v2 #734

Conversation

stefan11111 commented Aug 20, 2025

Uh oh!

stefan11111 commented Aug 20, 2025

Uh oh!

notbabaisyou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

notbabaisyou Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

stefan11111 Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stefan11111 commented Aug 20, 2025

Uh oh!

stefan11111 commented Aug 20, 2025

Uh oh!

Uh oh!

metux commented Aug 21, 2025

Uh oh!

stefan11111 commented Aug 21, 2025

Uh oh!

stefan11111 commented Aug 21, 2025

Uh oh!

metux commented Aug 21, 2025

Uh oh!

callmetango commented Aug 21, 2025

Uh oh!

stefan11111 commented Aug 21, 2025

Uh oh!

cepelinas9000 commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefan11111 commented Aug 21, 2025

Uh oh!

stefan11111 commented Aug 21, 2025

Uh oh!

Uh oh!

metux commented Aug 22, 2025

Uh oh!

stefan11111 commented Aug 22, 2025

Uh oh!

metux commented Aug 22, 2025

Uh oh!

metux commented Aug 22, 2025

Uh oh!

stefan11111 commented Aug 22, 2025

Uh oh!

stefan11111 commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metux commented Sep 26, 2025

Uh oh!

stefan11111 commented Sep 26, 2025

Uh oh!

github-actions bot commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cepelinas9000 commented Aug 21, 2025 •

edited

Loading

stefan11111 commented Aug 22, 2025 •

edited

Loading