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

[Casey Bowman]

On 3/30/22 10:25, Jani Nikula wrote:
> On Wed, 30 Mar 2022, Casey Bowman <casey.g.bowman at 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.
>
>
Ok, thank you for your perspectives!

Regards,
Casey