Re: [Intel-gfx] [PATCH 01/39] drm/i915: add display sub-struct to drm_i915_private

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

On Wed, 17 Aug 2022, Jani Nikula <jani.nikula@intel.com> wrote:
> On Tue, 16 Aug 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Thu, Aug 11, 2022 at 06:07:12PM +0300, Jani Nikula wrote:
>>>In another long-overdue cleanup, add a display sub-struct to
>>>drm_i915_private, and start moving display related members there. Start
>>>with display funcs that need a rename anyway to not collide with the new
>>>display member.
>>>
>>>Add a new header under display/ for defining struct intel_display.
>>>
>>>Rename struct drm_i915_display_funcs to intel_display_funcs while at it.

[...]

>>>+struct intel_display_funcs {
>>
>> in the same line as comment above. Maybe we could give this struct a
>> better name? Because it's already inside a intel_display.funcs.crtc
>>
>> display->funcs.crtc->get_pipe_config()
>> display->funcs.crtc->get_initial_plane_nfig()
>> display->funcs.crtc->enable()
>> display->funcs.crtc->disable()
>> display->funcs.crtc->commit_modeset_enables()
>>
>> and then call this `struct intel_crtc_funcs`.
>>
>> This can be done later as this commit itself is basically moving things
>> with the same name inside a substruct.
>
> I guess my question is, are the functions inside "crtc enough" to be
> called intel_crtc_funcs? Though intel_display_funcs is really too
> generic too.
>
> Maybe I'll just go with crtc.

Mmh, except we already have a bunch of struct drm_crtc_funcs with
<platform>_crtc_funcs in intel_crtc.c. Too confusing.

struct intel_random_collection_of_display_funcs. :p

The easy choice *for now* would be to stick with the struct
intel_display_funcs and live with the display->funcs.display tautology.

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Graphics Center