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

[Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>]

Hi,

Will keep WA number in commit message/WA location.
thanks,

Regards,

-Mahesh

On Thursday 24 November 2016 06:21 PM, Lankhorst, Maarten wrote:
> 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@xxxxxxxxx>
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > ---
> >   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-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx