| Message ID | 1495767859-10064-3-git-send-email-chuanxiao.dong@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 26, 2017 at 11:04:18AM +0800, Chuanxiao Dong wrote: > GVT-g relies on the enable_execlists parameter in i915. If this option > is not enabled for GVT-g, should return -EIO to make i915 driver loading > failed. > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com> > --- > drivers/gpu/drm/i915/intel_gvt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c > index dde9c78..c90d476 100644 > --- a/drivers/gpu/drm/i915/intel_gvt.c > +++ b/drivers/gpu/drm/i915/intel_gvt.c > @@ -97,7 +97,7 @@ int intel_gvt_init(struct drm_i915_private *dev_priv) > > if (!i915.enable_execlists) { > DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled execlist submission [i915.enable_execlists module parameter]\n"); > - goto bail; > + return -EIO; Hmm, interesting debate as to whether or not you still want to set i915.enable_gvt=0 to indicate gvt being lost. Since this will cause the driver to fail to load, it should be now DRM_ERROR("and the error message changed to reflect the fatal failure!") and having modprobe report -EIO with i915.enable_gvt still to 1 seems like the best approach. -Chris
> -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Friday, May 26, 2017 5:35 PM > To: Dong, Chuanxiao <chuanxiao.dong@intel.com> > Cc: intel-gvt-dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915/gvt: Return -EIO if host > enable_execlists not enabled when loading GVT-g > > On Fri, May 26, 2017 at 11:04:18AM +0800, Chuanxiao Dong wrote: > > GVT-g relies on the enable_execlists parameter in i915. If this option > > is not enabled for GVT-g, should return -EIO to make i915 driver > > loading failed. > > > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com> > > --- > > drivers/gpu/drm/i915/intel_gvt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c > > b/drivers/gpu/drm/i915/intel_gvt.c > > index dde9c78..c90d476 100644 > > --- a/drivers/gpu/drm/i915/intel_gvt.c > > +++ b/drivers/gpu/drm/i915/intel_gvt.c > > @@ -97,7 +97,7 @@ int intel_gvt_init(struct drm_i915_private > > *dev_priv) > > > > if (!i915.enable_execlists) { > > DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to > disabled execlist submission [i915.enable_execlists module parameter]\n"); > > - goto bail; > > + return -EIO; > > Hmm, interesting debate as to whether or not you still want to set > i915.enable_gvt=0 to indicate gvt being lost. Since this will cause the driver > to fail to load, it should be now DRM_ERROR("and the error message > changed to reflect the fatal failure!") and having modprobe report -EIO with > i915.enable_gvt still to 1 seems like the best approach. > -Chris Yes, agree with keeping i915.enable_gvt to be 1, this is also the current solution. For the error message, how about DRM_ERROR("i915 GVT-g driver loading failed due to disabled execlists mode.\n")? Regarding fail with -EIO, as we can take i915.enable_gvt as a single feature of intel i915 GPU, is it still reasonable to return -EIO to disable the whole i915 driver? It is something like two features A and B in i915, as B relies on A, so B should be disabled if detected feature A is not enable, but not just making the whole i915 stack disabled. Thanks Chuanxiao
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c index dde9c78..c90d476 100644 --- a/drivers/gpu/drm/i915/intel_gvt.c +++ b/drivers/gpu/drm/i915/intel_gvt.c @@ -97,7 +97,7 @@ int intel_gvt_init(struct drm_i915_private *dev_priv) if (!i915.enable_execlists) { DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled execlist submission [i915.enable_execlists module parameter]\n"); - goto bail; + return -EIO; } /*
GVT-g relies on the enable_execlists parameter in i915. If this option is not enabled for GVT-g, should return -EIO to make i915 driver loading failed. Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com> --- drivers/gpu/drm/i915/intel_gvt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)