All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH] drm/i915: clean up PIPECONF bpc #defines
Date: Mon, 17 Dec 2012 13:12:07 +0200	[thread overview]
Message-ID: <87k3shdjaw.fsf@intel.com> (raw)
In-Reply-To: <1355739698-20392-1-git-send-email-daniel.vetter@ffwll.ch>

On Mon, 17 Dec 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Ilk+ somehow used #defines in near the PIPESTAT definitions, which
> decently confused me. Earlier platforms called it BPP instead of
> BPC. Clean this all up.

This patch does what it says on the box, so

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

*BUT* see a related comment below.

> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   15 +++++----------
>  drivers/gpu/drm/i915/intel_display.c |   28 ++++++++++++++--------------
>  2 files changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 062ca21..596d629 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2648,11 +2648,11 @@
>  #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
>  #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
>  #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
> -#define   PIPECONF_BPP_MASK	(0x000000e0)
> -#define   PIPECONF_BPP_8	(0<<5)
> -#define   PIPECONF_BPP_10	(1<<5)
> -#define   PIPECONF_BPP_6	(2<<5)
> -#define   PIPECONF_BPP_12	(3<<5)
> +#define   PIPECONF_BPC_MASK	(0x7 << 5)
> +#define   PIPECONF_8BPC		(0<<5)
> +#define   PIPECONF_10BPC	(1<<5)
> +#define   PIPECONF_6BPC		(2<<5)
> +#define   PIPECONF_12BPC	(3<<5)
>  #define   PIPECONF_DITHER_EN	(1<<4)
>  #define   PIPECONF_DITHER_TYPE_MASK (0x0000000c)
>  #define   PIPECONF_DITHER_TYPE_SP (0<<2)
> @@ -2696,11 +2696,6 @@
>  #define   PIPE_START_VBLANK_INTERRUPT_STATUS	(1UL<<2) /* 965 or later */
>  #define   PIPE_VBLANK_INTERRUPT_STATUS		(1UL<<1)
>  #define   PIPE_OVERLAY_UPDATED_STATUS		(1UL<<0)
> -#define   PIPE_BPC_MASK				(7 << 5) /* Ironlake */
> -#define   PIPE_8BPC				(0 << 5)
> -#define   PIPE_10BPC				(1 << 5)
> -#define   PIPE_6BPC				(2 << 5)
> -#define   PIPE_12BPC				(3 << 5)
>  
>  #define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC)
>  #define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0eb8821..02cd329 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1633,8 +1633,8 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  		 * make the BPC in transcoder be consistent with
>  		 * that in pipeconf reg.
>  		 */
> -		val &= ~PIPE_BPC_MASK;
> -		val |= pipeconf_val & PIPE_BPC_MASK;
> +		val &= ~PIPECONF_BPC_MASK;
> +		val |= pipeconf_val & PIPECONF_BPC_MASK;
>  	}
>  
>  	val &= ~TRANS_INTERLACE_MASK;
> @@ -2712,7 +2712,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(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);

The magic shift by 11 made me look at FDI RX CTL specs, and turns out
the register has changed in LPT PCH, and the above no longer matches
spec. Port width is just one bit now, and color depth is nowhere to be
found.


BR,
Jani.


>  
>  	POSTING_READ(reg);
> @@ -2782,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(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp & ~FDI_RX_ENABLE);
>  
>  	POSTING_READ(reg);
> @@ -2811,7 +2811,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(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp);
>  
>  	POSTING_READ(reg);
> @@ -3046,7 +3046,7 @@ 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(pipe)) & PIPECONF_BPC_MASK) >> 5;
>  		reg = TRANS_DP_CTL(pipe);
>  		temp = I915_READ(reg);
>  		temp &= ~(TRANS_DP_PORT_SEL_MASK |
> @@ -4628,10 +4628,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	}
>  
>  	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPP_MASK | PIPECONF_DITHER_EN);
> +	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
>  	if (is_dp) {
>  		if (adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
> -			pipeconf |= PIPECONF_BPP_6 |
> +			pipeconf |= PIPECONF_6BPC |
>  				    PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
>  		}
> @@ -4639,7 +4639,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
>  		if (adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
> -			pipeconf |= PIPECONF_BPP_6 |
> +			pipeconf |= PIPECONF_6BPC |
>  					PIPECONF_ENABLE |
>  					I965_PIPECONF_ACTIVE;
>  		}
> @@ -5022,19 +5022,19 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>  
>  	val = I915_READ(PIPECONF(pipe));
>  
> -	val &= ~PIPE_BPC_MASK;
> +	val &= ~PIPECONF_BPC_MASK;
>  	switch (intel_crtc->bpp) {
>  	case 18:
> -		val |= PIPE_6BPC;
> +		val |= PIPECONF_6BPC;
>  		break;
>  	case 24:
> -		val |= PIPE_8BPC;
> +		val |= PIPECONF_8BPC;
>  		break;
>  	case 30:
> -		val |= PIPE_10BPC;
> +		val |= PIPECONF_10BPC;
>  		break;
>  	case 36:
> -		val |= PIPE_12BPC;
> +		val |= PIPECONF_12BPC;
>  		break;
>  	default:
>  		/* Case prevented by intel_choose_pipe_bpp_dither. */
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2012-12-17 11:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-17 10:21 [PATCH] drm/i915: clean up PIPECONF bpc #defines Daniel Vetter
2012-12-17 11:12 ` Jani Nikula [this message]
2012-12-17 13:22   ` Daniel Vetter

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=87k3shdjaw.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.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.