Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Sep 24, 2014 at 08:44:42AM -0700, Clint Taylor wrote:
> On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> >On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
> >>From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >>Haswell and later silicon has added a new pixel replication register
> >>to the pipe timings for each transcoder. Now in addition to the
> >>DPLL_A_MD register for the pixel clock double, we also need to write to
> >>the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> >>to the DPLL only double the pixel clock.
> >>
> >>Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_reg.h      |    3 +++
> >>  drivers/gpu/drm/i915/intel_display.c |    6 +++++-
> >>  2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>index 15c0eaa..7c078d9 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -2431,6 +2431,7 @@ enum punit_power_well {
> >>  #define _PIPEASRC	0x6001c
> >>  #define _BCLRPAT_A	0x60020
> >>  #define _VSYNCSHIFT_A	0x60028
> >>+#define _MULTIPLY_A	0x6002c
> >>
> >>  /* Pipe B timing regs */
> >>  #define _HTOTAL_B	0x61000
> >>@@ -2442,6 +2443,7 @@ enum punit_power_well {
> >>  #define _PIPEBSRC	0x6101c
> >>  #define _BCLRPAT_B	0x61020
> >>  #define _VSYNCSHIFT_B	0x61028
> >>+#define _MULTIPLY_B	0x6102c
> >>
> >>  #define TRANSCODER_A_OFFSET 0x60000
> >>  #define TRANSCODER_B_OFFSET 0x61000
> >>@@ -2462,6 +2464,7 @@ enum punit_power_well {
> >>  #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> >>  #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> >>  #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> >>+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> >
> >MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> >with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
> >>
> >>  /* HSW+ eDP PSR registers */
> >>  #define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index c092ff4..e58fcde 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >>
> >>  	intel_set_pipe_timings(intel_crtc);
> >>
> >>+	I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> >
> >This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> >Same below.
> >
> The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes the
> register based on intel_crtc->pipe. This should be all that's required.

I don't see where it indexes with intel_crtc->pipe ...

But it doesn't matter since the register is clearly in the transcoder
block, and the reason why Bspec says is per-pipe is that the edp
transcoder doesn't have it. So on second consideration I guess we can keep
this part as-is then.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch