On Mon, 17 Dec 2012, Daniel Vetter <daniel.vetter at 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 at intel.com>
*BUT* see a related comment below.
> Signed-off-by: Daniel Vetter <daniel.vetter at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
