[PATCH xserver 1/5] xfree86/modes: Refactor xf86_use_hw_cursor_argb to use xf86_use_hw_cursor

[Michel Dänzer]

On 25.12.2015 03:06, Keith Packard wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> 
>> From: Michel Dänzer <michel.daenzer at amd.com>
>>
>> This reduces code duplication.
>>
>> One side effect of this change is that xf86_config->cursor will no longer
>> be updated for cursors which cannot use the HW cursor.
> 
> That means we'll be holding on to the last HW cursor in use on the
> screen 'forever'; not a big deal, but doesn't seem great.

Fixed in v2 of patch 1 of this series. If you or anyone could review
that (and possibly v2 of patch 2, though that's mostly identical with
v1, which you reviewed), I'll send a pull request for patches 1-3 & 5.


> The referencing counting was added to xf86Cursors.c (by me) back in 2007
> to avoid having the cursor get freed at the wrong time.
> 
> xf86_config->cursor is read in only two places: xf86_reload_cursors and
> xf86_set_cursor_colors. xf86_set_cursor_colors is only called from the
> ramdac cursor code, and is never called when a SW cursor is
> displayed. However, xf86_reload_cursors is currently called from drivers
> during modesetting, and so may well be called when a SW cursor is
> displayed.
> 
> Reading the code, I can't see any reason we can't just use
> ScreenPriv->cursor instead of saving another reference in this code; any
> time we're using the HW cursor, that will be correct, and anytime we're
> not using the HW cursor, we won't be doing anything with it. This will
> let unused cursors get freed sooner, and eliminate this twisty bit of
> extra code here.
> 
> This is untested, but 'should' work.

It seems to work fine in my quick testing. However, I'm not sure it's
worth it, given that v2 of patch 1 fixes holding on to the last HW
cursor indefinitely, and given the churn it would cause for external
drivers.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer