Commit 42f96e5b authored by Daniele Ceraolo Spurio's avatar Daniele Ceraolo Spurio Committed by Chris Wilson
Browse files

drm/i915/uc: consolidate firmware cleanup 



We are quite trigger happy in cleaning up the firmware blobs, as we do
so from several error/fini paths in GuC/HuC/uC code. We do have the
__uc_cleanup_firmwares cleanup function, which unwinds
__uc_fetch_firmwares and is already called both from the error path of
gem_init and from gem_driver_release, so let's stop cleaning up from
all the other paths.

The fact that we're not cleaning the firmware immediately means that
we can't consider firmware availability as an indication of
initialization success. A "LOADABLE" status has been added to
indicate that the initialization was successful, to be used to
selectively load HuC only if HuC init has completed (HuC init failure
is not considered a fatal error).

v2: s/ready_to_load/loadable (Michal), only run guc/huc_fini if the
    fw is in loadable state

Signed-off-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> #v1
Reviewed-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20200218223327.11058-9-daniele.ceraolospurio@intel.com
parent 3acffa8c

drivers/gpu/drm/i915/gt/uc/intel_guc.c

+5 −7
Original line number Diff line number Diff line
@@ -333,7 +333,7 @@ int intel_guc_init(struct intel_guc *guc) 


	ret = intel_uc_fw_init(&guc->fw);

	if (ret)

		goto err_fetch;

		goto out;



	ret = intel_guc_log_create(&guc->log);

	if (ret)
@@ -364,6 +364,8 @@ int intel_guc_init(struct intel_guc *guc) 
	/* We need to notify the guc whenever we change the GGTT */

	i915_ggtt_enable_guc(gt->ggtt);



	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_LOADABLE);



	return 0;



err_ct:
@@ -374,8 +376,7 @@ err_log: 
	intel_guc_log_destroy(&guc->log);

err_fw:

	intel_uc_fw_fini(&guc->fw);

err_fetch:

	intel_uc_fw_cleanup_fetch(&guc->fw);

out:

	i915_probe_error(gt->i915, "failed with %d\n", ret);

	return ret;

}
@@ -384,7 +385,7 @@ void intel_guc_fini(struct intel_guc *guc) 
{

	struct intel_gt *gt = guc_to_gt(guc);



	if (!intel_uc_fw_is_available(&guc->fw))

	if (!intel_uc_fw_is_loadable(&guc->fw))

		return;



	i915_ggtt_disable_guc(gt->ggtt);
@@ -397,9 +398,6 @@ void intel_guc_fini(struct intel_guc *guc) 
	intel_guc_ads_destroy(guc);

	intel_guc_log_destroy(&guc->log);

	intel_uc_fw_fini(&guc->fw);

	intel_uc_fw_cleanup_fetch(&guc->fw);



	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_DISABLED);

}



/*

drivers/gpu/drm/i915/gt/uc/intel_huc.c

+3 −2
Original line number Diff line number Diff line
@@ -121,19 +121,20 @@ int intel_huc_init(struct intel_huc *huc) 
	if (err)

		goto out_fini;



	intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_LOADABLE);



	return 0;



out_fini:

	intel_uc_fw_fini(&huc->fw);

out:

	intel_uc_fw_cleanup_fetch(&huc->fw);

	i915_probe_error(i915, "failed with %d\n", err);

	return err;

}



void intel_huc_fini(struct intel_huc *huc)

{

	if (!intel_uc_fw_is_available(&huc->fw))

	if (!intel_uc_fw_is_loadable(&huc->fw))

		return;



	intel_huc_rsa_data_destroy(huc);

drivers/gpu/drm/i915/gt/uc/intel_uc.c

+1 −1
Original line number Diff line number Diff line
@@ -417,7 +417,7 @@ static int __uc_init_hw(struct intel_uc *uc) 
	GEM_BUG_ON(!intel_uc_supports_guc(uc));

	GEM_BUG_ON(!intel_uc_wants_guc(uc));



	if (!intel_uc_fw_is_available(&guc->fw)) {

	if (!intel_uc_fw_is_loadable(&guc->fw)) {

		ret = __uc_check_hw(uc) ||

		      intel_uc_fw_is_overridden(&guc->fw) ||

		      intel_uc_wants_guc_submission(uc) ?

drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c

+5 −2
Original line number Diff line number Diff line
@@ -501,7 +501,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 dst_offset, u32 dma_flags) 
	if (err)

		return err;



	if (!intel_uc_fw_is_available(uc_fw))

	if (!intel_uc_fw_is_loadable(uc_fw))

		return -ENOEXEC;



	/* Call custom loader */
@@ -544,7 +544,10 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw) 


void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)

{

	intel_uc_fw_cleanup_fetch(uc_fw);

	if (i915_gem_object_has_pinned_pages(uc_fw->obj))

		i915_gem_object_unpin_pages(uc_fw->obj);



	intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_AVAILABLE);

}



/**

drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h

+15 −3
Original line number Diff line number Diff line
@@ -29,8 +29,11 @@ struct intel_gt; 
 * |            |                 SELECTED                          |

 * +------------+-               /   |   \                         -+

 * |            |    MISSING <--/    |    \--> ERROR                |

 * |   fetch    |                    |                              |

 * |            |        /------> AVAILABLE <---<-----------\       |

 * |   fetch    |                    V                              |

 * |            |                 AVAILABLE                         |

 * +------------+-                   |                             -+

 * |   init     |                    V                              |

 * |            |        /------> LOADABLE <----<-----------\       |

 * +------------+-       \         /    \        \           \     -+

 * |            |         FAIL <--<      \--> TRANSFERRED     \     |

 * |   upload   |                  \           /   \          /     |
@@ -46,6 +49,7 @@ enum intel_uc_fw_status { 
	INTEL_UC_FIRMWARE_MISSING, /* blob not found on the system */

	INTEL_UC_FIRMWARE_ERROR, /* invalid format or version */

	INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */

	INTEL_UC_FIRMWARE_LOADABLE, /* all fw-required objects are ready */

	INTEL_UC_FIRMWARE_FAIL, /* failed to xfer or init/auth the fw */

	INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */

	INTEL_UC_FIRMWARE_RUNNING /* init/auth done */
@@ -115,6 +119,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status) 
		return "ERROR";

	case INTEL_UC_FIRMWARE_AVAILABLE:

		return "AVAILABLE";

	case INTEL_UC_FIRMWARE_LOADABLE:

		return "LOADABLE";

	case INTEL_UC_FIRMWARE_FAIL:

		return "FAIL";

	case INTEL_UC_FIRMWARE_TRANSFERRED:
@@ -143,6 +149,7 @@ static inline int intel_uc_fw_status_to_error(enum intel_uc_fw_status status) 
	case INTEL_UC_FIRMWARE_SELECTED:

		return -ESTALE;

	case INTEL_UC_FIRMWARE_AVAILABLE:

	case INTEL_UC_FIRMWARE_LOADABLE:

	case INTEL_UC_FIRMWARE_TRANSFERRED:

	case INTEL_UC_FIRMWARE_RUNNING:

		return 0;
@@ -184,6 +191,11 @@ static inline bool intel_uc_fw_is_available(struct intel_uc_fw *uc_fw) 
	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_AVAILABLE;

}



static inline bool intel_uc_fw_is_loadable(struct intel_uc_fw *uc_fw)

{

	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_LOADABLE;

}



static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)

{

	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_TRANSFERRED;
@@ -202,7 +214,7 @@ static inline bool intel_uc_fw_is_overridden(const struct intel_uc_fw *uc_fw) 
static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)

{

	if (intel_uc_fw_is_loaded(uc_fw))

		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_AVAILABLE);

		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOADABLE);

}



static inline u32 __intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)

CRA Git | Maintained and supported by SUSTech CRA and CCSE