| Message ID | 1434624442-27413-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On to, 2015-06-18 at 13:47 +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The same dpll p2 divider selection is repeated three times in the > gen2-4 .find_dpll() functions. Factor it out. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Looks ok to me: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++---------------------- > 1 file changed, 30 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2fa81ed..2cc8ae7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev, > return true; > } > > -static bool > -i9xx_find_best_dpll(const intel_limit_t *limit, > - struct intel_crtc_state *crtc_state, > - int target, int refclk, intel_clock_t *match_clock, > - intel_clock_t *best_clock) > +static int > +i9xx_select_p2_div(const intel_limit_t *limit, > + const struct intel_crtc_state *crtc_state, > + int target) > { > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > - struct drm_device *dev = crtc->base.dev; > - intel_clock_t clock; > - int err = target; > + struct drm_device *dev = crtc_state->base.crtc->dev; > > if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > /* > @@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit, > * single/dual channel state, if we even can. > */ > if (intel_is_dual_link_lvds(dev)) > - clock.p2 = limit->p2.p2_fast; > + return limit->p2.p2_fast; > else > - clock.p2 = limit->p2.p2_slow; > + return limit->p2.p2_slow; > } else { > if (target < limit->p2.dot_limit) > - clock.p2 = limit->p2.p2_slow; > + return limit->p2.p2_slow; > else > - clock.p2 = limit->p2.p2_fast; > + return limit->p2.p2_fast; > } > +} > + > +static bool > +i9xx_find_best_dpll(const intel_limit_t *limit, > + struct intel_crtc_state *crtc_state, > + int target, int refclk, intel_clock_t *match_clock, > + intel_clock_t *best_clock) > +{ > + struct drm_device *dev = crtc_state->base.crtc->dev; > + intel_clock_t clock; > + int err = target; > > memset(best_clock, 0, sizeof(*best_clock)); > > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > + > for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; > clock.m1++) { > for (clock.m2 = limit->m2.min; > @@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit, > int target, int refclk, intel_clock_t *match_clock, > intel_clock_t *best_clock) > { > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > - struct drm_device *dev = crtc->base.dev; > + struct drm_device *dev = crtc_state->base.crtc->dev; > intel_clock_t clock; > int err = target; > > - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > - /* > - * For LVDS just rely on its current settings for dual-channel. > - * We haven't figured out how to reliably set up different > - * single/dual channel state, if we even can. > - */ > - if (intel_is_dual_link_lvds(dev)) > - clock.p2 = limit->p2.p2_fast; > - else > - clock.p2 = limit->p2.p2_slow; > - } else { > - if (target < limit->p2.dot_limit) > - clock.p2 = limit->p2.p2_slow; > - else > - clock.p2 = limit->p2.p2_fast; > - } > - > memset(best_clock, 0, sizeof(*best_clock)); > > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > + > for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; > clock.m1++) { > for (clock.m2 = limit->m2.min; > @@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit, > int target, int refclk, intel_clock_t *match_clock, > intel_clock_t *best_clock) > { > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > - struct drm_device *dev = crtc->base.dev; > + struct drm_device *dev = crtc_state->base.crtc->dev; > intel_clock_t clock; > int max_n; > - bool found; > + bool found = false; > /* approximately equals target * 0.00585 */ > int err_most = (target >> 8) + (target >> 9); > - found = false; > - > - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > - if (intel_is_dual_link_lvds(dev)) > - clock.p2 = limit->p2.p2_fast; > - else > - clock.p2 = limit->p2.p2_slow; > - } else { > - if (target < limit->p2.dot_limit) > - clock.p2 = limit->p2.p2_slow; > - else > - clock.p2 = limit->p2.p2_fast; > - } > > memset(best_clock, 0, sizeof(*best_clock)); > + > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > + > max_n = limit->n.max; > /* based on hardware requirement, prefer smaller n to precision */ > for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
On Thu, Jun 18, 2015 at 06:08:17PM +0300, Imre Deak wrote: > On to, 2015-06-18 at 13:47 +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The same dpll p2 divider selection is repeated three times in the > > gen2-4 .find_dpll() functions. Factor it out. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Looks ok to me: > Reviewed-by: Imre Deak <imre.deak@intel.com> Queued for -next, thanks for the patch. -Daniel > > > --- > > drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++---------------------- > > 1 file changed, 30 insertions(+), 48 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 2fa81ed..2cc8ae7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev, > > return true; > > } > > > > -static bool > > -i9xx_find_best_dpll(const intel_limit_t *limit, > > - struct intel_crtc_state *crtc_state, > > - int target, int refclk, intel_clock_t *match_clock, > > - intel_clock_t *best_clock) > > +static int > > +i9xx_select_p2_div(const intel_limit_t *limit, > > + const struct intel_crtc_state *crtc_state, > > + int target) > > { > > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > - struct drm_device *dev = crtc->base.dev; > > - intel_clock_t clock; > > - int err = target; > > + struct drm_device *dev = crtc_state->base.crtc->dev; > > > > if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > > /* > > @@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit, > > * single/dual channel state, if we even can. > > */ > > if (intel_is_dual_link_lvds(dev)) > > - clock.p2 = limit->p2.p2_fast; > > + return limit->p2.p2_fast; > > else > > - clock.p2 = limit->p2.p2_slow; > > + return limit->p2.p2_slow; > > } else { > > if (target < limit->p2.dot_limit) > > - clock.p2 = limit->p2.p2_slow; > > + return limit->p2.p2_slow; > > else > > - clock.p2 = limit->p2.p2_fast; > > + return limit->p2.p2_fast; > > } > > +} > > + > > +static bool > > +i9xx_find_best_dpll(const intel_limit_t *limit, > > + struct intel_crtc_state *crtc_state, > > + int target, int refclk, intel_clock_t *match_clock, > > + intel_clock_t *best_clock) > > +{ > > + struct drm_device *dev = crtc_state->base.crtc->dev; > > + intel_clock_t clock; > > + int err = target; > > > > memset(best_clock, 0, sizeof(*best_clock)); > > > > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > > + > > for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; > > clock.m1++) { > > for (clock.m2 = limit->m2.min; > > @@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit, > > int target, int refclk, intel_clock_t *match_clock, > > intel_clock_t *best_clock) > > { > > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > - struct drm_device *dev = crtc->base.dev; > > + struct drm_device *dev = crtc_state->base.crtc->dev; > > intel_clock_t clock; > > int err = target; > > > > - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > > - /* > > - * For LVDS just rely on its current settings for dual-channel. > > - * We haven't figured out how to reliably set up different > > - * single/dual channel state, if we even can. > > - */ > > - if (intel_is_dual_link_lvds(dev)) > > - clock.p2 = limit->p2.p2_fast; > > - else > > - clock.p2 = limit->p2.p2_slow; > > - } else { > > - if (target < limit->p2.dot_limit) > > - clock.p2 = limit->p2.p2_slow; > > - else > > - clock.p2 = limit->p2.p2_fast; > > - } > > - > > memset(best_clock, 0, sizeof(*best_clock)); > > > > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > > + > > for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; > > clock.m1++) { > > for (clock.m2 = limit->m2.min; > > @@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit, > > int target, int refclk, intel_clock_t *match_clock, > > intel_clock_t *best_clock) > > { > > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > - struct drm_device *dev = crtc->base.dev; > > + struct drm_device *dev = crtc_state->base.crtc->dev; > > intel_clock_t clock; > > int max_n; > > - bool found; > > + bool found = false; > > /* approximately equals target * 0.00585 */ > > int err_most = (target >> 8) + (target >> 9); > > - found = false; > > - > > - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { > > - if (intel_is_dual_link_lvds(dev)) > > - clock.p2 = limit->p2.p2_fast; > > - else > > - clock.p2 = limit->p2.p2_slow; > > - } else { > > - if (target < limit->p2.dot_limit) > > - clock.p2 = limit->p2.p2_slow; > > - else > > - clock.p2 = limit->p2.p2_fast; > > - } > > > > memset(best_clock, 0, sizeof(*best_clock)); > > + > > + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); > > + > > max_n = limit->n.max; > > /* based on hardware requirement, prefer smaller n to precision */ > > for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) { > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2fa81ed..2cc8ae7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -643,16 +643,12 @@ static bool intel_PLL_is_valid(struct drm_device *dev, return true; } -static bool -i9xx_find_best_dpll(const intel_limit_t *limit, - struct intel_crtc_state *crtc_state, - int target, int refclk, intel_clock_t *match_clock, - intel_clock_t *best_clock) +static int +i9xx_select_p2_div(const intel_limit_t *limit, + const struct intel_crtc_state *crtc_state, + int target) { - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - struct drm_device *dev = crtc->base.dev; - intel_clock_t clock; - int err = target; + struct drm_device *dev = crtc_state->base.crtc->dev; if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { /* @@ -661,18 +657,31 @@ i9xx_find_best_dpll(const intel_limit_t *limit, * single/dual channel state, if we even can. */ if (intel_is_dual_link_lvds(dev)) - clock.p2 = limit->p2.p2_fast; + return limit->p2.p2_fast; else - clock.p2 = limit->p2.p2_slow; + return limit->p2.p2_slow; } else { if (target < limit->p2.dot_limit) - clock.p2 = limit->p2.p2_slow; + return limit->p2.p2_slow; else - clock.p2 = limit->p2.p2_fast; + return limit->p2.p2_fast; } +} + +static bool +i9xx_find_best_dpll(const intel_limit_t *limit, + struct intel_crtc_state *crtc_state, + int target, int refclk, intel_clock_t *match_clock, + intel_clock_t *best_clock) +{ + struct drm_device *dev = crtc_state->base.crtc->dev; + intel_clock_t clock; + int err = target; memset(best_clock, 0, sizeof(*best_clock)); + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); + for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) { for (clock.m2 = limit->m2.min; @@ -712,30 +721,14 @@ pnv_find_best_dpll(const intel_limit_t *limit, int target, int refclk, intel_clock_t *match_clock, intel_clock_t *best_clock) { - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - struct drm_device *dev = crtc->base.dev; + struct drm_device *dev = crtc_state->base.crtc->dev; intel_clock_t clock; int err = target; - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { - /* - * For LVDS just rely on its current settings for dual-channel. - * We haven't figured out how to reliably set up different - * single/dual channel state, if we even can. - */ - if (intel_is_dual_link_lvds(dev)) - clock.p2 = limit->p2.p2_fast; - else - clock.p2 = limit->p2.p2_slow; - } else { - if (target < limit->p2.dot_limit) - clock.p2 = limit->p2.p2_slow; - else - clock.p2 = limit->p2.p2_fast; - } - memset(best_clock, 0, sizeof(*best_clock)); + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); + for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) { for (clock.m2 = limit->m2.min; @@ -773,28 +766,17 @@ g4x_find_best_dpll(const intel_limit_t *limit, int target, int refclk, intel_clock_t *match_clock, intel_clock_t *best_clock) { - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - struct drm_device *dev = crtc->base.dev; + struct drm_device *dev = crtc_state->base.crtc->dev; intel_clock_t clock; int max_n; - bool found; + bool found = false; /* approximately equals target * 0.00585 */ int err_most = (target >> 8) + (target >> 9); - found = false; - - if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) { - if (intel_is_dual_link_lvds(dev)) - clock.p2 = limit->p2.p2_fast; - else - clock.p2 = limit->p2.p2_slow; - } else { - if (target < limit->p2.dot_limit) - clock.p2 = limit->p2.p2_slow; - else - clock.p2 = limit->p2.p2_fast; - } memset(best_clock, 0, sizeof(*best_clock)); + + clock.p2 = i9xx_select_p2_div(limit, crtc_state, target); + max_n = limit->n.max; /* based on hardware requirement, prefer smaller n to precision */ for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {