[Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
No functional change. Introduce dg2_cdclk_squash_program and move squash_ctl register programming bits to this. v2: s/dg2_cdclk_squash_programming/dg2_cdclk_squash_program (Jani) Cc: Jani Nikula Cc: Balasubramani Vivekanandan Cc: Ville Syrjälä Signed-off-by: Anusha Srivatsa Reviewed-by: Balasubramani Vivekanandan --- drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 3893779e0c23..e21cd0fbe29a 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll_update(struct drm_i915_private *i915, int vco) } +static void dg2_cdclk_squash_program(struct drm_i915_private *i915, +u16 waveform) +{ + u32 squash_ctl = 0; + + if (waveform) + squash_ctl = CDCLK_SQUASH_ENABLE | +CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; + + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); +} + static void bxt_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *cdclk_config, enum pipe pipe) @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, else clock = cdclk; - if (HAS_CDCLK_SQUASH(dev_priv)) { - u32 squash_ctl = 0; - - if (waveform) - squash_ctl = CDCLK_SQUASH_ENABLE | - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; - - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); - } + if (HAS_CDCLK_SQUASH(dev_priv)) + dg2_cdclk_squash_program(dev_priv, waveform); val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | bxt_cdclk_cd2x_pipe(dev_priv, pipe) | -- 2.25.1
Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
On 21.10.2022 14:39, Anusha Srivatsa wrote: > No functional change. Introduce dg2_cdclk_squash_program and > move squash_ctl register programming bits to this. > > v2: s/dg2_cdclk_squash_programming/dg2_cdclk_squash_program (Jani) > > Cc: Jani Nikula > Cc: Balasubramani Vivekanandan > Cc: Ville Syrjälä > Signed-off-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- > 1 file changed, 14 insertions(+), 9 deletions(-) Reviewed-by: Balasubramani Vivekanandan > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 3893779e0c23..e21cd0fbe29a 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll_update(struct > drm_i915_private *i915, int vco) > > } > > +static void dg2_cdclk_squash_program(struct drm_i915_private *i915, > + u16 waveform) > +{ > + u32 squash_ctl = 0; > + > + if (waveform) > + squash_ctl = CDCLK_SQUASH_ENABLE | > + CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > + > + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); > +} > + > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_config *cdclk_config, > enum pipe pipe) > @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private > *dev_priv, > else > clock = cdclk; > > - if (HAS_CDCLK_SQUASH(dev_priv)) { > - u32 squash_ctl = 0; > - > - if (waveform) > - squash_ctl = CDCLK_SQUASH_ENABLE | > - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > - > - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); > - } > + if (HAS_CDCLK_SQUASH(dev_priv)) > + dg2_cdclk_squash_program(dev_priv, waveform); > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > bxt_cdclk_cd2x_pipe(dev_priv, pipe) | > -- > 2.25.1 >
[Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
No functional change. Introduce dg2_cdclk_squash_program and move squash_ctl register programming bits to this. v2: s/dg2_cdclk_squash_programming/dg2_cdclk_squash_program (Jani) Cc: Jani Nikula Cc: Balasubramani Vivekanandan Cc: Ville Syrjälä Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 3893779e0c23..e21cd0fbe29a 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll_update(struct drm_i915_private *i915, int vco) } +static void dg2_cdclk_squash_program(struct drm_i915_private *i915, +u16 waveform) +{ + u32 squash_ctl = 0; + + if (waveform) + squash_ctl = CDCLK_SQUASH_ENABLE | +CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; + + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); +} + static void bxt_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *cdclk_config, enum pipe pipe) @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, else clock = cdclk; - if (HAS_CDCLK_SQUASH(dev_priv)) { - u32 squash_ctl = 0; - - if (waveform) - squash_ctl = CDCLK_SQUASH_ENABLE | - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; - - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); - } + if (HAS_CDCLK_SQUASH(dev_priv)) + dg2_cdclk_squash_program(dev_priv, waveform); val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | bxt_cdclk_cd2x_pipe(dev_priv, pipe) | -- 2.25.1
Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
> -Original Message- > From: Vivekanandan, Balasubramani > > Sent: Friday, October 21, 2022 12:08 AM > To: Srivatsa, Anusha ; intel- > g...@lists.freedesktop.org > Cc: Ville Syrjälä > Subject: Re: [PATCH 4/4] drm/i915/display: Move squash_ctl register > programming to its own function > > On 20.10.2022 17:20, Anusha Srivatsa wrote: > > No functional change. Introduce dg2_cdclk_squash_programming and > move > > squash_ctl register programming bits to this. > > > > Cc: Balasubramani Vivekanandan > > > Cc: Ville Syrjälä > > Signed-off-by: Anusha Srivatsa > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 > > +- > > 1 file changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index 8701796788e3..b692186c8f02 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll(struct > > drm_i915_private *i915, int vco) > > > > } > > > > +static void dg2_cdclk_squash_programming(struct drm_i915_private > *i915, > > +u16 waveform) > > +{ > > + u32 squash_ctl = 0; > > + > > + if (waveform) > > + squash_ctl = CDCLK_SQUASH_ENABLE | > > +CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > > + > > + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); } > > + > > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_config *cdclk_config, > > enum pipe pipe) > > @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct > drm_i915_private *dev_priv, > > else > > clock = cdclk; > > > > - if (HAS_CDCLK_SQUASH(dev_priv)) { > > - u32 squash_ctl = 0; > > - > > - if (waveform) > > - squash_ctl = CDCLK_SQUASH_ENABLE | > > - CDCLK_SQUASH_WINDOW_SIZE(0xf) | > waveform; > > - > > - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); > > - } > > + if (HAS_CDCLK_SQUASH(dev_priv)) > > + dg2_cdclk_squash_programming(dev_priv, waveform); > > Is it possible to move also the function cdclk_squash_waveform() inside > dg2_cdclk_squash_programming()? Hmmm. Are you thinking it is better to have both grabbing the squash waveform and programming the squash_ctl registers in one place better? IMO the fact that they are separate makes it more readable. We will have to return waveform and calculate the clock anyway Anusha > Regards, > Bala > > > > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > > bxt_cdclk_cd2x_pipe(dev_priv, pipe) | > > -- > > 2.25.1 > >
Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
> -Original Message- > From: Jani Nikula > Sent: Friday, October 21, 2022 1:41 AM > To: Srivatsa, Anusha ; intel- > g...@lists.freedesktop.org > Cc: Vivekanandan, Balasubramani > > Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl > register programming to its own function > > On Thu, 20 Oct 2022, Anusha Srivatsa wrote: > > No functional change. Introduce dg2_cdclk_squash_programming and > move > > squash_ctl register programming bits to this. > > > > Cc: Balasubramani Vivekanandan > > > Cc: Ville Syrjälä > > Signed-off-by: Anusha Srivatsa > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 > > +- > > 1 file changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index 8701796788e3..b692186c8f02 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll(struct > > drm_i915_private *i915, int vco) > > > > } > > > > +static void dg2_cdclk_squash_programming(struct drm_i915_private > *i915, > > +u16 waveform) > > Function names that are verbs should preferrably use the imperative mood, > i.e. program() instead of programmed(), programs() or programming(). > > I'm also not sure "programming" or "program" is a very descriptive thing; > almost everything here is about "programming" something or other. Makes sense. Didn’t have a good feeling about the name in the first place. Thanks for the validation. Anusha > BR, > Jani. > > > > +{ > > + u32 squash_ctl = 0; > > + > > + if (waveform) > > + squash_ctl = CDCLK_SQUASH_ENABLE | > > +CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > > + > > + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); } > > + > > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_config *cdclk_config, > > enum pipe pipe) > > @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct > drm_i915_private *dev_priv, > > else > > clock = cdclk; > > > > - if (HAS_CDCLK_SQUASH(dev_priv)) { > > - u32 squash_ctl = 0; > > - > > - if (waveform) > > - squash_ctl = CDCLK_SQUASH_ENABLE | > > - CDCLK_SQUASH_WINDOW_SIZE(0xf) | > waveform; > > - > > - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); > > - } > > + if (HAS_CDCLK_SQUASH(dev_priv)) > > + dg2_cdclk_squash_programming(dev_priv, waveform); > > > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > > bxt_cdclk_cd2x_pipe(dev_priv, pipe) | > > -- > Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
On Thu, 20 Oct 2022, Anusha Srivatsa wrote: > No functional change. Introduce dg2_cdclk_squash_programming and > move squash_ctl register programming bits to this. > > Cc: Balasubramani Vivekanandan > Cc: Ville Syrjälä > Signed-off-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 8701796788e3..b692186c8f02 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll(struct drm_i915_private > *i915, int vco) > > } > > +static void dg2_cdclk_squash_programming(struct drm_i915_private *i915, > + u16 waveform) Function names that are verbs should preferrably use the imperative mood, i.e. program() instead of programmed(), programs() or programming(). I'm also not sure "programming" or "program" is a very descriptive thing; almost everything here is about "programming" something or other. BR, Jani. > +{ > + u32 squash_ctl = 0; > + > + if (waveform) > + squash_ctl = CDCLK_SQUASH_ENABLE | > + CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > + > + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); > +} > + > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_config *cdclk_config, > enum pipe pipe) > @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private > *dev_priv, > else > clock = cdclk; > > - if (HAS_CDCLK_SQUASH(dev_priv)) { > - u32 squash_ctl = 0; > - > - if (waveform) > - squash_ctl = CDCLK_SQUASH_ENABLE | > - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > - > - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); > - } > + if (HAS_CDCLK_SQUASH(dev_priv)) > + dg2_cdclk_squash_programming(dev_priv, waveform); > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > bxt_cdclk_cd2x_pipe(dev_priv, pipe) | -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
On 20.10.2022 17:20, Anusha Srivatsa wrote: > No functional change. Introduce dg2_cdclk_squash_programming and > move squash_ctl register programming bits to this. > > Cc: Balasubramani Vivekanandan > Cc: Ville Syrjälä > Signed-off-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 8701796788e3..b692186c8f02 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll(struct drm_i915_private > *i915, int vco) > > } > > +static void dg2_cdclk_squash_programming(struct drm_i915_private *i915, > + u16 waveform) > +{ > + u32 squash_ctl = 0; > + > + if (waveform) > + squash_ctl = CDCLK_SQUASH_ENABLE | > + CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > + > + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); > +} > + > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_config *cdclk_config, > enum pipe pipe) > @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private > *dev_priv, > else > clock = cdclk; > > - if (HAS_CDCLK_SQUASH(dev_priv)) { > - u32 squash_ctl = 0; > - > - if (waveform) > - squash_ctl = CDCLK_SQUASH_ENABLE | > - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; > - > - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); > - } > + if (HAS_CDCLK_SQUASH(dev_priv)) > + dg2_cdclk_squash_programming(dev_priv, waveform); Is it possible to move also the function cdclk_squash_waveform() inside dg2_cdclk_squash_programming()? Regards, Bala > > val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | > bxt_cdclk_cd2x_pipe(dev_priv, pipe) | > -- > 2.25.1 >
[Intel-gfx] [PATCH 4/4] drm/i915/display: Move squash_ctl register programming to its own function
No functional change. Introduce dg2_cdclk_squash_programming and move squash_ctl register programming bits to this. Cc: Balasubramani Vivekanandan Cc: Ville Syrjälä Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/display/intel_cdclk.c | 23 +- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 8701796788e3..b692186c8f02 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1705,6 +1705,18 @@ static void bxt_cdclk_pll(struct drm_i915_private *i915, int vco) } +static void dg2_cdclk_squash_programming(struct drm_i915_private *i915, +u16 waveform) +{ + u32 squash_ctl = 0; + + if (waveform) + squash_ctl = CDCLK_SQUASH_ENABLE | +CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; + + intel_de_write(i915, CDCLK_SQUASH_CTL, squash_ctl); +} + static void bxt_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *cdclk_config, enum pipe pipe) @@ -1752,15 +1764,8 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, else clock = cdclk; - if (HAS_CDCLK_SQUASH(dev_priv)) { - u32 squash_ctl = 0; - - if (waveform) - squash_ctl = CDCLK_SQUASH_ENABLE | - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform; - - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl); - } + if (HAS_CDCLK_SQUASH(dev_priv)) + dg2_cdclk_squash_programming(dev_priv, waveform); val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) | bxt_cdclk_cd2x_pipe(dev_priv, pipe) | -- 2.25.1