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@xxxxxxxxx> 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 >