Hi Am 12.07.21 um 16:26 schrieb Sumera Priyadarsini: > On Mon, Jul 12, 2021 at 6:53 PM Thomas Zimmermann wrote: >> >> Hi >> >> Am 12.07.21 um 13:56 schrieb Sumera Priyadarsini: >>> On Mon, Jul 5, 2021 at 1:16 PM 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 >>>> >>>> drivers/gpu/drm/drm_gem_atomic_helper.c | 55 ++++++++++++++++++++++-- >>>> drivers/gpu/drm/vkms/vkms_composer.c | 26 ++++++----- >>>> drivers/gpu/drm/vkms/vkms_drv.h | 6 ++- >>>> drivers/gpu/drm/vkms/vkms_plane.c | 57 ++++++------------------- >>>> include/drm/drm_gem_atomic_helper.h | 6 +++ >>>> 5 files changed, 86 insertions(+), 64 deletions(-) >>>> >>>> >>>> base-commit: 3d3b5479895dd6dd133571ded4318adf595708ba >>>> -- >>>> 2.32.0 >>>> >>> Hi, >>> >>> Thanks for the patches. The switch to shadow-plane helpers also solved >>> a bug that was causing a kernel >>> panic during some IGT kms_flip subtests on the vkms virtual hw patch. >> >> Melissa mention something like that as well and I don't really >> understand. Patch 3 removes an error message from the code, but is the >> actual bug also gone? > > Yes, I think so. Earlier, while testing the vkms virtual hw patch, the > tests were > not just failing, but the vmap fail also preceeded a page fault which required a > whole restart. Check these logs around line 303: > https://pastebin.pl/view/03b750be. > With the help of your log, I can see what's happening. The current vkms code reports an error in vmap, but does nothing about it. [1] So later during the commit, it operates with a bogus value for vaddr. The shared helper returns the error into the atomic modesetting machinery, [2] which then aborts the commit. It never gets to the point of using an invalid address. So no kernel panic occurs. > I could be wrong but I think if the same bug was still present, then > the kernel panic > would also happen even if the error message was not being returned. I'm pretty sure the vmap issue is still there. But as the shared code handles it correctly without a notice to the kernel log; and it doesn't crash the kernel any longer. But the vmap problem is caused by some other factor unrelated to vkms. Booting the test kernel with drm.debug=0x1ff on the kernel command line would probably turn up some sort of error message. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vkms/vkms_plane.c#L166 [2] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_gem_atomic_helper.c#L299 > > Cheers, > Sumera > >> >> There's little difference between vkms' original code and the shared >> helper; except for the order of operations in prepare_fb. The shared >> helper synchronizes fences before mapping; vkms mapped first. >> >> (Maybe the shared helper should warn about failed vmaps as well. But >> that's for another patch.) >> >> Best regards >> Thomas >> >>> >>> Tested-by: Sumera Priyadarsini >>> >>> Cheers, >>> Sumera >>> >> >> -- >> 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 >> -- 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