All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: Peter Antoine <peter.antoine@intel.com>, intel-gfx@lists.freedesktop.org
Cc: sean.v.kelley@intel.com, lawrence.t.li@intel.com, rodrigo.vivi@intel.com
Subject: Re: [PATCH 1/6] drm/i915/guc: Make the GuC fw loading helper functions general
Date: Tue, 28 Jun 2016 15:24:10 +0100	[thread overview]
Message-ID: <5772888A.9080004@intel.com> (raw)
In-Reply-To: <1466532685-5101-2-git-send-email-peter.antoine@intel.com>

On 21/06/16 19:11, Peter Antoine wrote:
> Rename some of the GuC fw loading code to make them more general. We
> will utilize them for HuC loading as well.
>      s/intel_guc_fw/intel_uc_fw/g
>      s/GUC_FIRMWARE/UC_FIRMWARE/g
>
> Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts members,
> such as 'guc' or 'guc_fw' either is renamed to 'uc' or removed for
> same purpose.
>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c        |  12 +-
>   drivers/gpu/drm/i915/i915_guc_submission.c |   4 +-
>   drivers/gpu/drm/i915/intel_guc.h           |  39 ++---
>   drivers/gpu/drm/i915/intel_guc_loader.c    | 221 ++++++++++++++++-------------
>   4 files changed, 151 insertions(+), 125 deletions(-)

The renaming is fine, but this patch also includes other changes 
(flagged below) which constitute a regression. It looks like it's based 
on (and is reinstating code from) Alex's original patch
- 33a732f drm/i915: GuC-specific firmware loader
thus reverting some of the changes in the more recent
+ fce91f2 drm/i915/guc: add enable_guc_loading parameter

I suggest the purely mechanical search-and-replace should be done in a 
first preliminary patch, and then any remaining changes should be 
carefully examined to see whether they are still wanted -- I suspect 
they aren't.

> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7d63af0..69964c2 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2483,7 +2483,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = m->private;
>   	struct drm_i915_private *dev_priv = node->minor->dev->dev_private;
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
>   	u32 tmp, i;
>
>   	if (!HAS_GUC_UCODE(dev_priv))
> @@ -2491,15 +2491,15 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
>
>   	seq_printf(m, "GuC firmware status:\n");
>   	seq_printf(m, "\tpath: %s\n",
> -		guc_fw->guc_fw_path);
> +		guc_fw->uc_fw_path);
>   	seq_printf(m, "\tfetch: %s\n",
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status));
> +		intel_uc_fw_status_repr(guc_fw->fetch_status));
>   	seq_printf(m, "\tload: %s\n",
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> +		intel_uc_fw_status_repr(guc_fw->load_status));
>   	seq_printf(m, "\tversion wanted: %d.%d\n",
> -		guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> +		guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
>   	seq_printf(m, "\tversion found: %d.%d\n",
> -		guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found);
> +		guc_fw->major_ver_found, guc_fw->minor_ver_found);
>   	seq_printf(m, "\theader: offset is %d; size = %d\n",
>   		guc_fw->header_offset, guc_fw->header_size);
>   	seq_printf(m, "\tuCode: offset is %d; size = %d\n",
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 01c7cdf..bfb8400 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1039,7 +1039,7 @@ int intel_guc_suspend(struct drm_device *dev)
>   	struct i915_gem_context *ctx;
>   	u32 data[3];
>
> -	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
> +	if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS)
>   		return 0;
>
>   	ctx = dev_priv->kernel_context;
> @@ -1065,7 +1065,7 @@ int intel_guc_resume(struct drm_device *dev)
>   	struct i915_gem_context *ctx;
>   	u32 data[3];
>
> -	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
> +	if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS)
>   		return 0;
>
>   	ctx = dev_priv->kernel_context;
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 3e3e743..836420b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -90,29 +90,29 @@ struct i915_guc_client {
>   	uint64_t submissions[I915_NUM_ENGINES];
>   };
>
> -enum intel_guc_fw_status {
> -	GUC_FIRMWARE_FAIL = -1,
> -	GUC_FIRMWARE_NONE = 0,
> -	GUC_FIRMWARE_PENDING,
> -	GUC_FIRMWARE_SUCCESS
> +enum intel_uc_fw_status {
> +	UC_FIRMWARE_FAIL = -1,
> +	UC_FIRMWARE_NONE = 0,
> +	UC_FIRMWARE_PENDING,
> +	UC_FIRMWARE_SUCCESS
>   };
>
>   /*
>    * This structure encapsulates all the data needed during the process
>    * of fetching, caching, and loading the firmware image into the GuC.
>    */
> -struct intel_guc_fw {
> -	struct drm_device *		guc_dev;
> -	const char *			guc_fw_path;
> -	size_t				guc_fw_size;
> -	struct drm_i915_gem_object *	guc_fw_obj;
> -	enum intel_guc_fw_status	guc_fw_fetch_status;
> -	enum intel_guc_fw_status	guc_fw_load_status;
> -
> -	uint16_t			guc_fw_major_wanted;
> -	uint16_t			guc_fw_minor_wanted;
> -	uint16_t			guc_fw_major_found;
> -	uint16_t			guc_fw_minor_found;
> +struct intel_uc_fw {
> +	struct drm_device *uc_dev;
> +	const char *uc_fw_path;
> +	size_t uc_fw_size;
> +	struct drm_i915_gem_object *uc_fw_obj;
> +	enum intel_uc_fw_status fetch_status;
> +	enum intel_uc_fw_status	load_status;
> +
> +	uint16_t major_ver_wanted;
> +	uint16_t minor_ver_wanted;
> +	uint16_t major_ver_found;
> +	uint16_t minor_ver_found;
>
>   	uint32_t header_size;
>   	uint32_t header_offset;
> @@ -123,7 +123,7 @@ struct intel_guc_fw {
>   };
>
>   struct intel_guc {
> -	struct intel_guc_fw guc_fw;
> +	struct intel_uc_fw guc_fw;
>   	uint32_t log_flags;
>   	struct drm_i915_gem_object *log_obj;
>
> @@ -152,9 +152,10 @@ struct intel_guc {
>   extern void intel_guc_init(struct drm_device *dev);
>   extern int intel_guc_setup(struct drm_device *dev);
>   extern void intel_guc_fini(struct drm_device *dev);
> -extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status);
> +extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
>   extern int intel_guc_suspend(struct drm_device *dev);
>   extern int intel_guc_resume(struct drm_device *dev);
> +void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw);
>
>   /* i915_guc_submission.c */
>   int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8fe96a2..ff054f5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -66,16 +66,16 @@ MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
>   MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
>
>   /* User-friendly representation of an enum */
> -const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status)
> +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>   {
>   	switch (status) {
> -	case GUC_FIRMWARE_FAIL:
> +	case UC_FIRMWARE_FAIL:
>   		return "FAIL";
> -	case GUC_FIRMWARE_NONE:
> +	case UC_FIRMWARE_NONE:
>   		return "NONE";
> -	case GUC_FIRMWARE_PENDING:
> +	case UC_FIRMWARE_PENDING:
>   		return "PENDING";
> -	case GUC_FIRMWARE_SUCCESS:
> +	case UC_FIRMWARE_SUCCESS:
>   		return "SUCCESS";
>   	default:
>   		return "UNKNOWN!";
> @@ -238,8 +238,8 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
>    */
>   static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
>   {
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> -	struct drm_i915_gem_object *fw_obj = guc_fw->guc_fw_obj;
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct drm_i915_gem_object *fw_obj = guc_fw->uc_fw_obj;
>   	unsigned long offset;
>   	struct sg_table *sg = fw_obj->pages;
>   	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
> @@ -311,17 +311,17 @@ static u32 guc_wopcm_size(struct drm_i915_private *dev_priv)
>    */
>   static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
>   {
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
>   	struct drm_device *dev = dev_priv->dev;
>   	int ret;
>
> -	ret = i915_gem_object_set_to_gtt_domain(guc_fw->guc_fw_obj, false);
> +	ret = i915_gem_object_set_to_gtt_domain(guc_fw->uc_fw_obj, false);
>   	if (ret) {
>   		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
>   		return ret;
>   	}
>
> -	ret = i915_gem_obj_ggtt_pin(guc_fw->guc_fw_obj, 0, 0);
> +	ret = i915_gem_obj_ggtt_pin(guc_fw->uc_fw_obj, 0, 0);
>   	if (ret) {
>   		DRM_DEBUG_DRIVER("pin failed %d\n", ret);
>   		return ret;
> @@ -373,7 +373,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
>   	 * We keep the object pages for reuse during resume. But we can unpin it
>   	 * now that DMA has completed, so it doesn't continue to take up space.
>   	 */
> -	i915_gem_object_ggtt_unpin(guc_fw->guc_fw_obj);
> +	i915_gem_object_ggtt_unpin(guc_fw->uc_fw_obj);
>
>   	return ret;
>   }
> @@ -412,46 +412,58 @@ static int i915_reset_guc(struct drm_i915_private *dev_priv)
>   int intel_guc_setup(struct drm_device *dev)

This section is the bit that looks like it's based on an old version of 
this function ...

>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> -	const char *fw_path = guc_fw->guc_fw_path;
> -	int retries, ret, err;
> -
> -	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
> -		fw_path,
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> -
> -	/* Loading forbidden, or no firmware to load? */
> -	if (!i915.enable_guc_loading) {
> -		err = 0;
> -		goto fail;
> -	} else if (fw_path == NULL) {
> -		/* Device is known to have no uCode (e.g. no GuC) */
> -		err = -ENXIO;
> -		goto fail;
> -	} else if (*fw_path == '\0') {
> -		/* Device has a GuC but we don't know what f/w to load? */
> -		DRM_INFO("No GuC firmware known for this platform\n");
> -		err = -ENODEV;
> -		goto fail;
> -	}
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	int retries, ret, err = 0;
> +
> +	if (!i915.enable_guc_loading)
> +		return 0;
> +
> +	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> +		intel_uc_fw_status_repr(guc_fw->fetch_status),
> +		intel_uc_fw_status_repr(guc_fw->load_status));
> +
> +	direct_interrupts_to_host(dev_priv);
> +
> +	if (guc_fw->fetch_status == UC_FIRMWARE_NONE)
> +		return 0;
> +
> +	if (guc_fw->fetch_status == UC_FIRMWARE_SUCCESS &&
> +	    guc_fw->load_status == UC_FIRMWARE_FAIL)
> +		return -ENOEXEC;
>
> -	/* Fetch failed, or already fetched but failed to load? */
> -	if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) {
> +	guc_fw->load_status = UC_FIRMWARE_PENDING;
> +
> +	DRM_DEBUG_DRIVER("GuC fw fetch status %s\n",
> +		intel_uc_fw_status_repr(guc_fw->fetch_status));
> +
> +	switch (guc_fw->fetch_status) {
> +	case UC_FIRMWARE_FAIL:
> +		/* something went wrong :( */
>   		err = -EIO;
>   		goto fail;
> -	} else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) {
> -		err = -ENOEXEC;
> +
> +	case UC_FIRMWARE_NONE:
> +	case UC_FIRMWARE_PENDING:
> +	default:
> +		/* "can't happen" */
> +		WARN_ONCE(1, "GuC fw %s invalid guc_fw_fetch_status %s [%d]\n",
> +			guc_fw->uc_fw_path,
> +			intel_uc_fw_status_repr(guc_fw->fetch_status),
> +			guc_fw->fetch_status);
> +		err = -ENXIO;
>   		goto fail;
> +
> +	case UC_FIRMWARE_SUCCESS:
> +		break;
>   	}

After this we're mostly back to search-and-replace, I think.

.Dave.

>   	direct_interrupts_to_host(dev_priv);
>
> -	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
> +	guc_fw->load_status = UC_FIRMWARE_PENDING;
>
>   	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> +		intel_uc_fw_status_repr(guc_fw->fetch_status),
> +		intel_uc_fw_status_repr(guc_fw->load_status));
>
>   	err = i915_guc_submission_init(dev_priv);
>   	if (err)
> @@ -485,11 +497,11 @@ int intel_guc_setup(struct drm_device *dev)
>   			 "retry %d more time(s)\n", err, retries);
>   	}
>
> -	guc_fw->guc_fw_load_status = GUC_FIRMWARE_SUCCESS;
> +	guc_fw->load_status = UC_FIRMWARE_SUCCESS;
>
>   	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> +		intel_uc_fw_status_repr(guc_fw->fetch_status),
> +		intel_uc_fw_status_repr(guc_fw->load_status));
>
>   	if (i915.enable_guc_submission) {
>   		err = i915_guc_submission_enable(dev_priv);
> @@ -501,8 +513,8 @@ int intel_guc_setup(struct drm_device *dev)
>   	return 0;
>
>   fail:
> -	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
> -		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
> +	if (guc_fw->load_status == UC_FIRMWARE_PENDING)
> +		guc_fw->load_status = UC_FIRMWARE_FAIL;
>
>   	direct_interrupts_to_host(dev_priv);
>   	i915_guc_submission_disable(dev_priv);
> @@ -535,7 +547,7 @@ fail:
>   		DRM_ERROR("GuC firmware load failed: %d\n", err);
>
>   	if (i915.enable_guc_submission) {
> -		if (fw_path == NULL)
> +		if (guc_fw->uc_fw_path == NULL)
>   			DRM_INFO("GuC submission without firmware not supported\n");
>   		if (ret == 0)
>   			DRM_INFO("Falling back from GuC submission to execlist mode\n");
> @@ -547,7 +559,18 @@ fail:
>   	return ret;
>   }
>
> -static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
> +/**
> + * intel_uc_fw_fetch() - fetch fw blob and save it to internal obj
> + * @dev:	drm device
> + * @uc_fw:	the intel_uc_fw to be setup
> + *
> + * The caller should have setup fw path and fw version required. This function
> + * first fetch the fw blob from file system. If succeed, it will do some basic
> + * check based on css header information. At last, a GEM obj is created and
> + * filled with the fw data. This obj will be loaded to HW at later stage of
> + * driver init process.
> + */
> +void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
>   {
>   	struct drm_i915_gem_object *obj;
>   	const struct firmware *fw;
> @@ -555,17 +578,17 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
>   	size_t size;
>   	int err;
>
> -	DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> -		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status));
> +	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
> +		intel_uc_fw_status_repr(uc_fw->fetch_status));
>
> -	err = request_firmware(&fw, guc_fw->guc_fw_path, &dev->pdev->dev);
> +	err = request_firmware(&fw, uc_fw->uc_fw_path, &dev->pdev->dev);
>   	if (err)
>   		goto fail;
>   	if (!fw)
>   		goto fail;
>
> -	DRM_DEBUG_DRIVER("fetch GuC fw from %s succeeded, fw %p\n",
> -		guc_fw->guc_fw_path, fw);
> +	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
> +		uc_fw->uc_fw_path, fw);
>
>   	/* Check the size of the blob before examining buffer contents */
>   	if (fw->size < sizeof(struct guc_css_header)) {
> @@ -576,62 +599,64 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
>   	css = (struct guc_css_header *)fw->data;
>
>   	/* Firmware bits always start from header */
> -	guc_fw->header_offset = 0;
> -	guc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
> +	uc_fw->header_offset = 0;
> +	uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
>   		css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
>
> -	if (guc_fw->header_size != sizeof(struct guc_css_header)) {
> +	if (uc_fw->header_size != sizeof(struct guc_css_header)) {
>   		DRM_ERROR("CSS header definition mismatch\n");
>   		goto fail;
>   	}
>
>   	/* then, uCode */
> -	guc_fw->ucode_offset = guc_fw->header_offset + guc_fw->header_size;
> -	guc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
> +	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
> +	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
>
>   	/* now RSA */
>   	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
>   		DRM_ERROR("RSA key size is bad\n");
>   		goto fail;
>   	}
> -	guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size;
> -	guc_fw->rsa_size = css->key_size_dw * sizeof(u32);
> +	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
> +	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
>
>   	/* At least, it should have header, uCode and RSA. Size of all three. */
> -	size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size;
> +	size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
>   	if (fw->size < size) {
>   		DRM_ERROR("Missing firmware components\n");
>   		goto fail;
>   	}
>
>   	/* Header and uCode will be loaded to WOPCM. Size of the two. */
> -	size = guc_fw->header_size + guc_fw->ucode_size;
> +	size = uc_fw->header_size + uc_fw->ucode_size;
> +
> +	/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
>   	if (size > guc_wopcm_size(dev->dev_private)) {
>   		DRM_ERROR("Firmware is too large to fit in WOPCM\n");
>   		goto fail;
>   	}
>
>   	/*
> -	 * The GuC firmware image has the version number embedded at a well-known
> +	 * The uC firmware image has the version number embedded at a well-known
>   	 * offset within the firmware blob; note that major / minor version are
>   	 * TWO bytes each (i.e. u16), although all pointers and offsets are defined
>   	 * in terms of bytes (u8).
>   	 */
> -	guc_fw->guc_fw_major_found = css->guc_sw_version >> 16;
> -	guc_fw->guc_fw_minor_found = css->guc_sw_version & 0xFFFF;
> -
> -	if (guc_fw->guc_fw_major_found != guc_fw->guc_fw_major_wanted ||
> -	    guc_fw->guc_fw_minor_found < guc_fw->guc_fw_minor_wanted) {
> -		DRM_ERROR("GuC firmware version %d.%d, required %d.%d\n",
> -			guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
> -			guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> +	uc_fw->major_ver_found = css->guc_sw_version >> 16;
> +	uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
> +
> +	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> +	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> +		DRM_ERROR("Firmware version %d.%d, required %d.%d\n",
> +			uc_fw->major_ver_found, uc_fw->minor_ver_found,
> +			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
>   		err = -ENOEXEC;
>   		goto fail;
>   	}
>
>   	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
> -			guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
> -			guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> +			uc_fw->major_ver_found, uc_fw->minor_ver_found,
> +			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
>
>   	mutex_lock(&dev->struct_mutex);
>   	obj = i915_gem_object_create_from_data(dev, fw->data, fw->size);
> @@ -641,31 +666,31 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw)
>   		goto fail;
>   	}
>
> -	guc_fw->guc_fw_obj = obj;
> -	guc_fw->guc_fw_size = fw->size;
> +	uc_fw->uc_fw_obj = obj;
> +	uc_fw->uc_fw_size = fw->size;
>
>   	DRM_DEBUG_DRIVER("GuC fw fetch status SUCCESS, obj %p\n",
> -			guc_fw->guc_fw_obj);
> +			uc_fw->uc_fw_obj);
>
>   	release_firmware(fw);
> -	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_SUCCESS;
> +	uc_fw->fetch_status = UC_FIRMWARE_SUCCESS;
>   	return;
>
>   fail:
>   	DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
> -		err, fw, guc_fw->guc_fw_obj);
> +		err, fw, uc_fw->uc_fw_obj);
>   	DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
> -		  guc_fw->guc_fw_path, err);
> +		  uc_fw->uc_fw_path, err);
>
>   	mutex_lock(&dev->struct_mutex);
> -	obj = guc_fw->guc_fw_obj;
> +	obj = uc_fw->uc_fw_obj;
>   	if (obj)
>   		drm_gem_object_unreference(&obj->base);
> -	guc_fw->guc_fw_obj = NULL;
> +	uc_fw->uc_fw_obj = NULL;
>   	mutex_unlock(&dev->struct_mutex);
>
>   	release_firmware(fw);		/* OK even if fw is NULL */
> -	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL;
> +	uc_fw->fetch_status = UC_FIRMWARE_FAIL;
>   }
>
>   /**
> @@ -680,7 +705,7 @@ fail:
>   void intel_guc_init(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
>   	const char *fw_path;
>
>   	/* A negative value means "use platform default" */
> @@ -693,20 +718,20 @@ void intel_guc_init(struct drm_device *dev)
>   		fw_path = NULL;
>   	} else if (IS_SKYLAKE(dev)) {
>   		fw_path = I915_SKL_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 6;
> -		guc_fw->guc_fw_minor_wanted = 1;
> +		guc_fw->major_ver_wanted = 6;
> +		guc_fw->minor_ver_wanted = 1;
>   	} else if (IS_BROXTON(dev)) {
>   		fw_path = I915_BXT_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 8;
> -		guc_fw->guc_fw_minor_wanted = 7;
> +		guc_fw->major_ver_wanted = 8;
> +		guc_fw->minor_ver_wanted = 7;
>   	} else {
>   		fw_path = "";	/* unknown device */
>   	}
>
> -	guc_fw->guc_dev = dev;
> -	guc_fw->guc_fw_path = fw_path;
> -	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> -	guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE;
> +	guc_fw->uc_dev = dev;
> +	guc_fw->uc_fw_path = fw_path;
> +	guc_fw->fetch_status = UC_FIRMWARE_NONE;
> +	guc_fw->load_status = UC_FIRMWARE_NONE;
>
>   	/* Early (and silent) return if GuC loading is disabled */
>   	if (!i915.enable_guc_loading)
> @@ -716,9 +741,9 @@ void intel_guc_init(struct drm_device *dev)
>   	if (*fw_path == '\0')
>   		return;
>
> -	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING;
> +	guc_fw->fetch_status = UC_FIRMWARE_PENDING;
>   	DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
> -	guc_fw_fetch(dev, guc_fw);
> +	intel_uc_fw_fetch(dev, guc_fw);
>   	/* status must now be FAIL or SUCCESS */
>   }
>
> @@ -729,17 +754,17 @@ void intel_guc_init(struct drm_device *dev)
>   void intel_guc_fini(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> +	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
>
>   	mutex_lock(&dev->struct_mutex);
>   	direct_interrupts_to_host(dev_priv);
>   	i915_guc_submission_disable(dev_priv);
>   	i915_guc_submission_fini(dev_priv);
>
> -	if (guc_fw->guc_fw_obj)
> -		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
> -	guc_fw->guc_fw_obj = NULL;
> +	if (guc_fw->uc_fw_obj)
> +		drm_gem_object_unreference(&guc_fw->uc_fw_obj->base);
> +	guc_fw->uc_fw_obj = NULL;
>   	mutex_unlock(&dev->struct_mutex);
>
> -	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> +	guc_fw->fetch_status = UC_FIRMWARE_NONE;
>   }
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-06-28 14:24 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 18:11 [PATCH 0/6] HuC Loading Patches Peter Antoine
2016-06-21 18:11 ` [PATCH 1/6] drm/i915/guc: Make the GuC fw loading helper functions general Peter Antoine
2016-06-28 14:24   ` Dave Gordon [this message]
2016-06-21 18:11 ` [PATCH 2/6] drm/i915/huc: Unified css_header struct for GuC and HuC Peter Antoine
2016-06-28 14:32   ` Dave Gordon
2016-06-30 10:39     ` Antoine, Peter
2016-06-21 18:11 ` [PATCH 3/6] drm/i915/huc: Add HuC fw loading support Peter Antoine
2016-06-22  8:31   ` Daniel Vetter
2016-06-23 10:14     ` Dave Gordon
2016-06-23 13:52       ` Peter Antoine
2016-07-13 12:48         ` Daniel Vetter
2016-07-13 14:52           ` Peter Antoine
2016-07-14 14:16             ` Daniel Vetter
2016-07-14 14:39               ` Dave Gordon
2016-07-14 14:43               ` Peter Antoine
2016-07-14 14:08           ` Dave Gordon
2016-07-14 14:26             ` Daniel Vetter
2016-07-15  7:33               ` Dave Gordon
2016-06-28 14:54   ` Dave Gordon
2016-06-28 15:45     ` Dave Gordon
2016-06-28 23:03       ` Rodrigo Vivi
2016-06-29 14:31         ` Dave Gordon
2016-06-29 17:59           ` Rodrigo Vivi
2016-07-05 14:41             ` Dave Gordon
2016-06-21 18:11 ` [PATCH 4/6] drm/i915/huc: Add debugfs for HuC loading status check Peter Antoine
2016-06-23  8:47   ` Xiang, Haihao
2016-06-23  9:51     ` Peter Antoine
2016-06-23 10:01     ` Peter Antoine
2016-06-23 10:48       ` Michel Thierry
2016-06-23 22:04         ` Kelley, Sean V
2016-07-13  8:13           ` Xiang, Haihao
2016-06-28 14:57   ` Dave Gordon
2016-06-21 18:11 ` [PATCH 5/6] drm/i915/huc: Support HuC authentication Peter Antoine
2016-06-28 15:08   ` Dave Gordon
2016-06-30 10:42     ` Antoine, Peter
2016-06-21 18:11 ` [PATCH 6/6] drm/i915/huc: Add BXT HuC Loading Support Peter Antoine
2016-06-21 18:26   ` Vivi, Rodrigo
2016-06-21 21:28     ` Antoine, Peter
2016-06-28 15:20   ` Dave Gordon
2016-06-22 13:02 ` ✗ Ro.CI.BAT: warning for HuC Loading Patches Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-01-11 21:46 [PATCH 1/6] drm/i915/guc: Make the GuC fw loading helper functions general yu.dai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5772888A.9080004@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lawrence.t.li@intel.com \
    --cc=peter.antoine@intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=sean.v.kelley@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.