| Message ID | 1490133430-13999-1-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2017-03-21 at 18:57 -0300, Paulo Zanoni wrote: > Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same thing > in slightly different ways. Replace everything with a simple copy of > the function and inline it inside intle_ddi_pll_select(). > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------------------- > 1 file changed, 7 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index d8214ba..bd6fd0b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, > bxt_ddi_clock_get(encoder, pipe_config); > } > > -static bool > -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - struct intel_shared_dpll *pll; > - > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > - encoder); I have a related question, not a patch review :) Looks like we care only about the boolean returned from intel_get_shared_dpll(). Could not find any caller that uses the returned 'struct intel_shared_dpll *'. Should it's signature be simply changed to return a bool? -DK > - if (!pll) > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > - pipe_name(intel_crtc->pipe)); > - > - return pll; > -} > - > -static bool > -skl_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - struct intel_shared_dpll *pll; > - > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); > - if (pll == NULL) { > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > - pipe_name(intel_crtc->pipe)); > - return false; > - } > - > - return true; > -} > - > -static bool > -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - return !!intel_get_shared_dpll(intel_crtc, crtc_state, encoder); > -} > - > /* > * Tries to find a *shared* PLL for the CRTC and store it in > * intel_crtc->ddi_pll_sel. > @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, > struct intel_crtc_state *crtc_state) > { > - struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > struct intel_encoder *encoder = > intel_ddi_get_crtc_new_encoder(crtc_state); > + struct intel_shared_dpll *pll; > > - if (IS_GEN9_BC(dev_priv)) > - return skl_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > - else if (IS_GEN9_LP(dev_priv)) > - return bxt_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > - else > - return hsw_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > + pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); > + if (!pll) > + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > + pipe_name(intel_crtc->pipe)); > + > + return pll; > } > > void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
On Tue, Mar 21, 2017 at 06:57:10PM -0300, Paulo Zanoni wrote: > Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same thing > in slightly different ways. Replace everything with a simple copy of > the function and inline it inside intle_ddi_pll_select(). > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------------------- > 1 file changed, 7 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index d8214ba..bd6fd0b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, > bxt_ddi_clock_get(encoder, pipe_config); > } > > -static bool > -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - struct intel_shared_dpll *pll; > - > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > - encoder); > - if (!pll) > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > - pipe_name(intel_crtc->pipe)); > - > - return pll; > -} > - > -static bool > -skl_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - struct intel_shared_dpll *pll; > - > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); > - if (pll == NULL) { > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > - pipe_name(intel_crtc->pipe)); > - return false; > - } > - > - return true; > -} > - > -static bool > -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_encoder *encoder) > -{ > - return !!intel_get_shared_dpll(intel_crtc, crtc_state, encoder); > -} > - > /* > * Tries to find a *shared* PLL for the CRTC and store it in > * intel_crtc->ddi_pll_sel. > @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, > struct intel_crtc_state *crtc_state) > { > - struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > struct intel_encoder *encoder = > intel_ddi_get_crtc_new_encoder(crtc_state); > + struct intel_shared_dpll *pll; > > - if (IS_GEN9_BC(dev_priv)) > - return skl_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > - else if (IS_GEN9_LP(dev_priv)) > - return bxt_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > - else > - return hsw_ddi_pll_select(intel_crtc, crtc_state, > - encoder); > + pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); I wonder if intel_ddi_pll_select() has any good reason to even exist anymore. It's just calling this one function, so maybe we should just call it directly? > + if (!pll) > + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > + pipe_name(intel_crtc->pipe)); > + > + return pll; I would probably write 'pll != NULL' for a bit of extra clarity. Either way Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em Ter, 2017-03-21 às 23:05 +0000, Pandiyan, Dhinakaran escreveu: > On Tue, 2017-03-21 at 18:57 -0300, Paulo Zanoni wrote: > > > > Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same > > thing > > in slightly different ways. Replace everything with a simple copy > > of > > the function and inline it inside intle_ddi_pll_select(). > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------- > > ------------ > > 1 file changed, 7 insertions(+), 51 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index d8214ba..bd6fd0b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct > > intel_encoder *encoder, > > bxt_ddi_clock_get(encoder, pipe_config); > > } > > > > -static bool > > -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - struct intel_shared_dpll *pll; > > - > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > - encoder); > > I have a related question, not a patch review :) Looks like we care > only > about the boolean returned from intel_get_shared_dpll(). Could not > find > any caller that uses the returned 'struct intel_shared_dpll *'. > Should > it's signature be simply changed to return a bool? There are only two callers. It would be possible to change the signature, although I don't see many benefits from doing it now. Perhaps it would be better to unify the error handling. > > -DK > > > > - if (!pll) > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > - pipe_name(intel_crtc->pipe)); > > - > > - return pll; > > -} > > - > > -static bool > > -skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - struct intel_shared_dpll *pll; > > - > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > - if (pll == NULL) { > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > - pipe_name(intel_crtc->pipe)); > > - return false; > > - } > > - > > - return true; > > -} > > - > > -static bool > > -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - return !!intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > -} > > - > > /* > > * Tries to find a *shared* PLL for the CRTC and store it in > > * intel_crtc->ddi_pll_sel. > > @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc > > *intel_crtc, > > bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, > > struct intel_crtc_state *crtc_state) > > { > > - struct drm_i915_private *dev_priv = to_i915(intel_crtc- > > >base.dev); > > struct intel_encoder *encoder = > > intel_ddi_get_crtc_new_encoder(crtc_state); > > + struct intel_shared_dpll *pll; > > > > - if (IS_GEN9_BC(dev_priv)) > > - return skl_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > - else if (IS_GEN9_LP(dev_priv)) > > - return bxt_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > - else > > - return hsw_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > + pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > + if (!pll) > > + DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > + pipe_name(intel_crtc->pipe)); > > + > > + return pll; > > } > > > > void intel_ddi_set_pipe_settings(const struct intel_crtc_state > > *crtc_state) >
Em Qua, 2017-03-22 às 15:21 +0200, Ville Syrjälä escreveu: > On Tue, Mar 21, 2017 at 06:57:10PM -0300, Paulo Zanoni wrote: > > > > Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same > > thing > > in slightly different ways. Replace everything with a simple copy > > of > > the function and inline it inside intle_ddi_pll_select(). > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------- > > ------------ > > 1 file changed, 7 insertions(+), 51 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index d8214ba..bd6fd0b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct > > intel_encoder *encoder, > > bxt_ddi_clock_get(encoder, pipe_config); > > } > > > > -static bool > > -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - struct intel_shared_dpll *pll; > > - > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > - encoder); > > - if (!pll) > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > - pipe_name(intel_crtc->pipe)); > > - > > - return pll; > > -} > > - > > -static bool > > -skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - struct intel_shared_dpll *pll; > > - > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > - if (pll == NULL) { > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > - pipe_name(intel_crtc->pipe)); > > - return false; > > - } > > - > > - return true; > > -} > > - > > -static bool > > -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_encoder *encoder) > > -{ > > - return !!intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > -} > > - > > /* > > * Tries to find a *shared* PLL for the CRTC and store it in > > * intel_crtc->ddi_pll_sel. > > @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc > > *intel_crtc, > > bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, > > struct intel_crtc_state *crtc_state) > > { > > - struct drm_i915_private *dev_priv = to_i915(intel_crtc- > > >base.dev); > > struct intel_encoder *encoder = > > intel_ddi_get_crtc_new_encoder(crtc_state); > > + struct intel_shared_dpll *pll; > > > > - if (IS_GEN9_BC(dev_priv)) > > - return skl_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > - else if (IS_GEN9_LP(dev_priv)) > > - return bxt_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > - else > > - return hsw_ddi_pll_select(intel_crtc, crtc_state, > > - encoder); > > + pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > encoder); > > I wonder if intel_ddi_pll_select() has any good reason to even exist > anymore. It's just calling this one function, so maybe we should just > call it directly? You're right here. Then the two callers would be haswell_crtc_compute_clock() and ironlake_crtc_compute_clock(), which makes even more sene. I'll submit another patch. > > > > > + if (!pll) > > + DRM_DEBUG_DRIVER("failed to find PLL for pipe > > %c\n", > > + pipe_name(intel_crtc->pipe)); > > + > > + return pll; > > I would probably write 'pll != NULL' for a bit of extra clarity. Are you talking about the return statement or the if condition? I'll assume you're talking about the return statement. I can fix this. Another thing would be "return !!pll". I'm fine with all approaches. > > Either way > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks! > > > > > } > > > > void intel_ddi_set_pipe_settings(const struct intel_crtc_state > > *crtc_state) > > -- > > 2.7.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, Mar 22, 2017 at 02:40:01PM -0300, Paulo Zanoni wrote: > Em Qua, 2017-03-22 às 15:21 +0200, Ville Syrjälä escreveu: > > On Tue, Mar 21, 2017 at 06:57:10PM -0300, Paulo Zanoni wrote: > > > > > > Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same > > > thing > > > in slightly different ways. Replace everything with a simple copy > > > of > > > the function and inline it inside intle_ddi_pll_select(). > > > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------- > > > ------------ > > > 1 file changed, 7 insertions(+), 51 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > index d8214ba..bd6fd0b 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct > > > intel_encoder *encoder, > > > bxt_ddi_clock_get(encoder, pipe_config); > > > } > > > > > > -static bool > > > -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_encoder *encoder) > > > -{ > > > - struct intel_shared_dpll *pll; > > > - > > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > > - encoder); > > > - if (!pll) > > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > > %c\n", > > > - pipe_name(intel_crtc->pipe)); > > > - > > > - return pll; > > > -} > > > - > > > -static bool > > > -skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_encoder *encoder) > > > -{ > > > - struct intel_shared_dpll *pll; > > > - > > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > > encoder); > > > - if (pll == NULL) { > > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > > > %c\n", > > > - pipe_name(intel_crtc->pipe)); > > > - return false; > > > - } > > > - > > > - return true; > > > -} > > > - > > > -static bool > > > -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_encoder *encoder) > > > -{ > > > - return !!intel_get_shared_dpll(intel_crtc, crtc_state, > > > encoder); > > > -} > > > - > > > /* > > > * Tries to find a *shared* PLL for the CRTC and store it in > > > * intel_crtc->ddi_pll_sel. > > > @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc > > > *intel_crtc, > > > bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, > > > struct intel_crtc_state *crtc_state) > > > { > > > - struct drm_i915_private *dev_priv = to_i915(intel_crtc- > > > >base.dev); > > > struct intel_encoder *encoder = > > > intel_ddi_get_crtc_new_encoder(crtc_state); > > > + struct intel_shared_dpll *pll; > > > > > > - if (IS_GEN9_BC(dev_priv)) > > > - return skl_ddi_pll_select(intel_crtc, crtc_state, > > > - encoder); > > > - else if (IS_GEN9_LP(dev_priv)) > > > - return bxt_ddi_pll_select(intel_crtc, crtc_state, > > > - encoder); > > > - else > > > - return hsw_ddi_pll_select(intel_crtc, crtc_state, > > > - encoder); > > > + pll = intel_get_shared_dpll(intel_crtc, crtc_state, > > > encoder); > > > > I wonder if intel_ddi_pll_select() has any good reason to even exist > > anymore. It's just calling this one function, so maybe we should just > > call it directly? > > You're right here. Then the two callers would be > haswell_crtc_compute_clock() and ironlake_crtc_compute_clock(), which > makes even more sene. I'll submit another patch. > > > > > > > > > + if (!pll) > > > + DRM_DEBUG_DRIVER("failed to find PLL for pipe > > > %c\n", > > > + pipe_name(intel_crtc->pipe)); > > > + > > > + return pll; > > > > I would probably write 'pll != NULL' for a bit of extra clarity. > > Are you talking about the return statement or the if condition? The return. > > I'll assume you're talking about the return statement. I can fix this. > Another thing would be "return !!pll". I'm fine with all approaches. For whatever reason I've started to prefer !=NULL type of stuff. Not really sure why since I used to dislike it when I was younger. > > > > > Either way > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Thanks! > > > > > > > > > } > > > > > > void intel_ddi_set_pipe_settings(const struct intel_crtc_state > > > *crtc_state) > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index d8214ba..bd6fd0b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1127,47 +1127,6 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, bxt_ddi_clock_get(encoder, pipe_config); } -static bool -hsw_ddi_pll_select(struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state, - struct intel_encoder *encoder) -{ - struct intel_shared_dpll *pll; - - pll = intel_get_shared_dpll(intel_crtc, crtc_state, - encoder); - if (!pll) - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", - pipe_name(intel_crtc->pipe)); - - return pll; -} - -static bool -skl_ddi_pll_select(struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state, - struct intel_encoder *encoder) -{ - struct intel_shared_dpll *pll; - - pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); - if (pll == NULL) { - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", - pipe_name(intel_crtc->pipe)); - return false; - } - - return true; -} - -static bool -bxt_ddi_pll_select(struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state, - struct intel_encoder *encoder) -{ - return !!intel_get_shared_dpll(intel_crtc, crtc_state, encoder); -} - /* * Tries to find a *shared* PLL for the CRTC and store it in * intel_crtc->ddi_pll_sel. @@ -1178,19 +1137,16 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, bool intel_ddi_pll_select(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state) { - struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); struct intel_encoder *encoder = intel_ddi_get_crtc_new_encoder(crtc_state); + struct intel_shared_dpll *pll; - if (IS_GEN9_BC(dev_priv)) - return skl_ddi_pll_select(intel_crtc, crtc_state, - encoder); - else if (IS_GEN9_LP(dev_priv)) - return bxt_ddi_pll_select(intel_crtc, crtc_state, - encoder); - else - return hsw_ddi_pll_select(intel_crtc, crtc_state, - encoder); + pll = intel_get_shared_dpll(intel_crtc, crtc_state, encoder); + if (!pll) + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", + pipe_name(intel_crtc->pipe)); + + return pll; } void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
Because {hsw,skl,bxt}_ddi_pll_select all pretty much do the same thing in slightly different ways. Replace everything with a simple copy of the function and inline it inside intle_ddi_pll_select(). Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 58 +++++----------------------------------- 1 file changed, 7 insertions(+), 51 deletions(-)