I'll first make the argument that drm_i915_private should not have a
single member that is not a named sub-struct defined separately. It's
just too big to have individual members like, say, framestart_delay. All
of these need context that is created using C constructs, not comments.
Ideas have been thrown around about splitting display data more, perhaps
to a separate "display private" struct. I'll also make the argument that
it is not fine-grained enough. Here, I'm taking the audio parts and
shoving them under intel_audio_private struct. It's still defined in
i915_drv.h, but it could be trivially turned into a completely opaque
type defined and allocated in intel_audio.c.
I think the driver is big enough to start requiring this kind of
abstractions, even at the cost of dynamic allocations and pointer
chasing that opaque types require. It's just too easy to poke at
drm_i915_private from wherever, breaking all abstractions.
Sure, it's no coincidence that I chose audio, it's fairly well isolated
already. Not all parts are going to be that easy. But I'm not sure I
like the thought of doing a massive display private struct refactoring
and not take one step further. Or, indeed, take this step *first*.
BR,
Jani.
Jani Nikula (4):
drm/i915/audio: group audio under anonymous struct in drm_i915_private
drm/i915/audio: name the audio sub-struct in drm_i915_private
drm/i915/audio: define the audio struct separately from
drm_i915_private
drm/i915/audio: move intel_audio_funcs internal to intel_audio.c
drivers/gpu/drm/i915/display/intel_audio.c | 99 ++++++++++---------
.../gpu/drm/i915/display/intel_lpe_audio.c | 42 ++++----
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 56 +++++------
4 files changed, 101 insertions(+), 98 deletions(-)
--
2.30.2
