On 2022-02-07 at 08:55:20 -0800, Daniele Ceraolo Spurio wrote:
>
>
> On 1/28/2022 10:52 AM, Ramalingam C wrote:
> > From: Stuart Summers <stuart.summ...@intel.com>
> >
> > The driver is set currently to fail modprobe when GuC is disabled
> > (enable_guc=0) after GuC has been loaded on a previous modprobe.
> > For GuC deprivilege, the BIOS is setting the locked bit, so the
> > driver always considers the GuC to have been loaded and thus does
> > not support enable_guc=0 on these platforms.
> >
> > There are some debug scenarios where loading without GuC can be
> > interesting. Add a new feature flag for GuC deprivilege and a mode
> > (enable_guc=0x80) which can be exclusively set to skip the locked
> > bit check.
>
> This is a debug-only patch, so IMO it should definitely not be merged as-is,
> because we don't want normal users having access to this option as it can
> lead to an hard gpu hangs if misused. I'm honestly not convinced we want
> this in the tree at all, because you can still run without GuC submission by
> setting enable_guc=2; the only thing this patch adds is the ability to skip
> the GuC/HuC load entirely. If you think there is still value in having this
> ability for debug, then the patch should be updated to only allow the new
> option when one of the debug flags is set; I'd go with DEBUG_GEM as we have
> that enabled by default in our CI builds.
Thank you daniele. Dropping this patch as i dont see any usecase as of
now.
Ram.
>
> Daniele
>
> > cc: Radhakrishna Sripada <radhakrishna.srip...@intel.com>
> > Signed-off-by: Stuart Summers <stuart.summ...@intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> > Signed-off-by: Ramalingam C <ramalinga...@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++--
> > drivers/gpu/drm/i915/i915_params.h | 1 +
> > 2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index da199aa6989f..a1376dbd04fe 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -108,7 +108,7 @@ static void __confirm_options(struct intel_uc *uc)
> > "Incompatible option enable_guc=%d - %s\n",
> > i915->params.enable_guc, "GuC submission is N/A");
> > - if (i915->params.enable_guc & ~ENABLE_GUC_MASK)
> > + if (i915->params.enable_guc & ~(ENABLE_GUC_MASK |
> > ENABLE_GUC_DO_NOT_LOAD_GUC))
> > drm_info(&i915->drm,
> > "Incompatible option enable_guc=%d - %s\n",
> > i915->params.enable_guc, "undocumented flag");
> > @@ -416,8 +416,21 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
> > (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) &
> > GUC_WOPCM_OFFSET_VALID);
> > }
> > +static inline bool skip_lock_check(struct drm_i915_private *i915)
> > +{
> > + /*
> > + * For platforms with GuC deprivilege, if a user *really* wants
> > + * to run without GuC, let that happen by setting enable_guc=0x80.
> > + */
> > + return (HAS_GUC_DEPRIVILEGE(i915) &&
> > + (i915->params.enable_guc & ENABLE_GUC_DO_NOT_LOAD_GUC) &&
> > + !(i915->params.enable_guc & ~ENABLE_GUC_DO_NOT_LOAD_GUC));
> > +}
> > +
> > static int __uc_check_hw(struct intel_uc *uc)
> > {
> > + struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> > +
> > if (!intel_uc_supports_guc(uc))
> > return 0;
> > @@ -426,7 +439,7 @@ static int __uc_check_hw(struct intel_uc *uc)
> > * before on this system after reboot, otherwise we risk GPU hangs.
> > * To check if GuC was loaded before we look at WOPCM registers.
> > */
> > - if (uc_is_wopcm_locked(uc))
> > + if (uc_is_wopcm_locked(uc) && likely(!skip_lock_check(i915)))
> > return -EIO;
> > return 0;
> > diff --git a/drivers/gpu/drm/i915/i915_params.h
> > b/drivers/gpu/drm/i915/i915_params.h
> > index c9d53ff910a0..8996ba2cc3a8 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -32,6 +32,7 @@ struct drm_printer;
> > #define ENABLE_GUC_SUBMISSION BIT(0)
> > #define ENABLE_GUC_LOAD_HUC BIT(1)
> > +#define ENABLE_GUC_DO_NOT_LOAD_GUC BIT(7)
> > #define ENABLE_GUC_MASK GENMASK(1, 0)
> > /*
>
