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

From: Daniel Vetter <daniel@ffwll.ch>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: "Maxime Ripard" <maxime.ripard@bootlin.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Dave Airlie" <airlied@redhat.com>, "Sean Paul" <sean@poorly.run>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH 6/8] drm/mgag200: Rewrite cursor handling
Date: Tue, 11 Jun 2019 17:33:01 +0200	[thread overview]
Message-ID: <CAKMK7uEwoCFWxtD-ktZSxjhS2TyOEoRovBX18gk3doGyCCrS2Q@mail.gmail.com> (raw)
In-Reply-To: <81937cd8-1b1f-007b-97e3-18a3b586b87f@suse.de>

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.

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)
>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel