Re: [PATCH 1/5] drm/vmwgfx: Implement virtual kms

2024-04-05 Thread Zack Rusin 
On Fri, Apr 5, 2024 at 5:53 PM Maaz Mombasawala
 wrote:
>
> On 4/2/24 16:28, Zack Rusin wrote:
> >
> > @@ -541,6 +518,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, 
> > unsigned unit)
> >dev_priv->implicit_placement_property,
> >1);
> >
> > + vmw_du_init(>base);
> > +
> >   return 0;
> >
> >  err_free_unregister:
>
> > @@ -905,6 +900,9 @@ static int vmw_sou_init(struct vmw_private *dev_priv, 
> > unsigned unit)
> >  dev->mode_config.suggested_x_property, 0);
> >   drm_object_attach_property(>base,
> >  dev->mode_config.suggested_y_property, 0);
> > +
> > + vmw_du_init(>base);
> > +
> >   return 0;
> >
> >  err_free_unregister:
>
> > @@ -1575,6 +1576,9 @@ static int vmw_stdu_init(struct vmw_private 
> > *dev_priv, unsigned unit)
> >  dev->mode_config.suggested_x_property, 0);
> >   drm_object_attach_property(>base,
> >  dev->mode_config.suggested_y_property, 0);
> > +
> > + vmw_du_init(>base);
> > +
> >   return 0;
> >
> >  err_free_unregister:
>
> Shouldn't calls to vmw_du_init() be behind an if(vkms_enabled) condition?

So the vmw_du_init is supposed to initialize the base, so that's
unconditional. To match the unconditional vmw_du_cleanup. There's an
argument to be made whether both of those should unconditionally  call
vmw_vkms_crtc_init and vmw_vkms_crtc_cleanup. My opinion was that
they're not doing anything costly and just initialize members and
having the members of vmw_display_unit initialized whether vkms is
enabled or not still makes sense.

z

Re: [PATCH 1/5] drm/vmwgfx: Implement virtual kms

2024-04-05 Thread Maaz Mombasawala 
On 4/2/24 16:28, Zack Rusin wrote:
>  
> @@ -541,6 +518,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, 
> unsigned unit)
>dev_priv->implicit_placement_property,
>1);
>  
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

> @@ -905,6 +900,9 @@ static int vmw_sou_init(struct vmw_private *dev_priv, 
> unsigned unit)
>  dev->mode_config.suggested_x_property, 0);
>   drm_object_attach_property(>base,
>  dev->mode_config.suggested_y_property, 0);
> +
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

> @@ -1575,6 +1576,9 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, 
> unsigned unit)
>  dev->mode_config.suggested_x_property, 0);
>   drm_object_attach_property(>base,
>  dev->mode_config.suggested_y_property, 0);
> +
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

Shouldn't calls to vmw_du_init() be behind an if(vkms_enabled) condition?

Thanks,

Maaz Mombasawala

[PATCH 1/5] drm/vmwgfx: Implement virtual kms

2024-04-02 Thread Zack Rusin 
By default vmwgfx doesn't support vblanking or crc generation which
makes it impossible to use various IGT tests to validate vmwgfx.
Implement virtual kernel mode setting, which is mainly related to
simulated vblank support.

Code is very similar to amd's vkms and the vkms module itself, except
that it's integrated with vmwgfx three different output technologies -
legacy, screen object and screen targets.

Make IGT's kms_vblank pass on vmwgfx and allows a lot of other IGT
tests to run with vmwgfx.

Support for vkms needs to be manually enabled by adding:
guestinfo.vmwgfx.vkms_enable = "TRUE"
somewhere in the vmx file, otherwise it's off by default.

Signed-off-by: Zack Rusin 
---
 drivers/gpu/drm/vmwgfx/Makefile  |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   3 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   2 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  15 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |   9 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |  39 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  28 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  22 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 193 +++
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.h |  53 
 10 files changed, 302 insertions(+), 64 deletions(-)
 create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
 create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.h

diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index e94479d9cd5b..46a4ab688a7f 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -10,6 +10,6 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o 
vmwgfx_drv.o \
vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
vmwgfx_devcaps.o ttm_object.o vmwgfx_system_manager.o \
-   vmwgfx_gem.o
+   vmwgfx_gem.o vmwgfx_vkms.o
 
 obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c7d90f96d16a..e34c48fd25d4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -32,6 +32,7 @@
 #include "vmwgfx_binding.h"
 #include "vmwgfx_devcaps.h"
 #include "vmwgfx_mksstat.h"
+#include "vmwgfx_vkms.h"
 #include "ttm_object.h"
 
 #include 
@@ -910,6 +911,8 @@ static int vmw_driver_load(struct vmw_private *dev_priv, 
u32 pci_id)
 "Please switch to a supported graphics device to 
avoid problems.");
}
 
+   vmw_vkms_init(dev_priv);
+
ret = vmw_dma_select_mode(dev_priv);
if (unlikely(ret != 0)) {
drm_info(_priv->drm,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 01f41fbb9c3b..4f5d7d13c4aa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -615,6 +615,8 @@ struct vmw_private {
 
uint32 *devcaps;
 
+   bool vkms_enabled;
+
/*
 * mksGuestStat instance-descriptor and pid arrays
 */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 09214f9339b2..e763cf0e6cfc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -27,6 +27,7 @@
 #include "vmwgfx_kms.h"
 
 #include "vmwgfx_bo.h"
+#include "vmwgfx_vkms.h"
 #include "vmw_surface_cache.h"
 
 #include 
@@ -37,9 +38,16 @@
 #include 
 #include 
 
+void vmw_du_init(struct vmw_display_unit *du)
+{
+   hrtimer_init(>vkms.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+   du->vkms.timer.function = _vkms_vblank_simulate;
+}
+
 void vmw_du_cleanup(struct vmw_display_unit *du)
 {
struct vmw_private *dev_priv = vmw_priv(du->primary.dev);
+   hrtimer_cancel(>vkms.timer);
drm_plane_cleanup(>primary);
if (vmw_cmd_supported(dev_priv))
drm_plane_cleanup(>cursor.base);
@@ -957,13 +965,6 @@ void vmw_du_crtc_atomic_begin(struct drm_crtc *crtc,
 {
 }
 
-
-void vmw_du_crtc_atomic_flush(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
-{
-}
-
-
 /**
  * vmw_du_crtc_duplicate_state - duplicate crtc state
  * @crtc: DRM crtc
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 4a2e3cac1c22..9e83a1553286 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -376,6 +376,12 @@ struct vmw_display_unit {
bool is_implicit;
int set_gui_x;
int set_gui_y;
+
+   struct {
+   struct hrtimer timer;
+   ktime_t period_ns;
+   struct drm_pending_vblank_event *event;
+   } vkms;
 };
 
 #define vmw_crtc_to_du(x) \
@@ -387,6 +393,7 @@ struct vmw_display_unit {
 /*
  * Shared display unit functions - vmwgfx_kms.c
  */
+void vmw_du_init(struct vmw_display_unit *du);
 void vmw_du_cleanup(struct vmw_display_unit *du);
 void