Re: [PATCH 01/12] drm/i915: Reduce CHV DDL multiplier to 16/8

Ville Syrjälä
2015-02-27T18:02:10Z

On Fri, Feb 27, 2015 at 09:36:58AM -0800, Jesse Barnes wrote:
> On 02/10/2015 05:28 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Apparently we must yet halve the DDL drain latency from what we're
> > using currently. This little nugget is not in any spec, but came
> > down through the grapevine.
> >
> > This makes the displays a bit more stable. Not quite fully stable but at
> > least they don't fall over immediately on driver load.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 6 +++---
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 4ee1964..d8a0205 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4166,6 +4166,7 @@ enum skl_disp_power_wells {
> > #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
> >
> > /* drain latency register values*/
> > +#define DRAIN_LATENCY_PRECISION_8 8
> > #define DRAIN_LATENCY_PRECISION_16 16
> > #define DRAIN_LATENCY_PRECISION_32 32
> > #define DRAIN_LATENCY_PRECISION_64 64
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 3c64810..a70bce4 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -728,8 +728,8 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> >
> > entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> > if (IS_CHERRYVIEW(dev))
> > - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> > - DRAIN_LATENCY_PRECISION_16;
> > + *prec_mult = (entries > 32) ? DRAIN_LATENCY_PRECISION_16 :
> > + DRAIN_LATENCY_PRECISION_8;
> > else
> > *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > DRAIN_LATENCY_PRECISION_32;
> > @@ -759,7 +759,7 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > enum pipe pipe = intel_crtc->pipe;
> > int plane_prec, prec_mult, plane_dl;
> > const int high_precision = IS_CHERRYVIEW(dev) ?
> > - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64;
> > + DRAIN_LATENCY_PRECISION_16 : DRAIN_LATENCY_PRECISION_64;
> >
> > plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
> > DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> >
>
> You're also changing the entries threshold; is that due to the reduced
> drain latency precsion also?
>
> Would be good to file a bspec bug on this so we don't lose it.

The spec just says we should choose the precision so that we don't exceed
7bits in the final value. So the 128 was just needleslly large and we
were just throwing away a useful bit by choosing to use the smaller precision
factor when we didn't have to. But I'm killing off these constants anyway
in a later patch to make the code less confusing.

>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

--
Ville Syrjälä
Intel OTC