[Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv

[Lucas De Marchi]

On Fri, Oct 22, 2021 at 07:27:54PM +0300, Jani Nikula wrote:
>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*.

I like it, thanks.

I only have a small comment on first patch about commit message. Also
about intel_audio_private vs intel_audio I had the same thought as
Ville, but I get your point too. And I don't have any better name
suggestion. So this is 


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>
>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
>