| Message ID | 1435926637-30892-3-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On 06/07/15 15:06, Daniel Vetter wrote: > 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 Well, I /could/ squash these functions into the GuC-specific loader and put a "g" on the front of all the names, but that would just result in the whole lot getting copypasted when we come to do the next letter of the alphabet :( Or worse still, someone else would write a completely new version from scratch, which would be a waste of effort both for them and for those who would then have to review the new code, spot the similarities and differences, and ask for it all to be rewritten to be more like the by-then-existing GuC code. It's also not quite as simple as your paraphrase; even without the prefetch code, it also deals with the do-this-once logic, and correctly freeing resources in all error paths; not enormously complicated but why repeat similar code in several places when you can just share the one instance (and there /will/ be other users for this code). And we may well add other common functionality here, should it turn out useful for more than one uC. You're just asking to increase the maintenance effort by duplicating the loader code rather than sharing it :( Conversely, if you think it adds little functionality, it also adds little overhead. I wouldn't much want to inline the currently-generic functions into their callers also because that would bloat them too much; at the moment, with the split between GuC-specific and common functions, the biggest functions turn out to be about one screenful. If we "open-code" it all we end up with monstrous 300-line-plus functions with any number of error paths, which is just not nice. So I'd really prefer to keep the utility code and the GuC-specific stuff tidily separated even before we have the other potential users of this support code in upstream. .Dave.
On Mon, Jul 06, 2015 at 07:24:37PM +0100, Dave Gordon wrote: > On 06/07/15 15:06, Daniel Vetter wrote: > >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 > > Well, I /could/ squash these functions into the GuC-specific loader and put > a "g" on the front of all the names, but that would just result in the whole > lot getting copypasted when we come to do the next letter of the alphabet :( > > Or worse still, someone else would write a completely new version from > scratch, which would be a waste of effort both for them and for those who > would then have to review the new code, spot the similarities and > differences, and ask for it all to be rewritten to be more like the > by-then-existing GuC code. > > It's also not quite as simple as your paraphrase; even without the prefetch > code, it also deals with the do-this-once logic, and correctly freeing > resources in all error paths; not enormously complicated but why repeat > similar code in several places when you can just share the one instance (and > there /will/ be other users for this code). And we may well add other common > functionality here, should it turn out useful for more than one uC. You're > just asking to increase the maintenance effort by duplicating the loader > code rather than sharing it :( do-this-once is solved easily by moving the do-it-once parts into gem_init, which is called once at driver load. The complexity seems to easy to reduce by dropping all the fancy loader status logic - the driver only cares about success y/n. And for "no" we just declare the gpu dead. Anything in addition to that can be dumped into dmesg using appropriate DRM_DEBUG* for developers consumption. No need for debugfs or anything else really imo. That leaves a small functions which computes the firmware name per platfrom, does the request_firmware, allocates a gem object, copies the firmware data over, releases the firmware and then returns the resulting gem object (or NULL when anything went wrong). guc init in gem_init_hw can then consume that. Once we have the guc+1 landing we can just export that or rename intel_guc_* to intel_gt_* and that's it - the loader here isn't generic because it copies the firmware to gem objects, which isn't actually what the dmc loader needs. > Conversely, if you think it adds little functionality, it also adds little > overhead. I wouldn't much want to inline the currently-generic functions > into their callers also because that would bloat them too much; at the > moment, with the split between GuC-specific and common functions, the > biggest functions turn out to be about one screenful. If we "open-code" it > all we end up with monstrous 300-line-plus functions with any number of > error paths, which is just not nice. It adds a fairly complex interface (and a non-standard one on top with all the custom state defines), and it starts out with generic code instead of the more common extract common code after the fact. Desinging interface before the users are really clear tends to result in overfitted design (you wanted to use this for dmc too after all where it doesn't really fit that well). > So I'd really prefer to keep the utility code and the GuC-specific stuff > tidily separated even before we have the other potential users of this > support code in upstream. I'm not opposed to a small helper which essentially would do request_firmware+copy to gem bo. This here is a lot more than that. -Daniel
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; +} + +/** + * 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
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