[Intel-gfx] [PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 03:26:18PM +0200, Michal Wajdeczko wrote:
> On Tue, 17 Jul 2018 14:53:20 +0200, Ville Syrjala  
> <ville.syrjala at linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > If there's no guc don't try to initialize it even if the user asked for
> > it.
> >
> > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> > b/drivers/gpu/drm/i915/intel_uc.c
> > index 7c95697e1a35..2765808b01e0 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -106,6 +106,11 @@ static void sanitize_options_early(struct  
> > drm_i915_private *i915)
> >  	struct intel_uc_fw *guc_fw = &i915->guc.fw;
> >  	struct intel_uc_fw *huc_fw = &i915->huc.fw;
> > +	if (!HAS_GUC(i915)) {
> > +		i915_modparams.enable_guc = 0;
> > +		return;
> > +	}
> > +
> 
> This will silently switch from user requested GuC-submission to
> execlist-mode which we wanted to stop.
> 
> If user don't know what is available on given platform then auto(-1)
> mode should be used instead. If user has decided to explicitly specify
> invalid enable_guc !0 mode on non-GuC platform why do we want to ignore
> that and continue as nothing happened?

If we want to fail then we should at least fail nicer and tell the user
they're trying something that's not possible.

> 
> Michal
> 
> ps. what is your expectation if there is GuC HW but no FW was defined?

I just bury my head in the sand whenever a guc approaches.

> 
> >  	/* A negative value means "use platform default" */
> >  	if (i915_modparams.enable_guc < 0)
> >  		i915_modparams.enable_guc = __get_platform_enable_guc(i915);

-- 
Ville Syrjälä
Intel