With enable_guc_loading=2 and enable_guc_submission=0 I get HuC authentication failure and with enable_guc_loading and enable_guc_submisssion both set to 2 the device does not show any display. Anusha >-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Michal Wajdeczko >Sent: Wednesday, December 21, 2016 8:31 AM >To: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; >Hiler, Arkadiusz <arkadiusz.hiler@xxxxxxxxx> >Subject: Re: [PATCH] drm/i915/guc: Reserve the upper end of the >Global GTT for the GuC > >On Wed, Dec 21, 2016 at 04:03:30PM +0000, Chris Wilson wrote: >> On Wed, Dec 21, 2016 at 04:30:14PM +0100, Michal Wajdeczko wrote: >> > On Wed, Dec 21, 2016 at 02:11:26PM +0000, Chris Wilson wrote: >> > > The GuC would like to own the upper portion of the GTT for itself, >> > > so exclude it from our drm_mm to prevent us using it. >> > > >> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> > > Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> >> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx> >> > > --- >> > > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++++ >> > > drivers/gpu/drm/i915/intel_guc_fwif.h | 3 +++ >> > > 2 files changed, 8 insertions(+) >> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c >> > > b/drivers/gpu/drm/i915/i915_gem_gtt.c >> > > index 6af9311f72f5..96bc0e83286a 100644 >> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> > > @@ -3176,6 +3176,11 @@ int i915_ggtt_probe_hw(struct >drm_i915_private *dev_priv) >> > > if (ret) >> > > return ret; >> > > >> > > + if (HAS_GUC_SCHED(dev_priv)) { >> > >> > Btw, shouldn't we also modify HAS_GUC_SCHED macro to look at >> > i915.enable_guc_submission instead of .has_guc ? Note that Guc may be >present but disabled... >> >> And we don't really know at this point, since >> i915.enable_guc_submission is resolved later. >> >> if (HAS_GUC_SCHED(dev_priv) && i915.enable_guc_submission) is >> conservative enough. >> >> > > + ggtt->base.total -= GUC_GGTT_RESERVED_TOP; >> > >> > Hmm, while unlikely, what if RESERVED_TOP is larger than detected >base.total ? >> >> Then we are screwed :) >> >> - if (HAS_GUC_SCHED(dev_priv)) { >> + if (HAS_GUC_SCHED(dev_priv) && i915.enable_guc_submission) { > >Assuming that in the future HAS_GUC_SCHED will be corrected to take use >sanitized value of .enable_guc_submission, to avoid any mislead, I would rather >go with > >- if (HAS_GUC_SCHED(dev_priv) && i915.enable_guc_submission) { >+ if (HAS_GUC(dev_priv) && i915.enable_guc_submission) { > > >> + if (WARN(ggtt->base.total < GUC_GGTT_RESERVED_TOP, >> + "Only found %lldMiB, but need %dMiB for the >> + GuC", > >Please update message with "GGTT" or similar to capture context what is missing >;) > >> + ggtt->base.total >> 20, GUC_GGTT_RESERVED_TOP >> 20)) >> + return -ENODEV; >> + >> ggtt->base.total -= GUC_GGTT_RESERVED_TOP; >> ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total); >> } >> >> > > + ggtt->mappable_end = min(ggtt->mappable_end, ggtt- >>base.total); >> > > + } >> > > + >> > > if ((ggtt->base.total - 1) >> 32) { >> > > DRM_ERROR("We never expected a Global GTT with more than >32bits" >> > > " of address space! Found %lldM!\n", diff --git >> > > a/drivers/gpu/drm/i915/intel_guc_fwif.h >> > > b/drivers/gpu/drm/i915/intel_guc_fwif.h >> > > index 3202b32b5638..3361d38ed859 100644 >> > > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h >> > > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h >> > > @@ -23,6 +23,9 @@ >> > > #ifndef _INTEL_GUC_FWIF_H >> > > #define _INTEL_GUC_FWIF_H >> > > >> > > +/* A small region at the top of the global GTT is reserved for use by the >GuC */ >> > > +#define GUC_GGTT_RESERVED_TOP 0x1200000 >> > >> > Is this region size fixed (part of Guc HW assumptions), or it can change per >different FW? >> > If former, then maybe this macro should be moved to i915_guc_reg.h? >> > If latter, then maybe it is worth to explictly state that in the comment? >> >> This is purely guess work. Feel free to supply the mising details... > >If only I knew ;) maybe another day. > >With the proposed changes, > >Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> > >Thanks, >Michal >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
