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

[Tian, Kevin]

> 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?

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?

> >
> > 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