On Mon, Sep 29, 2014 at 02:31:17PM +0800, Zhiyuan Lv wrote:
> 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@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Signed-off-by: Jike Song <jike.song@xxxxxxxxx>
> > > Signed-off-by: Zhiyuan Lv <zhiyuan.lv@xxxxxxxxx>
> > > ---
> > > 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!
I guess the question is what exactly you want to signal to the hyporvisor
with this. I guess I need to dig a bit into the sourcecode for the
hyperviros part, and you need to make a really clear specification of when
guests should call this and what the hyporvisor must to in reaction of
this.
I don't think we'll have any issues with a bit of overhead in the
frontbuffer flip/flush functions, they're not called too often. Aside:
There's no nice kerneldoc for this stuff:
http://people.freedesktop.org/~danvet/drm/drmI915.html#idp54709056
Cheers, Daniel
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> > > 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@xxxxxxxxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
