[Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[Chris Wilson]

On Thu, Mar 09, 2017 at 01:03:17PM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> > Behalf Of Chris Wilson
> > Sent: Thursday, March 9, 2017 8:45 PM
> > To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> > Cc: intel-gfx at lists.freedesktop.org; intel-gvt-dev at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check
> > before enable gvt
> > 
> > On Thu, Mar 09, 2017 at 12:32:18PM +0000, Dong, Chuanxiao wrote:
> > > Hi,
> > >
> > > Any comments to this patch?
> > >
> > > Thanks
> > > Chuanxiao
> > >
> > > > -----Original Message-----
> > > > From: intel-gvt-dev
> > > > [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of
> > > > Chuanxiao Dong
> > > > Sent: Monday, March 6, 2017 1:16 PM
> > > > To: intel-gfx at lists.freedesktop.org;
> > > > intel-gvt-dev at lists.freedesktop.org
> > > > Subject: [PATCH] drm/i915/gvt: add enable_execlists check before
> > > > enable gvt
> > > >
> > > > The GVT-g needs execlists to be enabled otherwise gvt should be disabled.
> > > > Add a check for enable_execlists before enabling gvt.
> > > >
> > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > > > b/drivers/gpu/drm/i915/intel_gvt.c
> > > > index d23c0fc..3799cb3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > > @@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private
> > *dev_priv)
> > > >  		goto bail;
> > > >  	}
> > > >
> > > > +	if (!i915.enable_execlists) {
> > > > +		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is
> > > > disabled\n");
> > 
> > Should be DRM_INFO() in response to the user action, and should be treated
> > as a user visible string.
> > 
> > DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled
> > execlist submission [i915.enable_execlists module parameter]\n"); -Chris
> 
> Thanks Chris for the comments. Use "DRM_DEBUG_DRIVER" is just following the same print level with the other fail case in intel_gvt_init(). Anyway, I will take your suggestion to use DRM_INFO instead.

Just a challenge to identify something that may be in response to user
action, typically module parameters or quirks, that may either have
intended or unintended side-effects.

> The message printed is longer than 80 characters, and checkpatch tool also complain with " quoted string split across lines" by splitting to multiple lines. So should I use the message print like below?
>                 
> DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to ");
> DRM_INFO("disabled execlist submission ");
> DRM_INFO("[i915.enable_execlists module parameter]\n");
> 
> Or any better idea?

Just ignore checkpatch. Either split the line or not, that's up to you :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre