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

[Zhiyuan Lv <zhiyuan.lv@intel.com>]

Hi Daniel,

On Fri, Sep 19, 2014 at 06:09:37PM +0200, Daniel Vetter wrote:
> On Sat, Sep 20, 2014 at 02:47:05AM +0800, Jike Song wrote:
> > From: Yu Zhang <yu.c.zhang@intel.com>
> > 
> > Display switch logic is added to notify the vgt mediator 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 vgt mediator. 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
> > 
> > Signed-off-by: Yu Zhang <yu.c.zhang@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      |  8 ++++++++
> >  drivers/gpu/drm/i915/i915_reg.h      |  7 +++++++
> >  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
> >  3 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index bb4ad52..d9462e1 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1790,6 +1790,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >  	} else {
> >  		/* Start out suspended in ums mode. */
> >  		dev_priv->ums.mm_suspended = 1;
> > +		if (intel_vgpu_active(dev)) {
> > +			/*
> > +			 * Notify a valid surface after modesetting,
> > +			 * when running inside a VM.
> > +			 */
> > +			I915_WRITE(vgt_info_off(display_ready),
> > +				VGT_DRV_DISPLAY_READY);
> > +		}
> >  	}
> >  
> >  	i915_setup_sysfs(dev);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a70f12e..38d606c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6673,5 +6673,12 @@ enum punit_power_well {
> >  #define vgt_info_off(x) \
> >  	(VGT_PVINFO_PAGE + (long)&((struct vgt_if *) NULL)->x)
> >  
> > +/*
> > + * The information set by the guest gfx driver, through the display_ready field
> > + */
> > +enum vgt_display_status {
> > +	VGT_DRV_DISPLAY_NOT_READY = 0,
> > +	VGT_DRV_DISPLAY_READY,	/* ready for display switch */
> > +};
> >  
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b78f00a..3af9259 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11642,6 +11642,16 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >  			intel_modeset_check_state(set->crtc->dev);
> >  	}
> >  
> > +	if (intel_vgpu_active(dev)) {
> > +		/*
> > +		 * Notify a valid surface after modesetting,
> > +		 * when running inside a VM.
> > +		 */
> > +		struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > +		I915_WRITE(vgt_info_off(display_ready), VGT_DRV_DISPLAY_READY);
> > +	}
> 
> Since very shortly we now have the frontbuffer tracking support code.
> Should we move it there? See intel_frontbuffer_flush&flip.

Thank you very much for the comments and sorry for the delayed reply!

For virtual machines we only need such notification once for guest system
booting, would that be too heavy to add code into the flush&flip function,
consider that they are to be called many times?

Going forward, we want to use the same "display_ready" for i915 running in host
machines so that we can capture display status changes. Would that be good to
keep the code in "intel_crtc_set_config"? Appreciate your inputs. Thanks!

Regards,
-Zhiyuan

> -Daniel
> 
> > +
> >  	if (ret) {
> >  		DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
> >  			      set->crtc->base.id, ret);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx