[Intel-gfx] [PATCH v5 1/8] drm/i915/skl: Add support to load SKL CSR firmware.
Daniel Vetter
daniel at ffwll.ch
Mon May 4 05:54:35 PDT 2015
On Wed, Apr 29, 2015 at 10:59:20PM +0530, Animesh Manna wrote:
> From: "A.Sunil Kamath" <sunil.kamath at intel.com>
>
> Display Context Save and Restore support is needed for
> various SKL Display C states like DC5, DC6.
>
> This implementation is added based on first version of DMC CSR program
> that we received from h/w team.
>
> Here we are using request_firmware based design.
> Finally this firmware should end up in linux-firmware tree.
>
> For SKL platform its mandatory to ensure that we load this
> csr program before enabling DC states like DC5/DC6.
>
> As CSR program gets reset on various conditions, we should ensure
> to load it during boot and in future change to be added to load
> this system resume sequence too.
>
> v1: Initial relese as RFC patch
>
> v2: Design change as per Daniel, Damien and Shobit's review comments
> request firmware method followed.
>
> v3: Some optimization and functional changes.
> Pulled register defines into drivers/gpu/drm/i915/i915_reg.h
> Used kmemdup to allocate and duplicate firmware content.
> Ensured to free allocated buffer.
>
> v4: Modified as per review comments from Satheesh and Daniel
> Removed temporary buffer.
> Optimized number of writes by replacing I915_WRITE with I915_WRITE64.
>
> v5:
> Modified as per review comemnts from Damien.
> - Changed name for functions and firmware.
> - Introduced HAS_CSR.
> - Reverted back previous change and used csr_buf with u8 size.
> - Using cpu_to_be64 for endianness change.
>
> Modified as per review comments from Imre.
> - Modified registers and macro names to be a bit closer to bspec terminology
> and the existing register naming in the driver.
> - Early return for non SKL platforms in intel_load_csr_program function.
> - Added locking around CSR program load function as it may be called
> concurrently during system/runtime resume.
> - Releasing the fw before loading the program for consistency
> - Handled error path during f/w load.
>
> v6: Modified as per review comments from Imre.
> - Corrected out_freecsr sequence.
>
> v7: Modified as per review comments from Imre.
> Fail loading fw if fw->size%8!=0.
>
> v8: Rebase to latest.
>
> v9: Rebase on top of -nightly (Damien)
>
> v10: Enabled support for dmc firmware ver 1.0.
> According to ver 1.0 in a single binary package all the firmware's that are
> required for different stepping's of the product will be stored. The package
> contains the css header, followed by the package header and the actual dmc
> firmwares. Package header contains the firmware/stepping mapping table and
> the corresponding firmware offsets to the individual binaries, within the
> package. Each individual program binary contains the header and the payload
> sections whose size is specified in the header section. This changes are done
> to extract the specific firmaware from the package. (Animesh)
>
> v11: Modified as per review comemnts from Imre.
> - Added code comment from bpec for header structure elements.
> - Added __packed to avoid structure padding.
> - Added helper functions for stepping and substepping info.
> - Added code comment for CSR_MAX_FW_SIZE.
> - Disabled BXT firmware loading, will be enabled with dmc 1.0 support.
> - Changed skl_stepping_info based on bspec, earlier used from config DB.
> - Removed duplicate call of cpu_to_be* from intel_csr_load_program function.
> - Used cpu_to_be32 instead of cpu_to_be64 as firmware binary in dword aligned.
> - Added sanity check for header length.
> - Added sanity check for mmio address got from firmware binary.
> - kmalloc done separately for dmc header and dmc firmware. (Animesh)
>
> v12: Modified as per review comemnts from Imre.
> - Corrected the typo error in skl stepping info structure.
> - Added out-of-bound access for skl_stepping_info.
> - Sanity check for mmio address modified.
> - Sanity check added for stepping and substeppig.
> - Modified the intel_dmc_info structure, cache only the required header info. (Animesh)
>
> v13: clarify firmware load error message.
> The reason for a firmware loading failure can be obscure if the driver
> is built-in. Provide an explanation to the user about the likely reason for
> the failure and how to resolve it. (Imre)
>
> v14: Suggested by Jani.
> - fix s/I915/CONFIG_DRM_I915/ typo
> - add fw_path to the firmware object instead of using a static ptr (Jani)
>
> v15:
> 1) Changed the firmware name as dmc_gen9.bin, everytime for a new firmware version a symbolic link
> with same name will help not to build kernel again.
> 2) Changes done as per review comments from Imre.
> - Error check removed for intel_csr_ucode_init.
> - Moved csr-specific data structure to intel_csr.h and optimization done on structure definition.
> - fw->data used directly for parsing the header info & memory allocation
> only done separately for payload. (Animesh)
>
> v16:
> - No need for out_regs label in i915_driver_load(), so removed it.
> - Changed the firmware name as skl_dmc_ver1.bin, followed naming convention <platform>_dmc_<api-version>.bin (Animesh)
>
> Issue: VIZ-2569
> Signed-off-by: A.Sunil Kamath <sunil.kamath at intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_dma.c | 11 +-
> drivers/gpu/drm/i915/i915_drv.c | 20 +++
> drivers/gpu/drm/i915/i915_drv.h | 17 ++
> drivers/gpu/drm/i915/intel_csr.c | 367 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 5 +
> 6 files changed, 420 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_csr.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index a69002e..5238deb 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -12,7 +12,8 @@ i915-y := i915_drv.o \
> i915_suspend.o \
> i915_sysfs.o \
> intel_pm.o \
> - intel_runtime_pm.o
> + intel_runtime_pm.o \
> + intel_csr.o
>
> i915-$(CONFIG_COMPAT) += i915_ioc32.o
> i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e44116f..a238889 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -816,6 +816,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> spin_lock_init(&dev_priv->mmio_flip_lock);
> mutex_init(&dev_priv->dpio_lock);
> mutex_init(&dev_priv->modeset_restore_lock);
> + mutex_init(&dev_priv->csr_lock);
>
> intel_pm_setup(dev);
>
> @@ -861,9 +862,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>
> intel_uncore_init(dev);
>
> + /* Load CSR Firmware for SKL */
> + intel_csr_ucode_init(dev);
> +
> ret = i915_gem_gtt_init(dev);
> if (ret)
> - goto out_regs;
> + goto out_freecsr;
>
> /* WARNING: Apparently we must kick fbdev drivers before vgacon,
> * otherwise the vga fbdev driver falls over. */
> @@ -1033,7 +1037,8 @@ out_mtrrfree:
> io_mapping_free(dev_priv->gtt.mappable);
> out_gtt:
> i915_global_gtt_cleanup(dev);
> -out_regs:
> +out_freecsr:
> + intel_csr_ucode_fini(dev);
> intel_uncore_fini(dev);
> pci_iounmap(dev->pdev, dev_priv->regs);
> put_bridge:
> @@ -1113,6 +1118,8 @@ int i915_driver_unload(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
> i915_gem_cleanup_stolen(dev);
>
> + intel_csr_ucode_fini(dev);
> +
> intel_teardown_gmbus(dev);
> intel_teardown_mchbar(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c3fdbb0..acd0e2b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -556,6 +556,26 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
> }
>
> +void i915_firmware_load_error_print(const char *fw_path, int err)
> +{
> + DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> +
> + /*
> + * If the reason is not known assume -ENOENT since that's the most
> + * usual failure mode.
> + */
> + if (!err)
> + err = -ENOENT;
> +
> + if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
> + return;
> +
> + DRM_ERROR(
> + "The driver is built-in, so to load the firmware you need to\n"
> + "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> + "in your initrd/initramfs image.\n");
> +}
> +
> static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 47be4a5..90e47a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -667,6 +667,15 @@ struct intel_uncore {
> #define for_each_fw_domain(domain__, dev_priv__, i__) \
> for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
>
> +struct intel_csr {
> + const char *fw_path;
> + __be32 *dmc_payload;
> + uint32_t dmc_fw_size;
> + uint32_t mmio_count;
> + uint32_t mmioaddr[8];
> + uint32_t mmiodata[8];
> +};
> +
> #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> func(is_mobile) sep \
> func(is_i85x) sep \
> @@ -1573,6 +1582,11 @@ struct drm_i915_private {
>
> struct i915_virtual_gpu vgpu;
>
> + struct intel_csr csr;
> +
> + /* Display CSR-related protection */
> + struct mutex csr_lock;
Another small one: If we have a substruct we put the lock withing that
substruct, e.g. dev_priv->psr.lock. Can you pls do another patch on top to
move that?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list