Re: [PATCH v5 1/8] drm/i915/skl: Add support to load SKL CSR firmware.

[Imre Deak <imre.deak@intel.com>]

On ma, 2015-05-04 at 11:30 +0200, Daniel Vetter wrote:
> On Thu, Apr 30, 2015 at 04:02:34PM +0300, Imre Deak wrote:
> > On ke, 2015-04-29 at 22:59 +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.
> > > - Changed the firmware name as skl_dmc_ver1.bin, followed naming convention <platform>_dmc_<api-version>.bin (Animesh)
> > > 
> > > Issue: VIZ-2569
> > > Signed-off-by: A.Sunil Kamath <sunil.kamath@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>
> > 
> > For the future: scripts/checkpatch.pl has a valid warning about the
> > commit message, please fix the issues reported by this tool next time.
> 
> Hm, what does checkpatch complain about - mine didn't?
> I did apply the patch manually though.

Too long commit message lines. Though one of those was from me:/

--Imre


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx