Re: [PATCH 6/8] drm/mgag200: Rewrite cursor handling

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 12, 2019 at 09:27:21AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.06.19 um 17:33 schrieb Daniel Vetter:
> > On Tue, Jun 11, 2019 at 2:32 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 05.06.19 um 11:58 schrieb Gerd Hoffmann:
> >>> On Tue, Jun 04, 2019 at 05:41:59PM +0200, Thomas Zimmermann wrote:
> >>>> The cursor handling in mgag200 is complicated to understand. It touches a
> >>>> number of different BOs, but doesn't really use all of them.
> >>>>
> >>>> Rewriting the cursor update reduces the amount of cursor state. There are
> >>>> two BOs for double-buffered HW updates. The source BO updates the one that
> >>>> is currently not displayed and then switches buffers. Explicit BO locking
> >>>> has been removed from the code. BOs are simply pinned and unpinned in video
> >>>> RAM.
> >>>
> >>> Cursors are not that big after all, so maybe pin the two BOs for
> >>> double-buffering permanently in vram to simplify things further?
> >>>
> >>> Also factoring out the code which updates the two BOs to a separate
> >>> function should help making the code more readable.
> >>
> >> The cursor handling in the ast driver is similar, but uses one single BO
> >> to hold both cursor buffers. I'm thinking about how to turn both
> >> implementations into a generic helper for legacy cursors (i.e., low
> >> number of colors or palette). This would also be helpful for my work on
> >> fbdev porting.
> >>
> >> One idea is to adapt deferred I/O. DRM would expose an ARGB shadow
> >> buffer to userspace and let the mmap implementation update the HW
> >> buffers (including dithering, palette setup, etc.).
> > 
> > No mmap games needed with kms, we expect userspace to give us an ioctl
> > call in all cases. fbdev is the legacy horrors that works differently.
> 
> Thanks for clarifying this. Conversion should be much easier this way. I
> saw the dirty callback and the DIRTYFB ioctl, but I don't saw anything
> in Weston that calls it. So I assumed that it's obsolete or optional.

It's not optional nor obsolete, but you only need to call dirtyfb if you
do frontbuffer rendering. Which weston doesnt do. As long as you pageflip,
the pageflip will make sure the buffer contents gets updated.
-Daniel

> 
> Best regards
> Thomas
> 
> > So for cursors, assuming you have universal cursors, you just need to
> > update the shadowed copy in the prepare_fb plane hook. For
> > non-universal plane drivers you need to do that somewhere in your
> > set/move_cursor hooks (I think both of them). Aside: For non-cursors
> > there's also the dirtyfb ioctl, which serves the same purpose.
> > 
> > Cheers, Daniel
> > 
> >>
> >> Best regards
> >> Thomas
> >>
> >>> But even as-is the patch is a step into the right direction.
> >>>
> >>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> >>>
> >>> cheers,
> >>>   Gerd
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> >> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> >> HRB 21284 (AG Nürnberg)
> >>
> > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel