Message ID | 20171024105056.43276-1-michal.wajdeczko@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Michal Wajdeczko (2017-10-24 11:50:56) > Waiting for DMA status register can be done with dedicated function. > Lets use it as additional bonus will be smaller driver footprint. > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_huc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c > index c8a48cb..98d1725 100644 > --- a/drivers/gpu/drm/i915/intel_huc.c > +++ b/drivers/gpu/drm/i915/intel_huc.c > @@ -151,7 +151,7 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma) > I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA)); > > /* Wait for DMA to finish */ > - ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100); > + ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100); > > DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret); Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Aside, what's the serialisation so that we only try to load one fw at a time? -Chris
On Tue, 24 Oct 2017 13:23:45 +0200, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Michal Wajdeczko (2017-10-24 11:50:56) >> Waiting for DMA status register can be done with dedicated function. >> Lets use it as additional bonus will be smaller driver footprint. >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_huc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_huc.c >> b/drivers/gpu/drm/i915/intel_huc.c >> index c8a48cb..98d1725 100644 >> --- a/drivers/gpu/drm/i915/intel_huc.c >> +++ b/drivers/gpu/drm/i915/intel_huc.c >> @@ -151,7 +151,7 @@ static int huc_ucode_xfer(struct intel_uc_fw >> *huc_fw, struct i915_vma *vma) >> I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | >> START_DMA)); >> >> /* Wait for DMA to finish */ >> - ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100); >> + ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, >> 0, 100); >> >> DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", >> ret); > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Aside, what's the serialisation so that we only try to load one fw at a > time? It looks that we just wait right after starting DMA. And if we attempt to next DMA while previous is still running then it will be likely silently ignored by HW as control bit START_DMA would be still on. If we want to be bullet-proof then maybe before we start programming new DMA xfer we should verify that DMA hw is idle (and then wait little more or just abort immediately) Michal
Quoting Patchwork (2017-10-24 13:27:27) > == Series Details == > > Series: drm/i915/huc: Use helper function while waiting for DMA completion > URL : https://patchwork.freedesktop.org/series/32528/ > State : failure > > == Summary == > > Series 32528v1 drm/i915/huc: Use helper function while waiting for DMA completion > https://patchwork.freedesktop.org/api/1.0/series/32528/revisions/1/mbox/ > > Test kms_pipe_crc_basic: > Subgroup read-crc-pipe-a: > pass -> INCOMPLETE (fi-glk-1) Unrelated, but unusual GEM_BUG_ON, which I hope I've already fixed... Pushed, thanks for the patch. -Chris
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index c8a48cb..98d1725 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -151,7 +151,7 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma) I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA)); /* Wait for DMA to finish */ - ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100); + ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100); DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
Waiting for DMA status register can be done with dedicated function. Lets use it as additional bonus will be smaller driver footprint. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/intel_huc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)