Re: [PATCH 8/9] drm/i915: Implement WaPixelRepeatModeFixForC0:chv

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> 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>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
>   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
>   drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
>   3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 37cc653..adaa656 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1851,7 +1851,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;
>   	struct i915_suspend_saved_registers regfile;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f08f729..2361347 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4580,6 +4580,9 @@ enum skl_disp_power_wells {
>   
>   #define CBR1_VLV			(VLV_DISPLAY_BASE + 0x70400)
>   #define  CBR_PND_DEADLINE_DISABLE	(1<<31)
> +#define CBR4_VLV			(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
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dec36a2..b862307 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1667,9 +1667,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)
> @@ -8065,7 +8083,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;
correct me if my understanding is wrong, the only place we used to read 
pixel_multiplier was in
i9xx_get_pipe_config and we have replaced even that with dev_priv 
variables ? so the first time
we get pipe_config will it return 0 ?
i assume the right thing here is to read the CBR4_VLV and check if the 
current pipe is enabled,
if so return the value in DPLL_MD(PIPE_B) as required.
On a side note, we should calculate pixel_multiplier as part of 
compute_dpll instead of depending
on GOP/VBIOS programmed values.

-- 
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx