On Wed, Oct 04, 2017 at 06:39:19AM +0000, Mika Kahola wrote:
> On Tue, 2017-10-03 at 00:06 -0700, Rodrigo Vivi wrote:
> > On Cannonlake the DVFS level selection depends on the
> > port clock.
> >
> > So let's re-org in a way that we can easily export without
> > duplicating any code.
> >
> > v2: Rebased on changes on previous patches
> >
> > Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > 2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 92eabb6cc1ab..ee64b1a50453 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1297,7 +1297,7 @@ static void cnl_ddi_clock_get(struct
> > intel_encoder *encoder,
> >
> > pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config-
> > >shared_dpll);
> >
> > - pipe_config->port_clock = cnl_calc_pll_link(dev_priv,
> > pll_id);
> > + pipe_config->port_clock = intel_ddi_port_clock(dev_priv,
> > pll_id);
> >
> > ddi_dotclock_get(pipe_config);
> > }
> > @@ -1353,7 +1353,7 @@ static void skl_ddi_clock_get(struct
> > intel_encoder *encoder,
> >
> > pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config-
> > >shared_dpll);
> >
> > - pipe_config->port_clock = skl_calc_pll_link(dev_priv,
> > pll_id);
> > + pipe_config->port_clock = intel_ddi_port_clock(dev_priv,
> > pll_id);
> >
> > ddi_dotclock_get(pipe_config);
> > }
> > @@ -1437,11 +1437,26 @@ static void bxt_ddi_clock_get(struct
> > intel_encoder *encoder,
> > enum port port = intel_ddi_get_encoder_port(encoder);
> > enum intel_dpll_id pll_id = port;
> >
> > - pipe_config->port_clock = bxt_calc_pll_link(dev_priv,
> > pll_id);
> > + pipe_config->port_clock = intel_ddi_port_clock(dev_priv,
> > pll_id);
> >
> > ddi_dotclock_get(pipe_config);
> > }
> >
> > +int intel_ddi_port_clock(struct drm_i915_private *dev_priv,
> > + enum intel_dpll_id pll_id)
> > +{
> > + if (IS_GEN9_BC(dev_priv)) {
> > + return skl_calc_pll_link(dev_priv, pll_id);
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > + return bxt_calc_pll_link(dev_priv, pll_id);
> > + } else if (IS_CANNONLAKE(dev_priv)) {
> > + return cnl_calc_pll_link(dev_priv, pll_id);
> > + } else {
> > + MISSING_CASE(INTEL_GEN(dev_priv));
> > + return 0;
> > + }
> > +}
> Personally, I feel that we wouldn't need to unify this. We call
> platform specific functions from this intel_ddi_port_clock() and this
> is called from platform specific *_ddi_clock_get() routine. Why not
> just keep all platform specific functions in one place?
Yeap, it is indeed silly to unify since we are just calling it
from the specific functions.
Let's drop this patch ;)
>
> Paolo, any opinions on this one?
>
> > +
> > void intel_ddi_clock_get(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 0cab667fff57..fe4650d6db03 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1293,7 +1293,8 @@ bool intel_ddi_is_audio_enabled(struct
> > drm_i915_private *dev_priv,
> > struct intel_crtc *intel_crtc);
> > void intel_ddi_get_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config);
> > -
> > +int intel_ddi_port_clock(struct drm_i915_private *dev_priv,
> > + enum intel_dpll_id pll_id);
> > void intel_ddi_clock_get(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config);
> > void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state
> > *crtc_state,
> --
> Mika Kahola - Intel OTC
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
