Re: [Intel-gfx] [PATCH 2/8] drm/i915: Merge ironlake_get_refclk() into its only caller

[Conselvan De Oliveira, Ander]

On Mon, 2016-03-14 at 15:55 +0200, Ville Syrjälä wrote:
> On Mon, Mar 14, 2016 at 10:55:41AM +0200, Ander Conselvan de Oliveira wrote:
> > A previous patche made ironlake_get_refclk() very simple, so merge
> > it into its only caller.
> 
> Again I'd like to keep the pch and gmch code as similar as possible.
> So could do the same for the gmch code.
> 
> I already had a patch in my lvds_downclock branch that moved some of
> the gmch platform differences out from i9xx_get_refclk() into the
> caller, so I guess might as well move the whole thing I suppose.

I think we should just split i9xx_crtc_compute_clock() into more platform
specific functions and kill i9xx_get_reclk(), intel_limit() and ->find_dpll().
We are jumping through hoops to make the code look like it's generic, but there
is a lot of platform specific details. IMO it would be a lot easier to read that
way.

I wrote some patches going into that direction today. The end result looks like
the following:

static int chv_crtc_compute_clock(struct intel_crtc *crtc,
  struct intel_crtc_state *crtc_state)
{
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

limit = _limits_chv;

if (!crtc_state->clock_set) {
refclk = 10;

ok = chv_find_best_dpll(limit, crtc_state,
crtc_state->port_clock, refclk,
NULL, _state->dpll);
if (!ok) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}
}

chv_compute_dpll(crtc, crtc_state);

return 0;
}

static int vlv_crtc_compute_clock(struct intel_crtc *crtc,
  struct intel_crtc_state *crtc_state)
{
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

limit = _limits_vlv;

if (!crtc_state->clock_set) {
refclk = 10;

ok = vlv_find_best_dpll(limit, crtc_state,
crtc_state->port_clock, refclk,
NULL, _state->dpll);
if (!ok) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}
}

vlv_compute_dpll(crtc, crtc_state);

return 0;
}

static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
   struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int refclk;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
intel_panel_use_ssc(dev_priv)) {
refclk = dev_priv->vbt.lvds_ssc_freq;
DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
} else {
refclk = 48000;
}

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
limit = _limits_i8xx_lvds;
else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
limit = _limits_i8xx_dvo;
else
limit = _limits_i8xx_dac;

if (!crtc_state->clock_set &&
!i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
 refclk, NULL, _state->dpll)) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}

i8xx_compute_dpll(crtc, crtc_state, NULL);

return 0;
}

static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
   struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
intel_panel_use_ssc(dev_priv)) {
refclk = dev_priv->vbt.lvds_ssc_freq;
DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
} else {
refclk = 96000;
}

if (!crtc_state->clock_set) {
/*
 * 

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Merge ironlake_get_refclk() into its only caller

[Ville Syrjälä]

On Mon, Mar 14, 2016 at 10:55:41AM +0200, Ander Conselvan de Oliveira wrote:
> A previous patche made ironlake_get_refclk() very simple, so merge
> it into its only caller.

Again I'd like to keep the pch and gmch code as similar as possible.
So could do the same for the gmch code.

I already had a patch in my lvds_downclock branch that moved some of
the gmch platform differences out from i9xx_get_refclk() into the
caller, so I guess might as well move the whole thing I suppose.

> 
> Signed-off-by: Ander Conselvan de Oliveira 
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 24 
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e7d6584..07b5244 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8589,21 +8589,6 @@ void intel_init_pch_refclk(struct drm_device *dev)
>   lpt_init_pch_refclk(dev);
>  }
>  
> -static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
> -{
> - struct drm_device *dev = crtc_state->base.crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> - intel_panel_use_ssc(dev_priv)) {
> - DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> -   dev_priv->vbt.lvds_ssc_freq);
> - return dev_priv->vbt.lvds_ssc_freq;
> - }
> -
> - return 12;
> -}
> -
>  static void ironlake_set_pipeconf(struct drm_crtc *crtc)
>  {
>   struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> @@ -8775,7 +8760,14 @@ static bool ironlake_compute_clocks(struct drm_crtc 
> *crtc,
>   const intel_limit_t *limit;
>   bool ret;
>  
> - refclk = ironlake_get_refclk(crtc_state);
> + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> + intel_panel_use_ssc(dev_priv)) {
> + DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> +   dev_priv->vbt.lvds_ssc_freq);
> + refclk = dev_priv->vbt.lvds_ssc_freq;
> + } else {
> + refclk = 12;
> + }
>  
>   /*
>* Returns a set of divisors for the desired target clock with the given
> -- 
> 2.4.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/8] drm/i915: Merge ironlake_get_refclk() into its only caller

[Ander Conselvan de Oliveira]

A previous patche made ironlake_get_refclk() very simple, so merge
it into its only caller.

Signed-off-by: Ander Conselvan de Oliveira 

---
 drivers/gpu/drm/i915/intel_display.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e7d6584..07b5244 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8589,21 +8589,6 @@ void intel_init_pch_refclk(struct drm_device *dev)
lpt_init_pch_refclk(dev);
 }
 
-static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
-{
-   struct drm_device *dev = crtc_state->base.crtc->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-
-   if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-   intel_panel_use_ssc(dev_priv)) {
-   DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
- dev_priv->vbt.lvds_ssc_freq);
-   return dev_priv->vbt.lvds_ssc_freq;
-   }
-
-   return 12;
-}
-
 static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 {
struct drm_i915_private *dev_priv = crtc->dev->dev_private;
@@ -8775,7 +8760,14 @@ static bool ironlake_compute_clocks(struct drm_crtc 
*crtc,
const intel_limit_t *limit;
bool ret;
 
-   refclk = ironlake_get_refclk(crtc_state);
+   if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+   intel_panel_use_ssc(dev_priv)) {
+   DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
+ dev_priv->vbt.lvds_ssc_freq);
+   refclk = dev_priv->vbt.lvds_ssc_freq;
+   } else {
+   refclk = 12;
+   }
 
/*
 * Returns a set of divisors for the desired target clock with the given
-- 
2.4.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx