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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 30, 2014 at 09:22:25PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 30, 2014 at 10:30:22AM -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.
> > 
> > ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> > ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
> > ver4: Macro name change to PIPE_MULT and default else pixel_multiplier
> > 
> > Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > 
> > 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 |   13 ++++++++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ad8179b..d428f7a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2443,6 +2443,7 @@ enum punit_power_well {
> >  #define _PIPEASRC	0x6001c
> >  #define _BCLRPAT_A	0x60020
> >  #define _VSYNCSHIFT_A	0x60028
> > +#define _PIPE_MULT_A	0x6002c
> >  
> >  /* Pipe B timing regs */
> >  #define _HTOTAL_B	0x61000
> > @@ -2454,6 +2455,7 @@ enum punit_power_well {
> >  #define _PIPEBSRC	0x6101c
> >  #define _BCLRPAT_B	0x61020
> >  #define _VSYNCSHIFT_B	0x61028
> > +#define _PIPE_MULT_B	0x6102c
> >  
> >  #define TRANSCODER_A_OFFSET 0x60000
> >  #define TRANSCODER_B_OFFSET 0x61000
> > @@ -2474,6 +2476,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 PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
> >  
> >  /* 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 858011d..617dad6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >  
> >  	intel_set_pipe_timings(intel_crtc);
> >  
> > +	if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> > +		I915_WRITE(PIPE_MULT(intel_crtc->config.cpu_transcoder),
> > +				intel_crtc->config.pixel_multiplier - 1);
> > +	}
> > +
> >  	if (intel_crtc->config.has_pch_encoder) {
> >  		intel_cpu_transcoder_set_m_n(intel_crtc,
> >  				     &intel_crtc->config.fdi_m_n, NULL);
> > @@ -7853,7 +7858,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  		pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> >  			(I915_READ(IPS_CTL) & IPS_ENABLE);
> >  
> > -	pipe_config->pixel_multiplier = 1;
> > +	if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> > +		pipe_config->pixel_multiplier =
> > +			I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
> > +	}
> > +	else {
> > +		pipe_config->pixel_multiplier = 1;
> > +	}
> 
> A bit too many curly braces for my taste but maybe that's just me.
> In any case looks like it will DTRT so:

Checkpatch actually complained about the misaligned in the first hunk and
about the } on a line of its own. And with proper kernel coding style
braces are allowed on single-statement multi-line blocks and should be
applied to all blocks of an if statement if one has them. So it's totally
ok.

> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next-fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch