Re: [PATCH 0/4] vkms: Switch to shadow-buffered plane state

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 06, 2021 at 06:59:07AM +0200, Thomas Zimmermann wrote:
> Am 05.07.21 um 16:20 schrieb Daniel Vetter:
> > On Mon, Jul 05, 2021 at 12:05:28PM +0200, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 05.07.21 um 11:27 schrieb Daniel Vetter:
> > > > On Mon, Jul 05, 2021 at 09:46:29AM +0200, Thomas Zimmermann wrote:
> > > > > Vkms copies each plane's framebuffer into the output buffer; essentially
> > > > > using a shadow buffer. DRM provides struct drm_shadow_plane_state, which
> > > > > handles the details of mapping/unmapping shadow buffers into memory for
> > > > > active planes.
> > > > > 
> > > > > Convert vkms to the helpers. Makes vkms use shared code and gives more
> > > > > test exposure to shadow-plane helpers.
> > > > > 
> > > > > Thomas Zimmermann (4):
> > > > >     drm/gem: Export implementation of shadow-plane helpers
> > > > >     drm/vkms: Inherit plane state from struct drm_shadow_plane_state
> > > > >     drm/vkms: Let shadow-plane helpers prepare the plane's FB
> > > > >     drm/vkms: Use dma-buf mapping from shadow-plane state for composing
> > > > 
> > > > So I think right now this fits, but I think it'll mismit going forward: We
> > > > don't really have a shadow-plane that we then toss to the hw, it's a
> > > > shadow-crtc-area. Right now there's no difference, because we don't
> > > > support positioning/scaling the primary plane. But that's all kinda stuff
> > > > that's on the table.
> > > > 
> > > > But conceptually at least the compositioning buffer should bet part of the
> > > > crtc, not of the primary plane.
> > > > 
> > > > So not sure what to do, but also coffee hasn't kicked in yet, so maybe I'm
> > > > just confused.
> > > 
> > > I'm not sure if I understand your concern. Can you elaborate? The
> > > compositing output buffer is not affected by this patchset. Only the input
> > > frambuffers of the planes. Those are shadow buffers. AFAICT the composer
> > > code memcpy's the primary plane and then blends the other planes on top.
> > > Supporting transformation of the primary plane doesn't really change much
> > > wrt to the vmaping of input fbs.
> > 
> > Yeah that's the current implementation, because that's easier. But
> > fundamentally we don't need a copy of the input shadow plane, we need a
> > scratch area that's sized for the crtc.
> 
> I stll don't understand. Are you talking about the whole patchset or just
> patch 4? Because if you want to do anything with vkms planes, you have to
> vmap the framebuffer BOs into memory. (right?) That's all that shadow-buffer
> planes do. Patches 1 to 3 have nothing to do with memcpy. Admittedly, Patch
> 4 does some minor refactoring, but only because it became a low-hanging
> fruit.

Oh man I need to hand in my patch reading license last few days, this aint
the first one :-(

Yeah looks all good to me, and makes total sense. Maybe I'll leave
detailed review to Melissa since I got this all wrong this much.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch