On 04/07, Pekka Paalanen wrote: > On Mon, 5 Apr 2021 11:41:50 +0530 > Sumera Priyadarsini <sylphrenadin@xxxxxxxxx> wrote: > > > This patchset adds support for emulating virtual hardware with VKMS. > > The virtual hardware mode can be enabled by using the following command > > while loading the module: > > sudo modprobe vkms enable_virtual_hw=1 > > Hi, > > every time I see this cover letter subject, I start wondering "what is > this virtual hardware module, yet another one?" and then I read the > cover letter and realise it is about adding an option to VKMS. > > The next time you revise this series, could you perhaps clarify the > subject? +1 > > The idea of having a mode where VKMS behaves like a virtual hardware > driver is good, IMO. I do think "vblank-less mode" describes it better > though, because I would assume things like USB display drivers to work > like this too, and VKMS is already a virtual driver anyway. > > To clarify, as a userspace programmer what I would expect "vblank-less > mode" to be is that the DRM driver completes pageflips and modesets at > arbitrary times, perhaps always immediately or perhaps with a variable > delay that depends on how much processing is needed for the update. > Also vblank events do not fire and vblank counters do not advance. Is > this correct? > yes. And I think this description should be clear in both the cover letter and also the commit message of the patch that add the module option to vkms. > > Thanks, > pq > > > > > The first patch is prep work for adding virtual_hw mode and refactors > > the plane composition in vkms by adding a helper function vkms_composer_common() > > which can be used for both vblank mode and virtual mode. > > > > The second patch adds virtual hardware support as a module option. It > > adds new atomic helper functions for the virtual mode > > and modifies the existing atomic helpers for usage by the vblank mode > > This gives us two sets of drm_crtc_helper_funcs struct for both modes, > > making the code flow cleaner and easier to debug. > > > > This patchset has been tested with the igt tests- kms_writeback, kms_atomic, > > kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for > > subtests related to crc reads and skips tests that rely on vertical > > blanking. This patchset must be tested after incorporating the > > igt-tests patch: https://lists.freedesktop.org/archives/igt-dev/2021-February/029355.html Sumera, Thanks for your patches. In addition to Pekka's comments, consider what I comment in each patch of the series for a next version. Best regards, Melissa > > > > Sumera Priyadarsini (2): > > drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode > > drm/vkms: Add support for virtual hardware mode > > > > drivers/gpu/drm/vkms/vkms_composer.c | 88 +++++++++++++++++----------- > > drivers/gpu/drm/vkms/vkms_crtc.c | 51 +++++++++++----- > > drivers/gpu/drm/vkms/vkms_drv.c | 18 ++++-- > > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++ > > 4 files changed, 109 insertions(+), 52 deletions(-) > > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel