| Message ID | 1507712056-25030-3-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 11 Oct 2017 10:53:57 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > Parameter guc_log_level needs to be sanitized based on GuC support and > enable_guc_loading parameter since it depends on them like > enable_guc_submission. This will make GuC logging paths independent of > enable_guc_submission parameter in further patches. > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_uc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index b33d469..3cf3cbd 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -56,6 +56,7 @@ void intel_uc_sanitize_options(struct drm_i915_private > *dev_priv) Btw, I think the message "Ignoring GuC options, no hardware\n" is also applicable when user specified guc_log_level>=0. Please update condition that controls that message. > i915_modparams.enable_guc_loading = 0; > i915_modparams.enable_guc_submission = 0; > + i915_modparams.guc_log_level = -1; > return; > } > @@ -72,9 +73,11 @@ void intel_uc_sanitize_options(struct > drm_i915_private *dev_priv) > i915_modparams.enable_guc_loading = 0; > } > - /* Can't enable guc submission without guc loaded */ > - if (!i915_modparams.enable_guc_loading) > + /* Can't enable guc submission and logging without guc loaded */ > + if (!i915_modparams.enable_guc_loading) { > i915_modparams.enable_guc_submission = 0; > + i915_modparams.guc_log_level = -1; > + } > /* A negative value means "use platform default" */ > if (i915_modparams.enable_guc_submission < 0) Looks good, but please also update condition in i915_guc_log_register() as we may now rely only on sanitized guc_log_level: void i915_guc_log_register(struct drm_i915_private *dev_priv) { if (!i915_modparams.enable_guc_submission || ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (i915_modparams.guc_log_level < 0)) return; Also, maybe it is worth to update DOC in intel_guc_log.c and param description in i915_params.c to indicate (obvious) dependency on guc. Michal
On 10/11/2017 8:21 PM, Michal Wajdeczko wrote: > On Wed, 11 Oct 2017 10:53:57 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> Parameter guc_log_level needs to be sanitized based on GuC support and >> enable_guc_loading parameter since it depends on them like >> enable_guc_submission. This will make GuC logging paths independent of >> enable_guc_submission parameter in further patches. >> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_uc.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index b33d469..3cf3cbd 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -56,6 +56,7 @@ void intel_uc_sanitize_options(struct >> drm_i915_private *dev_priv) > > Btw, I think the message "Ignoring GuC options, no hardware\n" is also > applicable when user specified guc_log_level>=0. Please update condition > that controls that message. > Yes. Will update that condition. >> i915_modparams.enable_guc_loading = 0; >> i915_modparams.enable_guc_submission = 0; >> + i915_modparams.guc_log_level = -1; >> return; >> } >> @@ -72,9 +73,11 @@ void intel_uc_sanitize_options(struct >> drm_i915_private *dev_priv) >> i915_modparams.enable_guc_loading = 0; >> } >> - /* Can't enable guc submission without guc loaded */ >> - if (!i915_modparams.enable_guc_loading) >> + /* Can't enable guc submission and logging without guc loaded */ >> + if (!i915_modparams.enable_guc_loading) { >> i915_modparams.enable_guc_submission = 0; >> + i915_modparams.guc_log_level = -1; >> + } >> /* A negative value means "use platform default" */ >> if (i915_modparams.enable_guc_submission < 0) > > Looks good, but please also update condition in i915_guc_log_register() > as we may now rely only on sanitized guc_log_level: > > void i915_guc_log_register(struct drm_i915_private *dev_priv) > { > if (!i915_modparams.enable_guc_submission || > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > (i915_modparams.guc_log_level < 0)) > return; > This is updated in the later patches in the series. > Also, maybe it is worth to update DOC in intel_guc_log.c and param > description in i915_params.c to indicate (obvious) dependency on guc. > Sure. Will add this. > Michal
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index b33d469..3cf3cbd 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -56,6 +56,7 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) i915_modparams.enable_guc_loading = 0; i915_modparams.enable_guc_submission = 0; + i915_modparams.guc_log_level = -1; return; } @@ -72,9 +73,11 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) i915_modparams.enable_guc_loading = 0; } - /* Can't enable guc submission without guc loaded */ - if (!i915_modparams.enable_guc_loading) + /* Can't enable guc submission and logging without guc loaded */ + if (!i915_modparams.enable_guc_loading) { i915_modparams.enable_guc_submission = 0; + i915_modparams.guc_log_level = -1; + } /* A negative value means "use platform default" */ if (i915_modparams.enable_guc_submission < 0)
Parameter guc_log_level needs to be sanitized based on GuC support and enable_guc_loading parameter since it depends on them like enable_guc_submission. This will make GuC logging paths independent of enable_guc_submission parameter in further patches. Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/intel_uc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)