Re: [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private

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

On Fri, 22 Oct 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> On Fri, 22 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Fri, Oct 22, 2021 at 07:27:57PM +0300, Jani Nikula wrote:
>>> Add a standalone definition of struct intel_audio_private, and note that
>>> all of it is private to intel_audio.c.
>>> 
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_drv.h | 45 ++++++++++++++++++---------------
>>>  1 file changed, 24 insertions(+), 21 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 9c77610acf23..ed86633a587b 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -828,6 +828,29 @@ struct i915_selftest_stash {
>>>  	struct ida mock_region_instances;
>>>  };
>>>  
>>> +/* intel_audio.c private */
>>> +struct intel_audio_private {
>>
>> Not sure the "_private" is actually useful. I'd just call it
>> intel_audio. The fact that struct drm_i915_private
>> already says "private" doesn't mean anything to anyone anyway.
>
> I first named it just intel_audio. Then I added intel_hdcp too, and
> realized it means something else, in intel_display_types.h. Then there's
> intel_gmbus. Probably others. I'd kind of like to have some
> consistency. I'm not hung up on "_private", but I don't like to call
> this intel_audio and then have intel_hdcp_foo and intel_gmbus_bar. Ideas
> welcome.

I ended up keeping the intel_audio_private name for now in v2. It's a
two-line patch to rename later. Though I admit it sets a precedent.

BR,
Jani.

>
> And, yeah, we could just rename drm_i915_private drm_i915_public. :p
>
>
> BR,
> Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center