All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 6/9] drm/i915: Enable DisplayPort in Valleyview
Date: Fri, 28 Sep 2012 17:03:59 +0200	[thread overview]
Message-ID: <20120928150359.GM2098@bremse> (raw)
In-Reply-To: <20120927082337.1cac0574@jbarnes-desktop>

On Thu, Sep 27, 2012 at 08:23:37AM -0700, Jesse Barnes wrote:
> On Thu, 27 Sep 2012 19:13:06 +0530
> Vijay Purushothaman <vijay.a.purushothaman@intel.com> wrote:
> 
> > In valleyview voltageswing, pre-emphasis and lane control registers can
> > be programmed only through the h/w side band fabric.
> > 
> > Cleaned up DPLL calculations for Valleyview to support multi display
> > configurations.
> > 
> > v2: Based on Daniel's feedbacak, moved crt hotplug detect work around as separate
> > patch. Also moved i9xx_update_pll_dividers to i8xx_update_pll and
> > i9xx_update_pll.
> > 
> > Signed-off-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
> > Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>

Small bikeshed: I've killed some spurious whitespace changs while
applying ...
-Daniel

> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |    8 +--
> >  drivers/gpu/drm/i915/intel_display.c |   90 ++++++++++++++++++++++++----------
> >  2 files changed, 66 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 3f75ee6..0fe4aad 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -385,12 +385,8 @@
> >  
> >  #define DPIO_FASTCLK_DISABLE		0x8100
> >  
> > -#define _DPIO_DATA_LANE0		0x0220
> > -#define _DPIO_DATA_LANE1		0x0420
> > -#define _DPIO_DATA_LANE2		0x2620
> > -#define _DPIO_DATA_LANE3		0x2820
> > -#define DPIO_DATA_LANE_A(pipe) _PIPE(pipe, _DPIO_DATA_LANE0, _DPIO_DATA_LANE2)
> > -#define DPIO_DATA_LANE_B(pipe) _PIPE(pipe, _DPIO_DATA_LANE1, _DPIO_DATA_LANE3)
> > +#define DPIO_DATA_CHANNEL1		0x8220
> > +#define DPIO_DATA_CHANNEL2		0x8420
> >  
> >  /*
> >   * Fence registers
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 68828e7..ed749c4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4017,7 +4017,7 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >  			   struct drm_display_mode *mode,
> >  			   struct drm_display_mode *adjusted_mode,
> >  			   intel_clock_t *clock, intel_clock_t *reduced_clock,
> > -			   int refclk, int num_connectors)
> > +			   int num_connectors)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -4025,9 +4025,19 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >  	int pipe = intel_crtc->pipe;
> >  	u32 dpll, mdiv, pdiv;
> >  	u32 bestn, bestm1, bestm2, bestp1, bestp2;
> > -	bool is_hdmi;
> > +	bool is_sdvo;
> > +	u32 temp;
> > +
> > +	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> >  
> > -	is_hdmi = intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> > +	dpll = DPLL_VGA_MODE_DIS;
> > +	dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> > +	dpll |= DPLL_REFA_CLK_ENABLE_VLV;
> > +	dpll |= DPLL_INTEGRATED_CLOCK_VLV;
> > +
> > +	I915_WRITE(DPLL(pipe), dpll);
> > +	POSTING_READ(DPLL(pipe));
> >  
> >  	bestn = clock->n;
> >  	bestm1 = clock->m1;
> > @@ -4035,12 +4045,10 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >  	bestp1 = clock->p1;
> >  	bestp2 = clock->p2;
> >  
> > -	/* Enable DPIO clock input */
> > -	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
> > -		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
> > -	I915_WRITE(DPLL(pipe), dpll);
> > -	POSTING_READ(DPLL(pipe));
> > -
> > +	/*
> > +	 * In Valleyview PLL and program lane counter registers are exposed
> > +	 * through DPIO interface
> > +	 */
> >  	mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
> >  	mdiv |= ((bestp1 << DPIO_P1_SHIFT) | (bestp2 << DPIO_P2_SHIFT));
> >  	mdiv |= ((bestn << DPIO_N_SHIFT));
> > @@ -4051,12 +4059,13 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >  
> >  	intel_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), 0x01000000);
> >  
> > -	pdiv = DPIO_REFSEL_OVERRIDE | (5 << DPIO_PLL_MODESEL_SHIFT) |
> > +	pdiv = (1 << DPIO_REFSEL_OVERRIDE) | (5 << DPIO_PLL_MODESEL_SHIFT) |
> >  		(3 << DPIO_BIAS_CURRENT_CTL_SHIFT) | (1<<20) |
> > -		(8 << DPIO_DRIVER_CTL_SHIFT) | (5 << DPIO_CLK_BIAS_CTL_SHIFT);
> > +		(7 << DPIO_PLL_REFCLK_SEL_SHIFT) | (8 << DPIO_DRIVER_CTL_SHIFT) |
> > +		(5 << DPIO_CLK_BIAS_CTL_SHIFT);
> >  	intel_dpio_write(dev_priv, DPIO_REFSFR(pipe), pdiv);
> >  
> > -	intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x009f0051);
> > +	intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x005f003b);
> >  
> >  	dpll |= DPLL_VCO_ENABLE;
> >  	I915_WRITE(DPLL(pipe), dpll);
> > @@ -4064,21 +4073,47 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >  	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> >  		DRM_ERROR("DPLL %d failed to lock\n", pipe);
> >  
> > -	if (is_hdmi) {
> > -		u32 temp = intel_mode_get_pixel_multiplier(adjusted_mode);
> > +	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
> > +
> > +	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> > +		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> > +
> > +	I915_WRITE(DPLL(pipe), dpll);
> > +
> > +	/* Wait for the clocks to stabilize. */
> > +	POSTING_READ(DPLL(pipe));
> > +	udelay(150);
> >  
> > +	temp = 0;
> > +	if (is_sdvo) {
> > +		temp = intel_mode_get_pixel_multiplier(adjusted_mode);
> >  		if (temp > 1)
> >  			temp = (temp - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> >  		else
> >  			temp = 0;
> > -
> > -		I915_WRITE(DPLL_MD(pipe), temp);
> > -		POSTING_READ(DPLL_MD(pipe));
> >  	}
> > +	I915_WRITE(DPLL_MD(pipe), temp);
> > +	POSTING_READ(DPLL_MD(pipe));
> >  
> > -	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x641); /* ??? */
> > +	/* Now program lane control registers */
> > +	if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> > +			|| intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> > +	{
> > +		temp = 0x1000C4;
> > +		if(pipe == 1)
> > +			temp |= (1 << 21);
> > +		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
> > +	}
> > +	if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> > +	{
> > +		temp = 0x1000C4;
> > +		if(pipe == 1)
> > +			temp |= (1 << 21);
> > +		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL2, temp);
> > +	}
> >  }
> >  
> > +
> >  static void i9xx_update_pll(struct drm_crtc *crtc,
> >  			    struct drm_display_mode *mode,
> >  			    struct drm_display_mode *adjusted_mode,
> > @@ -4092,9 +4127,10 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> >  	u32 dpll;
> >  	bool is_sdvo;
> >  
> > +	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> > +
> >  	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> >  		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> > -
> >  	dpll = DPLL_VGA_MODE_DIS;
> >  
> >  	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> > @@ -4192,7 +4228,7 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> >  
> >  static void i8xx_update_pll(struct drm_crtc *crtc,
> >  			    struct drm_display_mode *adjusted_mode,
> > -			    intel_clock_t *clock,
> > +			    intel_clock_t *clock, intel_clock_t *reduced_clock,
> >  			    int num_connectors)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> > @@ -4201,6 +4237,8 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
> >  	int pipe = intel_crtc->pipe;
> >  	u32 dpll;
> >  
> > +	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> > +
> >  	dpll = DPLL_VGA_MODE_DIS;
> >  
> >  	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> > @@ -4327,14 +4365,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  	if (is_sdvo && is_tv)
> >  		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> >  
> > -	i9xx_update_pll_dividers(crtc, &clock, has_reduced_clock ?
> > -				 &reduced_clock : NULL);
> > -
> >  	if (IS_GEN2(dev))
> > -		i8xx_update_pll(crtc, adjusted_mode, &clock, num_connectors);
> > +		i8xx_update_pll(crtc, adjusted_mode, &clock,
> > +				has_reduced_clock ? &reduced_clock : NULL,
> > +				num_connectors);
> >  	else if (IS_VALLEYVIEW(dev))
> > -		vlv_update_pll(crtc, mode,adjusted_mode, &clock, NULL,
> > -			       refclk, num_connectors);
> > +		vlv_update_pll(crtc, mode, adjusted_mode, &clock,
> > +				has_reduced_clock ? &reduced_clock : NULL,
> > +				num_connectors);
> >  	else
> >  		i9xx_update_pll(crtc, mode, adjusted_mode, &clock,
> >  				has_reduced_clock ? &reduced_clock : NULL,
> 
> Looks good.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2012-09-28 15:04 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-27 13:43 [PATCH v2 0/9] Enable all display interfaces in Valleyview Vijay Purushothaman
2012-09-27 13:43 ` [PATCH v2 1/9] drm/i915: Set aux clk to 100MHz for Valleyview Vijay Purushothaman
2012-09-27 15:11   ` Jesse Barnes
2012-09-27 13:43 ` [PATCH v2 2/9] drm/i915: Fix SDVO IER and status bits " Vijay Purushothaman
2012-09-27 15:13   ` Jesse Barnes
2012-09-27 13:43 ` [PATCH v2 3/9] drm/i915: Add Valleyview lane control definitions Vijay Purushothaman
2012-09-27 15:17   ` Jesse Barnes
2012-09-27 13:43 ` [PATCH v2 4/9] drm/i915: Program correct m n tu register for Valleyview Vijay Purushothaman
2012-09-27 15:18   ` Jesse Barnes
2012-09-27 13:43 ` [PATCH v2 5/9] drm/i915: Disable CRT hotplug detection for valleyview Vijay Purushothaman
2012-09-27 15:20   ` Jesse Barnes
2012-09-28 14:51     ` Daniel Vetter
2012-09-27 15:34   ` Jesse Barnes
2012-09-27 13:43 ` [PATCH v2 6/9] drm/i915: Enable DisplayPort in Valleyview Vijay Purushothaman
2012-09-27 15:23   ` Jesse Barnes
2012-09-28 15:03     ` Daniel Vetter [this message]
2012-09-27 13:43 ` [PATCH v2 7/9] drm/i915: Add eDP support for Valleyview Vijay Purushothaman
2012-09-27 15:24   ` Jesse Barnes
2012-09-28 15:08     ` Daniel Vetter
2012-09-27 13:43 ` [PATCH v2 8/9] drm/i915: panel power sequencing for VLV eDP Vijay Purushothaman
2012-09-27 15:26   ` Jesse Barnes
2012-09-27 16:59     ` Daniel Vetter
2012-09-27 13:43 ` [PATCH v2 9/9] drm/i915: Fixup HDMI output on Valleyview Vijay Purushothaman
2012-09-27 15:26   ` Jesse Barnes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120928150359.GM2098@bremse \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jbarnes@virtuousgeek.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.