Re: [PATCH v5 2/8] drm/i915/bxt: IPC WA for Broxton

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Sex, 2016-11-18 às 20:39 +0530, Mahesh Kumar escreveu:
> 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@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h |  3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++--
>  3 files changed, 14 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..394d7ea 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2128,6 +2128,9 @@ struct drm_i915_private {
>  	/* perform PHY state sanity checks? */
>  	bool chv_phy_assert[2];
>  
> +	/* To apply IPC WA */

The comment is not exactly necessary since we can just look where
ipc_enabled is used to find out why it's here. As a bonus, whenever we
add more ipc_enabled readers we won't need to update the comment.


> +	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);

Weird indentation here again. Is your editor configured to show tabs as
4 spaces?

> +	if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)

Once we merge Gemnilake, we'll need to write a patch checking for it
here.


Anyway, my two suggestions can be fixed while merging, so:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +		linetime_wm = DIV_ROUND_UP(linetime_wm, 2);
> +
> +	return linetime_wm;
>  }
>  
>  static void skl_compute_transition_wm(struct intel_crtc_state
> *cstate,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx