[Intel-gfx] [PATCH 2/2]intel_display.c:create corresponding functions to find best pll on different platform.
Ma Ling
ling.ma at intel.com
Mon Mar 9 07:01:13 CET 2009
On Sat, 2009-03-07 at 04:49 +0800, Eric Anholt wrote:
> On Thu, 2009-03-05 at 21:04 +0800, Ma Ling wrote:
> > according to reference spreadsheet, use different algorithm to find best PLL.
> >
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 99 ++++++++++++++++++++++++++++++----
> > 1 files changed, 89 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 463caeb..9ed90cb 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -57,9 +57,11 @@ typedef struct {
> >
> > #define INTEL_P2_NUM 2
> >
> > -typedef struct {
> > +typedef struct intel_limit {
> > intel_range_t dot, vco, n, m, m1, m2, p, p1;
> > intel_p2_t p2;
> > + bool (* find_pll)(struct intel_limit *, struct drm_crtc *,
> > + int, int, intel_clock_t *);
> > } intel_limit_t;
> >
>
> I'd prefer to see either:
>
> typedef struct intel_limit intel_limit_t;
> struct intel_limit {
> intel_range_t dot, vco, n, m, m1, m2, p, p1;
> intel_p2_t p2;
> bool (*find_pll)(intel_limit_t *, struct drm_crtc *,
> int, int, intel_clock_t *);
> } intel_limit_t;
>
done
> or removal of intel_limit_t. Using two different names for the same
> struct is ugly.
>
> > #define I8XX_DOT_MIN 25000
> > @@ -198,6 +200,12 @@ typedef struct {
> > #define IG4X_P2_DUA_LVDS_FAST 7
> > #define IG4X_P2_DUA_LVDS_LIMIT 0
> >
> > +static bool
> > +intel_find_best_PLL(intel_limit_t *limit, struct drm_crtc *crtc,
> > + int target, int refclk, intel_clock_t *best_clock);
> > +static bool
> > +intel_g4x_find_best_PLL(intel_limit_t *limit, struct drm_crtc *crtc,
> > + int target, int refclk, intel_clock_t *best_clock);
> >
> > static const intel_limit_t intel_limits[] = {
> > { /* INTEL_LIMIT_I8XX_DVO_DAC */
> > @@ -211,6 +219,7 @@ static const intel_limit_t intel_limits[] = {
> > .p1 = { .min = I8XX_P1_MIN, .max = I8XX_P1_MAX },
> > .p2 = { .dot_limit = I8XX_P2_SLOW_LIMIT,
> > .p2_slow = I8XX_P2_SLOW, .p2_fast = I8XX_P2_FAST },
> > + .find_pll = intel_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_I8XX_LVDS */
> > .dot = { .min = I8XX_DOT_MIN, .max = I8XX_DOT_MAX },
> > @@ -223,6 +232,7 @@ static const intel_limit_t intel_limits[] = {
> > .p1 = { .min = I8XX_P1_LVDS_MIN, .max = I8XX_P1_LVDS_MAX },
> > .p2 = { .dot_limit = I8XX_P2_SLOW_LIMIT,
> > .p2_slow = I8XX_P2_LVDS_SLOW, .p2_fast = I8XX_P2_LVDS_FAST },
> > + .find_pll = intel_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_I9XX_SDVO_DAC */
> > .dot = { .min = I9XX_DOT_MIN, .max = I9XX_DOT_MAX },
> > @@ -235,6 +245,7 @@ static const intel_limit_t intel_limits[] = {
> > .p1 = { .min = I9XX_P1_MIN, .max = I9XX_P1_MAX },
> > .p2 = { .dot_limit = I9XX_P2_SDVO_DAC_SLOW_LIMIT,
> > .p2_slow = I9XX_P2_SDVO_DAC_SLOW, .p2_fast = I9XX_P2_SDVO_DAC_FAST },
> > + .find_pll = intel_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_I9XX_LVDS */
> > .dot = { .min = I9XX_DOT_MIN, .max = I9XX_DOT_MAX },
> > @@ -250,6 +261,7 @@ static const intel_limit_t intel_limits[] = {
> > */
> > .p2 = { .dot_limit = I9XX_P2_LVDS_SLOW_LIMIT,
> > .p2_slow = I9XX_P2_LVDS_SLOW, .p2_fast = I9XX_P2_LVDS_FAST },
> > + .find_pll = intel_find_best_PLL,
> > },
> > /* below parameter and function is for EagleLake and Cantig Chipset Family*/
> > { /* INTEL_LIMIT_EL_AND_CTG_SDVO */
> > @@ -265,6 +277,7 @@ static const intel_limit_t intel_limits[] = {
> > .p2_slow = IG4X_P2_SDVO_SLOW,
> > .p2_fast = IG4X_P2_SDVO_FAST
> > },
> > + .find_pll = intel_g4x_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_EL_AND_CTG_HDMI_DAC */
> > .dot = { .min = IG4X_DOT_HDMI_DAC_MIN, .max = IG4X_DOT_HDMI_DAC_MAX },
> > @@ -279,6 +292,7 @@ static const intel_limit_t intel_limits[] = {
> > .p2_slow = IG4X_P2_HDMI_DAC_SLOW,
> > .p2_fast = IG4X_P2_HDMI_DAC_FAST
> > },
> > + .find_pll = intel_g4x_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_EL_AND_CTG_SIN_LVDS */
> > .dot = { .min = IG4X_DOT_SIN_LVDS_MIN, .max = IG4X_DOT_SIN_LVDS_MAX },
> > @@ -293,6 +307,7 @@ static const intel_limit_t intel_limits[] = {
> > .p2_slow = IG4X_P2_SIN_LVDS_SLOW,
> > .p2_fast = IG4X_P2_SIN_LVDS_FAST
> > },
> > + .find_pll = intel_g4x_find_best_PLL,
> > },
> > { /* INTEL_LIMIT_EL_AND_CTG_DUA_LVDS */
> > .dot = { .min = IG4X_DOT_DUA_LVDS_MIN, .max = IG4X_DOT_DUA_LVDS_MAX },
> > @@ -307,6 +322,7 @@ static const intel_limit_t intel_limits[] = {
> > .p2_slow = IG4X_P2_DUA_LVDS_SLOW,
> > .p2_fast = IG4X_P2_DUA_LVDS_FAST
> > },
> > + .find_pll = intel_g4x_find_best_PLL,
> > },
> > };
> >
> > @@ -426,18 +442,14 @@ static bool intel_PLL_is_valid(struct drm_crtc *crtc, intel_clock_t *clock)
> > return true;
> > }
> >
> > -/**
> > - * Returns a set of divisors for the desired target clock with the given
> > - * refclk, or FALSE. The returned values represent the clock equation:
> > - * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> > - */
> > -static bool intel_find_best_PLL(struct drm_crtc *crtc, int target,
> > - int refclk, intel_clock_t *best_clock)
> > +static bool
> > +intel_find_best_PLL(intel_limit_t *limit, struct drm_crtc *crtc,
> > + int target, int refclk, intel_clock_t *best_clock)
> > +
> > {
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > intel_clock_t clock;
> > - const intel_limit_t *limit = intel_limit(crtc);
> > int err = target;
> >
> > if (IS_I9XX(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> > @@ -489,6 +501,66 @@ static bool intel_find_best_PLL(struct drm_crtc *crtc, int target,
> > return (err != target);
> > }
> >
> > +static bool
> > +intel_g4x_find_best_PLL(intel_limit_t *limit, struct drm_crtc *crtc,
> > + int target, int refclk, intel_clock_t *best_clock)
> > +{
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + intel_clock_t clock;
> > + int max_n;
> > + bool found;
> > + /*approximately equals target * 0.00488*/
>
> /* approximately equals target * 0.00488 */
done
>
> > + int err_most = (target >> 8) + (target >> 10);
> > + /* return value defult false */
> > + found = false;
> > +
> > + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> > + if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
> > + LVDS_CLKB_POWER_UP)
> > + 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));
> > + max_n = limit->n.max;
> > + /* based on hardware requriment prefer smaller n to precision */
> > + for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
> > + /* based on hardware requirment prefere bigger m1,m2, p1*/
>
> Spelling: /* based on hardware requirement, prefer larger m1, m2, p1 */
done
>
> > + for (clock.m1 = limit->m1.max;
> > + clock.m1 >= limit->m1.min; clock.m1--) {
> > + for (clock.m2 = limit->m2.max;
> > + clock.m2 >= limit->m2.min; clock.m2--) {
> > + for (clock.p1 = limit->p1.max;
> > + clock.p1 >= limit->p1.min; clock.p1--) {
> > + int this_err;
> > +
> > + intel_clock (refclk, &clock);
> > + if (!intel_PLL_is_valid(crtc, &clock))
> > + continue;
> > + this_err = abs(clock.dot - target) ;
> > + if (this_err < err_most &&
> > + clock.n <= max_n) {
> > + *best_clock = clock;
> > + err_most = this_err;
> > + /*chose smaller n*/
> > + max_n = clock.n;
> > + found = true;
> > + }
> > + }
> > + }
> > + }
> > + }
> > +
> > + return found;
> > +}
> > +
>
> How would clock.n ever be > max_n?
>
> So if I'm reading this right, we won't accept any clocks that are more
> than .5% off of the target clock. As I recall (and granted, this was a
> long time ago that I was playing with this), this could cut down the
> number of pll values available to us, potentially leading to failure to
> find a PLL setting for a given dotclock at all.
>
> Have you verified that you don't reject any dotclock values that would
> have succeeded in the past? It shouldn't be hard to cook up a test in
> some C code. And if that's a problem, it seems like this loop could be
> fixed to look like the other one but without "upgrading" when the error
> was within tolerance.
I have tested HDMI&DAC, SDVO, Dual/Single channel lvds with independent C program respectively, all dotclock is produced within.5%.
output scope(scope is continuously increased one by one)
HDMI&DAC 22000 to 400000
SDVO 20000 to 270000 (x5 if less 25000, x4 if less 50000, x2 if less 100000)
Single LVDS 20000 to 115000
DUAL LVDS 80000 to 224000
(our original scope is from 20000 to 400000 for all outputs)
>
> (Note, though, that we've got people that want to get the closest
> possible dot clock to their target, to better match their video
> framerate. So even doing that has some downsides, as to those folks
> *any* amount of error is bad.)
>
> > void
> > intel_wait_for_vblank(struct drm_device *dev)
> > {
> > @@ -907,6 +979,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
> > bool is_crt = false, is_lvds = false, is_tv = false;
> > struct drm_mode_config *mode_config = &dev->mode_config;
> > struct drm_connector *connector;
> > + intel_limit_t *limit;
>
> gratuitous whitespace
done
>
> > int ret;
> >
> > drm_vblank_pre_modeset(dev, pipe);
> > @@ -950,7 +1023,13 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
> > refclk = 48000;
> > }
> >
> > - ok = intel_find_best_PLL(crtc, adjusted_mode->clock, refclk, &clock);
> > + /**
> > + * Returns a set of divisors for the desired target clock with the given
> > + * refclk, or FALSE. The returned values represent the clock equation:
> > + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> > + */
> > + limit = intel_limit (crtc);
> > + ok = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, &clock);
>
> Drop the double-star on the comment when it isn't a doxygen comment.
done
>
More information about the Intel-gfx
mailing list