* [PATCH 0/3] Small clocking code refactor, v2.
@ 2017-02-20 20:00 Paulo Zanoni
2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Paulo Zanoni @ 2017-02-20 20:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
With Ville's comments addressed. The main difference now is that what used to be
patches 1 and 2 are now just patch 1.
Paulo Zanoni (3):
drm/i915: unify the x_modeset_calc_cdclk() functions
drm/i915: remove potentially confusing IS_G4X checks
drm/i915: reorganize the get_cdclk assignment
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/intel_cdclk.c | 294 ++++++++++++++---------------------
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 127 insertions(+), 178 deletions(-)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions 2017-02-20 20:00 [PATCH 0/3] Small clocking code refactor, v2 Paulo Zanoni @ 2017-02-20 20:00 ` Paulo Zanoni 2017-03-02 20:28 ` Ville Syrjälä 2017-03-03 12:13 ` Ander Conselvan De Oliveira 2017-02-20 20:00 ` [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks Paulo Zanoni ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Paulo Zanoni @ 2017-02-20 20:00 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi There's a lot of duplicated platform-independent logic in the current modeset_calc_cdclk() functions. Adding cdclk support for more platforms will only add more copies of this code. To solve this problem, in this patch we create a new function called intel_modeset_calc_cdclk() which unifies the platform-independent logic, and we also create a new vfunc called calc_cdclk_state(), which is responsible to contain only the platform-dependent code. The code is now smaller and support for new platforms should be easier and not require duplicated platform-independent code. v2: Previously I had 2 different patches addressing these problems, but wiht Ville's suggestion I think it makes more sense to keep everything in a single patch (Ville). Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/intel_cdclk.c | 187 ++++++++++------------------------- drivers/gpu/drm/i915/intel_display.c | 6 +- drivers/gpu/drm/i915/intel_drv.h | 1 + 4 files changed, 60 insertions(+), 138 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4e7046d..f1c35c7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -644,7 +644,9 @@ struct drm_i915_display_funcs { struct intel_crtc_state *cstate); int (*compute_global_watermarks)(struct drm_atomic_state *state); void (*update_wm)(struct intel_crtc *crtc); - int (*modeset_calc_cdclk)(struct drm_atomic_state *state); + void (*calc_cdclk_state)(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state); /* Returns the active state of the crtc, and if the crtc is active, * fills out the pipe-config with the hw state. */ bool (*get_pipe_config)(struct intel_crtc *, diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index d643c0c..dd6fe25 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1496,148 +1496,69 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state) return max_pixel_rate; } -static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state) +static void vlv_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - int max_pixclk = intel_max_pixel_rate(state); - struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); - int cdclk; - - cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = vlv_calc_cdclk(dev_priv, 0); - - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } - - return 0; + cdclk_state->cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); } -static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state) +static void bdw_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - int max_pixclk = intel_max_pixel_rate(state); - int cdclk; - - /* - * FIXME should also account for plane ratio - * once 64bpp pixel formats are supported. - */ - cdclk = bdw_calc_cdclk(max_pixclk); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = bdw_calc_cdclk(0); - - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } - - return 0; + cdclk_state->cdclk = bdw_calc_cdclk(max_pixclk); } -static int skl_modeset_calc_cdclk(struct drm_atomic_state *state) +static void skl_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - struct drm_i915_private *dev_priv = to_i915(state->dev); - const int max_pixclk = intel_max_pixel_rate(state); - int cdclk, vco; + if (!cdclk_state->vco) + cdclk_state->vco = dev_priv->skl_preferred_vco_freq; - vco = intel_state->cdclk.logical.vco; - if (!vco) - vco = dev_priv->skl_preferred_vco_freq; - - /* - * FIXME should also account for plane ratio - * once 64bpp pixel formats are supported. - */ - cdclk = skl_calc_cdclk(max_pixclk, vco); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.vco = vco; - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = skl_calc_cdclk(0, vco); + cdclk_state->cdclk = skl_calc_cdclk(max_pixclk, cdclk_state->vco); +} - intel_state->cdclk.actual.vco = vco; - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } +static void bxt_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) +{ + cdclk_state->cdclk = bxt_calc_cdclk(max_pixclk); + cdclk_state->vco = bxt_de_pll_vco(dev_priv, cdclk_state->cdclk); +} - return 0; +static void glk_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) +{ + cdclk_state->cdclk = glk_calc_cdclk(max_pixclk); + cdclk_state->vco = glk_de_pll_vco(dev_priv, cdclk_state->cdclk); } -static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state) +int intel_modeset_calc_cdclk(struct intel_atomic_state *state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - int max_pixclk = intel_max_pixel_rate(state); - struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); - int cdclk, vco; + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_cdclk_state *logical = &state->cdclk.logical; + struct intel_cdclk_state *actual = &state->cdclk.actual; + int max_pixclk = intel_max_pixel_rate(&state->base); - if (IS_GEMINILAKE(dev_priv)) { - cdclk = glk_calc_cdclk(max_pixclk); - vco = glk_de_pll_vco(dev_priv, cdclk); - } else { - cdclk = bxt_calc_cdclk(max_pixclk); - vco = bxt_de_pll_vco(dev_priv, cdclk); - } + /* + * FIXME: should also account for plane ratio once 64bpp pixel formats + * are supported. + */ + dev_priv->display.calc_cdclk_state(dev_priv, max_pixclk, logical); - if (cdclk > dev_priv->max_cdclk_freq) { + if (logical->cdclk > dev_priv->max_cdclk_freq) { DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); + logical->cdclk, dev_priv->max_cdclk_freq); return -EINVAL; } - intel_state->cdclk.logical.vco = vco; - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - if (IS_GEMINILAKE(dev_priv)) { - cdclk = glk_calc_cdclk(0); - vco = glk_de_pll_vco(dev_priv, cdclk); - } else { - cdclk = bxt_calc_cdclk(0); - vco = bxt_de_pll_vco(dev_priv, cdclk); - } - - intel_state->cdclk.actual.vco = vco; - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } + if (!state->active_crtcs) + dev_priv->display.calc_cdclk_state(dev_priv, 0, actual); + else + *actual = *logical; return 0; } @@ -1823,24 +1744,22 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) { if (IS_CHERRYVIEW(dev_priv)) { dev_priv->display.set_cdclk = chv_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - vlv_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; } else if (IS_VALLEYVIEW(dev_priv)) { dev_priv->display.set_cdclk = vlv_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - vlv_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; } else if (IS_BROADWELL(dev_priv)) { dev_priv->display.set_cdclk = bdw_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - bdw_modeset_calc_cdclk; - } else if (IS_GEN9_LP(dev_priv)) { + dev_priv->display.calc_cdclk_state = bdw_calc_cdclk_state; + } else if (IS_BROXTON(dev_priv)) { + dev_priv->display.set_cdclk = bxt_set_cdclk; + dev_priv->display.calc_cdclk_state = bxt_calc_cdclk_state; + } else if (IS_GEMINILAKE(dev_priv)) { dev_priv->display.set_cdclk = bxt_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - bxt_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = glk_calc_cdclk_state; } else if (IS_GEN9_BC(dev_priv)) { dev_priv->display.set_cdclk = skl_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - skl_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; } if (IS_GEN9_BC(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f6feb93..1d2cb491 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12414,8 +12414,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state) * mode set on this crtc. For other crtcs we need to use the * adjusted_mode bits in the crtc directly. */ - if (dev_priv->display.modeset_calc_cdclk) { - ret = dev_priv->display.modeset_calc_cdclk(state); + if (dev_priv->display.calc_cdclk_state) { + ret = intel_modeset_calc_cdclk(intel_state); if (ret < 0) return ret; @@ -15468,7 +15468,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) pixclk = crtc_state->pixel_rate; else - WARN_ON(dev_priv->display.modeset_calc_cdclk); + WARN_ON(dev_priv->display.calc_cdclk_state); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 821c57c..3e112fe 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1260,6 +1260,7 @@ bool intel_cdclk_state_compare(const struct intel_cdclk_state *a, const struct intel_cdclk_state *b); void intel_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_state *cdclk_state); +int intel_modeset_calc_cdclk(struct intel_atomic_state *state); /* intel_display.c */ enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions 2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni @ 2017-03-02 20:28 ` Ville Syrjälä 2017-03-03 12:13 ` Ander Conselvan De Oliveira 1 sibling, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2017-03-02 20:28 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx, Rodrigo Vivi On Mon, Feb 20, 2017 at 05:00:40PM -0300, Paulo Zanoni wrote: > There's a lot of duplicated platform-independent logic in the current > modeset_calc_cdclk() functions. Adding cdclk support for more > platforms will only add more copies of this code. > > To solve this problem, in this patch we create a new function called > intel_modeset_calc_cdclk() which unifies the platform-independent > logic, and we also create a new vfunc called calc_cdclk_state(), which > is responsible to contain only the platform-dependent code. > > The code is now smaller and support for new platforms should be easier > and not require duplicated platform-independent code. > > v2: Previously I had 2 different patches addressing these problems, > but wiht Ville's suggestion I think it makes more sense to keep > everything in a single patch (Ville). > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +- > drivers/gpu/drm/i915/intel_cdclk.c | 187 ++++++++++------------------------- > drivers/gpu/drm/i915/intel_display.c | 6 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 4 files changed, 60 insertions(+), 138 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4e7046d..f1c35c7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -644,7 +644,9 @@ struct drm_i915_display_funcs { > struct intel_crtc_state *cstate); > int (*compute_global_watermarks)(struct drm_atomic_state *state); > void (*update_wm)(struct intel_crtc *crtc); > - int (*modeset_calc_cdclk)(struct drm_atomic_state *state); > + void (*calc_cdclk_state)(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state); > /* Returns the active state of the crtc, and if the crtc is active, > * fills out the pipe-config with the hw state. */ > bool (*get_pipe_config)(struct intel_crtc *, > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index d643c0c..dd6fe25 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1496,148 +1496,69 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state) > return max_pixel_rate; > } > > -static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void vlv_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - int max_pixclk = intel_max_pixel_rate(state); > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(state); > - int cdclk; > - > - cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = vlv_calc_cdclk(dev_priv, 0); > - > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > - > - return 0; > + cdclk_state->cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); > } > > -static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void bdw_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > - int max_pixclk = intel_max_pixel_rate(state); > - int cdclk; > - > - /* > - * FIXME should also account for plane ratio > - * once 64bpp pixel formats are supported. > - */ > - cdclk = bdw_calc_cdclk(max_pixclk); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = bdw_calc_cdclk(0); > - > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > - > - return 0; > + cdclk_state->cdclk = bdw_calc_cdclk(max_pixclk); > } > > -static int skl_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void skl_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - const int max_pixclk = intel_max_pixel_rate(state); > - int cdclk, vco; > + if (!cdclk_state->vco) > + cdclk_state->vco = dev_priv->skl_preferred_vco_freq; Hmm. To maintain the behaviour of the old code we would need to explicitly pluck the vco from the logicial cdclk state. Otherwise when we're computing the actual cdclk state we might choose the wrong vco. Well, I must admit I'm not 100% sure that we could get the wrong answer. I'd have to go through the entire thing again, thinking about all the cases when the vco might change. But since I'm feeling a little lazy I don't really want to do that. So I think it would be safer to keep to the current way of doing things. An alternative would be to always do the actual=logical assignment in intel_modeset_calc_cdclk() so that the actual.vco==logical.vco by the time we compute the actual cdclk state. This feels a little cleaner perhaps, but could probably use a comment to avoid someone optimizing the copy away again. > > - vco = intel_state->cdclk.logical.vco; > - if (!vco) > - vco = dev_priv->skl_preferred_vco_freq; > - > - /* > - * FIXME should also account for plane ratio > - * once 64bpp pixel formats are supported. > - */ > - cdclk = skl_calc_cdclk(max_pixclk, vco); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.vco = vco; > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = skl_calc_cdclk(0, vco); > + cdclk_state->cdclk = skl_calc_cdclk(max_pixclk, cdclk_state->vco); > +} > > - intel_state->cdclk.actual.vco = vco; > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > +static void bxt_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > +{ > + cdclk_state->cdclk = bxt_calc_cdclk(max_pixclk); > + cdclk_state->vco = bxt_de_pll_vco(dev_priv, cdclk_state->cdclk); > +} > > - return 0; > +static void glk_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > +{ > + cdclk_state->cdclk = glk_calc_cdclk(max_pixclk); > + cdclk_state->vco = glk_de_pll_vco(dev_priv, cdclk_state->cdclk); > } > > -static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state) > +int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - int max_pixclk = intel_max_pixel_rate(state); > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(state); > - int cdclk, vco; > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + struct intel_cdclk_state *logical = &state->cdclk.logical; > + struct intel_cdclk_state *actual = &state->cdclk.actual; > + int max_pixclk = intel_max_pixel_rate(&state->base); > > - if (IS_GEMINILAKE(dev_priv)) { > - cdclk = glk_calc_cdclk(max_pixclk); > - vco = glk_de_pll_vco(dev_priv, cdclk); > - } else { > - cdclk = bxt_calc_cdclk(max_pixclk); > - vco = bxt_de_pll_vco(dev_priv, cdclk); > - } > + /* > + * FIXME: should also account for plane ratio once 64bpp pixel formats > + * are supported. > + */ > + dev_priv->display.calc_cdclk_state(dev_priv, max_pixclk, logical); > > - if (cdclk > dev_priv->max_cdclk_freq) { > + if (logical->cdclk > dev_priv->max_cdclk_freq) { > DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > + logical->cdclk, dev_priv->max_cdclk_freq); > return -EINVAL; > } > > - intel_state->cdclk.logical.vco = vco; > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - if (IS_GEMINILAKE(dev_priv)) { > - cdclk = glk_calc_cdclk(0); > - vco = glk_de_pll_vco(dev_priv, cdclk); > - } else { > - cdclk = bxt_calc_cdclk(0); > - vco = bxt_de_pll_vco(dev_priv, cdclk); > - } > - > - intel_state->cdclk.actual.vco = vco; > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > + if (!state->active_crtcs) > + dev_priv->display.calc_cdclk_state(dev_priv, 0, actual); > + else > + *actual = *logical; > > return 0; > } > @@ -1823,24 +1744,22 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > { > if (IS_CHERRYVIEW(dev_priv)) { > dev_priv->display.set_cdclk = chv_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - vlv_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; > } else if (IS_VALLEYVIEW(dev_priv)) { > dev_priv->display.set_cdclk = vlv_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - vlv_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; > } else if (IS_BROADWELL(dev_priv)) { > dev_priv->display.set_cdclk = bdw_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - bdw_modeset_calc_cdclk; > - } else if (IS_GEN9_LP(dev_priv)) { > + dev_priv->display.calc_cdclk_state = bdw_calc_cdclk_state; > + } else if (IS_BROXTON(dev_priv)) { > + dev_priv->display.set_cdclk = bxt_set_cdclk; > + dev_priv->display.calc_cdclk_state = bxt_calc_cdclk_state; > + } else if (IS_GEMINILAKE(dev_priv)) { > dev_priv->display.set_cdclk = bxt_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - bxt_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = glk_calc_cdclk_state; > } else if (IS_GEN9_BC(dev_priv)) { > dev_priv->display.set_cdclk = skl_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - skl_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; > } > > if (IS_GEN9_BC(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f6feb93..1d2cb491 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12414,8 +12414,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state) > * mode set on this crtc. For other crtcs we need to use the > * adjusted_mode bits in the crtc directly. > */ > - if (dev_priv->display.modeset_calc_cdclk) { > - ret = dev_priv->display.modeset_calc_cdclk(state); > + if (dev_priv->display.calc_cdclk_state) { > + ret = intel_modeset_calc_cdclk(intel_state); > if (ret < 0) > return ret; > > @@ -15468,7 +15468,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > pixclk = crtc_state->pixel_rate; > else > - WARN_ON(dev_priv->display.modeset_calc_cdclk); > + WARN_ON(dev_priv->display.calc_cdclk_state); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 821c57c..3e112fe 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1260,6 +1260,7 @@ bool intel_cdclk_state_compare(const struct intel_cdclk_state *a, > const struct intel_cdclk_state *b); > void intel_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_state *cdclk_state); > +int intel_modeset_calc_cdclk(struct intel_atomic_state *state); > > /* intel_display.c */ > enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc); > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions 2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni 2017-03-02 20:28 ` Ville Syrjälä @ 2017-03-03 12:13 ` Ander Conselvan De Oliveira 1 sibling, 0 replies; 12+ messages in thread From: Ander Conselvan De Oliveira @ 2017-03-03 12:13 UTC (permalink / raw) To: Paulo Zanoni, intel-gfx, Pandiyan, Dhinakaran; +Cc: Rodrigo Vivi On Mon, 2017-02-20 at 17:00 -0300, Paulo Zanoni wrote: > There's a lot of duplicated platform-independent logic in the current > modeset_calc_cdclk() functions. Adding cdclk support for more > platforms will only add more copies of this code. > > To solve this problem, in this patch we create a new function called > intel_modeset_calc_cdclk() which unifies the platform-independent > logic, and we also create a new vfunc called calc_cdclk_state(), which > is responsible to contain only the platform-dependent code. > > The code is now smaller and support for new platforms should be easier > and not require duplicated platform-independent code. > > v2: Previously I had 2 different patches addressing these problems, > but wiht Ville's suggestion I think it makes more sense to keep > everything in a single patch (Ville). > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +- > drivers/gpu/drm/i915/intel_cdclk.c | 187 ++++++++++------------------------- > drivers/gpu/drm/i915/intel_display.c | 6 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 4 files changed, 60 insertions(+), 138 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4e7046d..f1c35c7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -644,7 +644,9 @@ struct drm_i915_display_funcs { > struct intel_crtc_state *cstate); > int (*compute_global_watermarks)(struct drm_atomic_state *state); > void (*update_wm)(struct intel_crtc *crtc); > - int (*modeset_calc_cdclk)(struct drm_atomic_state *state); > + void (*calc_cdclk_state)(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state); > /* Returns the active state of the crtc, and if the crtc is active, > * fills out the pipe-config with the hw state. */ > bool (*get_pipe_config)(struct intel_crtc *, > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index d643c0c..dd6fe25 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1496,148 +1496,69 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state) > return max_pixel_rate; > } > > -static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void vlv_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - int max_pixclk = intel_max_pixel_rate(state); > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(state); > - int cdclk; > - > - cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = vlv_calc_cdclk(dev_priv, 0); > - > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > - > - return 0; > + cdclk_state->cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); > } > > -static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void bdw_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > - int max_pixclk = intel_max_pixel_rate(state); > - int cdclk; > - > - /* > - * FIXME should also account for plane ratio > - * once 64bpp pixel formats are supported. > - */ > - cdclk = bdw_calc_cdclk(max_pixclk); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = bdw_calc_cdclk(0); > - > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > - > - return 0; > + cdclk_state->cdclk = bdw_calc_cdclk(max_pixclk); > } > > -static int skl_modeset_calc_cdclk(struct drm_atomic_state *state) > +static void skl_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > { > - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - const int max_pixclk = intel_max_pixel_rate(state); > - int cdclk, vco; > + if (!cdclk_state->vco) > + cdclk_state->vco = dev_priv->skl_preferred_vco_freq; > > - vco = intel_state->cdclk.logical.vco; > - if (!vco) > - vco = dev_priv->skl_preferred_vco_freq; > - > - /* > - * FIXME should also account for plane ratio > - * once 64bpp pixel formats are supported. > - */ > - cdclk = skl_calc_cdclk(max_pixclk, vco); > - > - if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - return -EINVAL; > - } > - > - intel_state->cdclk.logical.vco = vco; > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - cdclk = skl_calc_cdclk(0, vco); > + cdclk_state->cdclk = skl_calc_cdclk(max_pixclk, cdclk_state->vco); > +} > > - intel_state->cdclk.actual.vco = vco; > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > +static void bxt_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > +{ > + cdclk_state->cdclk = bxt_calc_cdclk(max_pixclk); > + cdclk_state->vco = bxt_de_pll_vco(dev_priv, cdclk_state->cdclk); > +} > > - return 0; > +static void glk_calc_cdclk_state(struct drm_i915_private *dev_priv, > + int max_pixclk, > + struct intel_cdclk_state *cdclk_state) > +{ > + cdclk_state->cdclk = glk_calc_cdclk(max_pixclk); > + cdclk_state->vco = glk_de_pll_vco(dev_priv, cdclk_state->cdclk); > } > > -static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state) > +int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > { > - struct drm_i915_private *dev_priv = to_i915(state->dev); > - int max_pixclk = intel_max_pixel_rate(state); > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(state); > - int cdclk, vco; > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + struct intel_cdclk_state *logical = &state->cdclk.logical; > + struct intel_cdclk_state *actual = &state->cdclk.actual; > + int max_pixclk = intel_max_pixel_rate(&state->base); > > - if (IS_GEMINILAKE(dev_priv)) { > - cdclk = glk_calc_cdclk(max_pixclk); > - vco = glk_de_pll_vco(dev_priv, cdclk); > - } else { > - cdclk = bxt_calc_cdclk(max_pixclk); > - vco = bxt_de_pll_vco(dev_priv, cdclk); > - } > + /* > + * FIXME: should also account for plane ratio once 64bpp pixel formats > + * are supported. > + */ > + dev_priv->display.calc_cdclk_state(dev_priv, max_pixclk, logical); I was looking at DK's patch adding minimum cdclk restrictions for glk [1] and that led me to think the code before the patch should look like cdclk = calc_cdclk() if (cdclk < min_cdclk) cdclk = min_cdclk; vco = de_pll_vco(cdclk); Now that patch will conflict with this and I think we either will have to replicate that min_cdclk check in every calc_cdclk_state() implementation or then split that into two hooks: one for the calc_cdclk() part and another for the vco part. Or something else? [1] https://patchwork.freedesktop.org/patch/141371/ Ander > > - if (cdclk > dev_priv->max_cdclk_freq) { > + if (logical->cdclk > dev_priv->max_cdclk_freq) { > DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > + logical->cdclk, dev_priv->max_cdclk_freq); > return -EINVAL; > } > > - intel_state->cdclk.logical.vco = vco; > - intel_state->cdclk.logical.cdclk = cdclk; > - > - if (!intel_state->active_crtcs) { > - if (IS_GEMINILAKE(dev_priv)) { > - cdclk = glk_calc_cdclk(0); > - vco = glk_de_pll_vco(dev_priv, cdclk); > - } else { > - cdclk = bxt_calc_cdclk(0); > - vco = bxt_de_pll_vco(dev_priv, cdclk); > - } > - > - intel_state->cdclk.actual.vco = vco; > - intel_state->cdclk.actual.cdclk = cdclk; > - } else { > - intel_state->cdclk.actual = > - intel_state->cdclk.logical; > - } > + if (!state->active_crtcs) > + dev_priv->display.calc_cdclk_state(dev_priv, 0, actual); > + else > + *actual = *logical; > > return 0; > } > @@ -1823,24 +1744,22 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > { > if (IS_CHERRYVIEW(dev_priv)) { > dev_priv->display.set_cdclk = chv_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - vlv_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; > } else if (IS_VALLEYVIEW(dev_priv)) { > dev_priv->display.set_cdclk = vlv_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - vlv_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; > } else if (IS_BROADWELL(dev_priv)) { > dev_priv->display.set_cdclk = bdw_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - bdw_modeset_calc_cdclk; > - } else if (IS_GEN9_LP(dev_priv)) { > + dev_priv->display.calc_cdclk_state = bdw_calc_cdclk_state; > + } else if (IS_BROXTON(dev_priv)) { > + dev_priv->display.set_cdclk = bxt_set_cdclk; > + dev_priv->display.calc_cdclk_state = bxt_calc_cdclk_state; > + } else if (IS_GEMINILAKE(dev_priv)) { > dev_priv->display.set_cdclk = bxt_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - bxt_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = glk_calc_cdclk_state; > } else if (IS_GEN9_BC(dev_priv)) { > dev_priv->display.set_cdclk = skl_set_cdclk; > - dev_priv->display.modeset_calc_cdclk = > - skl_modeset_calc_cdclk; > + dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; > } > > if (IS_GEN9_BC(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f6feb93..1d2cb491 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12414,8 +12414,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state) > * mode set on this crtc. For other crtcs we need to use the > * adjusted_mode bits in the crtc directly. > */ > - if (dev_priv->display.modeset_calc_cdclk) { > - ret = dev_priv->display.modeset_calc_cdclk(state); > + if (dev_priv->display.calc_cdclk_state) { > + ret = intel_modeset_calc_cdclk(intel_state); > if (ret < 0) > return ret; > > @@ -15468,7 +15468,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > pixclk = crtc_state->pixel_rate; > else > - WARN_ON(dev_priv->display.modeset_calc_cdclk); > + WARN_ON(dev_priv->display.calc_cdclk_state); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 821c57c..3e112fe 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1260,6 +1260,7 @@ bool intel_cdclk_state_compare(const struct intel_cdclk_state *a, > const struct intel_cdclk_state *b); > void intel_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_state *cdclk_state); > +int intel_modeset_calc_cdclk(struct intel_atomic_state *state); > > /* intel_display.c */ > enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks 2017-02-20 20:00 [PATCH 0/3] Small clocking code refactor, v2 Paulo Zanoni 2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni @ 2017-02-20 20:00 ` Paulo Zanoni 2017-02-21 11:45 ` Ville Syrjälä 2017-02-20 20:00 ` [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni 2017-02-20 20:52 ` ✓ Fi.CI.BAT: success for Small clocking code refactor, v2 Patchwork 3 siblings, 1 reply; 12+ messages in thread From: Paulo Zanoni @ 2017-02-20 20:00 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni The IS_G4X macro is defined as IS_G45 || IS_GM45. We have two points in our code where we have an if statement checking for GM45 followed by an else if statement checking for IS_G4X. This can be confusing since the IS_G4X check won't be catching the previously-checked GM45. Someone quickly trying to check which functions run on each platform may end up getting confused while reading the code. Fix the potential confusion by limiting the else if statements to only check for the platform that was not already checked earlier in the if ladder. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index dd6fe25..942adf0 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -223,7 +223,7 @@ static unsigned int intel_hpll_vco(struct drm_i915_private *dev_priv) /* FIXME other chipsets? */ if (IS_GM45(dev_priv)) vco_table = ctg_vco; - else if (IS_G4X(dev_priv)) + else if (IS_G45(dev_priv)) vco_table = elk_vco; else if (IS_I965GM(dev_priv)) vco_table = cl_vco; @@ -1778,7 +1778,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; else if (IS_GM45(dev_priv)) dev_priv->display.get_cdclk = gm45_get_cdclk; - else if (IS_G4X(dev_priv)) + else if (IS_G45(dev_priv)) dev_priv->display.get_cdclk = g33_get_cdclk; else if (IS_I965GM(dev_priv)) dev_priv->display.get_cdclk = i965gm_get_cdclk; -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks 2017-02-20 20:00 ` [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks Paulo Zanoni @ 2017-02-21 11:45 ` Ville Syrjälä 0 siblings, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2017-02-21 11:45 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx On Mon, Feb 20, 2017 at 05:00:41PM -0300, Paulo Zanoni wrote: > The IS_G4X macro is defined as IS_G45 || IS_GM45. We have two points > in our code where we have an if statement checking for GM45 followed > by an else if statement checking for IS_G4X. This can be confusing > since the IS_G4X check won't be catching the previously-checked GM45. > Someone quickly trying to check which functions run on each platform > may end up getting confused while reading the code. > > Fix the potential confusion by limiting the else if statements to only > check for the platform that was not already checked earlier in the if > ladder. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index dd6fe25..942adf0 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -223,7 +223,7 @@ static unsigned int intel_hpll_vco(struct drm_i915_private *dev_priv) > /* FIXME other chipsets? */ > if (IS_GM45(dev_priv)) > vco_table = ctg_vco; > - else if (IS_G4X(dev_priv)) > + else if (IS_G45(dev_priv)) > vco_table = elk_vco; > else if (IS_I965GM(dev_priv)) > vco_table = cl_vco; > @@ -1778,7 +1778,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; > else if (IS_GM45(dev_priv)) > dev_priv->display.get_cdclk = gm45_get_cdclk; > - else if (IS_G4X(dev_priv)) > + else if (IS_G45(dev_priv)) > dev_priv->display.get_cdclk = g33_get_cdclk; > else if (IS_I965GM(dev_priv)) > dev_priv->display.get_cdclk = i965gm_get_cdclk; > -- > 2.7.4 > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment 2017-02-20 20:00 [PATCH 0/3] Small clocking code refactor, v2 Paulo Zanoni 2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni 2017-02-20 20:00 ` [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks Paulo Zanoni @ 2017-02-20 20:00 ` Paulo Zanoni 2017-02-21 11:51 ` Ville Syrjälä 2017-02-21 12:26 ` Ander Conselvan De Oliveira 2017-02-20 20:52 ` ✓ Fi.CI.BAT: success for Small clocking code refactor, v2 Patchwork 3 siblings, 2 replies; 12+ messages in thread From: Paulo Zanoni @ 2017-02-20 20:00 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni Possible problems of the current if-ladder: - It's a huge if ladder with almost a different check for each of our platforms. - It mixes 3 different types of checks: IS_GENX, IS_PLATFORM and IS_GROUP_OF_PLATFORMS. - As demonstrated by the recent IS_G4X commit, it's not easy to be sure if a platform down on the list isn't also checked earlier. - As demonstrated by the WARN at the end, it's not easy to be sure if we're actually checking for every single platform. Possible advantages of the new switch statement: - It may be easier for the compiler to optimize stuff (I didn't check this), especially since the values are labels of an enum. - The compiler will tell us in case we miss some platform. - All platforms are explicitly there instead of maybe hidden in some check for a certain group of platforms such as IS_GEN9_BC. Possible disadvantages with the new code: - A few lines bigger. v2: Don't unsort the list. Now the list almost matches the enum definition, with the exception of CHV, KBL and GLK, which are listed along their predecessors (Ville). Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index 942adf0..e1921fe7 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1762,49 +1762,76 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; } - if (IS_GEN9_BC(dev_priv)) - dev_priv->display.get_cdclk = skl_get_cdclk; - else if (IS_GEN9_LP(dev_priv)) - dev_priv->display.get_cdclk = bxt_get_cdclk; - else if (IS_BROADWELL(dev_priv)) - dev_priv->display.get_cdclk = bdw_get_cdclk; - else if (IS_HASWELL(dev_priv)) - dev_priv->display.get_cdclk = hsw_get_cdclk; - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) - dev_priv->display.get_cdclk = vlv_get_cdclk; - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) + switch (INTEL_INFO(dev_priv)->platform) { + case INTEL_PLATFORM_UNINITIALIZED: + MISSING_CASE(INTEL_INFO(dev_priv)->platform); + /* Fall through. */ + case INTEL_I830: + dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; + break; + case INTEL_I845G: + dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; + break; + case INTEL_I85X: + dev_priv->display.get_cdclk = i85x_get_cdclk; + break; + case INTEL_I865G: + dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; + break; + case INTEL_I915G: + dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; + break; + case INTEL_I915GM: + dev_priv->display.get_cdclk = i915gm_get_cdclk; + break; + case INTEL_I945G: dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; - else if (IS_GEN5(dev_priv)) - dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; - else if (IS_GM45(dev_priv)) - dev_priv->display.get_cdclk = gm45_get_cdclk; - else if (IS_G45(dev_priv)) + break; + case INTEL_I945GM: + dev_priv->display.get_cdclk = i945gm_get_cdclk; + break; + case INTEL_G33: dev_priv->display.get_cdclk = g33_get_cdclk; - else if (IS_I965GM(dev_priv)) - dev_priv->display.get_cdclk = i965gm_get_cdclk; - else if (IS_I965G(dev_priv)) - dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; - else if (IS_PINEVIEW(dev_priv)) + break; + case INTEL_PINEVIEW: dev_priv->display.get_cdclk = pnv_get_cdclk; - else if (IS_G33(dev_priv)) + break; + case INTEL_I965G: + dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; + break; + case INTEL_I965GM: + dev_priv->display.get_cdclk = i965gm_get_cdclk; + break; + case INTEL_G45: dev_priv->display.get_cdclk = g33_get_cdclk; - else if (IS_I945GM(dev_priv)) - dev_priv->display.get_cdclk = i945gm_get_cdclk; - else if (IS_I945G(dev_priv)) + break; + case INTEL_GM45: + dev_priv->display.get_cdclk = gm45_get_cdclk; + break; + case INTEL_IRONLAKE: + dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; + break; + case INTEL_SANDYBRIDGE: + case INTEL_IVYBRIDGE: dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; - else if (IS_I915GM(dev_priv)) - dev_priv->display.get_cdclk = i915gm_get_cdclk; - else if (IS_I915G(dev_priv)) - dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; - else if (IS_I865G(dev_priv)) - dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; - else if (IS_I85X(dev_priv)) - dev_priv->display.get_cdclk = i85x_get_cdclk; - else if (IS_I845G(dev_priv)) - dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; - else { /* 830 */ - WARN(!IS_I830(dev_priv), - "Unknown platform. Assuming 133 MHz CDCLK\n"); - dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; + break; + case INTEL_VALLEYVIEW: + case INTEL_CHERRYVIEW: + dev_priv->display.get_cdclk = vlv_get_cdclk; + break; + case INTEL_HASWELL: + dev_priv->display.get_cdclk = hsw_get_cdclk; + break; + case INTEL_BROADWELL: + dev_priv->display.get_cdclk = bdw_get_cdclk; + break; + case INTEL_SKYLAKE: + case INTEL_KABYLAKE: + dev_priv->display.get_cdclk = skl_get_cdclk; + break; + case INTEL_BROXTON: + case INTEL_GEMINILAKE: + dev_priv->display.get_cdclk = bxt_get_cdclk; + break; } } -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment 2017-02-20 20:00 ` [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni @ 2017-02-21 11:51 ` Ville Syrjälä 2017-02-21 19:45 ` Paulo Zanoni 2017-02-21 12:26 ` Ander Conselvan De Oliveira 1 sibling, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2017-02-21 11:51 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx On Mon, Feb 20, 2017 at 05:00:42PM -0300, Paulo Zanoni wrote: > Possible problems of the current if-ladder: > - It's a huge if ladder with almost a different check for each of > our platforms. > - It mixes 3 different types of checks: IS_GENX, IS_PLATFORM and > IS_GROUP_OF_PLATFORMS. > - As demonstrated by the recent IS_G4X commit, it's not easy to be > sure if a platform down on the list isn't also checked earlier. > - As demonstrated by the WARN at the end, it's not easy to be sure > if we're actually checking for every single platform. > > Possible advantages of the new switch statement: > - It may be easier for the compiler to optimize stuff (I didn't > check this), especially since the values are labels of an enum. > - The compiler will tell us in case we miss some platform. > - All platforms are explicitly there instead of maybe hidden in some > check for a certain group of platforms such as IS_GEN9_BC. > > Possible disadvantages with the new code: > - A few lines bigger. > > v2: Don't unsort the list. Now the list almost matches the enum > definition, with the exception of CHV, KBL and GLK, which are listed > along their predecessors (Ville). > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++-------------- > 1 file changed, 66 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 942adf0..e1921fe7 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1762,49 +1762,76 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; > } > > - if (IS_GEN9_BC(dev_priv)) > - dev_priv->display.get_cdclk = skl_get_cdclk; > - else if (IS_GEN9_LP(dev_priv)) > - dev_priv->display.get_cdclk = bxt_get_cdclk; > - else if (IS_BROADWELL(dev_priv)) > - dev_priv->display.get_cdclk = bdw_get_cdclk; > - else if (IS_HASWELL(dev_priv)) > - dev_priv->display.get_cdclk = hsw_get_cdclk; > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - dev_priv->display.get_cdclk = vlv_get_cdclk; > - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) > + switch (INTEL_INFO(dev_priv)->platform) { > + case INTEL_PLATFORM_UNINITIALIZED: > + MISSING_CASE(INTEL_INFO(dev_priv)->platform); > + /* Fall through. */ > + case INTEL_I830: The diff might come out nicer if you didn't reverse the list. Right now it's quite hard to read. > + dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; > + break; > + case INTEL_I845G: > + dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; > + break; > + case INTEL_I85X: > + dev_priv->display.get_cdclk = i85x_get_cdclk; > + break; > + case INTEL_I865G: > + dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; > + break; > + case INTEL_I915G: > + dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; > + break; > + case INTEL_I915GM: > + dev_priv->display.get_cdclk = i915gm_get_cdclk; > + break; > + case INTEL_I945G: > dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_GEN5(dev_priv)) > - dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; > - else if (IS_GM45(dev_priv)) > - dev_priv->display.get_cdclk = gm45_get_cdclk; > - else if (IS_G45(dev_priv)) > + break; > + case INTEL_I945GM: > + dev_priv->display.get_cdclk = i945gm_get_cdclk; > + break; > + case INTEL_G33: > dev_priv->display.get_cdclk = g33_get_cdclk; > - else if (IS_I965GM(dev_priv)) > - dev_priv->display.get_cdclk = i965gm_get_cdclk; > - else if (IS_I965G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_PINEVIEW(dev_priv)) > + break; > + case INTEL_PINEVIEW: > dev_priv->display.get_cdclk = pnv_get_cdclk; > - else if (IS_G33(dev_priv)) > + break; > + case INTEL_I965G: > + dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > + break; > + case INTEL_I965GM: > + dev_priv->display.get_cdclk = i965gm_get_cdclk; > + break; > + case INTEL_G45: > dev_priv->display.get_cdclk = g33_get_cdclk; > - else if (IS_I945GM(dev_priv)) > - dev_priv->display.get_cdclk = i945gm_get_cdclk; > - else if (IS_I945G(dev_priv)) > + break; > + case INTEL_GM45: > + dev_priv->display.get_cdclk = gm45_get_cdclk; > + break; > + case INTEL_IRONLAKE: > + dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; > + break; > + case INTEL_SANDYBRIDGE: > + case INTEL_IVYBRIDGE: > dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_I915GM(dev_priv)) > - dev_priv->display.get_cdclk = i915gm_get_cdclk; > - else if (IS_I915G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; > - else if (IS_I865G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; > - else if (IS_I85X(dev_priv)) > - dev_priv->display.get_cdclk = i85x_get_cdclk; > - else if (IS_I845G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; > - else { /* 830 */ > - WARN(!IS_I830(dev_priv), > - "Unknown platform. Assuming 133 MHz CDCLK\n"); > - dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; > + break; > + case INTEL_VALLEYVIEW: > + case INTEL_CHERRYVIEW: > + dev_priv->display.get_cdclk = vlv_get_cdclk; > + break; > + case INTEL_HASWELL: > + dev_priv->display.get_cdclk = hsw_get_cdclk; > + break; > + case INTEL_BROADWELL: > + dev_priv->display.get_cdclk = bdw_get_cdclk; > + break; > + case INTEL_SKYLAKE: > + case INTEL_KABYLAKE: > + dev_priv->display.get_cdclk = skl_get_cdclk; > + break; > + case INTEL_BROXTON: > + case INTEL_GEMINILAKE: > + dev_priv->display.get_cdclk = bxt_get_cdclk; > + break; > } > } > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment 2017-02-21 11:51 ` Ville Syrjälä @ 2017-02-21 19:45 ` Paulo Zanoni 0 siblings, 0 replies; 12+ messages in thread From: Paulo Zanoni @ 2017-02-21 19:45 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx Em Ter, 2017-02-21 às 13:51 +0200, Ville Syrjälä escreveu: > On Mon, Feb 20, 2017 at 05:00:42PM -0300, Paulo Zanoni wrote: > > > > Possible problems of the current if-ladder: > > - It's a huge if ladder with almost a different check for each of > > our platforms. > > - It mixes 3 different types of checks: IS_GENX, IS_PLATFORM and > > IS_GROUP_OF_PLATFORMS. > > - As demonstrated by the recent IS_G4X commit, it's not easy to > > be > > sure if a platform down on the list isn't also checked earlier. > > - As demonstrated by the WARN at the end, it's not easy to be > > sure > > if we're actually checking for every single platform. > > > > Possible advantages of the new switch statement: > > - It may be easier for the compiler to optimize stuff (I didn't > > check this), especially since the values are labels of an enum. > > - The compiler will tell us in case we miss some platform. > > - All platforms are explicitly there instead of maybe hidden in > > some > > check for a certain group of platforms such as IS_GEN9_BC. > > > > Possible disadvantages with the new code: > > - A few lines bigger. > > > > v2: Don't unsort the list. Now the list almost matches the enum > > definition, with the exception of CHV, KBL and GLK, which are > > listed > > along their predecessors (Ville). > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++ > > -------------- > > 1 file changed, 66 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > b/drivers/gpu/drm/i915/intel_cdclk.c > > index 942adf0..e1921fe7 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1762,49 +1762,76 @@ void intel_init_cdclk_hooks(struct > > drm_i915_private *dev_priv) > > dev_priv->display.calc_cdclk_state = > > skl_calc_cdclk_state; > > } > > > > - if (IS_GEN9_BC(dev_priv)) > > - dev_priv->display.get_cdclk = skl_get_cdclk; > > - else if (IS_GEN9_LP(dev_priv)) > > - dev_priv->display.get_cdclk = bxt_get_cdclk; > > - else if (IS_BROADWELL(dev_priv)) > > - dev_priv->display.get_cdclk = bdw_get_cdclk; > > - else if (IS_HASWELL(dev_priv)) > > - dev_priv->display.get_cdclk = hsw_get_cdclk; > > - else if (IS_VALLEYVIEW(dev_priv) || > > IS_CHERRYVIEW(dev_priv)) > > - dev_priv->display.get_cdclk = vlv_get_cdclk; > > - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) > > + switch (INTEL_INFO(dev_priv)->platform) { > > + case INTEL_PLATFORM_UNINITIALIZED: > > + MISSING_CASE(INTEL_INFO(dev_priv)->platform); > > + /* Fall through. */ > > + case INTEL_I830: > > The diff might come out nicer if you didn't reverse the list. Right > now > it's quite hard to read. On the other hand, the labels in the switch statement match the enum definition (with the exception of CHV/KBL/GLK). So IMHO this is an "easier to review the patch now vs easier to review the code later" problem. Still, I can change this, it won't hurt. > > > > > + dev_priv->display.get_cdclk = > > fixed_133mhz_get_cdclk; > > + break; > > + case INTEL_I845G: > > + dev_priv->display.get_cdclk = > > fixed_200mhz_get_cdclk; > > + break; > > + case INTEL_I85X: > > + dev_priv->display.get_cdclk = i85x_get_cdclk; > > + break; > > + case INTEL_I865G: > > + dev_priv->display.get_cdclk = > > fixed_266mhz_get_cdclk; > > + break; > > + case INTEL_I915G: > > + dev_priv->display.get_cdclk = > > fixed_333mhz_get_cdclk; > > + break; > > + case INTEL_I915GM: > > + dev_priv->display.get_cdclk = i915gm_get_cdclk; > > + break; > > + case INTEL_I945G: > > dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_GEN5(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_450mhz_get_cdclk; > > - else if (IS_GM45(dev_priv)) > > - dev_priv->display.get_cdclk = gm45_get_cdclk; > > - else if (IS_G45(dev_priv)) > > + break; > > + case INTEL_I945GM: > > + dev_priv->display.get_cdclk = i945gm_get_cdclk; > > + break; > > + case INTEL_G33: > > dev_priv->display.get_cdclk = g33_get_cdclk; > > - else if (IS_I965GM(dev_priv)) > > - dev_priv->display.get_cdclk = i965gm_get_cdclk; > > - else if (IS_I965G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_PINEVIEW(dev_priv)) > > + break; > > + case INTEL_PINEVIEW: > > dev_priv->display.get_cdclk = pnv_get_cdclk; > > - else if (IS_G33(dev_priv)) > > + break; > > + case INTEL_I965G: > > + dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > + break; > > + case INTEL_I965GM: > > + dev_priv->display.get_cdclk = i965gm_get_cdclk; > > + break; > > + case INTEL_G45: > > dev_priv->display.get_cdclk = g33_get_cdclk; > > - else if (IS_I945GM(dev_priv)) > > - dev_priv->display.get_cdclk = i945gm_get_cdclk; > > - else if (IS_I945G(dev_priv)) > > + break; > > + case INTEL_GM45: > > + dev_priv->display.get_cdclk = gm45_get_cdclk; > > + break; > > + case INTEL_IRONLAKE: > > + dev_priv->display.get_cdclk = > > fixed_450mhz_get_cdclk; > > + break; > > + case INTEL_SANDYBRIDGE: > > + case INTEL_IVYBRIDGE: > > dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_I915GM(dev_priv)) > > - dev_priv->display.get_cdclk = i915gm_get_cdclk; > > - else if (IS_I915G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_333mhz_get_cdclk; > > - else if (IS_I865G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_266mhz_get_cdclk; > > - else if (IS_I85X(dev_priv)) > > - dev_priv->display.get_cdclk = i85x_get_cdclk; > > - else if (IS_I845G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_200mhz_get_cdclk; > > - else { /* 830 */ > > - WARN(!IS_I830(dev_priv), > > - "Unknown platform. Assuming 133 MHz > > CDCLK\n"); > > - dev_priv->display.get_cdclk = > > fixed_133mhz_get_cdclk; > > + break; > > + case INTEL_VALLEYVIEW: > > + case INTEL_CHERRYVIEW: > > + dev_priv->display.get_cdclk = vlv_get_cdclk; > > + break; > > + case INTEL_HASWELL: > > + dev_priv->display.get_cdclk = hsw_get_cdclk; > > + break; > > + case INTEL_BROADWELL: > > + dev_priv->display.get_cdclk = bdw_get_cdclk; > > + break; > > + case INTEL_SKYLAKE: > > + case INTEL_KABYLAKE: > > + dev_priv->display.get_cdclk = skl_get_cdclk; > > + break; > > + case INTEL_BROXTON: > > + case INTEL_GEMINILAKE: > > + dev_priv->display.get_cdclk = bxt_get_cdclk; > > + break; > > } > > } > > -- > > 2.7.4 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment 2017-02-20 20:00 ` [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni 2017-02-21 11:51 ` Ville Syrjälä @ 2017-02-21 12:26 ` Ander Conselvan De Oliveira 2017-02-21 19:56 ` Paulo Zanoni 1 sibling, 1 reply; 12+ messages in thread From: Ander Conselvan De Oliveira @ 2017-02-21 12:26 UTC (permalink / raw) To: Paulo Zanoni, intel-gfx On Mon, 2017-02-20 at 17:00 -0300, Paulo Zanoni wrote: > Possible problems of the current if-ladder: > - It's a huge if ladder with almost a different check for each of > our platforms. > - It mixes 3 different types of checks: IS_GENX, IS_PLATFORM and > IS_GROUP_OF_PLATFORMS. > - As demonstrated by the recent IS_G4X commit, it's not easy to be > sure if a platform down on the list isn't also checked earlier. > - As demonstrated by the WARN at the end, it's not easy to be sure > if we're actually checking for every single platform. > > Possible advantages of the new switch statement: > - It may be easier for the compiler to optimize stuff (I didn't > check this), especially since the values are labels of an enum. > - The compiler will tell us in case we miss some platform. > - All platforms are explicitly there instead of maybe hidden in some > check for a certain group of platforms such as IS_GEN9_BC. > > Possible disadvantages with the new code: > - A few lines bigger. I had suggested something different at some point, but didn't get around to implementing it yet: https://lists.freedesktop.org/archives/intel-gfx/2016-December/115078.html I think that if we need almost one case for each single platform, might as well put these in device info. Anyway, I didn't check how well that proposal works with the changes in this series, but figured I mention it anyway in case it helps. Ander > > v2: Don't unsort the list. Now the list almost matches the enum > definition, with the exception of CHV, KBL and GLK, which are listed > along their predecessors (Ville). > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++-------------- > 1 file changed, 66 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 942adf0..e1921fe7 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1762,49 +1762,76 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; > } > > - if (IS_GEN9_BC(dev_priv)) > - dev_priv->display.get_cdclk = skl_get_cdclk; > - else if (IS_GEN9_LP(dev_priv)) > - dev_priv->display.get_cdclk = bxt_get_cdclk; > - else if (IS_BROADWELL(dev_priv)) > - dev_priv->display.get_cdclk = bdw_get_cdclk; > - else if (IS_HASWELL(dev_priv)) > - dev_priv->display.get_cdclk = hsw_get_cdclk; > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - dev_priv->display.get_cdclk = vlv_get_cdclk; > - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) > + switch (INTEL_INFO(dev_priv)->platform) { > + case INTEL_PLATFORM_UNINITIALIZED: > + MISSING_CASE(INTEL_INFO(dev_priv)->platform); > + /* Fall through. */ > + case INTEL_I830: > + dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; > + break; > + case INTEL_I845G: > + dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; > + break; > + case INTEL_I85X: > + dev_priv->display.get_cdclk = i85x_get_cdclk; > + break; > + case INTEL_I865G: > + dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; > + break; > + case INTEL_I915G: > + dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; > + break; > + case INTEL_I915GM: > + dev_priv->display.get_cdclk = i915gm_get_cdclk; > + break; > + case INTEL_I945G: > dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_GEN5(dev_priv)) > - dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; > - else if (IS_GM45(dev_priv)) > - dev_priv->display.get_cdclk = gm45_get_cdclk; > - else if (IS_G45(dev_priv)) > + break; > + case INTEL_I945GM: > + dev_priv->display.get_cdclk = i945gm_get_cdclk; > + break; > + case INTEL_G33: > dev_priv->display.get_cdclk = g33_get_cdclk; > - else if (IS_I965GM(dev_priv)) > - dev_priv->display.get_cdclk = i965gm_get_cdclk; > - else if (IS_I965G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_PINEVIEW(dev_priv)) > + break; > + case INTEL_PINEVIEW: > dev_priv->display.get_cdclk = pnv_get_cdclk; > - else if (IS_G33(dev_priv)) > + break; > + case INTEL_I965G: > + dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > + break; > + case INTEL_I965GM: > + dev_priv->display.get_cdclk = i965gm_get_cdclk; > + break; > + case INTEL_G45: > dev_priv->display.get_cdclk = g33_get_cdclk; > - else if (IS_I945GM(dev_priv)) > - dev_priv->display.get_cdclk = i945gm_get_cdclk; > - else if (IS_I945G(dev_priv)) > + break; > + case INTEL_GM45: > + dev_priv->display.get_cdclk = gm45_get_cdclk; > + break; > + case INTEL_IRONLAKE: > + dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk; > + break; > + case INTEL_SANDYBRIDGE: > + case INTEL_IVYBRIDGE: > dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk; > - else if (IS_I915GM(dev_priv)) > - dev_priv->display.get_cdclk = i915gm_get_cdclk; > - else if (IS_I915G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk; > - else if (IS_I865G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk; > - else if (IS_I85X(dev_priv)) > - dev_priv->display.get_cdclk = i85x_get_cdclk; > - else if (IS_I845G(dev_priv)) > - dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk; > - else { /* 830 */ > - WARN(!IS_I830(dev_priv), > - "Unknown platform. Assuming 133 MHz CDCLK\n"); > - dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk; > + break; > + case INTEL_VALLEYVIEW: > + case INTEL_CHERRYVIEW: > + dev_priv->display.get_cdclk = vlv_get_cdclk; > + break; > + case INTEL_HASWELL: > + dev_priv->display.get_cdclk = hsw_get_cdclk; > + break; > + case INTEL_BROADWELL: > + dev_priv->display.get_cdclk = bdw_get_cdclk; > + break; > + case INTEL_SKYLAKE: > + case INTEL_KABYLAKE: > + dev_priv->display.get_cdclk = skl_get_cdclk; > + break; > + case INTEL_BROXTON: > + case INTEL_GEMINILAKE: > + dev_priv->display.get_cdclk = bxt_get_cdclk; > + break; > } > } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment 2017-02-21 12:26 ` Ander Conselvan De Oliveira @ 2017-02-21 19:56 ` Paulo Zanoni 0 siblings, 0 replies; 12+ messages in thread From: Paulo Zanoni @ 2017-02-21 19:56 UTC (permalink / raw) To: Ander Conselvan De Oliveira, intel-gfx Em Ter, 2017-02-21 às 14:26 +0200, Ander Conselvan De Oliveira escreveu: > On Mon, 2017-02-20 at 17:00 -0300, Paulo Zanoni wrote: > > > > Possible problems of the current if-ladder: > > - It's a huge if ladder with almost a different check for each of > > our platforms. > > - It mixes 3 different types of checks: IS_GENX, IS_PLATFORM and > > IS_GROUP_OF_PLATFORMS. > > - As demonstrated by the recent IS_G4X commit, it's not easy to > > be > > sure if a platform down on the list isn't also checked earlier. > > - As demonstrated by the WARN at the end, it's not easy to be > > sure > > if we're actually checking for every single platform. > > > > Possible advantages of the new switch statement: > > - It may be easier for the compiler to optimize stuff (I didn't > > check this), especially since the values are labels of an enum. > > - The compiler will tell us in case we miss some platform. > > - All platforms are explicitly there instead of maybe hidden in > > some > > check for a certain group of platforms such as IS_GEN9_BC. > > > > Possible disadvantages with the new code: > > - A few lines bigger. > > I had suggested something different at some point, but didn't get > around to > implementing it yet: > > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115078 > .html > > I think that if we need almost one case for each single platform, > might as well > put these in device info. Anyway, I didn't check how well that > proposal works > with the changes in this series, but figured I mention it anyway in > case it > helps. That approach would still work. I can drop this patch if you're still planning to implement that, no problem. The only patch from this series that I really want to upstream is patch 1. The value of this specific 3/3 patch was greatly decreased after Ville sorted the platform if ladder, but I decided to send it anyway since I already had it (and since I wanted to test people's reactions to using the platform enum in a switch statement). > > Ander > > > > > > > > v2: Don't unsort the list. Now the list almost matches the enum > > definition, with the exception of CHV, KBL and GLK, which are > > listed > > along their predecessors (Ville). > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++ > > -------------- > > 1 file changed, 66 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > b/drivers/gpu/drm/i915/intel_cdclk.c > > index 942adf0..e1921fe7 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1762,49 +1762,76 @@ void intel_init_cdclk_hooks(struct > > drm_i915_private *dev_priv) > > dev_priv->display.calc_cdclk_state = > > skl_calc_cdclk_state; > > } > > > > - if (IS_GEN9_BC(dev_priv)) > > - dev_priv->display.get_cdclk = skl_get_cdclk; > > - else if (IS_GEN9_LP(dev_priv)) > > - dev_priv->display.get_cdclk = bxt_get_cdclk; > > - else if (IS_BROADWELL(dev_priv)) > > - dev_priv->display.get_cdclk = bdw_get_cdclk; > > - else if (IS_HASWELL(dev_priv)) > > - dev_priv->display.get_cdclk = hsw_get_cdclk; > > - else if (IS_VALLEYVIEW(dev_priv) || > > IS_CHERRYVIEW(dev_priv)) > > - dev_priv->display.get_cdclk = vlv_get_cdclk; > > - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) > > + switch (INTEL_INFO(dev_priv)->platform) { > > + case INTEL_PLATFORM_UNINITIALIZED: > > + MISSING_CASE(INTEL_INFO(dev_priv)->platform); > > + /* Fall through. */ > > + case INTEL_I830: > > + dev_priv->display.get_cdclk = > > fixed_133mhz_get_cdclk; > > + break; > > + case INTEL_I845G: > > + dev_priv->display.get_cdclk = > > fixed_200mhz_get_cdclk; > > + break; > > + case INTEL_I85X: > > + dev_priv->display.get_cdclk = i85x_get_cdclk; > > + break; > > + case INTEL_I865G: > > + dev_priv->display.get_cdclk = > > fixed_266mhz_get_cdclk; > > + break; > > + case INTEL_I915G: > > + dev_priv->display.get_cdclk = > > fixed_333mhz_get_cdclk; > > + break; > > + case INTEL_I915GM: > > + dev_priv->display.get_cdclk = i915gm_get_cdclk; > > + break; > > + case INTEL_I945G: > > dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_GEN5(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_450mhz_get_cdclk; > > - else if (IS_GM45(dev_priv)) > > - dev_priv->display.get_cdclk = gm45_get_cdclk; > > - else if (IS_G45(dev_priv)) > > + break; > > + case INTEL_I945GM: > > + dev_priv->display.get_cdclk = i945gm_get_cdclk; > > + break; > > + case INTEL_G33: > > dev_priv->display.get_cdclk = g33_get_cdclk; > > - else if (IS_I965GM(dev_priv)) > > - dev_priv->display.get_cdclk = i965gm_get_cdclk; > > - else if (IS_I965G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_PINEVIEW(dev_priv)) > > + break; > > + case INTEL_PINEVIEW: > > dev_priv->display.get_cdclk = pnv_get_cdclk; > > - else if (IS_G33(dev_priv)) > > + break; > > + case INTEL_I965G: > > + dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > + break; > > + case INTEL_I965GM: > > + dev_priv->display.get_cdclk = i965gm_get_cdclk; > > + break; > > + case INTEL_G45: > > dev_priv->display.get_cdclk = g33_get_cdclk; > > - else if (IS_I945GM(dev_priv)) > > - dev_priv->display.get_cdclk = i945gm_get_cdclk; > > - else if (IS_I945G(dev_priv)) > > + break; > > + case INTEL_GM45: > > + dev_priv->display.get_cdclk = gm45_get_cdclk; > > + break; > > + case INTEL_IRONLAKE: > > + dev_priv->display.get_cdclk = > > fixed_450mhz_get_cdclk; > > + break; > > + case INTEL_SANDYBRIDGE: > > + case INTEL_IVYBRIDGE: > > dev_priv->display.get_cdclk = > > fixed_400mhz_get_cdclk; > > - else if (IS_I915GM(dev_priv)) > > - dev_priv->display.get_cdclk = i915gm_get_cdclk; > > - else if (IS_I915G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_333mhz_get_cdclk; > > - else if (IS_I865G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_266mhz_get_cdclk; > > - else if (IS_I85X(dev_priv)) > > - dev_priv->display.get_cdclk = i85x_get_cdclk; > > - else if (IS_I845G(dev_priv)) > > - dev_priv->display.get_cdclk = > > fixed_200mhz_get_cdclk; > > - else { /* 830 */ > > - WARN(!IS_I830(dev_priv), > > - "Unknown platform. Assuming 133 MHz > > CDCLK\n"); > > - dev_priv->display.get_cdclk = > > fixed_133mhz_get_cdclk; > > + break; > > + case INTEL_VALLEYVIEW: > > + case INTEL_CHERRYVIEW: > > + dev_priv->display.get_cdclk = vlv_get_cdclk; > > + break; > > + case INTEL_HASWELL: > > + dev_priv->display.get_cdclk = hsw_get_cdclk; > > + break; > > + case INTEL_BROADWELL: > > + dev_priv->display.get_cdclk = bdw_get_cdclk; > > + break; > > + case INTEL_SKYLAKE: > > + case INTEL_KABYLAKE: > > + dev_priv->display.get_cdclk = skl_get_cdclk; > > + break; > > + case INTEL_BROXTON: > > + case INTEL_GEMINILAKE: > > + dev_priv->display.get_cdclk = bxt_get_cdclk; > > + break; > > } > > } > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for Small clocking code refactor, v2. 2017-02-20 20:00 [PATCH 0/3] Small clocking code refactor, v2 Paulo Zanoni ` (2 preceding siblings ...) 2017-02-20 20:00 ` [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni @ 2017-02-20 20:52 ` Patchwork 3 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2017-02-20 20:52 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx == Series Details == Series: Small clocking code refactor, v2. URL : https://patchwork.freedesktop.org/series/19974/ State : success == Summary == Series 19974v1 Small clocking code refactor, v2. https://patchwork.freedesktop.org/api/1.0/series/19974/revisions/1/mbox/ fi-bdw-5557u total:253 pass:242 dwarn:0 dfail:0 fail:0 skip:11 fi-bsw-n3050 total:253 pass:214 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:253 pass:234 dwarn:0 dfail:0 fail:0 skip:19 fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:253 pass:226 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:253 pass:222 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:253 pass:237 dwarn:0 dfail:0 fail:0 skip:16 fi-hsw-4770r total:253 pass:237 dwarn:0 dfail:0 fail:0 skip:16 fi-ilk-650 total:253 pass:203 dwarn:0 dfail:0 fail:0 skip:50 fi-ivb-3520m total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18 fi-ivb-3770 total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18 fi-kbl-7500u total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18 fi-skl-6260u total:253 pass:243 dwarn:0 dfail:0 fail:0 skip:10 fi-skl-6700hq total:253 pass:236 dwarn:0 dfail:0 fail:0 skip:17 fi-skl-6700k total:253 pass:231 dwarn:4 dfail:0 fail:0 skip:18 fi-skl-6770hq total:253 pass:243 dwarn:0 dfail:0 fail:0 skip:10 fi-snb-2520m total:253 pass:225 dwarn:0 dfail:0 fail:0 skip:28 fi-snb-2600 total:253 pass:224 dwarn:0 dfail:0 fail:0 skip:29 0a03ea9496b49746b4964d05cc1f483385d1df8a drm-tip: 2017y-02m-20d-17h-19m-56s UTC integration manifest 172baa0 drm/i915: reorganize the get_cdclk assignment 25d43f8 drm/i915: remove potentially confusing IS_G4X checks 4f12bd0 drm/i915: unify the x_modeset_calc_cdclk() functions == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3904/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-03 12:13 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-20 20:00 [PATCH 0/3] Small clocking code refactor, v2 Paulo Zanoni 2017-02-20 20:00 ` [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions Paulo Zanoni 2017-03-02 20:28 ` Ville Syrjälä 2017-03-03 12:13 ` Ander Conselvan De Oliveira 2017-02-20 20:00 ` [PATCH 2/3] drm/i915: remove potentially confusing IS_G4X checks Paulo Zanoni 2017-02-21 11:45 ` Ville Syrjälä 2017-02-20 20:00 ` [PATCH 3/3] drm/i915: reorganize the get_cdclk assignment Paulo Zanoni 2017-02-21 11:51 ` Ville Syrjälä 2017-02-21 19:45 ` Paulo Zanoni 2017-02-21 12:26 ` Ander Conselvan De Oliveira 2017-02-21 19:56 ` Paulo Zanoni 2017-02-20 20:52 ` ✓ Fi.CI.BAT: success for Small clocking code refactor, v2 Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).