[PATCH v2] drm/i915/gvt: Deliver guest cursor hotspot info

[Zhang, Tina]

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
> Sent: Wednesday, March 28, 2018 2:18 PM
> To: Zhang, Tina <tina.zhang at intel.com>
> Cc: Gerd Hoffmann <kraxel at redhat.com>; intel-gvt-dev at lists.freedesktop.org;
> Wang, Zhi A <zhi.a.wang at intel.com>
> Subject: Re: [PATCH v2] drm/i915/gvt: Deliver guest cursor hotspot info
> 
> On 2018.03.28 06:10:33 +0000, Zhang, Tina wrote:
> > > > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > index 9a471b0..ef1937b 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > > > @@ -229,12 +229,10 @@ static int vgpu_get_plane_info(struct
> > > > drm_device
> > > *dev,
> > > >  		info->x_pos = c.x_pos;
> > > >  		info->y_pos = c.y_pos;
> > > >
> > > > -		/* The invalid cursor hotspot value is delivered to host
> > > > -		 * until we find a way to get the cursor hotspot info of
> > > > -		 * guest OS.
> > > > -		 */
> > > > -		info->x_hot = UINT_MAX;
> > > > -		info->y_hot = UINT_MAX;
> > > > +
> > > > +		info->x_hot = c.x_hot;
> > > > +		info->y_hot = c.y_hot;
> > >
> > > Do we need sanity check on guest setting? and fallback to invalid
> > > value if check failed.
> > X_hot/y_hot are initialized with UINT_MAX, during
> > "populate_pvinfo_page(...)", and the values won't be changed until the
> related mmio in pv_info is updated by guest driver.
> > And here just report the current values to userspace, not matter
> > they're updated with the new values or still being the invalidate one
> (UINT_MAX).
> > So, a sanity checking might not be so necessary here, as they should
> > respect to guest anyway, just like the framebuffer does.
> 
> As fb decoder has spec level definition to check, this hotspot value is from guest
> input directly, what if it's set inconsistent like over cursor plane size?
Then, it should be the userspace's responsibility to handle it, as only userspace knows
what the hotspots are.

BR,
Tina
> 
> >
> > >
> > > > +
> > > >  		info->size = (((info->stride * c.height * c.bpp) / 8)
> > > >  				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> > > >  	} else {
> > > > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > index 6b50fe7..14a7961 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > > @@ -36,6 +36,7 @@
> > > >  #include <uapi/drm/drm_fourcc.h>
> > > >  #include "i915_drv.h"
> > > >  #include "gvt.h"
> > > > +#include "i915_pvinfo.h"
> > > >
> > > >  #define PRIMARY_FORMAT_NUM	16
> > > >  struct pixel_format {
> > > > @@ -390,6 +391,8 @@ int intel_vgpu_decode_cursor_plane(struct
> > > intel_vgpu *vgpu,
> > > >  	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >>
> > > _CURSOR_POS_Y_SHIFT;
> > > >  	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >>
> > > _CURSOR_SIGN_Y_SHIFT;
> > > >
> > > > +	plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot));
> > > > +	plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot));
> > > >  	return 0;
> > > >  }
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > index fbb908e..0d7560a 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > @@ -1198,8 +1198,8 @@ static int pvinfo_mmio_write(struct
> > > > intel_vgpu
> > > *vgpu, unsigned int offset,
> > > >  		ret = handle_g2v_notification(vgpu, data);
> > > >  		break;
> > > >  	/* add xhot and yhot to handled list to avoid error log */
> > > > -	case 0x78830:
> > > > -	case 0x78834:
> > > > +	case _vgtif_reg(cursor_x_hot):
> > > > +	case _vgtif_reg(cursor_y_hot):
> > > >  	case _vgtif_reg(pdp[0].lo):
> > > >  	case _vgtif_reg(pdp[0].hi):
> > > >  	case _vgtif_reg(pdp[1].lo):
> > > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > b/drivers/gpu/drm/i915/gvt/vgpu.c index 41f76e8..c56e634 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > > > @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu
> > > > *vgpu)
> > > >
> > > >  	vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) =
> > > > vgpu_fence_sz(vgpu);
> > > >
> > > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX;
> > > > +	vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX;
> > > > +
> > > >  	gvt_dbg_core("Populate PVINFO PAGE for vGPU %d\n", vgpu->id);
> > > >  	gvt_dbg_core("aperture base [GMADR] 0x%llx size 0x%llx\n",
> > > >  		vgpu_aperture_gmadr_base(vgpu), vgpu_aperture_sz(vgpu));
> > > diff --git
> > > > a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > index 195203f..d61914a 100644
> > > > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > > > @@ -93,7 +93,10 @@ struct vgt_if {
> > > >  	u32 rsv5[4];
> > > >
> > > >  	u32 g2v_notify;
> > > > -	u32 rsv6[7];
> > > > +	u32 rsv6[5];
> > > > +
> > > > +	u32 cursor_x_hot;
> > > > +	u32 cursor_y_hot;
> > > >
> > > >  	struct {
> > > >  		u32 lo;
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > intel-gvt-dev mailing list
> > > > intel-gvt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> > >
> > > --
> > > Open Source Technology Center, Intel ltd.
> > >
> > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827