On Fri, 23 Mar 2012 10:51:03 +0100
Daniel Vetter <dan...@ffwll.ch> wrote:

> On Thu, Mar 22, 2012 at 08:29:30PM -0700, Ben Widawsky wrote:
> > On Thu, Mar 22, 2012 at 02:38:46PM -0700, Jesse Barnes wrote:
> > > Add support for ValleyView watermark handling.  It's like Cantiga
> > > with a few small differences (big FIFO mode and different WM
> > > limits).
> > > 
> > > Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h      |    9 +++++
> > >  drivers/gpu/drm/i915/intel_display.c |   65
> > > ++++++++++++++++++++++++++++++++++ 2 files changed, 74
> > > insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index f3609f2..0540099 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -1043,6 +1043,12 @@
> > >  #define RAMCLK_GATE_D            0x6210          /*
> > > CRL only */ #define DEUC
> > > 0x6214          /* CRL only */ 
> > > +#define FW_BLC_SELF_VLV          0x6500
> > > +#define  FW_CSPWRDWNEN           (1<<15)
> > > +#define MI_ARB_VLV               0x6504
> > > +#define  DISP_TRICKLE_FEED_DIS   (1<<2)
> > > +#define CZCLK_CDCLK_FREQ_RATIO_VLV       0x6508
> > > +
> > >  /*
> > >   * Palette regs
> > >   */
> > 
> > Seems like the bottom 3 of these aren't used anywhere.
> 
> I honestly don't mind a few extra #defines, as long as someone has
> bothered to cross check them with bspec (to avoid another MI_WTF). At
> least if they're sounding somewhat relevant.
> -Daniel

Okay, seriously - fix mutt so it actually responds To: me.

The reason I'm only acking most of the patches thus far is I can't find
the register definitions. Jesse said some exist in some doc, and
others were just told to him by the design team. OTOH, anything with my
r-b has been crossed checked with the bspec.

Also, since we got into the discussion RIGHT BEFORE the vlv patches
about how messy our code is, I was being extra diligent about cutting
out stuff we don't use, since the odds of someone doing it after the
fact are slim to none. Furthermore, these defines are so terribly
named, the odds of someone creating another set of defines for the same
purpose are greater than 0.

~Ben

> 
> > 
> > > @@ -2495,6 +2501,7 @@
> > >  #define I915_FIFO_LINE_SIZE      64
> > >  #define I830_FIFO_LINE_SIZE      32
> > >  
> > > +#define VALLEYVIEW_FIFO_SIZE     255
> > >  #define G4X_FIFO_SIZE            127
> > >  #define I965_FIFO_SIZE           512
> > >  #define I945_FIFO_SIZE           127
> > > @@ -2502,6 +2509,7 @@
> > >  #define I855GM_FIFO_SIZE 127 /* In cachelines */
> > >  #define I830_FIFO_SIZE           95
> > >  
> > > +#define VALLEYVIEW_MAX_WM        0xff
> > >  #define G4X_MAX_WM               0x3f
> > >  #define I915_MAX_WM              0x3f
> > >  
> > > @@ -2516,6 +2524,7 @@
> > >  #define PINEVIEW_CURSOR_DFT_WM   0
> > >  #define PINEVIEW_CURSOR_GUARD_WM 5
> > >  
> > > +#define VALLEYVIEW_CURSOR_MAX_WM 64
> > >  #define I965_CURSOR_FIFO 64
> > >  #define I965_CURSOR_MAX_WM       32
> > >  #define I965_CURSOR_DFT_WM       8
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c index de1ba19..daa8853
> > > 100644 --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3606,6 +3606,20 @@ static const struct intel_watermark_params
> > > g4x_cursor_wm_info = { 2,
> > >   G4X_FIFO_LINE_SIZE,
> > >  };
> > > +static const struct intel_watermark_params valleyview_wm_info = {
> > > + VALLEYVIEW_FIFO_SIZE,
> > > + VALLEYVIEW_MAX_WM,
> > > + VALLEYVIEW_MAX_WM,
> > > + 2,
> > > + G4X_FIFO_LINE_SIZE,
> > > +};
> > > +static const struct intel_watermark_params
> > > valleyview_cursor_wm_info = {
> > > + I965_CURSOR_FIFO,
> > > + VALLEYVIEW_CURSOR_MAX_WM,
> > > + I965_CURSOR_DFT_WM,
> > > + 2,
> > > + G4X_FIFO_LINE_SIZE,
> > > +};
> > >  static const struct intel_watermark_params i965_cursor_wm_info =
> > > { I965_CURSOR_FIFO,
> > >   I965_CURSOR_MAX_WM,
> > > @@ -4130,6 +4144,55 @@ static bool g4x_compute_srwm(struct
> > > drm_device *dev, 
> > >  #define single_plane_enabled(mask) is_power_of_2(mask)
> > >  
> > > +static void valleyview_update_wm(struct drm_device *dev)
> > > +{
> > > + static const int sr_latency_ns = 12000;
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > + int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
> > > + int plane_sr, cursor_sr;
> > > + unsigned int enabled = 0;
> > > +
> > > + if (g4x_compute_wm0(dev, 0,
> > > +                     &valleyview_wm_info, latency_ns,
> > > +                     &valleyview_cursor_wm_info,
> > > latency_ns,
> > > +                     &planea_wm, &cursora_wm))
> > > +         enabled |= 1;
> > > +
> > > + if (g4x_compute_wm0(dev, 1,
> > > +                     &valleyview_wm_info, latency_ns,
> > > +                     &valleyview_cursor_wm_info,
> > > latency_ns,
> > > +                     &planeb_wm, &cursorb_wm))
> > > +         enabled |= 2;
> > > +
> > > + plane_sr = cursor_sr = 0;
> > > + if (single_plane_enabled(enabled) &&
> > > +     g4x_compute_srwm(dev, ffs(enabled) - 1,
> > > +                      sr_latency_ns,
> > > +                      &valleyview_wm_info,
> > > +                      &valleyview_cursor_wm_info,
> > > +                      &plane_sr, &cursor_sr))
> > > +         I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN);
> > > + else
> > > +         I915_WRITE(FW_BLC_SELF_VLV,
> > > +                    I915_READ(FW_BLC_SELF_VLV) &
> > > ~FW_CSPWRDWNEN); +
> > > + DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d,
> > > cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
> > > +               planea_wm, cursora_wm,
> > > +               planeb_wm, cursorb_wm,
> > > +               plane_sr, cursor_sr);
> > > +
> > > + I915_WRITE(DSPFW1,
> > > +            (plane_sr << DSPFW_SR_SHIFT) |
> > > +            (cursorb_wm << DSPFW_CURSORB_SHIFT) |
> > > +            (planeb_wm << DSPFW_PLANEB_SHIFT) |
> > > +            planea_wm);
> > > + I915_WRITE(DSPFW2,
> > > +            (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) |
> > > +            (cursora_wm << DSPFW_CURSORA_SHIFT));
> > > + I915_WRITE(DSPFW3,
> > > +            (I915_READ(DSPFW3) | (cursor_sr <<
> > > DSPFW_CURSOR_SR_SHIFT))); +}
> > > +
> > >  static void g4x_update_wm(struct drm_device *dev)
> > >  {
> > >   static const int sr_latency_ns = 12000;
> > > @@ -8917,6 +8980,8 @@ static void intel_init_display(struct
> > > drm_device *dev) dev_priv->display.write_eld = ironlake_write_eld;
> > >           } else
> > >                   dev_priv->display.update_wm = NULL;
> > > + } else if (IS_VALLEYVIEW(dev)) {
> > > +         dev_priv->display.update_wm =
> > > valleyview_update_wm; } else if (IS_PINEVIEW(dev)) {
> > >           if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
> > >                                       dev_priv->is_ddr3,
> > 
> > Aside from the extraneous #defines
> > Acked-by: Ben Widawsky <b...@bwidawsk.net>
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

Reply via email to