[PATCH] drm/i915/gvt: Make MI_USER_INTERRUPT nop in cmd parser

[Gong, Zhipeng]

> -----Original Message-----
> From: Tian, Kevin
> Sent: Monday, March 26, 2018 11:08 AM
> To: Gong, Zhipeng <zhipeng.gong at intel.com>; Zhenyu Wang
> <zhenyuw at linux.intel.com>; Wang, Zhi A <zhi.a.wang at intel.com>
> Cc: He, Min <min.he at intel.com>; intel-gvt-dev at lists.freedesktop.org
> Subject: RE: [PATCH] drm/i915/gvt: Make MI_USER_INTERRUPT nop in cmd
> parser
> 
> > From: Gong, Zhipeng
> > Sent: Monday, March 26, 2018 10:53 AM
> 
> > Zhenyu and Zhi
> >
> > Could you please review this patch?
> >
> > Thanks
> > Zhipeng
> > > -----Original Message-----
> > > From: Gong, Zhipeng
> > > Sent: Thursday, March 22, 2018 9:44 AM
> > > To: intel-gvt-dev at lists.freedesktop.org
> > > Cc: Gong, Zhipeng <zhipeng.gong at intel.com>; He, Min
> > <min.he at intel.com>
> > > Subject: [PATCH] drm/i915/gvt: Make MI_USER_INTERRUPT nop in cmd
> > parser
> > >
> > > GVT-g cmd parser passes through MI_USER_INTERRUPT instruction, while
> > > i915 adds MI_USER_INTERRUPT again for each request from GVT-g,
> > > which causes some unnecessary interrupt handling.
> > > This patch makes MI_USER_INTERRUPT nop to save some interrupt
> > > handling.
> > >
> > > Here is test result to run glmark2 on guest:
> > > host master interrupts number is reduced from 16021 to 11162
> > > host user interrupts number is reduced from 7936 to 3536
> 
> the number is a nice improvement. however simply counting
> on current knowledge may get GVT-g broken in case future
> i915 is changed to not use MI_USER_INTERRUPT (e.g. using
> PIPE_CONTROL). Is there a way that we can detect and then
> optionally do patching only in case the assumption holds?
gvt-g dispatches request to host i915 and depends on i915 notify 
ring interrupt mechanism to check completion of request. 
gvt-g won't get broken if i915 is changed to not use MI_USER_INTERRUPT.
will revise the commit message.

> another thought is, what about guest cmd buffer contains
> multiple MI_USER_INTERRUPT then nop all of them may
> cause some latency issue (if one of them is in middle of
> execution)? should we restrict nop to last one only?
If several requests from guest are combined into one request in gvt-g 
and contain MI_USER_INTERRUPT in the middle of combined request.
gvt-g still has to wait on the whole request to complete to inject
user interrupt to guest. It won't cause the latency issue. 

> > >
> > > Signed-off-by: Zhipeng Gong <zhipeng.gong at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/cmd_parser.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > index febb814..ab4a670 100644
> > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > @@ -1064,6 +1064,7 @@ static int
> > cmd_handler_mi_user_interrupt(struct
> > > parser_exec_state *s)
> > >  {
> > >  	set_bit(cmd_interrupt_events[s->ring_id].mi_user_interrupt,
> > >  			s->workload->pending_events);
> > > +	patch_value(s, cmd_ptr(s, 0), MI_NOOP);
> > >  	return 0;
> > >  }
> > >
> > > --
> > > 2.7.4
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev