Re: [PATCH 02/15] drm/i915: Embedded microcontroller (uC) firmware loading support

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jul 03, 2015 at 01:30:24PM +0100, Dave Gordon wrote:
> Current devices may contain one or more programmable microcontrollers
> that need to have a firmware image (aka "binary blob") loaded from an
> external medium and transferred to the device's memory.
> 
> This file provides common support functions for doing this; they can
> then be used by each uC-specific loader, thus reducing code duplication
> and testing effort.
> 
> v2:
>     Local functions should pass dev_priv rather than dev [Chris Wilson]
>     Various other improvements per Chris Wilson's review comments
> 
> v3:
>     Defeatured version without async prefetch [Daniel Vetter]
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile          |   3 +
>  drivers/gpu/drm/i915/intel_uc_loader.c | 310 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc_loader.h |  92 ++++++++++
>  3 files changed, 405 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel_uc_loader.c
>  create mode 100644 drivers/gpu/drm/i915/intel_uc_loader.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index de21965..f1f80fc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -39,6 +39,9 @@ i915-y += i915_cmd_parser.o \
>  	  intel_ringbuffer.o \
>  	  intel_uncore.o
>  
> +# generic ancilliary microcontroller support
> +i915-y += intel_uc_loader.o
> +
>  # autogenerated null render state
>  i915-y += intel_renderstate_gen6.o \
>  	  intel_renderstate_gen7.o \
> diff --git a/drivers/gpu/drm/i915/intel_uc_loader.c b/drivers/gpu/drm/i915/intel_uc_loader.c
> new file mode 100644
> index 0000000..a8fc1dd
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc_loader.c
> @@ -0,0 +1,310 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Author:
> + *	Dave Gordon <david.s.gordon@intel.com>
> + */
> +#include <linux/firmware.h>
> +#include "i915_drv.h"
> +#include "intel_uc_loader.h"
> +
> +/**
> + * DOC: Common functions for embedded microcontroller (uC) firmware loading
> + *
> + * The functions in this file provide common support code for loading the
> + * firmware that may be required by an embedded microcontroller (uC).
> + *
> + * The function intel_uc_fw_init() should be called first; it requires no
> + * locking, and can be called even before GEM has been initialised. It just
> + * initialises the tracking data and stores its parameters for the subsequent
> + * call to intel_uc_fw_fetch().
> + *
> + * At some convenient point after GEM initialisation, the driver should call
> + * intel_uc_fw_fetch(). The first time, this will use the Linux kernel's
> + * request_firmware() call to open and read the firmware image into memory.
> + * (On subsequent calls, this is skipped, as either the firmware has already
> + * been fetched into memory, or it is already known that no valid firmware
> + * image could be found).
> + *
> + * The callback() function passed to intel_uc_fw_fetch() can further check
> + * the firmware image before it is saved. This function can reject the image
> + * by returning a negative error code (e.g. -ENOEXEC), or accept it. In the
> + * latter case, it can return INTEL_UC_FW_GOOD (which is also the default if
> + * no callback() is supplied), and the common code here will save the whole
> + * of the firmware image in a (pageable, shmfs-backed) GEM object.
> + *
> + * If saving the whole image unmodified is not appropriate (for example, if
> + * only a small part of the image is needed later, or the data needs to be
> + * reorganised before saving), the callback() function can instead make its
> + * own arrangements for saving the required data in a GEM object or otherwise
> + * and then return INTEL_UC_FW_SAVED.
> + *
> + * (If such a callback does stash (some of) the image data in a GEM object,
> + * it can use the uc_fw_obj and uc_fw_size fields; the common framework will
> + * then handle deallocating the object on failure or finalisation. Otherwise
> + * any allocated resources will have to be managed by the uC-specific code).
> + *
> + * After a successful call to intel_uc_fw_fetch(), the uC-specific code can
> + * transfer the data in the GEM object (or its own alternative) to the uC's
> + * memory (in some uC-specific way, not handled here).
> + *
> + * During driver shutdown, or if driver load is aborted, intel_uc_fw_fini()
> + * should be called to release any remaining resources.
> + */
> +
> +/* User-friendly representation of an enum */
> +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
> +{
> +	switch (status) {
> +	case INTEL_UC_FIRMWARE_FAIL:
> +		return "FAIL";
> +	case INTEL_UC_FIRMWARE_NONE:
> +		return "NONE";
> +	case INTEL_UC_FIRMWARE_PENDING:
> +		return "PENDING";
> +	case INTEL_UC_FIRMWARE_SUCCESS:
> +		return "SUCCESS";
> +	default:
> +		return "UNKNOWN!";
> +	}
> +};
> +
> +/*
> + * Called once per uC, late in driver initialisation, after GEM is set up.
> + * First, we fetch the firmware image using request_firmware(), then allow
> + * the optional callback() function to check it. If it's good, and callback()
> + * doesn't say it's already saved the image, we will save it here by copying
> + * the whole thing into a (pageable, shmfs-backed) GEM object,
> + */
> +static void
> +uc_fw_fetch(struct intel_uc_fw *uc_fw,
> +	    int callback(struct intel_uc_fw *))
> +{
> +	struct drm_device *dev = uc_fw->uc_dev;
> +	struct drm_i915_gem_object *obj;
> +	const struct firmware *fw;
> +	int cb_status = INTEL_UC_FW_GOOD;
> +
> +	DRM_DEBUG_DRIVER("before waiting: %s fw fetch status %s, fw %p\n",
> +		uc_fw->uc_name,
> +		intel_uc_fw_status_repr(uc_fw->uc_fw_fetch_status),
> +		uc_fw->uc_fw_blob);
> +
> +	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> +	WARN_ON(uc_fw->uc_fw_fetch_status != INTEL_UC_FIRMWARE_PENDING);
> +
> +	if (request_firmware(&fw, uc_fw->uc_fw_path, &dev->pdev->dev))
> +		goto fail;
> +	if (!fw)
> +		goto fail;
> +
> +	DRM_DEBUG_DRIVER("fetch %s fw from %s succeeded, fw %p\n",
> +		uc_fw->uc_name, uc_fw->uc_fw_path, fw);
> +	uc_fw->uc_fw_blob = fw;
> +
> +	/* Callback to the optional uC-specific function, if supplied */
> +	if (callback)
> +		cb_status = callback(uc_fw);
> +	if (cb_status < 0)
> +		goto fail;
> +	switch (cb_status) {
> +	default:
> +		WARN(1, "Invalid status %d from fw checking callback %pf\n",
> +				cb_status, callback);
> +		goto fail;
> +
> +	case INTEL_UC_FW_SAVED:
> +		// Good, already saved, nothing to do
> +		break;
> +
> +	case INTEL_UC_FW_GOOD:
> +		// Good, save in GEM object
> +		obj = i915_gem_object_create_from_data(dev, fw->data, fw->size);
> +		if (!obj)
> +			goto fail;
> +
> +		uc_fw->uc_fw_obj = obj;
> +		uc_fw->uc_fw_size = fw->size;
> +	}
> +
> +	DRM_DEBUG_DRIVER("%s fw fetch status SUCCESS, cb %d, obj %p\n",
> +			uc_fw->uc_name, cb_status, uc_fw->uc_fw_obj);
> +
> +	release_firmware(fw);
> +	uc_fw->uc_fw_blob = NULL;
> +	uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
> +	return;
> +
> +fail:
> +	DRM_DEBUG_DRIVER("%s fw fetch status FAIL; fw %p, obj %p\n",
> +		uc_fw->uc_name, fw, uc_fw->uc_fw_obj);
> +	DRM_ERROR("Failed to fetch %s firmware from %s\n",
> +		  uc_fw->uc_name, uc_fw->uc_fw_path);
> +
> +	obj = uc_fw->uc_fw_obj;
> +	if (obj)
> +		drm_gem_object_unreference(&obj->base);
> +	uc_fw->uc_fw_obj = NULL;
> +
> +	release_firmware(fw);		/* OK even if fw is NULL */
> +	uc_fw->uc_fw_blob = NULL;
> +	uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_FAIL;
> +}

So looking at this the entire library now boils down to a glorified
version of

	if (request_firmware != 0) {
		DRM_DEBUG_GEM("guc firmware loading failed\n");
		return fail;
	}

	gem_obj_create_from_date(fw)
	relase_firmware

Can I have that inlined (or as a simple static helper function) into the
callsite please? Yes that unfortunately means I ask you to can the really
great kerneldoc you've done (I'd really like all patches to look this
great wrt docs), but I also think that the abstraction really doesn't pay
off. Firmware-loading using request_firmware is not something tricky
enough to justify added abstraction. Yes there was the
synchronization/completion stuff in there before, but as explained in the
previous discussion I don't expect us to be able to share that part at
all, and I'd like any kind of synchronization to be open-coded using
normal kernel primitives (like flush_work or similar) to make it really
stick out to reviewers.

Thanks, Daniel


> +
> +/**
> + * intel_uc_fw_fetch() - fetch the firmware image
> + * @uc_fw:	intel_uc_fw structure
> + * @callback:	optional callback function to validate and/or save the image
> + *
> + * If the fetch is not PENDING (i.e. on the second and subsequent calls), this
> + * function will just return an approriate errno, based on the previously-set
> + * status.
> + *
> + * On the first call only, it will try to retrieve the firmaware image using
> + * the parameters set earlier. If an image is found, the optional @callback()
> + * will be called to further validate it.
> + *
> + * When it is called, @uc_fw->uc_fw_blob refers to the fetched firmware image,
> + * and @uc_fw->uc_fw_obj is NULL. The @callback() function can return an error
> + * code (any negative value), in which case the image will be rejected.  The
> + * fetch status will be set to FAIL, and this function will return -EIO.
> + *
> + * Or, @callback() can return INTEL_UC_FW_GOOD. The image is considered good,
> + * and it will be saved in a GEM object as described above. In this case,
> + * @uc_fw->uc_fw_obj will be set to point to the GEM object, and the size of
> + * the image will be in @uc_fw->uc_fw_size. This is also the default if no
> + * @callback() is supplied.
> + *
> + * Finally, @callback() can return INTEL_UC_FW_SAVED. The image is considered
> + * good, but @callback() has already deal with saving the content, so this
> + * common code will not allocate and fill a GEM object. @callback() may use
> + * @uc_fw->uc_fw_obj to hold a reference to its own GEM object, if it has
> + * allocated one, and in this case the common code will deal with disposing
> + * of it on error or finalisation; otherwise it can make its own arragements.
> + *
> + * After this call, @uc_fw->uc_fw_fetch_status will show whether the firmware
> + * image was successfully fetched and saved.
> + *
> + * In all cases the firmware blob is released before this function returns.
> + *
> + * Return:	non-zero code on error
> + */
> +int
> +intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,
> +		  int callback(struct intel_uc_fw *))
> +{
> +	WARN_ON(!mutex_is_locked(&uc_fw->uc_dev->struct_mutex));
> +
> +	if (uc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_PENDING) {
> +		/* We only come here once */
> +		uc_fw_fetch(uc_fw, callback);
> +		/* status must now be FAIL or SUCCESS */
> +	}
> +
> +	DRM_DEBUG_DRIVER("%s fw fetch status %s\n", uc_fw->uc_name,
> +		intel_uc_fw_status_repr(uc_fw->uc_fw_fetch_status));
> +
> +	switch (uc_fw->uc_fw_fetch_status) {
> +	case INTEL_UC_FIRMWARE_FAIL:
> +		/* something went wrong :( */
> +		return -EIO;
> +
> +	case INTEL_UC_FIRMWARE_NONE:
> +		/* no firmware, nothing to do (not an error) */
> +		return 0;
> +
> +	case INTEL_UC_FIRMWARE_PENDING:
> +	default:
> +		/* "can't happen" */
> +		WARN_ONCE(1, "%s fw %s invalid uc_fw_fetch_status %s [%d]\n",
> +			uc_fw->uc_name, uc_fw->uc_fw_path,
> +			intel_uc_fw_status_repr(uc_fw->uc_fw_fetch_status),
> +			uc_fw->uc_fw_fetch_status);
> +		return -ENXIO;
> +
> +	case INTEL_UC_FIRMWARE_SUCCESS:
> +		return 0;
> +	}
> +}
> +
> +/**
> + * intel_uc_fw_init() - define parameters for fetching firmware
> + * @dev:	drm device
> + * @uc_fw:	intel_uc_fw structure
> + * @name:	human-readable device name (e.g. "GuC") for messages
> + * @fw_path:	(trailing parts of) path to firmware (e.g. "i915/guc_fw.bin")
> + * 		@fw_path == NULL means "no firmware expected" (not an error),
> + * 		@fw_path == "" (empty string) means "firmware unknown" i.e.
> + * 		the uC requires firmware, but the driver doesn't know where
> + * 		to find the proper version. This will be logged as an error.
> + *
> + * This is called just once per uC, during driver loading. It is therefore
> + * automatically single-threaded and does not need to acquire any mutexes
> + * or spinlocks. OTOH, GEM is not yet fully initialised, so we can't do
> + * very much here.
> + *
> + * The main task here is to save the parameters for later. The actual fetching
> + * will happen when intel_uc_fw_init() is called, after GEM initialisation is
> + * complete.
> + */
> +void
> +intel_uc_fw_init(struct drm_device *dev, struct intel_uc_fw *uc_fw,
> +		 const char *name, const char *fw_path)
> +{
> +	uc_fw->uc_dev = dev;
> +	uc_fw->uc_name = name;
> +	uc_fw->uc_fw_path = fw_path;
> +	uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_NONE;
> +	uc_fw->uc_fw_load_status = INTEL_UC_FIRMWARE_NONE;
> +
> +	if (fw_path == NULL)
> +		return;
> +
> +	if (*fw_path == '\0') {
> +		DRM_ERROR("No %s firmware known for this platform\n", name);
> +		uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_FAIL;
> +		return;
> +	}
> +
> +	uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_PENDING;
> +	DRM_DEBUG_DRIVER("%s firmware pending, path %s\n",
> +		name, fw_path);
> +}
> +
> +/**
> + * intel_uc_fw_fini() - clean up all uC firmware-related data
> + * @uc_fw:	intel_uc_fw structure
> + */
> +void
> +intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
> +{
> +	WARN_ON(!mutex_is_locked(&uc_fw->uc_dev->struct_mutex));
> +
> +	if (uc_fw->uc_fw_obj)
> +		drm_gem_object_unreference(&uc_fw->uc_fw_obj->base);
> +	uc_fw->uc_fw_obj = NULL;
> +
> +	release_firmware(uc_fw->uc_fw_blob);	/* OK even if NULL */
> +	uc_fw->uc_fw_blob = NULL;
> +
> +	uc_fw->uc_fw_fetch_status = INTEL_UC_FIRMWARE_NONE;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc_loader.h b/drivers/gpu/drm/i915/intel_uc_loader.h
> new file mode 100644
> index 0000000..b710406
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc_loader.h
> @@ -0,0 +1,92 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Author:
> + *	Dave Gordon <david.s.gordon@intel.com>
> + */
> +#ifndef _INTEL_UC_LOADER_H
> +#define _INTEL_UC_LOADER_H
> +
> +/*
> + * Microcontroller (uC) firmware loading support
> + */
> +
> +/*
> + * These values are used to track the stages of getting the required firmware
> + * into an onboard microcontroller. The common code tracks the phases of
> + * fetching the firmware (aka "binary blob") from an external file into a GEM
> + * object in the 'uc_fw_fetch_status' field below; the uC-specific DMA code
> + * uses the 'uc_fw_load_status' field to track the transfer from GEM object
> + * to uC memory.
> + *
> + * For the first (fetch) stage, the interpretation of the values is:
> + * NONE - no firmware is being fetched e.g. because there is no uC
> + * PENDING - parameters initialised, pending call to intel_uc_fw_fetch()
> + * SUCCESS - uC firmware fetched into a GEM object and ready for use
> + * FAIL - something went wrong; uC firmware is not available
> + *
> + * The second (load) stage is similar:
> + * NONE - no firmware is being loaded e.g. because there is no uC
> + * PENDING - firmware DMA load in progress
> + * SUCCESS - uC firmware loaded into uC memory and ready for use
> + * FAIL - something went wrong; uC firmware is not available
> + *
> + * The function intel_uc_fw_status_repr() will convert this enum to a
> + * string representation suitable for use in log messages.
> + */
> +enum intel_uc_fw_status {
> +	INTEL_UC_FIRMWARE_FAIL = -1,
> +	INTEL_UC_FIRMWARE_NONE = 0,
> +	INTEL_UC_FIRMWARE_PENDING,
> +	INTEL_UC_FIRMWARE_SUCCESS
> +};
> +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
> +
> +/*
> + * This structure encapsulates all the data needed during the process of
> + * fetching, caching, and loading the firmware image into the uC.
> + */
> +struct intel_uc_fw {
> +	struct drm_device *		uc_dev;
> +	const char *			uc_name;
> +	const char *			uc_fw_path;
> +	const struct firmware *		uc_fw_blob;
> +	size_t				uc_fw_size;
> +	struct drm_i915_gem_object *	uc_fw_obj;
> +	enum intel_uc_fw_status		uc_fw_fetch_status;
> +	enum intel_uc_fw_status		uc_fw_load_status;
> +};
> +
> +void intel_uc_fw_init(struct drm_device *dev, struct intel_uc_fw *uc_fw,
> +		const char *uc_name, const char *fw_path);
> +int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,
> +		int callback(struct intel_uc_fw *));
> +void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
> +
> +/*
> + * The callback() function passed to intel_uc_fw_fetch() above can return
> + * a negative number (a standard error code), or one of the values below:
> + */
> +#define	INTEL_UC_FW_GOOD	1	/* f/w good, save in GEM object	*/
> +#define	INTEL_UC_FW_SAVED	2	/* f/w good, already saved	*/
> +
> +#endif
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx