On Wed, Mar 28, 2018 at 04:51:55PM +0300, Joonas Lahtinen wrote: > Quoting Daniele Ceraolo Spurio (2018-03-23 19:17:49) > > > > > > On 23/03/18 05:34, Michał Winiarski wrote: > > > We're seeing "RPM wakelock ref not held during HW access" warning > > > otherwise. And since IRQ are synced for runtime suspend, we can use the > > > variant without wakeref assert. > > > > > > Reported-by: Marta Löfstedt <marta.lofstedt@xxxxxxxxx> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105710 > > > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > Cc: Marta Löfstedt <marta.lofstedt@xxxxxxxxx> > > > Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/intel_guc.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c > > > index 8f93f5bef8fd..6787a3116783 100644 > > > --- a/drivers/gpu/drm/i915/intel_guc.c > > > +++ b/drivers/gpu/drm/i915/intel_guc.c > > > @@ -391,9 +391,9 @@ void intel_guc_to_host_event_handler(struct intel_guc *guc) > > > * clears out the bit on handling the 1st interrupt. > > > */ > > > spin_lock(&guc->irq_lock); > > > - val = I915_READ(SOFT_SCRATCH(15)); > > > + val = I915_READ_FW(SOFT_SCRATCH(15)); > > > > GuC registers are in forcewake range, so don't we need to manually grab > > forcewake if we use the _FW variant of the read/write macros? > > Hmm, with the Bugzilla tag and all, wasn't this patch tested > specifically to fix the bug? > > Regards, Joonas > > PS. If there's a bugfix, it should really be a separate patch that can > be immediately merged and the bug should get fixed by the patch with > Bugzilla: is merged. Daniele is correct - we do need the forcewake. The WARN is caused by the fact that we're not doing pm_get (and we don't need to do it). The correct fix would be to do the disable/enable_rpm_wakeref_asserts dance, however Chris suggested that since the forcewake takes time, maybe we could move the logic to tasklet. And... why not? I'll follow your advice and post it as a separate patch. -Michał > > > > > Daniele > > > > > msg = val & guc->msg_enabled_mask; > > > - I915_WRITE(SOFT_SCRATCH(15), val & ~msg); > > > + I915_WRITE_FW(SOFT_SCRATCH(15), val & ~msg); > > > spin_unlock(&guc->irq_lock); > > > > > > if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER | > > > > > _______________________________________________ > > 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
