Re: [PATCH v3 5/8] drm/i915: Add the display switch logic for vGPU in i915 driver

Daniel Vetter
2014-12-15T08:15:54Z

On Thu, Nov 13, 2014 at 08:02:46PM +0800, Yu Zhang wrote:
> Display switch logic is added to notify the host side that
> current vGPU have a valid surface to show. It does so by
> writing the display_ready field in PV INFO page, and then
> will be handled in the host side. This is useful to avoid
> trickiness when the VM's framebuffer is being accessed in
> the middle of VM modesetting, e.g. compositing the framebuffer
> in the host side.
>
> v2:
> - move the notification code outside the 'else' in load sequence
> - remove the notification code in intel_crtc_set_config()
>
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 11 +++++++++++
> drivers/gpu/drm/i915/i915_vgpu.h | 7 +++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 9a73533..06daa5c 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -36,6 +36,7 @@
> #include "intel_drv.h"
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
> +#include "i915_vgpu.h"
> #include "i915_trace.h"
> #include <linux/pci.h>
> #include <linux/console.h>
> @@ -1780,6 +1781,16 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> dev_priv->ums.mm_suspended = 1;
> }
>
> + if (intel_vgpu_active(dev)) {
> + /*
> + * Notify a valid surface after modesetting,
> + * when running inside a VM.
> + */
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
> + }
> +
> i915_setup_sysfs(dev);
>
> if (INTEL_INFO(dev)->num_pipes) {
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index f538b18..9d35fb8 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -80,6 +80,13 @@ struct vgt_if {
> #define vgtif_reg(x) \
> (VGT_PVINFO_PAGE + (long)&((struct vgt_if *) NULL)->x)
>
> +/* vGPU display status to be used by the host side */
> +enum vgt_display_status {
> + VGT_DRV_DISPLAY_NOT_READY = 0,
> + VGT_DRV_DISPLAY_READY, /* ready for display switch */
> +};

Nitpick: Since this is a virtual register I prefer we use plain #defines
with explicit codes for everything instead of enums. Makes it clearer
which values are used and all that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch