[05/14] drm/i915: convert PIPECONF to use transcoder instead of pipe
diff mbox

Message ID 1350595304-18237-6-git-send-email-przanoni@gmail.com
State New, archived
Headers show

Commit Message

Paulo Zanoni Oct. 18, 2012, 9:21 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because the PIPECONF register is actually part of the CPU transcoder,
not the CPU pipe.

Ideally we would also rename PIPECONF to TRANSCONF to remind people
that they should use the transcoder instead of the pipe, but let's
keep it like this for now since most Gens still name it PIPECONF.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      |  4 ++-
 drivers/gpu/drm/i915/i915_reg.h      |  2 +-
 drivers/gpu/drm/i915/intel_crt.c     |  6 ++--
 drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_sprite.c  |  3 +-
 drivers/gpu/drm/i915/intel_tv.c      |  4 +--
 6 files changed, 49 insertions(+), 31 deletions(-)

Comments

Daniel Vetter Oct. 18, 2012, 10:23 p.m. UTC | #1
On Thu, Oct 18, 2012 at 06:21:35PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Because the PIPECONF register is actually part of the CPU transcoder,
> not the CPU pipe.
> 
> Ideally we would also rename PIPECONF to TRANSCONF to remind people
> that they should use the transcoder instead of the pipe, but let's
> keep it like this for now since most Gens still name it PIPECONF.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I think as a general rule it makes sense to not convert codepaths that are
never run on haswell to cpu_transcoder, since that thing really doesn't
exist that much on earlier platforms. Also, it makes the patch smaller ;-)

Below some comments about which hunks I think we can drop.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c      |  4 ++-
>  drivers/gpu/drm/i915/i915_reg.h      |  2 +-
>  drivers/gpu/drm/i915/intel_crt.c     |  6 ++--
>  drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_sprite.c  |  3 +-
>  drivers/gpu/drm/i915/intel_tv.c      |  4 +--
>  6 files changed, 49 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d07c787..c9b186d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -123,7 +123,9 @@ static int
>  i915_pipe_enabled(struct drm_device *dev, int pipe)
>  {
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> -	return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
> +
> +	return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE;
>  }

Oh, how I hate our vblank code and it's insistency to deal with pipes
instead of crtc numbers.

>  
>  /* Called from drm generic code, passed a 'crtc', which
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 72a61b5..9fecd3b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2714,7 +2714,7 @@
>  #define   PIPE_12BPC				(3 << 5)
>  
>  #define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC)
> -#define PIPECONF(pipe) _PIPE(pipe, _PIPEACONF, _PIPEBCONF)
> +#define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF)
>  #define PIPEDSL(pipe)  _PIPE(pipe, _PIPEADSL, _PIPEBDSL)
>  #define PIPEFRAME(pipe) _PIPE(pipe, _PIPEAFRAMEHIGH, _PIPEBFRAMEHIGH)
>  #define PIPEFRAMEPIXEL(pipe)  _PIPE(pipe, _PIPEAFRAMEPIXEL, _PIPEBFRAMEPIXEL)
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 53f3e87..2a2c976 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -467,7 +467,9 @@ intel_crt_load_detect(struct intel_crt *crt)
>  {
>  	struct drm_device *dev = crt->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t pipe = to_intel_crtc(crt->base.base.crtc)->pipe;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crt->base.base.crtc);
> +	enum pipe pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	uint32_t save_bclrpat;
>  	uint32_t save_vtotal;
>  	uint32_t vtotal, vactive;
> @@ -489,7 +491,7 @@ intel_crt_load_detect(struct intel_crt *crt)
>  	vtotal_reg = VTOTAL(pipe);
>  	vblank_reg = VBLANK(pipe);
>  	vsync_reg = VSYNC(pipe);
> -	pipeconf_reg = PIPECONF(pipe);
> +	pipeconf_reg = PIPECONF(cpu_transcoder);
>  	pipe_dsl_reg = PIPEDSL(pipe);
>  
>  	save_bclrpat = I915_READ(bclrpat_reg);

Load detect is only used by gen2/3 vga connectors and by the tv out
connector, nothing else. So I think we can just leave this as-is.

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 827c5ba..dc93c39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1008,9 +1008,10 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
>  void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> -		int reg = PIPECONF(pipe);
> +		int reg = PIPECONF(cpu_transcoder);
>  
>  		/* Wait for the Pipe State to go off */
>  		if (wait_for((I915_READ(reg) & I965_PIPECONF_ACTIVE) == 0,
> @@ -1222,12 +1223,13 @@ void assert_pipe(struct drm_i915_private *dev_priv,
>  	int reg;
>  	u32 val;
>  	bool cur_state;
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>  
>  	/* if we need the pipe A quirk it must be always on */
>  	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
>  		state = true;
>  
> -	reg = PIPECONF(pipe);
> +	reg = PIPECONF(cpu_transcoder);
>  	val = I915_READ(reg);
>  	cur_state = !!(val & PIPECONF_ENABLE);
>  	WARN(cur_state != state,
> @@ -1661,6 +1663,7 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
>  	int reg;
>  	u32 val, pipeconf_val;
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>  
>  	/* PCH only available on ILK+ */
>  	BUG_ON(dev_priv->info->gen < 5);

Do we really need this on hsw for the pch vga port? Since the only pch
port we have is vga, and that is currently restricted to pipe 0 I'd prefer
if we fix this up once we really put the code to some use (i.e. lift the
pipe == 0 restriction).

> @@ -1680,7 +1683,7 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
>  	}
>  	reg = TRANSCONF(pipe);
>  	val = I915_READ(reg);
> -	pipeconf_val = I915_READ(PIPECONF(pipe));
> +	pipeconf_val = I915_READ(PIPECONF(cpu_transcoder));
>  
>  	if (HAS_PCH_IBX(dev_priv->dev)) {
>  		/*
> @@ -1745,6 +1748,7 @@ static void intel_disable_transcoder(struct drm_i915_private *dev_priv,
>  static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>  			      bool pch_port)
>  {
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>  	int reg;
>  	u32 val;
>  
> @@ -1764,7 +1768,7 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>  		/* FIXME: assert CPU port conditions for SNB+ */
>  	}
>  
> -	reg = PIPECONF(pipe);
> +	reg = PIPECONF(cpu_transcoder);
>  	val = I915_READ(reg);
>  	if (val & PIPECONF_ENABLE)
>  		return;
> @@ -1788,6 +1792,7 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>  static void intel_disable_pipe(struct drm_i915_private *dev_priv,
>  			       enum pipe pipe)
>  {
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>  	int reg;
>  	u32 val;
>  
> @@ -1801,7 +1806,7 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
>  	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
>  		return;
>  
> -	reg = PIPECONF(pipe);
> +	reg = PIPECONF(cpu_transcoder);
>  	val = I915_READ(reg);
>  	if ((val & PIPECONF_ENABLE) == 0)
>  		return;
> @@ -2679,6 +2684,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	u32 reg, temp;

Dito for all the fdi stuff here, I think that's better done when we
enable/fix-up VGA.

>  
>  	/* Write the TU size bits so error detection works */
> @@ -2690,7 +2696,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>  	temp = I915_READ(reg);
>  	temp &= ~((0x7 << 19) | (0x7 << 16));
>  	temp |= (intel_crtc->fdi_lanes - 1) << 19;
> -	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
> +	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>  
>  	POSTING_READ(reg);
> @@ -2764,6 +2770,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	u32 reg, temp;
>  
>  	/* disable CPU FDI tx and PCH FDI rx */
> @@ -2775,7 +2782,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>  	reg = FDI_RX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~(0x7 << 16);
> -	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
> +	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp & ~FDI_RX_ENABLE);
>  
>  	POSTING_READ(reg);
> @@ -2809,7 +2816,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>  	}
>  	/* BPC in FDI rx is consistent with that in PIPECONF */
>  	temp &= ~(0x07 << 16);
> -	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
> +	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp);
>  
>  	POSTING_READ(reg);
> @@ -2971,6 +2978,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	u32 reg, temp;
>  
>  	assert_transcoder_disabled(dev_priv, pipe);
> @@ -3027,7 +3035,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	if (HAS_PCH_CPT(dev) &&
>  	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>  	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
> -		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) >> 5;
> +		u32 bpc = (I915_READ(PIPECONF(cpu_transcoder)) &
> +			   PIPE_BPC_MASK) >> 5;
>  		reg = TRANS_DP_CTL(pipe);
>  		temp = I915_READ(reg);
>  		temp &= ~(TRANS_DP_PORT_SEL_MASK |
> @@ -4383,6 +4392,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
>  	u32 dspcntr, pipeconf;
> @@ -4463,7 +4473,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  				num_connectors);

i9xx stuff, I think pipeconf is totally ok for these ;-)

>  
>  	/* setup pipeconf */
> -	pipeconf = I915_READ(PIPECONF(pipe));
> +	pipeconf = I915_READ(PIPECONF(cpu_transcoder));
>  
>  	/* Set up the display plane register */
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
> @@ -4535,8 +4545,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		   (mode->hdisplay - 1));
>  	I915_WRITE(DSPPOS(plane), 0);
>  
> -	I915_WRITE(PIPECONF(pipe), pipeconf);
> -	POSTING_READ(PIPECONF(pipe));
> +	I915_WRITE(PIPECONF(cpu_transcoder), pipeconf);
> +	POSTING_READ(PIPECONF(cpu_transcoder));
>  	intel_enable_pipe(dev_priv, pipe, false);
>  
>  	intel_wait_for_vblank(dev, pipe);
> @@ -4704,10 +4714,10 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	uint32_t val;

ilk_set_pipeconf is not called on hsw, we have the special version now.

>  
> -	val = I915_READ(PIPECONF(pipe));
> +	val = I915_READ(PIPECONF(cpu_transcoder));
>  
>  	val &= ~PIPE_BPC_MASK;
>  	switch (intel_crtc->bpp) {
> @@ -4738,8 +4748,8 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
>  
> -	I915_WRITE(PIPECONF(pipe), val);
> -	POSTING_READ(PIPECONF(pipe));
> +	I915_WRITE(PIPECONF(cpu_transcoder), val);
> +	POSTING_READ(PIPECONF(cpu_transcoder));
>  }
>  
>  static void haswell_set_pipeconf(struct drm_crtc *crtc,
> @@ -4748,10 +4758,10 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	uint32_t val;
>  
> -	val = I915_READ(PIPECONF(pipe));
> +	val = I915_READ(PIPECONF(cpu_transcoder));
>  
>  	val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
>  	if (dither)
> @@ -4763,8 +4773,8 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
>  
> -	I915_WRITE(PIPECONF(pipe), val);
> -	POSTING_READ(PIPECONF(pipe));
> +	I915_WRITE(PIPECONF(cpu_transcoder), val);
> +	POSTING_READ(PIPECONF(cpu_transcoder));
>  }
>  
>  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
> @@ -5238,7 +5248,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
>  	     num_connectors, pipe_name(pipe));
>  
> -	WARN_ON(I915_READ(PIPECONF(pipe)) &
> +	WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
>  		(PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
>  
>  	WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
> @@ -8416,7 +8426,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  	u32 reg, val;
>  
>  	/* Clear any frame start delays used for debugging left by the BIOS */
> -	reg = PIPECONF(crtc->pipe);
> +	reg = PIPECONF(crtc->cpu_transcoder);
>  	I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>  
>  	/* We need to sanitize the plane -> pipe mapping first because this will
> @@ -8579,7 +8589,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev)
>  	for_each_pipe(pipe) {
>  		crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -		tmp = I915_READ(PIPECONF(pipe));
> +		tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
>  		if (tmp & PIPECONF_ENABLE)
>  			crtc->active = true;
>  		else
> @@ -8773,6 +8783,7 @@ intel_display_capture_error_state(struct drm_device *dev)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	struct intel_display_error_state *error;
> +	enum transcoder cpu_transcoder;
>  	int i;
>  
>  	error = kmalloc(sizeof(*error), GFP_ATOMIC);
> @@ -8780,6 +8791,8 @@ intel_display_capture_error_state(struct drm_device *dev)
>  		return NULL;
>  
>  	for_each_pipe(i) {
> +		cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, i);
> +
>  		error->cursor[i].control = I915_READ(CURCNTR(i));
>  		error->cursor[i].position = I915_READ(CURPOS(i));
>  		error->cursor[i].base = I915_READ(CURBASE(i));
> @@ -8794,7 +8807,7 @@ intel_display_capture_error_state(struct drm_device *dev)
>  			error->plane[i].tile_offset = I915_READ(DSPTILEOFF(i));
>  		}
>  
> -		error->pipe[i].conf = I915_READ(PIPECONF(i));
> +		error->pipe[i].conf = I915_READ(PIPECONF(cpu_transcoder));
>  		error->pipe[i].source = I915_READ(PIPESRC(i));
>  		error->pipe[i].htotal = I915_READ(HTOTAL(i));
>  		error->pipe[i].hblank = I915_READ(HBLANK(i));
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 7644f31..f2ca943 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -422,6 +422,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	struct intel_framebuffer *intel_fb;
>  	struct drm_i915_gem_object *obj, *old_obj;
>  	int pipe = intel_plane->pipe;
> +	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);

I think we can leave that until we enable sprite support on hsw, too.

>  	int ret = 0;
>  	int x = src_x >> 16, y = src_y >> 16;
>  	int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
> @@ -436,7 +437,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	src_h = src_h >> 16;
>  
>  	/* Pipe must be running... */
> -	if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
> +	if (!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE))
>  		return -EINVAL;
>  
>  	if (crtc_x >= primary_w || crtc_y >= primary_h)
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index d2c5c8f..fcdbb66 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -941,7 +941,7 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
>  	const struct video_levels *video_levels;
>  	const struct color_conversion *color_conversion;
>  	bool burst_ena;
> -	int pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;

TV-out is non-existing on hsw ...
>  
>  	if (!tv_mode)
>  		return;	/* can't happen (mode_prepare prevents this) */
> @@ -1085,7 +1085,7 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
>  			   ((video_levels->black << TV_BLACK_LEVEL_SHIFT) |
>  			    (video_levels->blank << TV_BLANK_LEVEL_SHIFT)));
>  	{
> -		int pipeconf_reg = PIPECONF(pipe);
> +		int pipeconf_reg = PIPECONF(cpu_transcoder);
>  		int dspcntr_reg = DSPCNTR(intel_crtc->plane);
>  		int pipeconf = I915_READ(pipeconf_reg);
>  		int dspcntr = I915_READ(dspcntr_reg);
> -- 
> 1.7.11.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Paulo Zanoni Oct. 19, 2012, 6:36 p.m. UTC | #2
Hi

2012/10/18 Daniel Vetter <daniel@ffwll.ch>:
> On Thu, Oct 18, 2012 at 06:21:35PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> Because the PIPECONF register is actually part of the CPU transcoder,
>> not the CPU pipe.
>>
>> Ideally we would also rename PIPECONF to TRANSCONF to remind people
>> that they should use the transcoder instead of the pipe, but let's
>> keep it like this for now since most Gens still name it PIPECONF.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> I think as a general rule it makes sense to not convert codepaths that are
> never run on haswell to cpu_transcoder, since that thing really doesn't
> exist that much on earlier platforms. Also, it makes the patch smaller ;-)

Just making the discussion public...

I also thought about that, but my fear is that as we move code from
one place to another, in the future we might end moving/copying some
code and making Haswell use pipe instead of transcoder.

I will write the smaller version with only haswell-specific bits and send V2.

>
> Below some comments about which hunks I think we can drop.
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c      |  4 ++-
>>  drivers/gpu/drm/i915/i915_reg.h      |  2 +-
>>  drivers/gpu/drm/i915/intel_crt.c     |  6 ++--
>>  drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++++++++--------------
>>  drivers/gpu/drm/i915/intel_sprite.c  |  3 +-
>>  drivers/gpu/drm/i915/intel_tv.c      |  4 +--
>>  6 files changed, 49 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index d07c787..c9b186d 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -123,7 +123,9 @@ static int
>>  i915_pipe_enabled(struct drm_device *dev, int pipe)
>>  {
>>       drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>> -     return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>> +
>> +     return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE;
>>  }
>
> Oh, how I hate our vblank code and it's insistency to deal with pipes
> instead of crtc numbers.
>
>>
>>  /* Called from drm generic code, passed a 'crtc', which
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 72a61b5..9fecd3b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2714,7 +2714,7 @@
>>  #define   PIPE_12BPC                         (3 << 5)
>>
>>  #define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC)
>> -#define PIPECONF(pipe) _PIPE(pipe, _PIPEACONF, _PIPEBCONF)
>> +#define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF)
>>  #define PIPEDSL(pipe)  _PIPE(pipe, _PIPEADSL, _PIPEBDSL)
>>  #define PIPEFRAME(pipe) _PIPE(pipe, _PIPEAFRAMEHIGH, _PIPEBFRAMEHIGH)
>>  #define PIPEFRAMEPIXEL(pipe)  _PIPE(pipe, _PIPEAFRAMEPIXEL, _PIPEBFRAMEPIXEL)
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index 53f3e87..2a2c976 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -467,7 +467,9 @@ intel_crt_load_detect(struct intel_crt *crt)
>>  {
>>       struct drm_device *dev = crt->base.base.dev;
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> -     uint32_t pipe = to_intel_crtc(crt->base.base.crtc)->pipe;
>> +     struct intel_crtc *intel_crtc = to_intel_crtc(crt->base.base.crtc);
>> +     enum pipe pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       uint32_t save_bclrpat;
>>       uint32_t save_vtotal;
>>       uint32_t vtotal, vactive;
>> @@ -489,7 +491,7 @@ intel_crt_load_detect(struct intel_crt *crt)
>>       vtotal_reg = VTOTAL(pipe);
>>       vblank_reg = VBLANK(pipe);
>>       vsync_reg = VSYNC(pipe);
>> -     pipeconf_reg = PIPECONF(pipe);
>> +     pipeconf_reg = PIPECONF(cpu_transcoder);
>>       pipe_dsl_reg = PIPEDSL(pipe);
>>
>>       save_bclrpat = I915_READ(bclrpat_reg);
>
> Load detect is only used by gen2/3 vga connectors and by the tv out
> connector, nothing else. So I think we can just leave this as-is.
>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 827c5ba..dc93c39 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -1008,9 +1008,10 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
>>  void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>>
>>       if (INTEL_INFO(dev)->gen >= 4) {
>> -             int reg = PIPECONF(pipe);
>> +             int reg = PIPECONF(cpu_transcoder);
>>
>>               /* Wait for the Pipe State to go off */
>>               if (wait_for((I915_READ(reg) & I965_PIPECONF_ACTIVE) == 0,
>> @@ -1222,12 +1223,13 @@ void assert_pipe(struct drm_i915_private *dev_priv,
>>       int reg;
>>       u32 val;
>>       bool cur_state;
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>>
>>       /* if we need the pipe A quirk it must be always on */
>>       if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
>>               state = true;
>>
>> -     reg = PIPECONF(pipe);
>> +     reg = PIPECONF(cpu_transcoder);
>>       val = I915_READ(reg);
>>       cur_state = !!(val & PIPECONF_ENABLE);
>>       WARN(cur_state != state,
>> @@ -1661,6 +1663,7 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
>>       int reg;
>>       u32 val, pipeconf_val;
>>       struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>>
>>       /* PCH only available on ILK+ */
>>       BUG_ON(dev_priv->info->gen < 5);
>
> Do we really need this on hsw for the pch vga port? Since the only pch
> port we have is vga, and that is currently restricted to pipe 0 I'd prefer
> if we fix this up once we really put the code to some use (i.e. lift the
> pipe == 0 restriction).
>
>> @@ -1680,7 +1683,7 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
>>       }
>>       reg = TRANSCONF(pipe);
>>       val = I915_READ(reg);
>> -     pipeconf_val = I915_READ(PIPECONF(pipe));
>> +     pipeconf_val = I915_READ(PIPECONF(cpu_transcoder));
>>
>>       if (HAS_PCH_IBX(dev_priv->dev)) {
>>               /*
>> @@ -1745,6 +1748,7 @@ static void intel_disable_transcoder(struct drm_i915_private *dev_priv,
>>  static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>>                             bool pch_port)
>>  {
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>>       int reg;
>>       u32 val;
>>
>> @@ -1764,7 +1768,7 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>>               /* FIXME: assert CPU port conditions for SNB+ */
>>       }
>>
>> -     reg = PIPECONF(pipe);
>> +     reg = PIPECONF(cpu_transcoder);
>>       val = I915_READ(reg);
>>       if (val & PIPECONF_ENABLE)
>>               return;
>> @@ -1788,6 +1792,7 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>>  static void intel_disable_pipe(struct drm_i915_private *dev_priv,
>>                              enum pipe pipe)
>>  {
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>>       int reg;
>>       u32 val;
>>
>> @@ -1801,7 +1806,7 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
>>       if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
>>               return;
>>
>> -     reg = PIPECONF(pipe);
>> +     reg = PIPECONF(cpu_transcoder);
>>       val = I915_READ(reg);
>>       if ((val & PIPECONF_ENABLE) == 0)
>>               return;
>> @@ -2679,6 +2684,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>>       struct drm_device *dev = intel_crtc->base.dev;
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       u32 reg, temp;
>
> Dito for all the fdi stuff here, I think that's better done when we
> enable/fix-up VGA.
>
>>
>>       /* Write the TU size bits so error detection works */
>> @@ -2690,7 +2696,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>>       temp = I915_READ(reg);
>>       temp &= ~((0x7 << 19) | (0x7 << 16));
>>       temp |= (intel_crtc->fdi_lanes - 1) << 19;
>> -     temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
>> +     temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>>       I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>>
>>       POSTING_READ(reg);
>> @@ -2764,6 +2770,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>       int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       u32 reg, temp;
>>
>>       /* disable CPU FDI tx and PCH FDI rx */
>> @@ -2775,7 +2782,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>>       reg = FDI_RX_CTL(pipe);
>>       temp = I915_READ(reg);
>>       temp &= ~(0x7 << 16);
>> -     temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
>> +     temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>>       I915_WRITE(reg, temp & ~FDI_RX_ENABLE);
>>
>>       POSTING_READ(reg);
>> @@ -2809,7 +2816,7 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
>>       }
>>       /* BPC in FDI rx is consistent with that in PIPECONF */
>>       temp &= ~(0x07 << 16);
>> -     temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
>> +     temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
>>       I915_WRITE(reg, temp);
>>
>>       POSTING_READ(reg);
>> @@ -2971,6 +2978,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>       int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       u32 reg, temp;
>>
>>       assert_transcoder_disabled(dev_priv, pipe);
>> @@ -3027,7 +3035,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>>       if (HAS_PCH_CPT(dev) &&
>>           (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
>>            intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
>> -             u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) >> 5;
>> +             u32 bpc = (I915_READ(PIPECONF(cpu_transcoder)) &
>> +                        PIPE_BPC_MASK) >> 5;
>>               reg = TRANS_DP_CTL(pipe);
>>               temp = I915_READ(reg);
>>               temp &= ~(TRANS_DP_PORT_SEL_MASK |
>> @@ -4383,6 +4392,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>       int pipe = intel_crtc->pipe;
>>       int plane = intel_crtc->plane;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       int refclk, num_connectors = 0;
>>       intel_clock_t clock, reduced_clock;
>>       u32 dspcntr, pipeconf;
>> @@ -4463,7 +4473,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>>                               num_connectors);
>
> i9xx stuff, I think pipeconf is totally ok for these ;-)
>
>>
>>       /* setup pipeconf */
>> -     pipeconf = I915_READ(PIPECONF(pipe));
>> +     pipeconf = I915_READ(PIPECONF(cpu_transcoder));
>>
>>       /* Set up the display plane register */
>>       dspcntr = DISPPLANE_GAMMA_ENABLE;
>> @@ -4535,8 +4545,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>>                  (mode->hdisplay - 1));
>>       I915_WRITE(DSPPOS(plane), 0);
>>
>> -     I915_WRITE(PIPECONF(pipe), pipeconf);
>> -     POSTING_READ(PIPECONF(pipe));
>> +     I915_WRITE(PIPECONF(cpu_transcoder), pipeconf);
>> +     POSTING_READ(PIPECONF(cpu_transcoder));
>>       intel_enable_pipe(dev_priv, pipe, false);
>>
>>       intel_wait_for_vblank(dev, pipe);
>> @@ -4704,10 +4714,10 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>>  {
>>       struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -     int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       uint32_t val;
>
> ilk_set_pipeconf is not called on hsw, we have the special version now.
>
>>
>> -     val = I915_READ(PIPECONF(pipe));
>> +     val = I915_READ(PIPECONF(cpu_transcoder));
>>
>>       val &= ~PIPE_BPC_MASK;
>>       switch (intel_crtc->bpp) {
>> @@ -4738,8 +4748,8 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>>       else
>>               val |= PIPECONF_PROGRESSIVE;
>>
>> -     I915_WRITE(PIPECONF(pipe), val);
>> -     POSTING_READ(PIPECONF(pipe));
>> +     I915_WRITE(PIPECONF(cpu_transcoder), val);
>> +     POSTING_READ(PIPECONF(cpu_transcoder));
>>  }
>>
>>  static void haswell_set_pipeconf(struct drm_crtc *crtc,
>> @@ -4748,10 +4758,10 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
>>  {
>>       struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -     int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>>       uint32_t val;
>>
>> -     val = I915_READ(PIPECONF(pipe));
>> +     val = I915_READ(PIPECONF(cpu_transcoder));
>>
>>       val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
>>       if (dither)
>> @@ -4763,8 +4773,8 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
>>       else
>>               val |= PIPECONF_PROGRESSIVE;
>>
>> -     I915_WRITE(PIPECONF(pipe), val);
>> -     POSTING_READ(PIPECONF(pipe));
>> +     I915_WRITE(PIPECONF(cpu_transcoder), val);
>> +     POSTING_READ(PIPECONF(cpu_transcoder));
>>  }
>>
>>  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>> @@ -5238,7 +5248,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>>       WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
>>            num_connectors, pipe_name(pipe));
>>
>> -     WARN_ON(I915_READ(PIPECONF(pipe)) &
>> +     WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
>>               (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
>>
>>       WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
>> @@ -8416,7 +8426,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>>       u32 reg, val;
>>
>>       /* Clear any frame start delays used for debugging left by the BIOS */
>> -     reg = PIPECONF(crtc->pipe);
>> +     reg = PIPECONF(crtc->cpu_transcoder);
>>       I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>>
>>       /* We need to sanitize the plane -> pipe mapping first because this will
>> @@ -8579,7 +8589,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev)
>>       for_each_pipe(pipe) {
>>               crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>>
>> -             tmp = I915_READ(PIPECONF(pipe));
>> +             tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
>>               if (tmp & PIPECONF_ENABLE)
>>                       crtc->active = true;
>>               else
>> @@ -8773,6 +8783,7 @@ intel_display_capture_error_state(struct drm_device *dev)
>>  {
>>       drm_i915_private_t *dev_priv = dev->dev_private;
>>       struct intel_display_error_state *error;
>> +     enum transcoder cpu_transcoder;
>>       int i;
>>
>>       error = kmalloc(sizeof(*error), GFP_ATOMIC);
>> @@ -8780,6 +8791,8 @@ intel_display_capture_error_state(struct drm_device *dev)
>>               return NULL;
>>
>>       for_each_pipe(i) {
>> +             cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, i);
>> +
>>               error->cursor[i].control = I915_READ(CURCNTR(i));
>>               error->cursor[i].position = I915_READ(CURPOS(i));
>>               error->cursor[i].base = I915_READ(CURBASE(i));
>> @@ -8794,7 +8807,7 @@ intel_display_capture_error_state(struct drm_device *dev)
>>                       error->plane[i].tile_offset = I915_READ(DSPTILEOFF(i));
>>               }
>>
>> -             error->pipe[i].conf = I915_READ(PIPECONF(i));
>> +             error->pipe[i].conf = I915_READ(PIPECONF(cpu_transcoder));
>>               error->pipe[i].source = I915_READ(PIPESRC(i));
>>               error->pipe[i].htotal = I915_READ(HTOTAL(i));
>>               error->pipe[i].hblank = I915_READ(HBLANK(i));
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index 7644f31..f2ca943 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -422,6 +422,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>>       struct intel_framebuffer *intel_fb;
>>       struct drm_i915_gem_object *obj, *old_obj;
>>       int pipe = intel_plane->pipe;
>> +     enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
>
> I think we can leave that until we enable sprite support on hsw, too.
>
>>       int ret = 0;
>>       int x = src_x >> 16, y = src_y >> 16;
>>       int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
>> @@ -436,7 +437,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>>       src_h = src_h >> 16;
>>
>>       /* Pipe must be running... */
>> -     if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
>> +     if (!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE))
>>               return -EINVAL;
>>
>>       if (crtc_x >= primary_w || crtc_y >= primary_h)
>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
>> index d2c5c8f..fcdbb66 100644
>> --- a/drivers/gpu/drm/i915/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/intel_tv.c
>> @@ -941,7 +941,7 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
>>       const struct video_levels *video_levels;
>>       const struct color_conversion *color_conversion;
>>       bool burst_ena;
>> -     int pipe = intel_crtc->pipe;
>> +     enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>
> TV-out is non-existing on hsw ...
>>
>>       if (!tv_mode)
>>               return; /* can't happen (mode_prepare prevents this) */
>> @@ -1085,7 +1085,7 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
>>                          ((video_levels->black << TV_BLACK_LEVEL_SHIFT) |
>>                           (video_levels->blank << TV_BLANK_LEVEL_SHIFT)));
>>       {
>> -             int pipeconf_reg = PIPECONF(pipe);
>> +             int pipeconf_reg = PIPECONF(cpu_transcoder);
>>               int dspcntr_reg = DSPCNTR(intel_crtc->plane);
>>               int pipeconf = I915_READ(pipeconf_reg);
>>               int dspcntr = I915_READ(dspcntr_reg);
>> --
>> 1.7.11.4
>>
>> _______________________________________________
>> 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

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d07c787..c9b186d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -123,7 +123,9 @@  static int
 i915_pipe_enabled(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
+
+	return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE;
 }
 
 /* Called from drm generic code, passed a 'crtc', which
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72a61b5..9fecd3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2714,7 +2714,7 @@ 
 #define   PIPE_12BPC				(3 << 5)
 
 #define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC)
-#define PIPECONF(pipe) _PIPE(pipe, _PIPEACONF, _PIPEBCONF)
+#define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF)
 #define PIPEDSL(pipe)  _PIPE(pipe, _PIPEADSL, _PIPEBDSL)
 #define PIPEFRAME(pipe) _PIPE(pipe, _PIPEAFRAMEHIGH, _PIPEBFRAMEHIGH)
 #define PIPEFRAMEPIXEL(pipe)  _PIPE(pipe, _PIPEAFRAMEPIXEL, _PIPEBFRAMEPIXEL)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 53f3e87..2a2c976 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -467,7 +467,9 @@  intel_crt_load_detect(struct intel_crt *crt)
 {
 	struct drm_device *dev = crt->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t pipe = to_intel_crtc(crt->base.base.crtc)->pipe;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crt->base.base.crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	uint32_t save_bclrpat;
 	uint32_t save_vtotal;
 	uint32_t vtotal, vactive;
@@ -489,7 +491,7 @@  intel_crt_load_detect(struct intel_crt *crt)
 	vtotal_reg = VTOTAL(pipe);
 	vblank_reg = VBLANK(pipe);
 	vsync_reg = VSYNC(pipe);
-	pipeconf_reg = PIPECONF(pipe);
+	pipeconf_reg = PIPECONF(cpu_transcoder);
 	pipe_dsl_reg = PIPEDSL(pipe);
 
 	save_bclrpat = I915_READ(bclrpat_reg);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 827c5ba..dc93c39 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1008,9 +1008,10 @@  void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 
 	if (INTEL_INFO(dev)->gen >= 4) {
-		int reg = PIPECONF(pipe);
+		int reg = PIPECONF(cpu_transcoder);
 
 		/* Wait for the Pipe State to go off */
 		if (wait_for((I915_READ(reg) & I965_PIPECONF_ACTIVE) == 0,
@@ -1222,12 +1223,13 @@  void assert_pipe(struct drm_i915_private *dev_priv,
 	int reg;
 	u32 val;
 	bool cur_state;
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 
 	/* if we need the pipe A quirk it must be always on */
 	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
 		state = true;
 
-	reg = PIPECONF(pipe);
+	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
 	cur_state = !!(val & PIPECONF_ENABLE);
 	WARN(cur_state != state,
@@ -1661,6 +1663,7 @@  static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
 	int reg;
 	u32 val, pipeconf_val;
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 
 	/* PCH only available on ILK+ */
 	BUG_ON(dev_priv->info->gen < 5);
@@ -1680,7 +1683,7 @@  static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
 	}
 	reg = TRANSCONF(pipe);
 	val = I915_READ(reg);
-	pipeconf_val = I915_READ(PIPECONF(pipe));
+	pipeconf_val = I915_READ(PIPECONF(cpu_transcoder));
 
 	if (HAS_PCH_IBX(dev_priv->dev)) {
 		/*
@@ -1745,6 +1748,7 @@  static void intel_disable_transcoder(struct drm_i915_private *dev_priv,
 static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
 			      bool pch_port)
 {
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 	int reg;
 	u32 val;
 
@@ -1764,7 +1768,7 @@  static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
 		/* FIXME: assert CPU port conditions for SNB+ */
 	}
 
-	reg = PIPECONF(pipe);
+	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
 	if (val & PIPECONF_ENABLE)
 		return;
@@ -1788,6 +1792,7 @@  static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
 static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 			       enum pipe pipe)
 {
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 	int reg;
 	u32 val;
 
@@ -1801,7 +1806,7 @@  static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
 		return;
 
-	reg = PIPECONF(pipe);
+	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
 	if ((val & PIPECONF_ENABLE) == 0)
 		return;
@@ -2679,6 +2684,7 @@  static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	u32 reg, temp;
 
 	/* Write the TU size bits so error detection works */
@@ -2690,7 +2696,7 @@  static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	temp = I915_READ(reg);
 	temp &= ~((0x7 << 19) | (0x7 << 16));
 	temp |= (intel_crtc->fdi_lanes - 1) << 19;
-	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
+	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
 	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
 
 	POSTING_READ(reg);
@@ -2764,6 +2770,7 @@  static void ironlake_fdi_disable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	u32 reg, temp;
 
 	/* disable CPU FDI tx and PCH FDI rx */
@@ -2775,7 +2782,7 @@  static void ironlake_fdi_disable(struct drm_crtc *crtc)
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(0x7 << 16);
-	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
+	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
 	I915_WRITE(reg, temp & ~FDI_RX_ENABLE);
 
 	POSTING_READ(reg);
@@ -2809,7 +2816,7 @@  static void ironlake_fdi_disable(struct drm_crtc *crtc)
 	}
 	/* BPC in FDI rx is consistent with that in PIPECONF */
 	temp &= ~(0x07 << 16);
-	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
+	temp |= (I915_READ(PIPECONF(cpu_transcoder)) & PIPE_BPC_MASK) << 11;
 	I915_WRITE(reg, temp);
 
 	POSTING_READ(reg);
@@ -2971,6 +2978,7 @@  static void ironlake_pch_enable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	u32 reg, temp;
 
 	assert_transcoder_disabled(dev_priv, pipe);
@@ -3027,7 +3035,8 @@  static void ironlake_pch_enable(struct drm_crtc *crtc)
 	if (HAS_PCH_CPT(dev) &&
 	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
 	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
-		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) >> 5;
+		u32 bpc = (I915_READ(PIPECONF(cpu_transcoder)) &
+			   PIPE_BPC_MASK) >> 5;
 		reg = TRANS_DP_CTL(pipe);
 		temp = I915_READ(reg);
 		temp &= ~(TRANS_DP_PORT_SEL_MASK |
@@ -4383,6 +4392,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	int refclk, num_connectors = 0;
 	intel_clock_t clock, reduced_clock;
 	u32 dspcntr, pipeconf;
@@ -4463,7 +4473,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 				num_connectors);
 
 	/* setup pipeconf */
-	pipeconf = I915_READ(PIPECONF(pipe));
+	pipeconf = I915_READ(PIPECONF(cpu_transcoder));
 
 	/* Set up the display plane register */
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
@@ -4535,8 +4545,8 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		   (mode->hdisplay - 1));
 	I915_WRITE(DSPPOS(plane), 0);
 
-	I915_WRITE(PIPECONF(pipe), pipeconf);
-	POSTING_READ(PIPECONF(pipe));
+	I915_WRITE(PIPECONF(cpu_transcoder), pipeconf);
+	POSTING_READ(PIPECONF(cpu_transcoder));
 	intel_enable_pipe(dev_priv, pipe, false);
 
 	intel_wait_for_vblank(dev, pipe);
@@ -4704,10 +4714,10 @@  static void ironlake_set_pipeconf(struct drm_crtc *crtc,
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	uint32_t val;
 
-	val = I915_READ(PIPECONF(pipe));
+	val = I915_READ(PIPECONF(cpu_transcoder));
 
 	val &= ~PIPE_BPC_MASK;
 	switch (intel_crtc->bpp) {
@@ -4738,8 +4748,8 @@  static void ironlake_set_pipeconf(struct drm_crtc *crtc,
 	else
 		val |= PIPECONF_PROGRESSIVE;
 
-	I915_WRITE(PIPECONF(pipe), val);
-	POSTING_READ(PIPECONF(pipe));
+	I915_WRITE(PIPECONF(cpu_transcoder), val);
+	POSTING_READ(PIPECONF(cpu_transcoder));
 }
 
 static void haswell_set_pipeconf(struct drm_crtc *crtc,
@@ -4748,10 +4758,10 @@  static void haswell_set_pipeconf(struct drm_crtc *crtc,
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 	uint32_t val;
 
-	val = I915_READ(PIPECONF(pipe));
+	val = I915_READ(PIPECONF(cpu_transcoder));
 
 	val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
 	if (dither)
@@ -4763,8 +4773,8 @@  static void haswell_set_pipeconf(struct drm_crtc *crtc,
 	else
 		val |= PIPECONF_PROGRESSIVE;
 
-	I915_WRITE(PIPECONF(pipe), val);
-	POSTING_READ(PIPECONF(pipe));
+	I915_WRITE(PIPECONF(cpu_transcoder), val);
+	POSTING_READ(PIPECONF(cpu_transcoder));
 }
 
 static bool ironlake_compute_clocks(struct drm_crtc *crtc,
@@ -5238,7 +5248,7 @@  static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
 	     num_connectors, pipe_name(pipe));
 
-	WARN_ON(I915_READ(PIPECONF(pipe)) &
+	WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
 		(PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
 
 	WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
@@ -8416,7 +8426,7 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 	u32 reg, val;
 
 	/* Clear any frame start delays used for debugging left by the BIOS */
-	reg = PIPECONF(crtc->pipe);
+	reg = PIPECONF(crtc->cpu_transcoder);
 	I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
@@ -8579,7 +8589,7 @@  void intel_modeset_setup_hw_state(struct drm_device *dev)
 	for_each_pipe(pipe) {
 		crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
-		tmp = I915_READ(PIPECONF(pipe));
+		tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
 		if (tmp & PIPECONF_ENABLE)
 			crtc->active = true;
 		else
@@ -8773,6 +8783,7 @@  intel_display_capture_error_state(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_display_error_state *error;
+	enum transcoder cpu_transcoder;
 	int i;
 
 	error = kmalloc(sizeof(*error), GFP_ATOMIC);
@@ -8780,6 +8791,8 @@  intel_display_capture_error_state(struct drm_device *dev)
 		return NULL;
 
 	for_each_pipe(i) {
+		cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, i);
+
 		error->cursor[i].control = I915_READ(CURCNTR(i));
 		error->cursor[i].position = I915_READ(CURPOS(i));
 		error->cursor[i].base = I915_READ(CURBASE(i));
@@ -8794,7 +8807,7 @@  intel_display_capture_error_state(struct drm_device *dev)
 			error->plane[i].tile_offset = I915_READ(DSPTILEOFF(i));
 		}
 
-		error->pipe[i].conf = I915_READ(PIPECONF(i));
+		error->pipe[i].conf = I915_READ(PIPECONF(cpu_transcoder));
 		error->pipe[i].source = I915_READ(PIPESRC(i));
 		error->pipe[i].htotal = I915_READ(HTOTAL(i));
 		error->pipe[i].hblank = I915_READ(HBLANK(i));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7644f31..f2ca943 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -422,6 +422,7 @@  intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj, *old_obj;
 	int pipe = intel_plane->pipe;
+	enum transcoder cpu_transcoder = pipe_to_cpu_transcoder(dev_priv, pipe);
 	int ret = 0;
 	int x = src_x >> 16, y = src_y >> 16;
 	int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
@@ -436,7 +437,7 @@  intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	src_h = src_h >> 16;
 
 	/* Pipe must be running... */
-	if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
+	if (!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE))
 		return -EINVAL;
 
 	if (crtc_x >= primary_w || crtc_y >= primary_h)
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index d2c5c8f..fcdbb66 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -941,7 +941,7 @@  intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	const struct video_levels *video_levels;
 	const struct color_conversion *color_conversion;
 	bool burst_ena;
-	int pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
 
 	if (!tv_mode)
 		return;	/* can't happen (mode_prepare prevents this) */
@@ -1085,7 +1085,7 @@  intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 			   ((video_levels->black << TV_BLACK_LEVEL_SHIFT) |
 			    (video_levels->blank << TV_BLANK_LEVEL_SHIFT)));
 	{
-		int pipeconf_reg = PIPECONF(pipe);
+		int pipeconf_reg = PIPECONF(cpu_transcoder);
 		int dspcntr_reg = DSPCNTR(intel_crtc->plane);
 		int pipeconf = I915_READ(pipeconf_reg);
 		int dspcntr = I915_READ(dspcntr_reg);