On Mon, 17 May 2021, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 17, 2021 at 7:05 PM Jason Ekstrand jason@jlekstrand.net wrote:
On Mon, May 17, 2021 at 8:40 AM Daniel Vetter daniel@ffwll.ch wrote:
On Fri, May 14, 2021 at 02:13:57PM -0500, Jason Ekstrand wrote:
I can add those. I just don't know where to put it. We don't have an i915_gem_vm.h. Suggestions?
gt/intel_gtt.h seems to be the header for i915_address_space stuff. Also contains the i915_vma_ops but not i915_vma.
It's a pretty good mess, but probably the best place for now for these :-/
The one for contexts is in i915_drv.h so I put the VM one there too. Feel free to tell me to move it. I don't care where it goes.
i915_drv.h is the og dumping ground and needs to die. Everything in there needs to be moved out/split/whatever for better code organization. If we have a place already that fits better (even if maybe misnamed) it's better to put it there.
I haven't really codified this anywhere, but this is what I've been trying to drive:
* All functions in a .c file are declared in the corresponding .h file. 1:1 relationship.
* Have _types.h headers separately for defining types that lead to deep include chains. (We have this in part because we have absolutely everything in struct drm_i915_private, and everything needs everything else to look inside i915.)
* Minimize includes from headers. Prefer forward declarations where possible. Prefer specific includes over generic includes.
* Each header is self-contained (this is build-tested with CONFIG_DRM_I915_WERROR=y).
* Avoid static inlines unless you have a performance need.
* Don't have any externs. Interfaces over data; data is not an interface.
* Prefix functions in a file according to the filename. intel_foo.[ch] would have functions intel_foo_*(). Ditto i915_bar.[ch] and i915_bar_*(). (Avoid non-static platform specific functions, but if you have them, you'd name them e.g. skl_foo_*().)
Basically the rationale is to have more order in the chaos that we've had for a long time. It's not so much about being pedantic about the naming, but rather the secondary effect of making people think about where they put stuff and how it's all grouped together.
IMO it's also easier to add file.[ch] and nuke it later than add stuff to some of our huge files and then clean it up later.
BR, Jani.