>-----Original Message----- >From: Ceraolo Spurio, Daniele >Sent: Friday, June 22, 2018 10:26 AM >To: Srivatsa, Anusha <[email protected]>; intel- >[email protected] >Cc: Spotswood, John A <[email protected]>; Mateo Lozano, Oscar ><[email protected]> >Subject: Re: [PATCH] firmware/guc: Remove USES_GUC_SUBMISSION for >suspend/resume > >Commit title is slightly misleading, as the USES_GUC_SUBMISSION is not removed >from a suspend/resume path. the firmware tag is also confusing since this fixes >an i915 bug. Maybe something like "drm/i915/guc: Remove >USES_GUC_SUBMISSION for ads programming" would be clearer Sure. Makes sense.
>On 22/06/18 10:05, Anusha Srivatsa wrote: >> In the guc_ctl_debug_flags, the ads struct is programmed only when >> USES_GUC_SUBMISSION is satisfied. But, this has to be programmed for >> all suspend/resume cases. >> Remove the condition and program the ads struct for both huc loading >> and guc submission. >> >> This issue was noticed when CI threw errors for enable_guc=2 (load >> huc; disable submission) >> > >Do we need a fixes: tag? Not sure we want this backported since GuC is off by >default. Hmm...so the patch - Load Guc,HuC on GLK is still not merged. Maybe skip the fixes tag? >> Credits to: Daniele Ceraolo Spurio <[email protected]> >> Cc: John Spotswood <[email protected]> >> Cc: Oscar Mateo <[email protected]> >> Cc: Daniele Ceraolo Spurio <[email protected]> >> Signed-off-by: Anusha Srivatsa <[email protected]> >> --- >> drivers/gpu/drm/i915/intel_guc.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc.c >> b/drivers/gpu/drm/i915/intel_guc.c >> index 1aff30b..b1d1a10 100644 >> --- a/drivers/gpu/drm/i915/intel_guc.c >> +++ b/drivers/gpu/drm/i915/intel_guc.c >> @@ -207,6 +207,7 @@ static u32 guc_ctl_debug_flags(struct intel_guc *guc) >> { >> u32 level = intel_guc_log_get_level(&guc->log); >> u32 flags = 0; >> + u32 ads = 0; >> >> if (!GUC_LOG_LEVEL_IS_ENABLED(level)) >> flags |= GUC_LOG_DEFAULT_DISABLED; @@ -217,12 +218,10 >@@ static >> u32 guc_ctl_debug_flags(struct intel_guc *guc) >> flags |= GUC_LOG_LEVEL_TO_VERBOSITY(level) << >> GUC_LOG_VERBOSITY_SHIFT; >> >> - if (USES_GUC_SUBMISSION(guc_to_i915(guc))) { >> - u32 ads = intel_guc_ggtt_offset(guc, guc->ads_vma) >> - >> PAGE_SHIFT; >> + ads = intel_guc_ggtt_offset(guc, guc->ads_vma) << > >You've flipped the shift here. With that fixed: Oops...thanks for pointing it out. >Reviewed-by: Daniele Ceraolo Spurio <[email protected]> And thanks for the review. Will re-spin the patch asap. Anusha >> + PAGE_SHIFT; >> >> - flags |= ads << GUC_ADS_ADDR_SHIFT | GUC_ADS_ENABLED; >> - } >> + flags |= ads << GUC_ADS_ADDR_SHIFT | GUC_ADS_ENABLED; >> >> return flags; >> } >> _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
