[PATCH 07/11] drm/i915/dpll: Change argument for enable hook in intel_global_dpll_funcs

[Kandpal, Suraj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Friday, March 7, 2025 7:36 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; Nautiyal,
> Ankit K <ankit.k.nautiyal at intel.com>; Shankar, Uma
> <uma.shankar at intel.com>; Kahola, Mika <mika.kahola at intel.com>
> Subject: Re: [PATCH 07/11] drm/i915/dpll: Change argument for enable hook in
> intel_global_dpll_funcs
> 
> On Tue, Feb 25, 2025 at 01:39:23PM +0530, Suraj Kandpal wrote:
> > Change the arguments for enable hook in intel_global_dpll_funcs to
> > only accept crtc_state. This is because we really don't need those
> > extra arguments everything can be derived from crtc_state and we need
> > intel_encoder for PLL enablement when DISPLAY_VER() >= 14.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 100
> > ++++++++++--------
> >  1 file changed, 54 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index bd623fdddfdc..c39f7d73a89f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -66,9 +66,8 @@ struct intel_global_dpll_funcs {
> >  	 * Hook for enabling the pll, called from intel_enable_global_dpll() if
> >  	 * the pll is not already enabled.
> >  	 */
> > -	void (*enable)(struct intel_display *display,
> > -		       struct intel_global_dpll *pll,
> > -		       const struct intel_dpll_hw_state *dpll_hw_state);
> > +	void (*enable)(const struct intel_crtc_state *state,
> 
> This feels like the wrong direction. We want the plls to be independent of crtcs.
> The crtc_state should only be necessary to compute a plls's state.

The problem is how much cx0 really depends on the parameters present in crtc_state which
are not a part of pll_state and wouldn't make sense to track them in pll state since they are not just pll specific.
Mika What do you think ? I feel this will be needed if you need to plug in the newer individual PHY PLL hooks 
In the current dpll framework.

Regards,
Suraj Kandpal

> 
> --
> Ville Syrjälä
> Intel