Re: [Intel-gfx] [PATCH v3 1/2] drm/i915/gt: Split intel-gtt functions by arch

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 30 Mar 2022, Casey Bowman <casey.g.bowman@intel.com> 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