[Intel-gfx] [PATCH 05/39] drm/i915: move clock_gating_funcs to display.funcs

[Lucas De Marchi]

On Wed, Aug 17, 2022 at 09:50:52AM +0300, Jani Nikula wrote:
>On Tue, 16 Aug 2022, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Thu, Aug 11, 2022 at 06:07:16PM +0300, Jani Nikula wrote:
>>>Move display related members under drm_i915_private display sub-struct.
>>>
>>>Rename struct i915_clock_gating_funcs to intel_clock_gating_funcs while
>>>at it.
>>>
>>>Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>>---
>>> .../gpu/drm/i915/display/intel_display_core.h |  4 ++
>>> drivers/gpu/drm/i915/i915_drv.h               |  4 --
>>> drivers/gpu/drm/i915/intel_pm.c               | 58 +++++++++----------
>>> 3 files changed, 33 insertions(+), 33 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
>>>index ff76bd4079e4..98c6ccdc9100 100644
>>>--- a/drivers/gpu/drm/i915/display/intel_display_core.h
>>>+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
>>>@@ -10,6 +10,7 @@
>>>
>>> struct intel_atomic_state;
>>> struct intel_cdclk_funcs;
>>>+struct intel_clock_gating_funcs;
>>> struct intel_crtc;
>>> struct intel_crtc_state;
>>> struct intel_dpll_funcs;
>>>@@ -44,6 +45,9 @@ struct intel_display {
>>>
>>> 		/* irq display functions */
>>> 		const struct intel_hotplug_funcs *hotplug;
>>>+
>>>+		/* pm private clock gating functions */
>>>+		const struct intel_clock_gating_funcs *clock_gating;
>>
>> did we get this correct moving clock_gating to display? The question I'd
>> ask is: if a platform doesn't have display, would it need to do
>> anything clock-gating related? Looking at the current functions e.g.
>> gen9_init_clock_gating setting some chicken bits, I'd say yes.
>>
>> Another reasoning I'd have is regarding the registers it touches.
>> And here they are not from display.
>>
>> So, I don't really understand the reason for moving clock_gating here,
>> except that there are indeed several functions doing display-related
>> things. Should we rather split one for i915 and one for i915-display?
>
>Mmmh, maybe. It's hard to split the driver nicely when the hardware
>isn't!

oh, maybe one alternative is to move most of those register settings
(that appear to be WAs) somewhere else. And then declare the
clock_gating stuff as "this is display stuff".

I don't mind if you move them inside display first just to get
the ball rolling and we move them to a better place later.

Lucas De Marchi

>
>BR,
>Jani.
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center