Hi Am 18.02.21 um 12:50 schrieb Gerd Hoffmann:
Hi,I'm still trying to wrap my head around the qxl cursor code. Getting vmap out of the commit tail is good, but I feel like this isn't going in the right direction overall. In ast, these helper functions were only good when converting the drvier to atomic modesetting. So I removed them in the latst patchset and did all the updates in the plane helpers directly.I see the helper functions more as a way to get some structure into the code flow. The callbacks are easier to read if they just call helper functions for stuff which needs more than a handful lines of code (patch 9/11 exists for the same reason). The helpers also make it easier move work from one callback to another, but that is just a useful side-effect. I had considered making that two separate patches, one factor out code into functions and one moving the calls. Turned out to not be that easy though, because the old qxl_cursor_atomic_update() code was a rather hairy mix of qxl_create_cursor() + qxl_primary_apply_cursor() + qxl_primary_move_cursor().For cursor_bo itself, it seems to be transitional state that is only used during the plane update and crtc update . It should probably be stored in a plane-state structure. Some of the primary plane's functions seem to deal with cursor handling. What's the role of the primary plane in cursor handling?It's a quirk. The qxl device will forget the cursor state on qxl_io_create_primary(), so I have to remember the cursor state and re-establish it by calling qxl_primary_apply_cursor() again. So I'm not sure sticking this into plane state would work. Because of the quirk this is more than just a handover from prepare to commit.For now, I suggest to merge patch 1 to 8 and 11; and move the cursor patches into a new patchset.I can merge 1-8, but 11 has to wait until the cursor is sorted. There is a reason why 11 is last in the series ;)I'd like ot hear Daniel's opinion on this. Do you have further plans here?Well. I suspect I could easily spend a month cleaning up and party redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). I'm not sure I'll find the time to actually do that anytime soon. I have plenty of other stuff on my TODO list, and given that the world is transitioning to virtio-gpu the priority for qxl isn't that high.
Well, in that case: Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> for patches 9 and 10. Having the vmap calls fixed is at least worth it. Best regards Thomas
So, no, I have no short-term plans for qxl beyond fixing pins + reservations + lockdep. take care, Gerd _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
