Re: [PATCH v3 3/6] drm/i915/huc: Add HuC fw loading support

Dave Gordon
2016-07-29T12:36:01Z

On 06/07/16 15:24, Peter Antoine wrote:
> The HuC loading process is similar to GuC. The intel_uc_fw_fetch()
> is used for both cases.
>
> HuC loading needs to be before GuC loading. The WOPCM setting must
> be done early before loading any of them.
>
> v2: rebased on-top of drm-intel-nightly.
> removed if(HAS_GUC()) before the guc call. (D.Gordon)
> update huc_version number of format.
> v3: rebased to drm-intel-nightly, changed the file name format to
> match the one in the huc package.
> Changed dev->dev_provate to to_i915()
>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 3 +
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/i915_guc_reg.h | 3 +
> drivers/gpu/drm/i915/intel_guc.h | 1 +
> drivers/gpu/drm/i915/intel_guc_loader.c | 26 ++--
> drivers/gpu/drm/i915/intel_huc.h | 44 ++++++
> drivers/gpu/drm/i915/intel_huc_loader.c | 267 ++++++++++++++++++++++++++++++++
> 8 files changed, 336 insertions(+), 12 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_huc.h
> create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c

I found another problem, which at root is because this was *copied* from
the GuC loading code some time ago, rather than being implemented in a
single generic function. The GuC code has been changed since it was
cloned, but of course the corresponding updates are missing from the HuC
version.

[snip]

> +
> + /* Start the DMA */
> + I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
> +
> + /* Wait for DMA to finish */
> + ret = wait_for_atomic((I915_READ(DMA_CTRL) & START_DMA) == 0, 50);

Can't use wait_for_atomic() here. The GuC version now reads:

/*
* Wait for the DMA to complete & the GuC to start up.
* NB: Docs recommend not using the interrupt for completion.
* Measurements indicate this should take no more than 20ms,
* so a timeout here indicates that the GuC has is unusable.
* (Higher levels of the driver will attempt to fall back to
* execlist mode if this happens.)
*/
ret = wait_for(guc_ucode_response(dev_priv, &status), 100);

.Dave.