Re: [PATCH 03/16] drm/i915: Implement WaPixelRepeatModeFixForC0:chv

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Mar 16, 2016 at 11:27:05AM +0200, Jani Nikula wrote:
> On Tue, 15 Mar 2016, ville.syrjala@linux.intel.com wrote:
> > [ text/plain ]
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DPLL_MD(PIPE_C) is AWOL on CHV. Instead of fixing it someone added
> > chicken bits to propagate the pixel multiplier from DPLL_MD(PIPE_B)
> > to either pipe B or C. So do that to make pixel repeat work on pipes
> > B and C. Pipe A is fine without any tricks.
> >
> > Fortunately the pixel repeat propagation appears to be a oneshot
> > operation, so once the value has been written we can clear the
> > chicken bits. So it is still possible to drive pipe B and C with
> > different pixel multipliers simultaneosly.
> >
> > Looks like DPLL_VGA_MODE_DIS must also be set in DPLL(PIPE_B)
> > for this to work. But since we keep that bit always set in all
> > DPLLs there's no problem.
> >
> > This of course means we can't reliably read out the pixel multiplier
> > for pipes B and C. That would make the state checker unhappy, so I
> > added shadow copies of those registers in to dev_priv. The other
> > option would have been to skip pixel multiplier, dpll_md an dotclock
> > checks entirely on CHV, but that feels like a serious loss of cross
> > checking, so just pretending that we have working DPLL MD registers
> > seemed better. Obviously with the shadow copies we can't detect if
> > the pixel multiplier was properly configured, nor can we take over
> > its state from the BIOS, but hopefully people won't have displays
> > that would be limitd to such crappy modes.
> >
> > There is one strange flicker still remaining. It's visible on
> > pipe C/HDMID when HDMIB is enabled while driven by pipe B.
> > It doesn't occur if pipe A drives HDMIB, nor is there any glitch
> > on pipe B/HDMIB when port C/HDMID starts up. I don't have a board
> > with HDMIC so not sure if it happens there too. So I'm not sure
> > if it's somehow tied in with this strange linkage between pipe B
> > and C. Sadly I was unable to find an enable sequence that would
> > avoid the glitch, but at least it's not fatal ie. the output
> > recovers afterwards.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I didn't dig up the specs, but the code seems to do what the commit
> message says.

The "spec" wouldn't help much either, apart from showing where
the chicken bits live. I might have a powerpoint somewhere about
this, but it IIRC it didn't really tell you how to do things in
practice. As far as I can remember, I just used the standard
trial and error method to implement this.

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
> >  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
> >  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
> >  3 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 80b14f1ba302..31689e1b19e6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1871,7 +1871,14 @@ struct drm_i915_private {
> >  
> >  	u32 fdi_rx_config;
> >  
> > +	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
> >  	u32 chv_phy_control;
> > +	/*
> > +	 * Shadows for CHV DPLL_MD regs to keep the state
> > +	 * checker somewhat working in the presence hardware
> > +	 * crappiness (can't read out DPLL_MD for pipes B & C).
> > +	 */
> > +	u32 chv_dpll_md[I915_MAX_PIPES];
> >  
> >  	u32 suspend_count;
> >  	bool suspended_to_idle;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 7dfc4007f3fa..f138588a88cd 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4778,6 +4778,10 @@ enum skl_disp_power_wells {
> >  #define  CBR_PND_DEADLINE_DISABLE	(1<<31)
> >  #define  CBR_PWM_CLOCK_MUX_SELECT	(1<<30)
> >  
> > +#define CBR4_VLV			_MMIO(VLV_DISPLAY_BASE + 0x70450)
> > +#define  CBR_DPLLBMD_PIPE_C		(1<<29)
> > +#define  CBR_DPLLBMD_PIPE_B		(1<<18)
> > +
> >  /* FIFO watermark sizes etc */
> >  #define G4X_FIFO_LINE_SIZE	64
> >  #define I915_FIFO_LINE_SIZE	64
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 414ed5007e60..18158b0324ed 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1635,9 +1635,27 @@ static void chv_enable_pll(struct intel_crtc *crtc,
> >  	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> >  		DRM_ERROR("PLL %d failed to lock\n", pipe);
> >  
> > -	/* not sure when this should be written */
> > -	I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > -	POSTING_READ(DPLL_MD(pipe));
> > +	if (pipe != PIPE_A) {
> > +		/*
> > +		 * WaPixelRepeatModeFixForC0:chv
> > +		 *
> > +		 * DPLLCMD is AWOL. Use chicken bits to propagate
> > +		 * the value from DPLLBMD to either pipe B or C.
> > +		 */
> > +		I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B : CBR_DPLLBMD_PIPE_C);
> > +		I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
> > +		I915_WRITE(CBR4_VLV, 0);
> > +		dev_priv->chv_dpll_md[pipe] = pipe_config->dpll_hw_state.dpll_md;
> > +
> > +		/*
> > +		 * DPLLB VGA mode also seems to cause problems.
> > +		 * We should always have it disabled.
> > +		 */
> > +		WARN_ON((I915_READ(DPLL(PIPE_B)) & DPLL_VGA_MODE_DIS) == 0);
> > +	} else {
> > +		I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > +		POSTING_READ(DPLL_MD(pipe));
> > +	}
> >  }
> >  
> >  static int intel_num_dvo_pipes(struct drm_device *dev)
> > @@ -8096,7 +8114,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  	i9xx_get_pfit_config(crtc, pipe_config);
> >  
> >  	if (INTEL_INFO(dev)->gen >= 4) {
> > -		tmp = I915_READ(DPLL_MD(crtc->pipe));
> > +		/* No way to read it out on pipes B and C */
> > +		if (IS_CHERRYVIEW(dev) && crtc->pipe != PIPE_A)
> > +			tmp = dev_priv->chv_dpll_md[crtc->pipe];
> > +		else
> > +			tmp = I915_READ(DPLL_MD(crtc->pipe));
> >  		pipe_config->pixel_multiplier =
> >  			((tmp & DPLL_MD_UDI_MULTIPLIER_MASK)
> >  			 >> DPLL_MD_UDI_MULTIPLIER_SHIFT) + 1;
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx