[5/9] drm/i915: s/update/compute/ for gmch dpll register functions
diff mbox

Message ID 1434616228-28358-5-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter June 18, 2015, 8:30 a.m. UTC
I was momentarily confused until I've double-checked that these
functions really only compute state and don't update the hardware
state. They once did that, but since Ander's rework of the dpll
computation flow that's no longer the case.

Rename them to avoid further confusion.

Note that the ilk code already follows the compute_dpll naming scheme
for computing the actual register value. DDI code goes with _calc_,
but that is close enough.

Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Comments

Paulo Zanoni June 23, 2015, 8:26 p.m. UTC | #1
2015-06-18 5:30 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> I was momentarily confused until I've double-checked that these
> functions really only compute state and don't update the hardware
> state. They once did that, but since Ander's rework of the dpll
> computation flow that's no longer the case.
>
> Rename them to avoid further confusion.
>
> Note that the ilk code already follows the compute_dpll naming scheme
> for computing the actual register value. DDI code goes with _calc_,
> but that is close enough.

My only bikeshed would be to point that we now have x_prepare_pll(),
x_enable_pll() and x_compute_dpll(), with the "d" to break the
standard. OTOH the ILK function also has the "d" letter, so meh...

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45a93c87d48e..a8d9dbd5bd34 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7233,8 +7233,8 @@ void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
>                 intel_cpu_transcoder_set_m_n(crtc, dp_m_n, dp_m2_n2);
>  }
>
> -static void vlv_update_pll(struct intel_crtc *crtc,
> -                          struct intel_crtc_state *pipe_config)
> +static void vlv_compute_dpll(struct intel_crtc *crtc,
> +                            struct intel_crtc_state *pipe_config)
>  {
>         u32 dpll, dpll_md;
>
> @@ -7347,8 +7347,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>         mutex_unlock(&dev_priv->sb_lock);
>  }
>
> -static void chv_update_pll(struct intel_crtc *crtc,
> -                          struct intel_crtc_state *pipe_config)
> +static void chv_compute_dpll(struct intel_crtc *crtc,
> +                            struct intel_crtc_state *pipe_config)
>  {
>         pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
>                 DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> @@ -7487,11 +7487,11 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>         };
>
>         if (IS_CHERRYVIEW(dev)) {
> -               chv_update_pll(crtc, &pipe_config);
> +               chv_compute_dpll(crtc, &pipe_config);
>                 chv_prepare_pll(crtc, &pipe_config);
>                 chv_enable_pll(crtc, &pipe_config);
>         } else {
> -               vlv_update_pll(crtc, &pipe_config);
> +               vlv_compute_dpll(crtc, &pipe_config);
>                 vlv_prepare_pll(crtc, &pipe_config);
>                 vlv_enable_pll(crtc, &pipe_config);
>         }
> @@ -7513,10 +7513,10 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
>                 vlv_disable_pll(to_i915(dev), pipe);
>  }
>
> -static void i9xx_update_pll(struct intel_crtc *crtc,
> -                           struct intel_crtc_state *crtc_state,
> -                           intel_clock_t *reduced_clock,
> -                           int num_connectors)
> +static void i9xx_compute_dpll(struct intel_crtc *crtc,
> +                             struct intel_crtc_state *crtc_state,
> +                             intel_clock_t *reduced_clock,
> +                             int num_connectors)
>  {
>         struct drm_device *dev = crtc->base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7590,10 +7590,10 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>         }
>  }
>
> -static void i8xx_update_pll(struct intel_crtc *crtc,
> -                           struct intel_crtc_state *crtc_state,
> -                           intel_clock_t *reduced_clock,
> -                           int num_connectors)
> +static void i8xx_compute_dpll(struct intel_crtc *crtc,
> +                             struct intel_crtc_state *crtc_state,
> +                             intel_clock_t *reduced_clock,
> +                             int num_connectors)
>  {
>         struct drm_device *dev = crtc->base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7887,15 +7887,15 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>         }
>
>         if (IS_GEN2(dev)) {
> -               i8xx_update_pll(crtc, crtc_state, NULL,
> -                               num_connectors);
> +               i8xx_compute_dpll(crtc, crtc_state, NULL,
> +                                 num_connectors);
>         } else if (IS_CHERRYVIEW(dev)) {
> -               chv_update_pll(crtc, crtc_state);
> +               chv_compute_dpll(crtc, crtc_state);
>         } else if (IS_VALLEYVIEW(dev)) {
> -               vlv_update_pll(crtc, crtc_state);
> +               vlv_compute_dpll(crtc, crtc_state);
>         } else {
> -               i9xx_update_pll(crtc, crtc_state, NULL,
> -                               num_connectors);
> +               i9xx_compute_dpll(crtc, crtc_state, NULL,
> +                                 num_connectors);
>         }
>
>         return 0;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter June 23, 2015, 9:20 p.m. UTC | #2
On Tue, Jun 23, 2015 at 05:26:40PM -0300, Paulo Zanoni wrote:
> 2015-06-18 5:30 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > I was momentarily confused until I've double-checked that these
> > functions really only compute state and don't update the hardware
> > state. They once did that, but since Ander's rework of the dpll
> > computation flow that's no longer the case.
> >
> > Rename them to avoid further confusion.
> >
> > Note that the ilk code already follows the compute_dpll naming scheme
> > for computing the actual register value. DDI code goes with _calc_,
> > but that is close enough.
> 
> My only bikeshed would be to point that we now have x_prepare_pll(),
> x_enable_pll() and x_compute_dpll(), with the "d" to break the
> standard. OTOH the ILK function also has the "d" letter, so meh...

Thanks for your review. I fixed up patch 1, pinged Chris again on the lvds
drrs removal and merged all the others.
-Daniel

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------------
> >  1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 45a93c87d48e..a8d9dbd5bd34 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7233,8 +7233,8 @@ void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
> >                 intel_cpu_transcoder_set_m_n(crtc, dp_m_n, dp_m2_n2);
> >  }
> >
> > -static void vlv_update_pll(struct intel_crtc *crtc,
> > -                          struct intel_crtc_state *pipe_config)
> > +static void vlv_compute_dpll(struct intel_crtc *crtc,
> > +                            struct intel_crtc_state *pipe_config)
> >  {
> >         u32 dpll, dpll_md;
> >
> > @@ -7347,8 +7347,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> >         mutex_unlock(&dev_priv->sb_lock);
> >  }
> >
> > -static void chv_update_pll(struct intel_crtc *crtc,
> > -                          struct intel_crtc_state *pipe_config)
> > +static void chv_compute_dpll(struct intel_crtc *crtc,
> > +                            struct intel_crtc_state *pipe_config)
> >  {
> >         pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
> >                 DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> > @@ -7487,11 +7487,11 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> >         };
> >
> >         if (IS_CHERRYVIEW(dev)) {
> > -               chv_update_pll(crtc, &pipe_config);
> > +               chv_compute_dpll(crtc, &pipe_config);
> >                 chv_prepare_pll(crtc, &pipe_config);
> >                 chv_enable_pll(crtc, &pipe_config);
> >         } else {
> > -               vlv_update_pll(crtc, &pipe_config);
> > +               vlv_compute_dpll(crtc, &pipe_config);
> >                 vlv_prepare_pll(crtc, &pipe_config);
> >                 vlv_enable_pll(crtc, &pipe_config);
> >         }
> > @@ -7513,10 +7513,10 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
> >                 vlv_disable_pll(to_i915(dev), pipe);
> >  }
> >
> > -static void i9xx_update_pll(struct intel_crtc *crtc,
> > -                           struct intel_crtc_state *crtc_state,
> > -                           intel_clock_t *reduced_clock,
> > -                           int num_connectors)
> > +static void i9xx_compute_dpll(struct intel_crtc *crtc,
> > +                             struct intel_crtc_state *crtc_state,
> > +                             intel_clock_t *reduced_clock,
> > +                             int num_connectors)
> >  {
> >         struct drm_device *dev = crtc->base.dev;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -7590,10 +7590,10 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
> >         }
> >  }
> >
> > -static void i8xx_update_pll(struct intel_crtc *crtc,
> > -                           struct intel_crtc_state *crtc_state,
> > -                           intel_clock_t *reduced_clock,
> > -                           int num_connectors)
> > +static void i8xx_compute_dpll(struct intel_crtc *crtc,
> > +                             struct intel_crtc_state *crtc_state,
> > +                             intel_clock_t *reduced_clock,
> > +                             int num_connectors)
> >  {
> >         struct drm_device *dev = crtc->base.dev;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -7887,15 +7887,15 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> >         }
> >
> >         if (IS_GEN2(dev)) {
> > -               i8xx_update_pll(crtc, crtc_state, NULL,
> > -                               num_connectors);
> > +               i8xx_compute_dpll(crtc, crtc_state, NULL,
> > +                                 num_connectors);
> >         } else if (IS_CHERRYVIEW(dev)) {
> > -               chv_update_pll(crtc, crtc_state);
> > +               chv_compute_dpll(crtc, crtc_state);
> >         } else if (IS_VALLEYVIEW(dev)) {
> > -               vlv_update_pll(crtc, crtc_state);
> > +               vlv_compute_dpll(crtc, crtc_state);
> >         } else {
> > -               i9xx_update_pll(crtc, crtc_state, NULL,
> > -                               num_connectors);
> > +               i9xx_compute_dpll(crtc, crtc_state, NULL,
> > +                                 num_connectors);
> >         }
> >
> >         return 0;
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45a93c87d48e..a8d9dbd5bd34 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7233,8 +7233,8 @@  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
 		intel_cpu_transcoder_set_m_n(crtc, dp_m_n, dp_m2_n2);
 }
 
-static void vlv_update_pll(struct intel_crtc *crtc,
-			   struct intel_crtc_state *pipe_config)
+static void vlv_compute_dpll(struct intel_crtc *crtc,
+			     struct intel_crtc_state *pipe_config)
 {
 	u32 dpll, dpll_md;
 
@@ -7347,8 +7347,8 @@  static void vlv_prepare_pll(struct intel_crtc *crtc,
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
-static void chv_update_pll(struct intel_crtc *crtc,
-			   struct intel_crtc_state *pipe_config)
+static void chv_compute_dpll(struct intel_crtc *crtc,
+			     struct intel_crtc_state *pipe_config)
 {
 	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
 		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
@@ -7487,11 +7487,11 @@  void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
 	};
 
 	if (IS_CHERRYVIEW(dev)) {
-		chv_update_pll(crtc, &pipe_config);
+		chv_compute_dpll(crtc, &pipe_config);
 		chv_prepare_pll(crtc, &pipe_config);
 		chv_enable_pll(crtc, &pipe_config);
 	} else {
-		vlv_update_pll(crtc, &pipe_config);
+		vlv_compute_dpll(crtc, &pipe_config);
 		vlv_prepare_pll(crtc, &pipe_config);
 		vlv_enable_pll(crtc, &pipe_config);
 	}
@@ -7513,10 +7513,10 @@  void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
 		vlv_disable_pll(to_i915(dev), pipe);
 }
 
-static void i9xx_update_pll(struct intel_crtc *crtc,
-			    struct intel_crtc_state *crtc_state,
-			    intel_clock_t *reduced_clock,
-			    int num_connectors)
+static void i9xx_compute_dpll(struct intel_crtc *crtc,
+			      struct intel_crtc_state *crtc_state,
+			      intel_clock_t *reduced_clock,
+			      int num_connectors)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7590,10 +7590,10 @@  static void i9xx_update_pll(struct intel_crtc *crtc,
 	}
 }
 
-static void i8xx_update_pll(struct intel_crtc *crtc,
-			    struct intel_crtc_state *crtc_state,
-			    intel_clock_t *reduced_clock,
-			    int num_connectors)
+static void i8xx_compute_dpll(struct intel_crtc *crtc,
+			      struct intel_crtc_state *crtc_state,
+			      intel_clock_t *reduced_clock,
+			      int num_connectors)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7887,15 +7887,15 @@  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	}
 
 	if (IS_GEN2(dev)) {
-		i8xx_update_pll(crtc, crtc_state, NULL,
-				num_connectors);
+		i8xx_compute_dpll(crtc, crtc_state, NULL,
+				  num_connectors);
 	} else if (IS_CHERRYVIEW(dev)) {
-		chv_update_pll(crtc, crtc_state);
+		chv_compute_dpll(crtc, crtc_state);
 	} else if (IS_VALLEYVIEW(dev)) {
-		vlv_update_pll(crtc, crtc_state);
+		vlv_compute_dpll(crtc, crtc_state);
 	} else {
-		i9xx_update_pll(crtc, crtc_state, NULL,
-				num_connectors);
+		i9xx_compute_dpll(crtc, crtc_state, NULL,
+				  num_connectors);
 	}
 
 	return 0;