| Message ID | 1428920642-23712-1-git-send-email-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2015-04-13 at 15:54 +0530, Animesh Manna wrote: > From: "A.Sunil Kamath" <sunil.kamath@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. (Animesh) > > Issue: VIZ-2569 > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: Imre Deak <imre.deak@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 | 234 +++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_csr.h | 164 +++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 5 + > 7 files changed, 451 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_csr.c > create mode 100644 drivers/gpu/drm/i915/intel_csr.h > > 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; > + > struct intel_gmbus gmbus[GMBUS_NUM_PINS]; > > /** gmbus_mutex protects against concurrent usage of the single hw gmbus > @@ -2425,6 +2439,8 @@ struct drm_i915_cmd_table { > #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) > #define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev)) > > +#define HAS_CSR(dev) (IS_SKYLAKE(dev)) > + > #define INTEL_PCH_DEVICE_ID_MASK 0xff00 > #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 > #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 > @@ -2515,6 +2531,7 @@ extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); > extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); > int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); > void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); > +void i915_firmware_load_error_print(const char *fw_path, int err); > > /* i915_irq.c */ > void i915_queue_hangcheck(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > new file mode 100644 > index 0000000..ab9b16b > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -0,0 +1,234 @@ > +/* > + * 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. > + * > + */ > +#include <linux/firmware.h> > +#include "i915_drv.h" > +#include "i915_reg.h" > +#include "intel_csr.h" > + > +static struct stepping_info skl_stepping_info[] = { Nitpick: this should be const. > + {'A', '0'}, {'B', '0'}, {'C', '0'}, > + {'D', '0'}, {'E', '0'}, {'F', '0'}, > + {'G', '0'}, {'H', '0'}, {'I', '0'} > +}; > + > +static char intel_get_stepping(struct drm_device *dev) > +{ > + if (IS_SKYLAKE(dev) && (dev->pdev->revision < > + ARRAY_SIZE(skl_stepping_info))) > + return skl_stepping_info[dev->pdev->revision].stepping; Nitpick: at this point it's just if (dev->pdev->revision < 9) return 'A' + dev->pdev->revision; w/o the need for skl_stepping_info. > + else > + return -ENODATA; > +} > + > +static char intel_get_substepping(struct drm_device *dev) > +{ > + if (IS_SKYLAKE(dev) && (dev->pdev->revision < > + ARRAY_SIZE(skl_stepping_info))) > + return skl_stepping_info[dev->pdev->revision].substepping; and this one is simply return 0; > + else > + return -ENODATA; > +} Missing newline. > +void intel_csr_load_program(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + __be32 *payload = dev_priv->csr.dmc_payload; > + uint32_t i, fw_size; > + > + if (!IS_GEN9(dev)) { > + DRM_ERROR("No CSR support available for this platform\n"); > + return; > + } > + > + mutex_lock(&dev_priv->csr_lock); > + /* fw_size is dwords, converting it to bytes and nearest 8 multiple. */ The above comment is stale now. > + fw_size = dev_priv->csr.dmc_fw_size; > + for (i = 0; i < fw_size; i++) > + I915_WRITE(CSR_PROGRAM_BASE + i * 4, > + (u32 __force)payload[i]); > + > + for (i = 0; i < dev_priv->csr.mmio_count; i++) { > + I915_WRITE(dev_priv->csr.mmioaddr[i], > + dev_priv->csr.mmiodata[i]); > + } > + mutex_unlock(&dev_priv->csr_lock); > +} > + > +static void finish_csr_load(const struct firmware *fw, void *context) > +{ > + struct drm_i915_private *dev_priv = context; > + struct drm_device *dev = dev_priv->dev; > + struct intel_css_header *css_header; > + struct intel_package_header *package_header; > + struct intel_dmc_header *dmc_header; > + struct intel_csr *csr = &dev_priv->csr; > + char stepping = intel_get_stepping(dev); > + char substepping = intel_get_substepping(dev); > + uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; > + uint32_t i; > + __be32 *dmc_payload; > + > + if (!fw) { > + i915_firmware_load_error_print(csr->fw_path, 0); > + goto out; > + } > + > + if ((stepping == -ENODATA) || (substepping == -ENODATA)) { > + DRM_ERROR("Unknown stepping info, firmware loading failed\n"); > + goto out; > + } > + > + /* Extract CSS Header information*/ > + css_header = (struct intel_css_header *)fw->data; > + if (sizeof(struct intel_css_header) != > + (css_header->header_len * 4)) { > + DRM_ERROR("Firmware has wrong CSS header length %u bytes\n", > + (css_header->header_len * 4)); > + goto out; > + } > + readcount += sizeof(struct intel_css_header); > + > + /* Extract Package Header information*/ > + package_header = (struct intel_package_header *) > + &fw->data[readcount]; > + if (sizeof(struct intel_package_header) != > + (package_header->header_len * 4)) { > + DRM_ERROR("Firmware has wrong package header length %u bytes\n", > + (package_header->header_len * 4)); > + goto out; > + } > + readcount += sizeof(struct intel_package_header); > + > + /* Search for dmc_offset to find firware binary. */ > + for (i = 0; i < package_header->num_entries; i++) { > + if (package_header->fw_info[i].substepping == '*' && > + stepping == package_header->fw_info[i].stepping) { > + dmc_offset = package_header->fw_info[i].offset; > + break; > + } else if (stepping == package_header->fw_info[i].stepping && > + substepping == package_header->fw_info[i].substepping) { > + dmc_offset = package_header->fw_info[i].offset; > + break; > + } else if (package_header->fw_info[i].stepping == '*' && > + package_header->fw_info[i].substepping == '*') > + dmc_offset = package_header->fw_info[i].offset; > + } > + if (dmc_offset == CSR_DEFAULT_FW_OFFSET) { > + DRM_ERROR("Firmware not supported for %c stepping\n", stepping); > + goto out; > + } > + readcount += dmc_offset; > + > + /* Extract dmc_header information. */ > + dmc_header = (struct intel_dmc_header *)&fw->data[readcount]; > + if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) { > + DRM_ERROR("Firmware has wrong dmc header length %u bytes\n", > + (dmc_header->header_len)); > + goto out; > + } > + readcount += sizeof(struct intel_dmc_header); > + > + /* Cache the dmc header info. */ > + if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) { > + DRM_ERROR("Firmware has wrong mmio count %u\n", > + dmc_header->mmio_count); > + goto out; > + } > + csr->mmio_count = dmc_header->mmio_count; > + for (i = 0; i < dmc_header->mmio_count; i++) { > + if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE && > + dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) { > + DRM_ERROR(" Firmware has wrong mmio address 0x%x\n", > + dmc_header->mmioaddr[i]); > + goto out; > + } > + csr->mmioaddr[i] = dmc_header->mmioaddr[i]; > + csr->mmiodata[i] = dmc_header->mmiodata[i]; > + } > + > + /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */ > + nbytes = dmc_header->fw_size * 4; > + if (nbytes > CSR_MAX_FW_SIZE) { > + DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes); > + goto out; > + } > + csr->dmc_fw_size = dmc_header->fw_size; > + > + csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL); > + if (!csr->dmc_payload) { > + DRM_ERROR("Memory allocation failed for dmc payload\n"); > + goto out; > + } > + > + dmc_payload = csr->dmc_payload; > + for (i = 0; i < dmc_header->fw_size; i++) { > + uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4]; > + /* > + * The firmware payload is an array of 32 bit words stored in > + * little-endian format in the firmware image and programmed > + * as 32 bit big-endian format to memory. > + */ > + dmc_payload[i] = cpu_to_be32(*tmp); > + } > + > + /* load csr program during system boot, as needed for DC states */ > + intel_csr_load_program(dev); > +out: > + release_firmware(fw); > +} > + > +void intel_csr_ucode_init(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_csr *csr = &dev_priv->csr; > + int ret; > + > + if (!HAS_CSR(dev)) > + return; > + > + if (IS_SKYLAKE(dev)) > + csr->fw_path = I915_CSR_SKL; > + else { > + DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); > + return; > + } > + > + /* CSR supported for platform, load firmware */ > + ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path, > + &dev_priv->dev->pdev->dev, > + GFP_KERNEL, dev_priv, > + finish_csr_load); > + if (ret) > + i915_firmware_load_error_print(csr->fw_path, ret); > + > +} > + > +void intel_csr_ucode_fini(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!HAS_CSR(dev)) > + return; > + > + kfree(dev_priv->csr.dmc_payload); > +} > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > new file mode 100644 > index 0000000..c2a5a53 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_csr.h > @@ -0,0 +1,164 @@ > +/* > + * 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. > + * > + */ > + > +#ifndef _INTEL_CSR_H_ > +#define _INTEL_CSR_H_ > + > +#define I915_CSR_SKL "i915/dmc_gen9.bin" > + > +MODULE_FIRMWARE(I915_CSR_SKL); > + > +/* > +* SKL CSR registers for DC5 and DC6 > +*/ > +#define CSR_PROGRAM_BASE 0x80000 > +#define CSR_HEADER_OFFSET 128 > +#define CSR_SSP_BASE_ADDR_GEN9 0x00002FC0 > +#define CSR_HTP_ADDR_SKL 0x00500034 > +#define CSR_SSP_BASE 0x8F074 > +#define CSR_HTP_SKL 0x8F004 > +#define CSR_LAST_WRITE 0x8F034 > +#define CSR_LAST_WRITE_VALUE 0xc003b400 > +/* MMIO address range for CSR program (0x80000 - 0x82FFF) */ > +#define CSR_MAX_FW_SIZE 0x2FFF > +#define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF > +#define CSR_MMIO_START_RANGE 0x80000 > +#define CSR_MMIO_END_RANGE 0x8FFFF > + > +struct intel_css_header { > + /* 0x09 for DMC */ > + uint32_t module_type; > + > + /* Includes the DMC specific header in dwords */ > + uint32_t header_len; > + > + /* always value would be 0x10000 */ > + uint32_t header_ver; > + > + /* Not used */ > + uint32_t module_id; > + > + /* Not used */ > + uint32_t module_vendor; > + > + /* in YYYYMMDD format */ > + uint32_t date; > + > + /* Size in dwords (CSS_Headerlen + PackageHeaderLen + dmc FWsLen)/4 */ > + uint32_t size; > + > + /* Not used */ > + uint32_t key_size; > + > + /* Not used */ > + uint32_t modulus_size; > + > + /* Not used */ > + uint32_t exponent_size; > + > + /* Not used */ > + uint32_t reserved1[12]; > + > + /* Major Minor */ > + uint32_t version; > + > + /* Not used */ > + uint32_t reserved2[8]; > + > + /* Not used */ > + uint32_t kernel_header_info; > +} __packed; > + > +struct intel_fw_info { > + uint16_t reserved1; > + > + /* Stepping (A, B, C, ..., *). * is a wildcard */ > + char stepping; > + > + /* Sub-stepping (0, 1, ..., *). * is a wildcard */ > + char substepping; > + > + uint32_t offset; > + uint32_t reserved2; > +} __packed; > + > +struct intel_package_header { > + /* DMC container header length in dwords */ > + unsigned char header_len; > + > + /* always value would be 0x01 */ > + unsigned char header_ver; > + > + unsigned char reserved[10]; > + > + /* Number of valid entries in the FWInfo array below */ > + uint32_t num_entries; > + > + struct intel_fw_info fw_info[20]; > +} __packed; > + > +struct intel_dmc_header { > + /* always value would be 0x40403E3E */ > + uint32_t signature; > + > + /* DMC binary header length */ > + unsigned char header_len; > + > + /* 0x01 */ > + unsigned char header_ver; > + > + /* Reserved */ > + uint16_t dmcc_ver; > + > + /* Major, Minor */ > + uint32_t project; > + > + /* Firmware program size (excluding header) in dwords */ > + uint32_t fw_size; > + > + /* Major Minor version */ > + uint32_t fw_version; > + > + /* Number of valid MMIO cycles present. */ > + uint32_t mmio_count; > + > + /* MMIO address */ > + uint32_t mmioaddr[8]; > + > + /* MMIO data */ > + uint32_t mmiodata[8]; > + > + /* FW filename */ > + unsigned char dfile[32]; > + > + uint32_t reserved1[2]; > +} __packed; > + > +struct stepping_info { > + char stepping; > + char substepping; > +} __packed; stepping_info doesn't need to be packed. What I meant is to add all these to intel_csr.c. I don't think there is any reason for a new header file unless you need to share something among multiple .c modules. --Imre > + > + > +#endif /* _INTEL_CSR_H_ */ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 7a0aa24..f3a2d88 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1062,6 +1062,11 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); > unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane, > struct drm_i915_gem_object *obj); > > +/* intel_csr.c */ > +void intel_csr_ucode_init(struct drm_device *dev); > +void intel_csr_load_program(struct drm_device *dev); > +void intel_csr_ucode_fini(struct drm_device *dev); > + > /* intel_dp.c */ > void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); > bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
On ma, 2015-04-13 at 18:37 +0530, Animesh Manna wrote: > > On 04/13/2015 04:33 PM, Imre Deak wrote: > > On ma, 2015-04-13 at 15:54 +0530, Animesh Manna wrote: > >> From: "A.Sunil Kamath" <sunil.kamath@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. (Animesh) > >> > >> Issue: VIZ-2569 > >> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > >> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> > >> Signed-off-by: Imre Deak <imre.deak@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 | 234 +++++++++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_csr.h | 164 +++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_drv.h | 5 + > >> 7 files changed, 451 insertions(+), 3 deletions(-) > >> create mode 100644 drivers/gpu/drm/i915/intel_csr.c > >> create mode 100644 drivers/gpu/drm/i915/intel_csr.h > >> > >> 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; > >> + > >> struct intel_gmbus gmbus[GMBUS_NUM_PINS]; > >> > >> /** gmbus_mutex protects against concurrent usage of the single hw gmbus > >> @@ -2425,6 +2439,8 @@ struct drm_i915_cmd_table { > >> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) > >> #define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev)) > >> > >> +#define HAS_CSR(dev) (IS_SKYLAKE(dev)) > >> + > >> #define INTEL_PCH_DEVICE_ID_MASK 0xff00 > >> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 > >> #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 > >> @@ -2515,6 +2531,7 @@ extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); > >> extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); > >> int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); > >> void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); > >> +void i915_firmware_load_error_print(const char *fw_path, int err); > >> > >> /* i915_irq.c */ > >> void i915_queue_hangcheck(struct drm_device *dev); > >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > >> new file mode 100644 > >> index 0000000..ab9b16b > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i915/intel_csr.c > >> @@ -0,0 +1,234 @@ > >> +/* > >> + * 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. > >> + * > >> + */ > >> +#include <linux/firmware.h> > >> +#include "i915_drv.h" > >> +#include "i915_reg.h" > >> +#include "intel_csr.h" > >> + > >> +static struct stepping_info skl_stepping_info[] = { > > Nitpick: this should be const. > > > >> + {'A', '0'}, {'B', '0'}, {'C', '0'}, > >> + {'D', '0'}, {'E', '0'}, {'F', '0'}, > >> + {'G', '0'}, {'H', '0'}, {'I', '0'} > >> +}; > >> + > >> +static char intel_get_stepping(struct drm_device *dev) > >> +{ > >> + if (IS_SKYLAKE(dev) && (dev->pdev->revision < > >> + ARRAY_SIZE(skl_stepping_info))) > >> + return skl_stepping_info[dev->pdev->revision].stepping; > > Nitpick: at this point it's just > > if (dev->pdev->revision < 9) > > return 'A' + dev->pdev->revision; > > > > w/o the need for skl_stepping_info. > > If we have different firmwares for different substepping for example > A0,A1,A3 then stepping will be same but substepping will be different > and I assume revision_id also will be different. And if we want to put > some logic like ('A' + dev->pdev->revision) then we can do it but > should be based on bspec, Why I am telling because if later we get some > table and not able to derive a common logic then again may need this > kind of lookup table. > > And using lookuo table comes with advantage > - to extend support for future platform only have to add new structure or > change existing structure to incorporate any future changes in stepping > info which got from bspec. - time complexity will be less. > > Yes agree space might be an issue. Suggest me trade off between time and > space. It would be just simpler in the above way, it's not a space issue. I think once we have platforms with substeppings other than 0, we will anyway have to change the above struct stepping_info and the logic to look up the proper entry in it. But the above works too, so I'm ok if you keep it as-is for now. Just make the struct const please. > >> + else > >> + return -ENODATA; > >> +} > >> + > >> +static char intel_get_substepping(struct drm_device *dev) > >> +{ > >> + if (IS_SKYLAKE(dev) && (dev->pdev->revision < > >> + ARRAY_SIZE(skl_stepping_info))) > >> + return skl_stepping_info[dev->pdev->revision].substepping; > > and this one is simply > > return 0; > > For now all values are 0 for skl, but implementation done thinking of > supporting future changes. We may have multiple firmware for different > substepping. Ok, let's keep it as-is. > >> + else > >> + return -ENODATA; > >> +} > > Missing newline. > > > >> +void intel_csr_load_program(struct drm_device *dev) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + __be32 *payload = dev_priv->csr.dmc_payload; > >> + uint32_t i, fw_size; > >> + > >> + if (!IS_GEN9(dev)) { > >> + DRM_ERROR("No CSR support available for this platform\n"); > >> + return; > >> + } > >> + > >> + mutex_lock(&dev_priv->csr_lock); > >> + /* fw_size is dwords, converting it to bytes and nearest 8 multiple. */ > > The above comment is stale now. > > Agree. > > > > >> + fw_size = dev_priv->csr.dmc_fw_size; > >> + for (i = 0; i < fw_size; i++) > >> + I915_WRITE(CSR_PROGRAM_BASE + i * 4, > >> + (u32 __force)payload[i]); > >> + > >> + for (i = 0; i < dev_priv->csr.mmio_count; i++) { > >> + I915_WRITE(dev_priv->csr.mmioaddr[i], > >> + dev_priv->csr.mmiodata[i]); > >> + } > >> + mutex_unlock(&dev_priv->csr_lock); > >> +} > >> + > >> +static void finish_csr_load(const struct firmware *fw, void *context) > >> +{ > >> + struct drm_i915_private *dev_priv = context; > >> + struct drm_device *dev = dev_priv->dev; > >> + struct intel_css_header *css_header; > >> + struct intel_package_header *package_header; > >> + struct intel_dmc_header *dmc_header; > >> + struct intel_csr *csr = &dev_priv->csr; > >> + char stepping = intel_get_stepping(dev); > >> + char substepping = intel_get_substepping(dev); > >> + uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; > >> + uint32_t i; > >> + __be32 *dmc_payload; > >> + > >> + if (!fw) { > >> + i915_firmware_load_error_print(csr->fw_path, 0); > >> + goto out; > >> + } > >> + > >> + if ((stepping == -ENODATA) || (substepping == -ENODATA)) { > >> + DRM_ERROR("Unknown stepping info, firmware loading failed\n"); > >> + goto out; > >> + } > >> + > >> + /* Extract CSS Header information*/ > >> + css_header = (struct intel_css_header *)fw->data; > >> + if (sizeof(struct intel_css_header) != > >> + (css_header->header_len * 4)) { > >> + DRM_ERROR("Firmware has wrong CSS header length %u bytes\n", > >> + (css_header->header_len * 4)); > >> + goto out; > >> + } > >> + readcount += sizeof(struct intel_css_header); > >> + > >> + /* Extract Package Header information*/ > >> + package_header = (struct intel_package_header *) > >> + &fw->data[readcount]; > >> + if (sizeof(struct intel_package_header) != > >> + (package_header->header_len * 4)) { > >> + DRM_ERROR("Firmware has wrong package header length %u bytes\n", > >> + (package_header->header_len * 4)); > >> + goto out; > >> + } > >> + readcount += sizeof(struct intel_package_header); > >> + > >> + /* Search for dmc_offset to find firware binary. */ > >> + for (i = 0; i < package_header->num_entries; i++) { > >> + if (package_header->fw_info[i].substepping == '*' && > >> + stepping == package_header->fw_info[i].stepping) { > >> + dmc_offset = package_header->fw_info[i].offset; > >> + break; > >> + } else if (stepping == package_header->fw_info[i].stepping && > >> + substepping == package_header->fw_info[i].substepping) { > >> + dmc_offset = package_header->fw_info[i].offset; > >> + break; > >> + } else if (package_header->fw_info[i].stepping == '*' && > >> + package_header->fw_info[i].substepping == '*') > >> + dmc_offset = package_header->fw_info[i].offset; > >> + } > >> + if (dmc_offset == CSR_DEFAULT_FW_OFFSET) { > >> + DRM_ERROR("Firmware not supported for %c stepping\n", stepping); > >> + goto out; > >> + } > >> + readcount += dmc_offset; > >> + > >> + /* Extract dmc_header information. */ > >> + dmc_header = (struct intel_dmc_header *)&fw->data[readcount]; > >> + if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) { > >> + DRM_ERROR("Firmware has wrong dmc header length %u bytes\n", > >> + (dmc_header->header_len)); > >> + goto out; > >> + } > >> + readcount += sizeof(struct intel_dmc_header); > >> + > >> + /* Cache the dmc header info. */ > >> + if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) { > >> + DRM_ERROR("Firmware has wrong mmio count %u\n", > >> + dmc_header->mmio_count); > >> + goto out; > >> + } > >> + csr->mmio_count = dmc_header->mmio_count; > >> + for (i = 0; i < dmc_header->mmio_count; i++) { > >> + if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE && > >> + dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) { > >> + DRM_ERROR(" Firmware has wrong mmio address 0x%x\n", > >> + dmc_header->mmioaddr[i]); > >> + goto out; > >> + } > >> + csr->mmioaddr[i] = dmc_header->mmioaddr[i]; > >> + csr->mmiodata[i] = dmc_header->mmiodata[i]; > >> + } > >> + > >> + /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */ > >> + nbytes = dmc_header->fw_size * 4; > >> + if (nbytes > CSR_MAX_FW_SIZE) { > >> + DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes); > >> + goto out; > >> + } > >> + csr->dmc_fw_size = dmc_header->fw_size; > >> + > >> + csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL); > >> + if (!csr->dmc_payload) { > >> + DRM_ERROR("Memory allocation failed for dmc payload\n"); > >> + goto out; > >> + } > >> + > >> + dmc_payload = csr->dmc_payload; > >> + for (i = 0; i < dmc_header->fw_size; i++) { > >> + uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4]; > >> + /* > >> + * The firmware payload is an array of 32 bit words stored in > >> + * little-endian format in the firmware image and programmed > >> + * as 32 bit big-endian format to memory. > >> + */ > >> + dmc_payload[i] = cpu_to_be32(*tmp); > >> + } > >> + > >> + /* load csr program during system boot, as needed for DC states */ > >> + intel_csr_load_program(dev); > >> +out: > >> + release_firmware(fw); > >> +} > >> + > >> +void intel_csr_ucode_init(struct drm_device *dev) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct intel_csr *csr = &dev_priv->csr; > >> + int ret; > >> + > >> + if (!HAS_CSR(dev)) > >> + return; > >> + > >> + if (IS_SKYLAKE(dev)) > >> + csr->fw_path = I915_CSR_SKL; > >> + else { > >> + DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); > >> + return; > >> + } > >> + > >> + /* CSR supported for platform, load firmware */ > >> + ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path, > >> + &dev_priv->dev->pdev->dev, > >> + GFP_KERNEL, dev_priv, > >> + finish_csr_load); > >> + if (ret) > >> + i915_firmware_load_error_print(csr->fw_path, ret); > >> + > >> +} > >> + > >> +void intel_csr_ucode_fini(struct drm_device *dev) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + > >> + if (!HAS_CSR(dev)) > >> + return; > >> + > >> + kfree(dev_priv->csr.dmc_payload); > >> +} > >> diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > >> new file mode 100644 > >> index 0000000..c2a5a53 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i915/intel_csr.h > >> @@ -0,0 +1,164 @@ > >> +/* > >> + * 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. > >> + * > >> + */ > >> + > >> +#ifndef _INTEL_CSR_H_ > >> +#define _INTEL_CSR_H_ > >> + > >> +#define I915_CSR_SKL "i915/dmc_gen9.bin" > >> + > >> +MODULE_FIRMWARE(I915_CSR_SKL); > >> + > >> +/* > >> +* SKL CSR registers for DC5 and DC6 > >> +*/ > >> +#define CSR_PROGRAM_BASE 0x80000 > >> +#define CSR_HEADER_OFFSET 128 > >> +#define CSR_SSP_BASE_ADDR_GEN9 0x00002FC0 > >> +#define CSR_HTP_ADDR_SKL 0x00500034 > >> +#define CSR_SSP_BASE 0x8F074 > >> +#define CSR_HTP_SKL 0x8F004 > >> +#define CSR_LAST_WRITE 0x8F034 > >> +#define CSR_LAST_WRITE_VALUE 0xc003b400 > >> +/* MMIO address range for CSR program (0x80000 - 0x82FFF) */ > >> +#define CSR_MAX_FW_SIZE 0x2FFF > >> +#define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF > >> +#define CSR_MMIO_START_RANGE 0x80000 > >> +#define CSR_MMIO_END_RANGE 0x8FFFF > >> + > >> +struct intel_css_header { > >> + /* 0x09 for DMC */ > >> + uint32_t module_type; > >> + > >> + /* Includes the DMC specific header in dwords */ > >> + uint32_t header_len; > >> + > >> + /* always value would be 0x10000 */ > >> + uint32_t header_ver; > >> + > >> + /* Not used */ > >> + uint32_t module_id; > >> + > >> + /* Not used */ > >> + uint32_t module_vendor; > >> + > >> + /* in YYYYMMDD format */ > >> + uint32_t date; > >> + > >> + /* Size in dwords (CSS_Headerlen + PackageHeaderLen + dmc FWsLen)/4 */ > >> + uint32_t size; > >> + > >> + /* Not used */ > >> + uint32_t key_size; > >> + > >> + /* Not used */ > >> + uint32_t modulus_size; > >> + > >> + /* Not used */ > >> + uint32_t exponent_size; > >> + > >> + /* Not used */ > >> + uint32_t reserved1[12]; > >> + > >> + /* Major Minor */ > >> + uint32_t version; > >> + > >> + /* Not used */ > >> + uint32_t reserved2[8]; > >> + > >> + /* Not used */ > >> + uint32_t kernel_header_info; > >> +} __packed; > >> + > >> +struct intel_fw_info { > >> + uint16_t reserved1; > >> + > >> + /* Stepping (A, B, C, ..., *). * is a wildcard */ > >> + char stepping; > >> + > >> + /* Sub-stepping (0, 1, ..., *). * is a wildcard */ > >> + char substepping; > >> + > >> + uint32_t offset; > >> + uint32_t reserved2; > >> +} __packed; > >> + > >> +struct intel_package_header { > >> + /* DMC container header length in dwords */ > >> + unsigned char header_len; > >> + > >> + /* always value would be 0x01 */ > >> + unsigned char header_ver; > >> + > >> + unsigned char reserved[10]; > >> + > >> + /* Number of valid entries in the FWInfo array below */ > >> + uint32_t num_entries; > >> + > >> + struct intel_fw_info fw_info[20]; > >> +} __packed; > >> + > >> +struct intel_dmc_header { > >> + /* always value would be 0x40403E3E */ > >> + uint32_t signature; > >> + > >> + /* DMC binary header length */ > >> + unsigned char header_len; > >> + > >> + /* 0x01 */ > >> + unsigned char header_ver; > >> + > >> + /* Reserved */ > >> + uint16_t dmcc_ver; > >> + > >> + /* Major, Minor */ > >> + uint32_t project; > >> + > >> + /* Firmware program size (excluding header) in dwords */ > >> + uint32_t fw_size; > >> + > >> + /* Major Minor version */ > >> + uint32_t fw_version; > >> + > >> + /* Number of valid MMIO cycles present. */ > >> + uint32_t mmio_count; > >> + > >> + /* MMIO address */ > >> + uint32_t mmioaddr[8]; > >> + > >> + /* MMIO data */ > >> + uint32_t mmiodata[8]; > >> + > >> + /* FW filename */ > >> + unsigned char dfile[32]; > >> + > >> + uint32_t reserved1[2]; > >> +} __packed; > >> + > >> +struct stepping_info { > >> + char stepping; > >> + char substepping; > >> +} __packed; > > stepping_info doesn't need to be packed. > > To avoid structure padding used __packed which will save some memory. No, the reason for __packed above is simply correctness for binary interfaces like what we have with the DMC firmware. For anything else we can trust the compiler to do the right thing wrt. to alignment/padding. > > What I meant is to add all these to intel_csr.c. I don't think > > there is any reason for a new header file unless you need to share > > somethinh among multiple .c modules. > > intel_runtime_pm.c is using few macro which is defined in this header > file in later patches. Then we can move all macro definition to > i915_drv.h or else can put few macro in intel_csr.c and few in > i915_drv.h. Give me your suggestion. Yes, I missed assert_csr_loaded(). As this is really something internal to the DMC firmware, I would just move assert_csr_loaded() to intel_csr.c and export that from intel_drv.h. --Imre
On 04/13/2015 04:33 PM, Imre Deak wrote: > On ma, 2015-04-13 at 15:54 +0530, Animesh Manna wrote: >> From: "A.Sunil Kamath" <sunil.kamath@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. (Animesh) >> >> Issue: VIZ-2569 >> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> >> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> Signed-off-by: Imre Deak <imre.deak@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 | 234 +++++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_csr.h | 164 +++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 5 + >> 7 files changed, 451 insertions(+), 3 deletions(-) >> create mode 100644 drivers/gpu/drm/i915/intel_csr.c >> create mode 100644 drivers/gpu/drm/i915/intel_csr.h >> >> 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; >> + >> struct intel_gmbus gmbus[GMBUS_NUM_PINS]; >> >> /** gmbus_mutex protects against concurrent usage of the single hw gmbus >> @@ -2425,6 +2439,8 @@ struct drm_i915_cmd_table { >> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) >> #define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev)) >> >> +#define HAS_CSR(dev) (IS_SKYLAKE(dev)) >> + >> #define INTEL_PCH_DEVICE_ID_MASK 0xff00 >> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 >> #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 >> @@ -2515,6 +2531,7 @@ extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); >> extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); >> int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); >> void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); >> +void i915_firmware_load_error_print(const char *fw_path, int err); >> >> /* i915_irq.c */ >> void i915_queue_hangcheck(struct drm_device *dev); >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> new file mode 100644 >> index 0000000..ab9b16b >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_csr.c >> @@ -0,0 +1,234 @@ >> +/* >> + * 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. >> + * >> + */ >> +#include <linux/firmware.h> >> +#include "i915_drv.h" >> +#include "i915_reg.h" >> +#include "intel_csr.h" >> + >> +static struct stepping_info skl_stepping_info[] = { > Nitpick: this should be const. > >> + {'A', '0'}, {'B', '0'}, {'C', '0'}, >> + {'D', '0'}, {'E', '0'}, {'F', '0'}, >> + {'G', '0'}, {'H', '0'}, {'I', '0'} >> +}; >> + >> +static char intel_get_stepping(struct drm_device *dev) >> +{ >> + if (IS_SKYLAKE(dev) && (dev->pdev->revision < >> + ARRAY_SIZE(skl_stepping_info))) >> + return skl_stepping_info[dev->pdev->revision].stepping; > Nitpick: at this point it's just > if (dev->pdev->revision < 9) > return 'A' + dev->pdev->revision; > > w/o the need for skl_stepping_info. If we have different firmwares for different substepping for example A0,A1,A3 then stepping will be same but substepping will be different and I assume revision_id also will be different. And if we want to put some logic like ('A' + dev->pdev->revision) then we can do it but should be based on bspec, Why I am telling because if later we get some table and not able to derive a common logic then again may need this kind of lookup table. And using lookuo table comes with advantage - to extend support for future platform only have to add new structure or change existing structure to incorporate any future changes in stepping info which got from bspec. - time complexity will be less. Yes agree space might be an issue. Suggest me trade off between time and space. > > >> + else >> + return -ENODATA; >> +} >> + >> +static char intel_get_substepping(struct drm_device *dev) >> +{ >> + if (IS_SKYLAKE(dev) && (dev->pdev->revision < >> + ARRAY_SIZE(skl_stepping_info))) >> + return skl_stepping_info[dev->pdev->revision].substepping; > and this one is simply > return 0; For now all values are 0 for skl, but implementation done thinking of supporting future changes. We may have multiple firmware for different substepping. >> + else >> + return -ENODATA; >> +} > Missing newline. > >> +void intel_csr_load_program(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + __be32 *payload = dev_priv->csr.dmc_payload; >> + uint32_t i, fw_size; >> + >> + if (!IS_GEN9(dev)) { >> + DRM_ERROR("No CSR support available for this platform\n"); >> + return; >> + } >> + >> + mutex_lock(&dev_priv->csr_lock); >> + /* fw_size is dwords, converting it to bytes and nearest 8 multiple. */ > The above comment is stale now. Agree. > >> + fw_size = dev_priv->csr.dmc_fw_size; >> + for (i = 0; i < fw_size; i++) >> + I915_WRITE(CSR_PROGRAM_BASE + i * 4, >> + (u32 __force)payload[i]); >> + >> + for (i = 0; i < dev_priv->csr.mmio_count; i++) { >> + I915_WRITE(dev_priv->csr.mmioaddr[i], >> + dev_priv->csr.mmiodata[i]); >> + } >> + mutex_unlock(&dev_priv->csr_lock); >> +} >> + >> +static void finish_csr_load(const struct firmware *fw, void *context) >> +{ >> + struct drm_i915_private *dev_priv = context; >> + struct drm_device *dev = dev_priv->dev; >> + struct intel_css_header *css_header; >> + struct intel_package_header *package_header; >> + struct intel_dmc_header *dmc_header; >> + struct intel_csr *csr = &dev_priv->csr; >> + char stepping = intel_get_stepping(dev); >> + char substepping = intel_get_substepping(dev); >> + uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; >> + uint32_t i; >> + __be32 *dmc_payload; >> + >> + if (!fw) { >> + i915_firmware_load_error_print(csr->fw_path, 0); >> + goto out; >> + } >> + >> + if ((stepping == -ENODATA) || (substepping == -ENODATA)) { >> + DRM_ERROR("Unknown stepping info, firmware loading failed\n"); >> + goto out; >> + } >> + >> + /* Extract CSS Header information*/ >> + css_header = (struct intel_css_header *)fw->data; >> + if (sizeof(struct intel_css_header) != >> + (css_header->header_len * 4)) { >> + DRM_ERROR("Firmware has wrong CSS header length %u bytes\n", >> + (css_header->header_len * 4)); >> + goto out; >> + } >> + readcount += sizeof(struct intel_css_header); >> + >> + /* Extract Package Header information*/ >> + package_header = (struct intel_package_header *) >> + &fw->data[readcount]; >> + if (sizeof(struct intel_package_header) != >> + (package_header->header_len * 4)) { >> + DRM_ERROR("Firmware has wrong package header length %u bytes\n", >> + (package_header->header_len * 4)); >> + goto out; >> + } >> + readcount += sizeof(struct intel_package_header); >> + >> + /* Search for dmc_offset to find firware binary. */ >> + for (i = 0; i < package_header->num_entries; i++) { >> + if (package_header->fw_info[i].substepping == '*' && >> + stepping == package_header->fw_info[i].stepping) { >> + dmc_offset = package_header->fw_info[i].offset; >> + break; >> + } else if (stepping == package_header->fw_info[i].stepping && >> + substepping == package_header->fw_info[i].substepping) { >> + dmc_offset = package_header->fw_info[i].offset; >> + break; >> + } else if (package_header->fw_info[i].stepping == '*' && >> + package_header->fw_info[i].substepping == '*') >> + dmc_offset = package_header->fw_info[i].offset; >> + } >> + if (dmc_offset == CSR_DEFAULT_FW_OFFSET) { >> + DRM_ERROR("Firmware not supported for %c stepping\n", stepping); >> + goto out; >> + } >> + readcount += dmc_offset; >> + >> + /* Extract dmc_header information. */ >> + dmc_header = (struct intel_dmc_header *)&fw->data[readcount]; >> + if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) { >> + DRM_ERROR("Firmware has wrong dmc header length %u bytes\n", >> + (dmc_header->header_len)); >> + goto out; >> + } >> + readcount += sizeof(struct intel_dmc_header); >> + >> + /* Cache the dmc header info. */ >> + if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) { >> + DRM_ERROR("Firmware has wrong mmio count %u\n", >> + dmc_header->mmio_count); >> + goto out; >> + } >> + csr->mmio_count = dmc_header->mmio_count; >> + for (i = 0; i < dmc_header->mmio_count; i++) { >> + if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE && >> + dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) { >> + DRM_ERROR(" Firmware has wrong mmio address 0x%x\n", >> + dmc_header->mmioaddr[i]); >> + goto out; >> + } >> + csr->mmioaddr[i] = dmc_header->mmioaddr[i]; >> + csr->mmiodata[i] = dmc_header->mmiodata[i]; >> + } >> + >> + /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */ >> + nbytes = dmc_header->fw_size * 4; >> + if (nbytes > CSR_MAX_FW_SIZE) { >> + DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes); >> + goto out; >> + } >> + csr->dmc_fw_size = dmc_header->fw_size; >> + >> + csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL); >> + if (!csr->dmc_payload) { >> + DRM_ERROR("Memory allocation failed for dmc payload\n"); >> + goto out; >> + } >> + >> + dmc_payload = csr->dmc_payload; >> + for (i = 0; i < dmc_header->fw_size; i++) { >> + uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4]; >> + /* >> + * The firmware payload is an array of 32 bit words stored in >> + * little-endian format in the firmware image and programmed >> + * as 32 bit big-endian format to memory. >> + */ >> + dmc_payload[i] = cpu_to_be32(*tmp); >> + } >> + >> + /* load csr program during system boot, as needed for DC states */ >> + intel_csr_load_program(dev); >> +out: >> + release_firmware(fw); >> +} >> + >> +void intel_csr_ucode_init(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_csr *csr = &dev_priv->csr; >> + int ret; >> + >> + if (!HAS_CSR(dev)) >> + return; >> + >> + if (IS_SKYLAKE(dev)) >> + csr->fw_path = I915_CSR_SKL; >> + else { >> + DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); >> + return; >> + } >> + >> + /* CSR supported for platform, load firmware */ >> + ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path, >> + &dev_priv->dev->pdev->dev, >> + GFP_KERNEL, dev_priv, >> + finish_csr_load); >> + if (ret) >> + i915_firmware_load_error_print(csr->fw_path, ret); >> + >> +} >> + >> +void intel_csr_ucode_fini(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + >> + if (!HAS_CSR(dev)) >> + return; >> + >> + kfree(dev_priv->csr.dmc_payload); >> +} >> diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h >> new file mode 100644 >> index 0000000..c2a5a53 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_csr.h >> @@ -0,0 +1,164 @@ >> +/* >> + * 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. >> + * >> + */ >> + >> +#ifndef _INTEL_CSR_H_ >> +#define _INTEL_CSR_H_ >> + >> +#define I915_CSR_SKL "i915/dmc_gen9.bin" >> + >> +MODULE_FIRMWARE(I915_CSR_SKL); >> + >> +/* >> +* SKL CSR registers for DC5 and DC6 >> +*/ >> +#define CSR_PROGRAM_BASE 0x80000 >> +#define CSR_HEADER_OFFSET 128 >> +#define CSR_SSP_BASE_ADDR_GEN9 0x00002FC0 >> +#define CSR_HTP_ADDR_SKL 0x00500034 >> +#define CSR_SSP_BASE 0x8F074 >> +#define CSR_HTP_SKL 0x8F004 >> +#define CSR_LAST_WRITE 0x8F034 >> +#define CSR_LAST_WRITE_VALUE 0xc003b400 >> +/* MMIO address range for CSR program (0x80000 - 0x82FFF) */ >> +#define CSR_MAX_FW_SIZE 0x2FFF >> +#define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF >> +#define CSR_MMIO_START_RANGE 0x80000 >> +#define CSR_MMIO_END_RANGE 0x8FFFF >> + >> +struct intel_css_header { >> + /* 0x09 for DMC */ >> + uint32_t module_type; >> + >> + /* Includes the DMC specific header in dwords */ >> + uint32_t header_len; >> + >> + /* always value would be 0x10000 */ >> + uint32_t header_ver; >> + >> + /* Not used */ >> + uint32_t module_id; >> + >> + /* Not used */ >> + uint32_t module_vendor; >> + >> + /* in YYYYMMDD format */ >> + uint32_t date; >> + >> + /* Size in dwords (CSS_Headerlen + PackageHeaderLen + dmc FWsLen)/4 */ >> + uint32_t size; >> + >> + /* Not used */ >> + uint32_t key_size; >> + >> + /* Not used */ >> + uint32_t modulus_size; >> + >> + /* Not used */ >> + uint32_t exponent_size; >> + >> + /* Not used */ >> + uint32_t reserved1[12]; >> + >> + /* Major Minor */ >> + uint32_t version; >> + >> + /* Not used */ >> + uint32_t reserved2[8]; >> + >> + /* Not used */ >> + uint32_t kernel_header_info; >> +} __packed; >> + >> +struct intel_fw_info { >> + uint16_t reserved1; >> + >> + /* Stepping (A, B, C, ..., *). * is a wildcard */ >> + char stepping; >> + >> + /* Sub-stepping (0, 1, ..., *). * is a wildcard */ >> + char substepping; >> + >> + uint32_t offset; >> + uint32_t reserved2; >> +} __packed; >> + >> +struct intel_package_header { >> + /* DMC container header length in dwords */ >> + unsigned char header_len; >> + >> + /* always value would be 0x01 */ >> + unsigned char header_ver; >> + >> + unsigned char reserved[10]; >> + >> + /* Number of valid entries in the FWInfo array below */ >> + uint32_t num_entries; >> + >> + struct intel_fw_info fw_info[20]; >> +} __packed; >> + >> +struct intel_dmc_header { >> + /* always value would be 0x40403E3E */ >> + uint32_t signature; >> + >> + /* DMC binary header length */ >> + unsigned char header_len; >> + >> + /* 0x01 */ >> + unsigned char header_ver; >> + >> + /* Reserved */ >> + uint16_t dmcc_ver; >> + >> + /* Major, Minor */ >> + uint32_t project; >> + >> + /* Firmware program size (excluding header) in dwords */ >> + uint32_t fw_size; >> + >> + /* Major Minor version */ >> + uint32_t fw_version; >> + >> + /* Number of valid MMIO cycles present. */ >> + uint32_t mmio_count; >> + >> + /* MMIO address */ >> + uint32_t mmioaddr[8]; >> + >> + /* MMIO data */ >> + uint32_t mmiodata[8]; >> + >> + /* FW filename */ >> + unsigned char dfile[32]; >> + >> + uint32_t reserved1[2]; >> +} __packed; >> + >> +struct stepping_info { >> + char stepping; >> + char substepping; >> +} __packed; > stepping_info doesn't need to be packed. To avoid structure padding used __packed which will save some memory. > > What I meant is to add all these to intel_csr.c. I don't think there is > any reason for a new header file unless you need to share something > among multiple .c modules. > > --Imre intel_runtime_pm.c is using few macro which is defined in this header file in later patches. Then we can move all macro definition to i915_drv.h or else can put few macro in intel_csr.c and few in i915_drv.h. Give me your suggestion. Regards, Animesh > >> + >> + >> +#endif /* _INTEL_CSR_H_ */ >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 7a0aa24..f3a2d88 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -1062,6 +1062,11 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); >> unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane, >> struct drm_i915_gem_object *obj); >> >> +/* intel_csr.c */ >> +void intel_csr_ucode_init(struct drm_device *dev); >> +void intel_csr_load_program(struct drm_device *dev); >> +void intel_csr_ucode_fini(struct drm_device *dev); >> + >> /* intel_dp.c */ >> void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); >> bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >
On Mon, Apr 13, 2015 at 03:54:02PM +0530, Animesh Manna wrote: > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > new file mode 100644 > index 0000000..c2a5a53 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_csr.h > + [...] > +#define I915_CSR_SKL "i915/dmc_gen9.bin" I'm guessing the BXT DMC firwmare will be different from SKL's? Remember that one of the requirements is to be able to have the same OS image boot on both SKL and BXT. That means the firmware names have to be different for SKL and BXT. We probably should have skl in the name here. Also, we need to be able to be able to support "Interface" versions. Ie. if the firmware interface changes in such a way a different loading code is needed, we need to be able to have both firmware on the disk so both an old kernel and a new kernel can work with the same user space. Right now, the naming scheme of the firmware does have a version on it, hopefully that's this "Interface" version. Thoughts?
On Mon, 2015-04-13 at 17:34 +0100, Damien Lespiau wrote: > On Mon, Apr 13, 2015 at 03:54:02PM +0530, Animesh Manna wrote: > > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > > new file mode 100644 > > index 0000000..c2a5a53 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_csr.h > > + > > [...] > > > +#define I915_CSR_SKL "i915/dmc_gen9.bin" > > I'm guessing the BXT DMC firwmare will be different from SKL's? Remember > that one of the requirements is to be able to have the same OS image > boot on both SKL and BXT. That means the firmware names have to be > different for SKL and BXT. We probably should have skl in the name here. > > Also, we need to be able to be able to support "Interface" versions. Ie. > if the firmware interface changes in such a way a different loading code > is needed, we need to be able to have both firmware on the disk so both > an old kernel and a new kernel can work with the same user space. > > Right now, the naming scheme of the firmware does have a version on it, > hopefully that's this "Interface" version. > > Thoughts? Yes, the above file name looks incorrect. "dmc_gen9.bin" was the name for the firmware image with the old layout. This patchset adds support for the new firmware layout starting from ver 1, while the old layout doesn't need to be supported. In an earlier version of this patch the filename was changed to "i915/skl_dmc_ver1.bin" not sure why that change got dropped. I think we could just use this latter name. --Imre
On Mon, Apr 13, 2015 at 07:52:54PM +0300, Imre Deak wrote: > On Mon, 2015-04-13 at 17:34 +0100, Damien Lespiau wrote: > > On Mon, Apr 13, 2015 at 03:54:02PM +0530, Animesh Manna wrote: > > > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > > > new file mode 100644 > > > index 0000000..c2a5a53 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/i915/intel_csr.h > > > + > > > > [...] > > > > > +#define I915_CSR_SKL "i915/dmc_gen9.bin" > > > > I'm guessing the BXT DMC firwmare will be different from SKL's? Remember > > that one of the requirements is to be able to have the same OS image > > boot on both SKL and BXT. That means the firmware names have to be > > different for SKL and BXT. We probably should have skl in the name here. > > > > Also, we need to be able to be able to support "Interface" versions. Ie. > > if the firmware interface changes in such a way a different loading code > > is needed, we need to be able to have both firmware on the disk so both > > an old kernel and a new kernel can work with the same user space. > > > > Right now, the naming scheme of the firmware does have a version on it, > > hopefully that's this "Interface" version. > > > > Thoughts? > > Yes, the above file name looks incorrect. "dmc_gen9.bin" was the name > for the firmware image with the old layout. This patchset adds support > for the new firmware layout starting from ver 1, while the old layout > doesn't need to be supported. In an earlier version of this patch the > filename was changed to "i915/skl_dmc_ver1.bin" not sure why that change > got dropped. I think we could just use this latter name. First public firmware is already v4 [1], no idea is the version bumps are actual API/interface changes.
On Mon, 2015-04-13 at 18:02 +0100, Damien Lespiau wrote: > On Mon, Apr 13, 2015 at 07:52:54PM +0300, Imre Deak wrote: > > On Mon, 2015-04-13 at 17:34 +0100, Damien Lespiau wrote: > > > On Mon, Apr 13, 2015 at 03:54:02PM +0530, Animesh Manna wrote: > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h > > > > new file mode 100644 > > > > index 0000000..c2a5a53 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/i915/intel_csr.h > > > > + > > > > > > [...] > > > > > > > +#define I915_CSR_SKL "i915/dmc_gen9.bin" > > > > > > I'm guessing the BXT DMC firwmare will be different from SKL's? Remember > > > that one of the requirements is to be able to have the same OS image > > > boot on both SKL and BXT. That means the firmware names have to be > > > different for SKL and BXT. We probably should have skl in the name here. > > > > > > Also, we need to be able to be able to support "Interface" versions. Ie. > > > if the firmware interface changes in such a way a different loading code > > > is needed, we need to be able to have both firmware on the disk so both > > > an old kernel and a new kernel can work with the same user space. > > > > > > Right now, the naming scheme of the firmware does have a version on it, > > > hopefully that's this "Interface" version. > > > > > > Thoughts? > > > > Yes, the above file name looks incorrect. "dmc_gen9.bin" was the name > > for the firmware image with the old layout. This patchset adds support > > for the new firmware layout starting from ver 1, while the old layout > > doesn't need to be supported. In an earlier version of this patch the > > filename was changed to "i915/skl_dmc_ver1.bin" not sure why that change > > got dropped. I think we could just use this latter name. > > First public firmware is already v4 [1], no idea is the version bumps > are actual API/interface changes. Ok, I haven't seen that. One question is if we need to support multiple interface versions or just the latest one. I would say only the latest one (for each platform) and so I915_CSR_SKL should be this latest firmware image filename, in this case i915/skl_dmc_ver4.bin. --Imre
On Mon, Apr 13, 2015 at 08:15:29PM +0300, Imre Deak wrote: > Ok, I haven't seen that. One question is if we need to support multiple > interface versions or just the latest one. I would say only the latest > one (for each platform) and so I915_CSR_SKL should be this latest > firmware image filename, in this case i915/skl_dmc_ver4.bin. Yup, I think just supporting the latest one in the driver is what we want. The filename versioning is there so different kernel versions, supporting different interfaces, can all boot with the same userspace, each kernel loading the appropriate firmware.
Hi Daniel, hi Ville, some success at last. I couldn't stop myself playing with the NatSemi 2501 DVO in my Fujitsu S6010 and I believe I finally got a hang on this chip. I believe I understand now most of the undocumented registers. There are also a couple of additional features that are, apparently, not used by the video BIOS of the S6010, namely the chip has a ditherer on board - quite like the Intel Video Controller hub in the IBM R31. Unfortunately, to enable the scaler, the bypass must be turned off, and hence, parameters for a 1:1 through-mapping of the scaler are required. After quite some experimenting, I believe I found now the right settings to enable the scaler and configure it to pass the 1024x768 input to the output. The chip is really a bit weird. It not only requires the scaling factors, but also the input timings, (sync width, front/back porch for both horizontal and vertical) and the output timing, and the configuration of its PLL to sample the incoming data. Currently, most of the data I obtained by "trail and error", at least for the 1024x768 mode in which the bios configures the DVO in bypass mode. It turned out we forgot to configure a couple of registers (and some others are pretty much blank). Thus, my question at this time is whether there is any interface how to get the precise timing of the loaded video mode from the i915 module directly instead of second-guessing the parameters, i.e. dimensions of the frame, porch sizes, size of the sync pulses, pixel clock and so on. Other than that, I'll try to clean up the code I have to so far in the next days and release it. Greetings, Thomas
On 04/13/2015 10:52 PM, Damien Lespiau wrote: > On Mon, Apr 13, 2015 at 08:15:29PM +0300, Imre Deak wrote: >> Ok, I haven't seen that. One question is if we need to support multiple >> interface versions or just the latest one. I would say only the latest >> one (for each platform) and so I915_CSR_SKL should be this latest >> firmware image filename, in this case i915/skl_dmc_ver4.bin. > Yup, I think just supporting the latest one in the driver is what we > want. The filename versioning is there so different kernel versions, > supporting different interfaces, can all boot with the same userspace, > each kernel loading the appropriate firmware. > Can we have a symbolic link which can be hardcoded in intel_csr.c and option will be given to user during installing the firmware to create a symbolic link for the firmware wanted to load. Want to avoid version number mentioned in firmware file name as it getting change with latest fixes, bot not any API/inteface changes. Agree as bxt and skl both are gen9 we can name as skl_dmc_gen9.bin for now and discussion is going to finalize the name. Is it ok?
On Tue, Apr 14, 2015 at 02:46:56PM +0530, Animesh Manna wrote: > > > On 04/13/2015 10:52 PM, Damien Lespiau wrote: > >On Mon, Apr 13, 2015 at 08:15:29PM +0300, Imre Deak wrote: > >>Ok, I haven't seen that. One question is if we need to support multiple > >>interface versions or just the latest one. I would say only the latest > >>one (for each platform) and so I915_CSR_SKL should be this latest > >>firmware image filename, in this case i915/skl_dmc_ver4.bin. > >Yup, I think just supporting the latest one in the driver is what we > >want. The filename versioning is there so different kernel versions, > >supporting different interfaces, can all boot with the same userspace, > >each kernel loading the appropriate firmware. > > > Can we have a symbolic link which can be hardcoded in intel_csr.c > and option will be given to user during installing the firmware > to create a symbolic link for the firmware wanted to load. > Want to avoid version number mentioned in firmware file name as it > getting change with latest fixes, bot not any API/inteface changes. > Agree as bxt and skl both are gen9 we can name as skl_dmc_gen9.bin > for now and discussion is going to finalize the name. Is it ok? Why would we need a symlink? what would be its name? skl_dmc_gen9.bin does not answer the requirement that we need to encode the API/interface version in the filename. The firmware on 01.org skl_dmc_ver4.bin seems to be what we want. HTH,
On Mon, Apr 13, 2015 at 09:00:48PM +0200, Thomas Richter wrote: > Hi Daniel, hi Ville, > > some success at last. I couldn't stop myself playing with the NatSemi 2501 > DVO in my Fujitsu S6010 and I believe I finally got a hang on this chip. I > believe I understand now most of the undocumented registers. > > There are also a couple of additional features that are, apparently, not > used by the video BIOS of the S6010, namely the chip has a ditherer on board > - quite like the Intel Video Controller hub in the IBM R31. > Unfortunately, to enable the scaler, the bypass must be turned off, and > hence, parameters for a 1:1 through-mapping of the scaler are required. > > After quite some experimenting, I believe I found now the right settings to > enable the scaler and configure it to pass the 1024x768 input to the output. > > The chip is really a bit weird. It not only requires the scaling factors, > but also the input timings, (sync width, front/back porch for both > horizontal and vertical) and the output timing, and the configuration of its > PLL to sample the incoming data. Currently, most of the data I obtained by > "trail and error", at least for the 1024x768 mode in which the bios > configures the DVO in bypass mode. > > It turned out we forgot to configure a couple of registers (and some others > are pretty much blank). > > Thus, my question at this time is whether there is any interface how to get > the precise timing of the loaded video mode from the i915 module directly > instead of second-guessing the parameters, i.e. dimensions of the frame, > porch sizes, size of the sync pulses, pixel clock and so on. > > Other than that, I'll try to clean up the code I have to so far in the next > days and release it. In the mode structure that gets passed to your dvo driver look for the crtc_* values, those are the exact timings you need to set up. You want to look at the adjusted_mode since that's the one that actually gets sent to the dvo port, the other mode is the one userspace request and will get munged a bit. btw the sdvo code works really similar and also has input and output timings for the transcoder chip. You could peak at that code to see how it's all done. Cheers, Daniel PS: You're replies are still attached to some random thread, which makes them harder to spot and not miss ...
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; + struct intel_gmbus gmbus[GMBUS_NUM_PINS]; /** gmbus_mutex protects against concurrent usage of the single hw gmbus @@ -2425,6 +2439,8 @@ struct drm_i915_cmd_table { #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) #define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev)) +#define HAS_CSR(dev) (IS_SKYLAKE(dev)) + #define INTEL_PCH_DEVICE_ID_MASK 0xff00 #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 @@ -2515,6 +2531,7 @@ extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); +void i915_firmware_load_error_print(const char *fw_path, int err); /* i915_irq.c */ void i915_queue_hangcheck(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c new file mode 100644 index 0000000..ab9b16b --- /dev/null +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -0,0 +1,234 @@ +/* + * 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. + * + */ +#include <linux/firmware.h> +#include "i915_drv.h" +#include "i915_reg.h" +#include "intel_csr.h" + +static struct stepping_info skl_stepping_info[] = { + {'A', '0'}, {'B', '0'}, {'C', '0'}, + {'D', '0'}, {'E', '0'}, {'F', '0'}, + {'G', '0'}, {'H', '0'}, {'I', '0'} +}; + +static char intel_get_stepping(struct drm_device *dev) +{ + if (IS_SKYLAKE(dev) && (dev->pdev->revision < + ARRAY_SIZE(skl_stepping_info))) + return skl_stepping_info[dev->pdev->revision].stepping; + else + return -ENODATA; +} + +static char intel_get_substepping(struct drm_device *dev) +{ + if (IS_SKYLAKE(dev) && (dev->pdev->revision < + ARRAY_SIZE(skl_stepping_info))) + return skl_stepping_info[dev->pdev->revision].substepping; + else + return -ENODATA; +} +void intel_csr_load_program(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + __be32 *payload = dev_priv->csr.dmc_payload; + uint32_t i, fw_size; + + if (!IS_GEN9(dev)) { + DRM_ERROR("No CSR support available for this platform\n"); + return; + } + + mutex_lock(&dev_priv->csr_lock); + /* fw_size is dwords, converting it to bytes and nearest 8 multiple. */ + fw_size = dev_priv->csr.dmc_fw_size; + for (i = 0; i < fw_size; i++) + I915_WRITE(CSR_PROGRAM_BASE + i * 4, + (u32 __force)payload[i]); + + for (i = 0; i < dev_priv->csr.mmio_count; i++) { + I915_WRITE(dev_priv->csr.mmioaddr[i], + dev_priv->csr.mmiodata[i]); + } + mutex_unlock(&dev_priv->csr_lock); +} + +static void finish_csr_load(const struct firmware *fw, void *context) +{ + struct drm_i915_private *dev_priv = context; + struct drm_device *dev = dev_priv->dev; + struct intel_css_header *css_header; + struct intel_package_header *package_header; + struct intel_dmc_header *dmc_header; + struct intel_csr *csr = &dev_priv->csr; + char stepping = intel_get_stepping(dev); + char substepping = intel_get_substepping(dev); + uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; + uint32_t i; + __be32 *dmc_payload; + + if (!fw) { + i915_firmware_load_error_print(csr->fw_path, 0); + goto out; + } + + if ((stepping == -ENODATA) || (substepping == -ENODATA)) { + DRM_ERROR("Unknown stepping info, firmware loading failed\n"); + goto out; + } + + /* Extract CSS Header information*/ + css_header = (struct intel_css_header *)fw->data; + if (sizeof(struct intel_css_header) != + (css_header->header_len * 4)) { + DRM_ERROR("Firmware has wrong CSS header length %u bytes\n", + (css_header->header_len * 4)); + goto out; + } + readcount += sizeof(struct intel_css_header); + + /* Extract Package Header information*/ + package_header = (struct intel_package_header *) + &fw->data[readcount]; + if (sizeof(struct intel_package_header) != + (package_header->header_len * 4)) { + DRM_ERROR("Firmware has wrong package header length %u bytes\n", + (package_header->header_len * 4)); + goto out; + } + readcount += sizeof(struct intel_package_header); + + /* Search for dmc_offset to find firware binary. */ + for (i = 0; i < package_header->num_entries; i++) { + if (package_header->fw_info[i].substepping == '*' && + stepping == package_header->fw_info[i].stepping) { + dmc_offset = package_header->fw_info[i].offset; + break; + } else if (stepping == package_header->fw_info[i].stepping && + substepping == package_header->fw_info[i].substepping) { + dmc_offset = package_header->fw_info[i].offset; + break; + } else if (package_header->fw_info[i].stepping == '*' && + package_header->fw_info[i].substepping == '*') + dmc_offset = package_header->fw_info[i].offset; + } + if (dmc_offset == CSR_DEFAULT_FW_OFFSET) { + DRM_ERROR("Firmware not supported for %c stepping\n", stepping); + goto out; + } + readcount += dmc_offset; + + /* Extract dmc_header information. */ + dmc_header = (struct intel_dmc_header *)&fw->data[readcount]; + if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) { + DRM_ERROR("Firmware has wrong dmc header length %u bytes\n", + (dmc_header->header_len)); + goto out; + } + readcount += sizeof(struct intel_dmc_header); + + /* Cache the dmc header info. */ + if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) { + DRM_ERROR("Firmware has wrong mmio count %u\n", + dmc_header->mmio_count); + goto out; + } + csr->mmio_count = dmc_header->mmio_count; + for (i = 0; i < dmc_header->mmio_count; i++) { + if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE && + dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) { + DRM_ERROR(" Firmware has wrong mmio address 0x%x\n", + dmc_header->mmioaddr[i]); + goto out; + } + csr->mmioaddr[i] = dmc_header->mmioaddr[i]; + csr->mmiodata[i] = dmc_header->mmiodata[i]; + } + + /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */ + nbytes = dmc_header->fw_size * 4; + if (nbytes > CSR_MAX_FW_SIZE) { + DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes); + goto out; + } + csr->dmc_fw_size = dmc_header->fw_size; + + csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL); + if (!csr->dmc_payload) { + DRM_ERROR("Memory allocation failed for dmc payload\n"); + goto out; + } + + dmc_payload = csr->dmc_payload; + for (i = 0; i < dmc_header->fw_size; i++) { + uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4]; + /* + * The firmware payload is an array of 32 bit words stored in + * little-endian format in the firmware image and programmed + * as 32 bit big-endian format to memory. + */ + dmc_payload[i] = cpu_to_be32(*tmp); + } + + /* load csr program during system boot, as needed for DC states */ + intel_csr_load_program(dev); +out: + release_firmware(fw); +} + +void intel_csr_ucode_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_csr *csr = &dev_priv->csr; + int ret; + + if (!HAS_CSR(dev)) + return; + + if (IS_SKYLAKE(dev)) + csr->fw_path = I915_CSR_SKL; + else { + DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); + return; + } + + /* CSR supported for platform, load firmware */ + ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path, + &dev_priv->dev->pdev->dev, + GFP_KERNEL, dev_priv, + finish_csr_load); + if (ret) + i915_firmware_load_error_print(csr->fw_path, ret); + +} + +void intel_csr_ucode_fini(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!HAS_CSR(dev)) + return; + + kfree(dev_priv->csr.dmc_payload); +} diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h new file mode 100644 index 0000000..c2a5a53 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_csr.h @@ -0,0 +1,164 @@ +/* + * 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. + * + */ + +#ifndef _INTEL_CSR_H_ +#define _INTEL_CSR_H_ + +#define I915_CSR_SKL "i915/dmc_gen9.bin" + +MODULE_FIRMWARE(I915_CSR_SKL); + +/* +* SKL CSR registers for DC5 and DC6 +*/ +#define CSR_PROGRAM_BASE 0x80000 +#define CSR_HEADER_OFFSET 128 +#define CSR_SSP_BASE_ADDR_GEN9 0x00002FC0 +#define CSR_HTP_ADDR_SKL 0x00500034 +#define CSR_SSP_BASE 0x8F074 +#define CSR_HTP_SKL 0x8F004 +#define CSR_LAST_WRITE 0x8F034 +#define CSR_LAST_WRITE_VALUE 0xc003b400 +/* MMIO address range for CSR program (0x80000 - 0x82FFF) */ +#define CSR_MAX_FW_SIZE 0x2FFF +#define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF +#define CSR_MMIO_START_RANGE 0x80000 +#define CSR_MMIO_END_RANGE 0x8FFFF + +struct intel_css_header { + /* 0x09 for DMC */ + uint32_t module_type; + + /* Includes the DMC specific header in dwords */ + uint32_t header_len; + + /* always value would be 0x10000 */ + uint32_t header_ver; + + /* Not used */ + uint32_t module_id; + + /* Not used */ + uint32_t module_vendor; + + /* in YYYYMMDD format */ + uint32_t date; + + /* Size in dwords (CSS_Headerlen + PackageHeaderLen + dmc FWsLen)/4 */ + uint32_t size; + + /* Not used */ + uint32_t key_size; + + /* Not used */ + uint32_t modulus_size; + + /* Not used */ + uint32_t exponent_size; + + /* Not used */ + uint32_t reserved1[12]; + + /* Major Minor */ + uint32_t version; + + /* Not used */ + uint32_t reserved2[8]; + + /* Not used */ + uint32_t kernel_header_info; +} __packed; + +struct intel_fw_info { + uint16_t reserved1; + + /* Stepping (A, B, C, ..., *). * is a wildcard */ + char stepping; + + /* Sub-stepping (0, 1, ..., *). * is a wildcard */ + char substepping; + + uint32_t offset; + uint32_t reserved2; +} __packed; + +struct intel_package_header { + /* DMC container header length in dwords */ + unsigned char header_len; + + /* always value would be 0x01 */ + unsigned char header_ver; + + unsigned char reserved[10]; + + /* Number of valid entries in the FWInfo array below */ + uint32_t num_entries; + + struct intel_fw_info fw_info[20]; +} __packed; + +struct intel_dmc_header { + /* always value would be 0x40403E3E */ + uint32_t signature; + + /* DMC binary header length */ + unsigned char header_len; + + /* 0x01 */ + unsigned char header_ver; + + /* Reserved */ + uint16_t dmcc_ver; + + /* Major, Minor */ + uint32_t project; + + /* Firmware program size (excluding header) in dwords */ + uint32_t fw_size; + + /* Major Minor version */ + uint32_t fw_version; + + /* Number of valid MMIO cycles present. */ + uint32_t mmio_count; + + /* MMIO address */ + uint32_t mmioaddr[8]; + + /* MMIO data */ + uint32_t mmiodata[8]; + + /* FW filename */ + unsigned char dfile[32]; + + uint32_t reserved1[2]; +} __packed; + +struct stepping_info { + char stepping; + char substepping; +} __packed; + + +#endif /* _INTEL_CSR_H_ */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7a0aa24..f3a2d88 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1062,6 +1062,11 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane, struct drm_i915_gem_object *obj); +/* intel_csr.c */ +void intel_csr_ucode_init(struct drm_device *dev); +void intel_csr_load_program(struct drm_device *dev); +void intel_csr_ucode_fini(struct drm_device *dev); + /* intel_dp.c */ void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,