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

[Melissa Wen]

On 07/06, Thomas Zimmermann wrote:
> Hi
> 
> Am 05.07.21 um 23:29 schrieb Melissa Wen:
> > On 07/05, Daniel Vetter wrote:
> > > 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.
> > 
> > Maybe I'm missing something, but I am not sure the relevance for vkms to
> > switch to shadow-buffered plane. (?)
> 
> It replaces the vkms code with shared code. Nothing else. For the shared
> shadow-buffer code it means more testing. If vkms ever supports color
> formats that use multiple planes, the new code will be ready.
Hi,

lgtm.

For debug history, can you please include in the description of the
third patch (shadow-plane helpers to prepare fb) that vmap failure is no
longer displayed in the execution some IGT kms_flip testcases?

For the series:
Reviewed-by: Melissa Wen <melissa.srw at gmail.com>

Thanks,
Melissa
> 
> Best regards
> Thomas
> 
> 
> -- 
> 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
>