[Intel-gfx] [PATCH 1/2]intel_display.c:define related parameters and limit items for G4x platform
Ma Ling
ling.ma at intel.com
Mon Mar 9 07:01:02 CET 2009
hi Eric
thanks for your reply.
On Sat, 2009-03-07 at 04:38 +0800, Eric Anholt wrote:
> On Thu, 2009-03-05 at 21:04 +0800, Ma Ling wrote:
> > append new limit items for SDVO, HDVI&DAC, Single LVDS, and Dual
> LVDS on G4 platform.
>
> I think the commit message here should be:
>
> "
> drm/i915: Use documented PLL timing limits for G4X chipsets
>
> These timings were specified by an internal spreadsheet provided by
> the
> chipset group.
>
> <text here as to the problems that using these timing limits has
> fixed.
> Bugzilla references if applicable.>
> "
>
> I never want to see the filename in a commit message. That
> information
> shows up in the diffstat. I do want to see "drm/i915:" or "drm:" if
> it's generic, as that's the prefix we're using for all commits.
>
> Also, be sure to include Signed-off-by: on all commit messages for the
> kernel. Check out git-commit(1) for info on how to append it with
> your
> commits, and Documentation/SubmittingPatches for what Signed-off-by
> means.
>
done!
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 176
> +++++++++++++++++++++++++++++++++-
> > 1 files changed, 175 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > index a283427..463caeb 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -115,6 +115,89 @@ typedef struct {
> > #define INTEL_LIMIT_I8XX_LVDS 1
> > #define INTEL_LIMIT_I9XX_SDVO_DAC 2
> > #define INTEL_LIMIT_I9XX_LVDS 3
> > +#define INTEL_LIMIT_IG4X_SDVO 4
> > +#define INTEL_LIMIT_IG4X_HDMI_DAC 5
> > +#define INTEL_LIMIT_IG4X_SIN_LVDS 6
> > +#define INTEL_LIMIT_IG4X_DUA_LVDS 7
> > +
> > +/*The parameter is for SDVO on G4x platform*/
> > +#define IG4X_DOT_SDVO_MIN 25000
> > +#define IG4X_DOT_SDVO_MAX 270000
> > +#define IG4X_VCO_MIN 1750000
> > +#define IG4X_VCO_MAX 3500000
> > +#define IG4X_N_SDVO_MIN 1
> > +#define IG4X_N_SDVO_MAX 4
> > +#define IG4X_M_SDVO_MIN 104
> > +#define IG4X_M_SDVO_MAX 138
> > +#define IG4X_M1_SDVO_MIN 17
> > +#define IG4X_M1_SDVO_MAX 23
> > +#define IG4X_M2_SDVO_MIN 5
> > +#define IG4X_M2_SDVO_MAX 11
> > +#define IG4X_P_SDVO_MIN 10
> > +#define IG4X_P_SDVO_MAX 30
> > +#define IG4X_P1_SDVO_MIN 1
> > +#define IG4X_P1_SDVO_MAX 3
> > +#define IG4X_P2_SDVO_SLOW 10
> > +#define IG4X_P2_SDVO_FAST 10
> > +#define IG4X_P2_SDVO_LIMIT 270000
> > +
> > +/*The parameter is for HDMI_DAC on G4x platform*/
> > +#define IG4X_DOT_HDMI_DAC_MIN 20000
> > +#define IG4X_DOT_HDMI_DAC_MAX 400000
> > +#define IG4X_N_HDMI_DAC_MIN 1
> > +#define IG4X_N_HDMI_DAC_MAX 4
> > +#define IG4X_M_HDMI_DAC_MIN 104
> > +#define IG4X_M_HDMI_DAC_MAX 138
> > +#define IG4X_M1_HDMI_DAC_MIN 16
> > +#define IG4X_M1_HDMI_DAC_MAX 23
> > +#define IG4X_M2_HDMI_DAC_MIN 5
> > +#define IG4X_M2_HDMI_DAC_MAX 11
> > +#define IG4X_P_HDMI_DAC_MIN 5
> > +#define IG4X_P_HDMI_DAC_MAX 80
> > +#define IG4X_P1_HDMI_DAC_MIN 1
> > +#define IG4X_P1_HDMI_DAC_MAX 8
> > +#define IG4X_P2_HDMI_DAC_SLOW 10
> > +#define IG4X_P2_HDMI_DAC_FAST 5
> > +#define IG4X_P2_HDMI_DAC_LIMIT 165000
> > +
> > +/*The parameter is for SIN_LVDS on G4x platform*/
> > +#define IG4X_DOT_SIN_LVDS_MIN 20000
> > +#define IG4X_DOT_SIN_LVDS_MAX 115000
> > +#define IG4X_N_SIN_LVDS_MIN 1
> > +#define IG4X_N_SIN_LVDS_MAX 3
> > +#define IG4X_M_SIN_LVDS_MIN 104
> > +#define IG4X_M_SIN_LVDS_MAX 138
> > +#define IG4X_M1_SIN_LVDS_MIN 17
> > +#define IG4X_M1_SIN_LVDS_MAX 23
> > +#define IG4X_M2_SIN_LVDS_MIN 5
> > +#define IG4X_M2_SIN_LVDS_MAX 11
> > +#define IG4X_P_SIN_LVDS_MIN 28
> > +#define IG4X_P_SIN_LVDS_MAX 112
> > +#define IG4X_P1_SIN_LVDS_MIN 2
> > +#define IG4X_P1_SIN_LVDS_MAX 8
> > +#define IG4X_P2_SIN_LVDS_SLOW 14
> > +#define IG4X_P2_SIN_LVDS_FAST 14
> > +#define IG4X_P2_SIN_LVDS_LIMIT 0
> > +
> > +/*The parameter is for DUA_LVDS on G4x platform*/
> > +#define IG4X_DOT_DUA_LVDS_MIN 80000
> > +#define IG4X_DOT_DUA_LVDS_MAX 224000
> > +#define IG4X_N_DUA_LVDS_MIN 1
> > +#define IG4X_N_DUA_LVDS_MAX 3
> > +#define IG4X_M_DUA_LVDS_MIN 104
> > +#define IG4X_M_DUA_LVDS_MAX 138
> > +#define IG4X_M1_DUA_LVDS_MIN 17
> > +#define IG4X_M1_DUA_LVDS_MAX 23
> > +#define IG4X_M2_DUA_LVDS_MIN 5
> > +#define IG4X_M2_DUA_LVDS_MAX 11
> > +#define IG4X_P_DUA_LVDS_MIN 14
> > +#define IG4X_P_DUA_LVDS_MAX 42
> > +#define IG4X_P1_DUA_LVDS_MIN 2
> > +#define IG4X_P1_DUA_LVDS_MAX 6
> > +#define IG4X_P2_DUA_LVDS_SLOW 7
> > +#define IG4X_P2_DUA_LVDS_FAST 7
> > +#define IG4X_P2_DUA_LVDS_LIMIT 0
>
> Please call these G4X, not IG4X. We've got enough incoherent names
> already, no need to introduce new ones. Also, s/DUA/DUAL_CHANNEL/ and
> s/SIN/SINGLE_CHANNEL/
done
>
> > static const intel_limit_t intel_limits[] = {
> > { /* INTEL_LIMIT_I8XX_DVO_DAC */
> > @@ -168,14 +251,105 @@ 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 },
> > },
> > + /* below parameter and function is for EagleLake and Cantig
> Chipset Family*/
> > + { /* INTEL_LIMIT_EL_AND_CTG_SDVO */
>
> Please use our name for these chipsets (G4X) instead of codenames.
done
>
> > + .dot = { .min = IG4X_DOT_SDVO_MIN, .max = IG4X_DOT_SDVO_MAX },
> > + .vco = { .min = IG4X_VCO_MIN, .max = IG4X_VCO_MAX},
> > + .n = { .min = IG4X_N_SDVO_MIN, .max = IG4X_N_SDVO_MAX },
> > + .m = { .min = IG4X_M_SDVO_MIN, .max = IG4X_M_SDVO_MAX },
> > + .m1 = { .min = IG4X_M1_SDVO_MIN, .max = IG4X_M1_SDVO_MAX },
> > + .m2 = { .min = IG4X_M2_SDVO_MIN, .max = IG4X_M2_SDVO_MAX },
> > + .p = { .min = IG4X_P_SDVO_MIN, .max = IG4X_P_SDVO_MAX },
> > + .p1 = { .min = IG4X_P1_SDVO_MIN, .max = IG4X_P1_SDVO_MAX},
> > + .p2 = { .dot_limit = IG4X_P2_SDVO_LIMIT,
> > + .p2_slow = IG4X_P2_SDVO_SLOW,
> > + .p2_fast = IG4X_P2_SDVO_FAST
> > + },
> > + },
> > + { /* INTEL_LIMIT_EL_AND_CTG_HDMI_DAC */
> > + .dot = { .min = IG4X_DOT_HDMI_DAC_MIN, .max =
> IG4X_DOT_HDMI_DAC_MAX },
> > + .vco = { .min = IG4X_VCO_MIN, .max = IG4X_VCO_MAX},
> > + .n = { .min = IG4X_N_HDMI_DAC_MIN, .max = IG4X_N_HDMI_DAC_MAX },
> > + .m = { .min = IG4X_M_HDMI_DAC_MIN, .max = IG4X_M_HDMI_DAC_MAX },
> > + .m1 = { .min = IG4X_M1_HDMI_DAC_MIN, .max =
> IG4X_M1_HDMI_DAC_MAX },
> > + .m2 = { .min = IG4X_M2_HDMI_DAC_MIN, .max =
> IG4X_M2_HDMI_DAC_MAX },
> > + .p = { .min = IG4X_P_HDMI_DAC_MIN, .max = IG4X_P_HDMI_DAC_MAX },
> > + .p1 = { .min = IG4X_P1_HDMI_DAC_MIN, .max =
> IG4X_P1_HDMI_DAC_MAX},
> > + .p2 = { .dot_limit = IG4X_P2_HDMI_DAC_LIMIT,
> > + .p2_slow = IG4X_P2_HDMI_DAC_SLOW,
> > + .p2_fast = IG4X_P2_HDMI_DAC_FAST
> > + },
> > + },
> > + { /* INTEL_LIMIT_EL_AND_CTG_SIN_LVDS */
> > + .dot = { .min = IG4X_DOT_SIN_LVDS_MIN, .max =
> IG4X_DOT_SIN_LVDS_MAX },
> > + .vco = { .min = IG4X_VCO_MIN, .max = IG4X_VCO_MAX},
> > + .n = { .min = IG4X_N_SIN_LVDS_MIN, .max = IG4X_N_SIN_LVDS_MAX },
> > + .m = { .min = IG4X_M_SIN_LVDS_MIN, .max = IG4X_M_SIN_LVDS_MAX },
> > + .m1 = { .min = IG4X_M1_SIN_LVDS_MIN, .max =
> IG4X_M1_SIN_LVDS_MAX },
> > + .m2 = { .min = IG4X_M2_SIN_LVDS_MIN, .max =
> IG4X_M2_SIN_LVDS_MAX },
> > + .p = { .min = IG4X_P_SIN_LVDS_MIN, .max = IG4X_P_SIN_LVDS_MAX },
> > + .p1 = { .min = IG4X_P1_SIN_LVDS_MIN, .max =
> IG4X_P1_SIN_LVDS_MAX},
> > + .p2 = { .dot_limit = IG4X_P2_SIN_LVDS_LIMIT,
> > + .p2_slow = IG4X_P2_SIN_LVDS_SLOW,
> > + .p2_fast = IG4X_P2_SIN_LVDS_FAST
> > + },
> > + },
> > + { /* INTEL_LIMIT_EL_AND_CTG_DUA_LVDS */
> > + .dot = { .min = IG4X_DOT_DUA_LVDS_MIN, .max =
> IG4X_DOT_DUA_LVDS_MAX },
> > + .vco = { .min = IG4X_VCO_MIN, .max = IG4X_VCO_MAX},
> > + .n = { .min = IG4X_N_DUA_LVDS_MIN, .max =
> IG4X_N_DUA_LVDS_MAX },
> > + .m = { .min = IG4X_M_DUA_LVDS_MIN, .max =
> IG4X_M_DUA_LVDS_MAX },
> > + .m1 = { .min = IG4X_M1_DUA_LVDS_MIN, .max =
> IG4X_M1_DUA_LVDS_MAX },
> > + .m2 = { .min = IG4X_M2_DUA_LVDS_MIN, .max =
> IG4X_M2_DUA_LVDS_MAX },
> > + .p = { .min = IG4X_P_DUA_LVDS_MIN, .max =
> IG4X_P_DUA_LVDS_MAX },
> > + .p1 = { .min = IG4X_P1_DUA_LVDS_MIN, .max =
> IG4X_P1_DUA_LVDS_MAX},
> > + .p2 = { .dot_limit = IG4X_P2_DUA_LVDS_LIMIT,
> > + .p2_slow = IG4X_P2_DUA_LVDS_SLOW,
> > + .p2_fast = IG4X_P2_DUA_LVDS_FAST
> > + },
> > + },
> > };
> >
> > +static intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
> > +{
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + const intel_limit_t *limit;
> > +
> > + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +
> > + if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
> > + LVDS_CLKB_POWER_UP)
> > + /*LVDS with dual channel*/
>
> Better formatted as /* LVDS with dual channel */
done
>
> > + limit = &intel_limits[INTEL_LIMIT_IG4X_DUA_LVDS];
> > + else
> > + /*LVDS with dual channel*/
> > + limit = &intel_limits[INTEL_LIMIT_IG4X_SIN_LVDS];
> > + } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI) ||
> > + intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG)) {
> > +
> > + limit = &intel_limits[INTEL_LIMIT_IG4X_HDMI_DAC];
> > +
> > + } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
> > +
> > + limit = &intel_limits[INTEL_LIMIT_IG4X_SDVO];
> > + } else /*The option is for other outputs*/
> > +
> > + limit = &intel_limits[INTEL_LIMIT_I9XX_SDVO_DAC];
>
> gratuitous newlines in these blocks, please clean it up.
>
> > + return limit;
> > +}
> > +
> > static const intel_limit_t *intel_limit(struct drm_crtc *crtc)
> > {
> > struct drm_device *dev = crtc->dev;
> > const intel_limit_t *limit;
> >
> > - if (IS_I9XX(dev)) {
> > + if (IS_G4X(dev)){
>
> space between ')' and '{'
done
>
> >
> > + limit = intel_g4x_limit(crtc);
> > + }else if (IS_I9XX(dev)) {
>
> space between '}' and "else"
done
>
> >
> > +
> > if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> > limit = &intel_limits[INTEL_LIMIT_I9XX_LVDS];
> > else
More information about the Intel-gfx
mailing list