[Intel-gfx] [PATCH 1/2]intel_display.c:define related parameters and limit items for G4x platform

Eric Anholt eric at anholt.net
Fri Mar 6 21:38:02 CET 2009


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.

> ---
>  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/

>  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.

> +	.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 */

> +			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 '{'

> 
> +	    limit = intel_g4x_limit(crtc);
> +	}else if (IS_I9XX(dev)) {

space between '}' and "else"

> 
> +
>  		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits[INTEL_LIMIT_I9XX_LVDS];
>  		else
-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090306/c996bd33/attachment.sig>


More information about the Intel-gfx mailing list