[Intel-gfx] [PATCH 13/15] drm/i915: Split alds/rkl from icl_ddi_combo_{enable, disable}_clock()

[Ville Syrjälä]

On Mon, Feb 01, 2021 at 09:31:49PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 01, 2021 at 11:22:39AM -0800, Lucas De Marchi wrote:
> > On Mon, Feb 01, 2021 at 08:33:41PM +0200, Ville Syrjälä wrote:
> > >From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > >Since .{enable,disable}_clock() are already vfuncs it's a bit silly to
> > >have if-ladders inside them. Just provide specialized version for adlp
> > >and rkl so we don't need any of that.
> > 
> > s/alds/adl-s/
> > 
> > s/adlp/adl-s/
> > 
> > 
> > >
> > >Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 93 ++++++++++++++++--------
> > > 1 file changed, 62 insertions(+), 31 deletions(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > >index 1bd2aa86183d..bafb754d1b66 100644
> > >--- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > >+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > >@@ -3127,28 +3127,6 @@ static u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
> > > 	return 0;
> > > }
> > >
> > >-static u32 icl_dpclka_cfgcr0_clk_sel(struct drm_i915_private *dev_priv,
> > >-				     enum intel_dpll_id id, enum phy phy)
> > 
> > ok, but why do we even add them in this series if we are going to
> > remove. Just makes it harder to review.
> 
> I had to increase the SNR before I could see what the code was
> trying to do. I guess I could now go back and drop the first
> two patches.

One counter argument would be that we already had
icl_dpclka_cfgcr0_clk_off(), so not unifying the approach first
means the other refactorings have to deal with two different
styles, and thus could end up looking even more confusing.

-- 
Ville Syrjälä
Intel