[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
Tue Mar 10 13:50:24 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;
>
> 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 */
>
> > + 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 */
>
> > + 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?
yes, the condition is redundant.
>
> 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.
attach the c program, scope is from 22000 to 400000 for HDMI and VGA
output. It shows we will find all clocks from 22000 to 400000 with less
0.5% tolerance.
Thanks
Ma Ling
>
> (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
>
> > 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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dpll_hdmi.c
Type: text/x-csrc
Size: 2087 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090310/fd7b9ded/attachment.c>
More information about the Intel-gfx
mailing list