[Intel-gfx] [PATCH v6 2/8] drm/i915/bxt: IPC WA for Broxton

[Lankhorst, Maarten]

Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]:
> If IPC is enabled in BXT, display underruns are observed.
> WA: The Line Time programmed in the WM_LINETIME register should be
> half of the actual calculated Line Time.
> 
> Programmed Line Time = 1/2*Calculated Line Time
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 445fec9..1b0a589 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1243,6 +1243,8 @@ int i915_driver_load(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  
>  	intel_runtime_pm_enable(dev_priv);
>  
> +	dev_priv->ipc_enabled = false;
> +
>  	/* Everything is in place, we can now relax! */
>  	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>  		 driver.name, driver.major, driver.minor,
> driver.patchlevel,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 63c0ea0..6691a4e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2128,6 +2128,8 @@ struct drm_i915_private {
>  	/* perform PHY state sanity checks? */
>  	bool chv_phy_assert[2];
>  
> +	bool ipc_enabled;
> +
>  	/* Used to save the pipe-to-encoder mapping for audio */
>  	struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 8908736..7090a7c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct
> drm_i915_private *dev_priv,
>  static uint32_t
>  skl_compute_linetime_wm(struct intel_crtc_state *cstate)
>  {
> +	struct drm_atomic_state *state = cstate->base.state;
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>  	uint32_t pixel_rate;
> +	uint32_t linetime_wm;
>  
>  	if (!cstate->base.active)
>  		return 0;
> @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct
> intel_crtc_state *cstate)
>  	if (WARN_ON(pixel_rate == 0))
>  		return 0;
>  
> -	return DIV_ROUND_UP(8 * cstate-
> >base.adjusted_mode.crtc_htotal * 1000,
> -			    pixel_rate);
> +	linetime_wm = DIV_ROUND_UP(8 * cstate-
> >base.adjusted_mode.crtc_htotal *
> +						1000, pixel_rate);
> +	if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
> +		linetime_wm = DIV_ROUND_UP(linetime_wm, 2);
I've asked on irc to be sure, but this needs slightly more info in the
code as well. Same for 3/8 and 8/8.

12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a
name, do we still have to write comments about them in the code?
12:48 < danvet> at least a bspec reference
12:48 < danvet> some hint to make it possible to find where it's from
12:49 < danvet> e.g. mesa has traditionally just pasted the entire
bspec paragraph
12:49 < mlankhorst> ok
12:49 < danvet> we've done that too in the past, but then the wa db
mostly made that redundant
12:49 < danvet> but if there's no wa entry, then full deal
12:49 < danvet> also, maybe a bspec bug notice to pls add one
12:49 < danvet> you should be able to do that over the web, and afaik
there should be one for every w/a
---------------------------------------------------------------------
Intel International B.V.
Registered in The Netherlands under number 34098535
Statutory seat: Haarlemmermeer
Registered address: Capronilaan 37, 1119NG Schiphol-Rijk

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.