On Wed, 30 Mar 2022, Casey Bowman <casey.g.bowman@xxxxxxxxx> wrote: > On 3/30/22 02:55, Tvrtko Ursulin wrote: >> I mean I could suggest to do something about the incosistency of: >> >> static inline void intel_gt_gmch_gen5_chipset_flush(struct intel_gt *gt) >> >> vs: >> >> static inline int intel_gt_gmch_gen5_probe(struct i915_ggtt *ggtt) >> >> Since I value more for function name to tell me on what it operates >> instead where it is placed. So I'd personally rename the second class >> to i915_ggtt. It would also be consistent with other exported >> functions which take struct i915_ggtt like i915_ggtt_enable_guc, >> i915_ggtt_resume and so. But opinions will differ so I can live with >> it as is as well. >> > > @Jani/Lucas, do you guys have any opinion on changing the prefix to a > functionality-based name over location-based? > > If we have any standard we're trying to adhere to here, I can change it > for the standard. > I'd like to make everyone happy, if possible. :P For display code I'm pretty strongly in favour of file name based symbol prefixes. And basically a file should be a collection of related functionality around a theme, so in that sense it's not merely location based. Indeed the file name should be functionality based! Also for display code we tend to have tons of functions that take similar first (context) parameters, everywhere, and we also change the parameters being passed while refactoring. It would be counter productive to name the functions based on the first parameter. Random example, display/intel_dp.h, it's all about display port, almost all functions are named intel_dp_*, but if they were named by first parameter, we'd have intel_dp, intel_encoder, intel_atomic_state, drm_i915_private, intel_crtc_state, intel_digital_port, etc. It's the intel_dp that best describes them as a group, so that's in the file and function names. Now, I'm not going to put my foot down on gem or gt stuff, but I *personally* find the logic there confusing. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
