[Intel-gfx] [PATCH 04/15] drm/i915: Add GuC-related header files

[Dave Gordon]

On 17/06/15 16:01, Dave Gordon wrote:
> On 15/06/15 21:20, Chris Wilson wrote:
>> On Mon, Jun 15, 2015 at 07:36:22PM +0100, Dave Gordon wrote:
>>> From: Alex Dai <yu.dai at intel.com>
>>>
>>> intel_guc_api.h contains the subset of the GuC interface that we
>>> will need for submission of commands through the GuC. These MUST
>>> be kept in sync with the definitions used by the GuC firmware.
>>
>> intel_guc_hw.h or intel_guc_abi.h then. Calling it API doesn't make it
>> clear whose API you are talking about.
> 
> It's not 'hw' -- the hw register definitions are elsewhere, because they
> don't depend on the firmware. What it defines is a set of interfaces
> between the GuC firmware and the KMD, so I'll rename it to reflect that
> ("intel_guc_fwif.h", for FirmWareInterFace).
> 
>>> intel_guc.h defines structures and parameters relevant to loading
>>> the GuC firmware and setting it running. Some of these also need
>>> to be kept in sync with the firmware.
>>
>> intel_guc.h should be developed organically as features are added in the
>> series so that it is possible to track against implementation.
> 
>> Certainly not in a patch that adds the entirety of the firmware ABI.
> 
> What may not be obvious is that intel_guc_api.h (or intel_guc_fwif.h, as
> I'm now going to call it) is autogenerated from the non-Linux-friendly
> version actually used in building the GuC firmware. (Or at least, that's
> the PoR; in practice Alex has hacked^W hand-tuned this version.) So it
> makes no sense to break it into parts.
> 
> We /could/ do that with the purely KMD-defined structures in intel_guc.h
> such as intel_guc, and /maybe/ i915_guc_client. OTOH when it's a new
> file, containing a new structure, it's easier to see that the layout is
> sensible when it's all added in one go, rather than repeatedly adding
> bits here and there, especially if the logical order of fields in a
> structure isn't going to be the same as the order of addition of the
> code that uses them.
> 
> I'll see how it looks ...
> 
> .Dave.

So ... I've renamed the sort-of-autogenerated header containing f/w i/f
definitions, to intel_guc_fwif.h, and put all the invariant h/w-related
defines into i915_guc_reg.h (which could be merged into i915_reg.h if
required, but for now it's easier to keep them separate). There is
probably unused stuff in both of these but there's really no point in
removing it, as we may just have to add it back someday.

This leaves only the driver-defined data structures & related stuff in
intel_guc.h, which is now delivered in three tranches (loader, ctx pool
setup, client).

.Dave.