On 25.12.2015 03:06, Keith Packard wrote: > Michel Dänzer <mic...@daenzer.net> writes: > >> From: Michel Dänzer <michel.daen...@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 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
