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
