On 22/06/18 10:38, Srivatsa, Anusha wrote:
-----Original Message-----
From: Ceraolo Spurio, Daniele
Sent: Friday, June 22, 2018 10:26 AM
To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; intel-
gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Spotswood, John A <john.a.spotswood@xxxxxxxxx>; Mateo Lozano, Oscar
<oscar.mateo@xxxxxxxxx>
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?
This fails on SKL as well and we had FW there for a while. I still think
we can skip the tag because guc is off by default, but don't take my
word as assurance :)
Daniele
Credits to: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: John Spotswood <john.a.spotswood@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
---
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 <daniele.ceraolospurio@xxxxxxxxx>
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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
