On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> Current chv spec teels we can only use either 16 or 32 bits as precision.
> 
> Although in the past VLV went from 16/32 to 32/64 and spec might not be 
> updated,
> these precision values brings stability and fixes some issues Wayne was 
> facing.

Lies, damned lies, specs!

Well in this case I guess the spec might be correct since it helps
Wayne.

> 
> Cc: Wayne Boyer <wayne.bo...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
>  drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
>  2 files changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6db369a..dcd5650 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4054,17 +4054,18 @@ enum punit_power_well {
>  #define   DSPFW_PLANEA_WM1_HI_MASK   (1<<0)
>  
>  /* drain latency register values*/
> +#define DRAIN_LATENCY_PRECISION_16   16
>  #define DRAIN_LATENCY_PRECISION_32   32
>  #define DRAIN_LATENCY_PRECISION_64   64

While you're poking at this stuff, could I trouble you to remove these
defines and just use the values directly? Could be a separate patch if
you prefer.

>  #define VLV_DDL(pipe)                        (VLV_DISPLAY_BASE + 0x70050 + 4 
> * (pipe))
> -#define DDL_CURSOR_PRECISION_64              (1<<31)
> -#define DDL_CURSOR_PRECISION_32              (0<<31)
> +#define DDL_CURSOR_PRECISION_HIGH    (1<<31)
> +#define DDL_CURSOR_PRECISION_LOW     (0<<31)
>  #define DDL_CURSOR_SHIFT             24
> -#define DDL_SPRITE_PRECISION_64(sprite)      (1<<(15+8*(sprite)))
> -#define DDL_SPRITE_PRECISION_32(sprite)      (0<<(15+8*(sprite)))
> +#define DDL_SPRITE_PRECISION_HIGH(sprite)    (1<<(15+8*(sprite)))
> +#define DDL_SPRITE_PRECISION_LOW(sprite)     (0<<(15+8*(sprite)))
>  #define DDL_SPRITE_SHIFT(sprite)     (8+8*(sprite))
> -#define DDL_PLANE_PRECISION_64               (1<<7)
> -#define DDL_PLANE_PRECISION_32               (0<<7)
> +#define DDL_PLANE_PRECISION_HIGH     (1<<7)
> +#define DDL_PLANE_PRECISION_LOW              (0<<7)
>  #define DDL_PLANE_SHIFT                      0
>  #define DRAIN_LATENCY_MASK           0x7f
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index daa99e7..50c3512 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc 
> *crtc,
>                                     int *prec_mult,
>                                     int *drain_latency)
>  {
> +     struct drm_device *dev = crtc->dev;
>       int entries;
>       int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
>  
> @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc 
> *crtc,
>               return false;
>  
>       entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> -     *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> -                                    DRAIN_LATENCY_PRECISION_32;
> +     if (IS_CHERRYVIEW(dev))
> +             *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> +                                            DRAIN_LATENCY_PRECISION_16;
> +     else
> +             *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> +                                            DRAIN_LATENCY_PRECISION_32;
>       *drain_latency = (64 * (*prec_mult) * 4) / entries;
>  
>       if (*drain_latency > DRAIN_LATENCY_MASK)
> @@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc 
> *crtc,
>  
>  static void vlv_update_drain_latency(struct drm_crtc *crtc)
>  {
> -     struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +     struct drm_device *dev = crtc->dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>       int pixel_size;
>       int drain_latency;
>       enum pipe pipe = intel_crtc->pipe;
>       int plane_prec, prec_mult, plane_dl;
> +     int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> +                                               DRAIN_LATENCY_PRECISION_64;

Maybe const just help the reader remember that it's a constant.

>  
> -     plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
> -                DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
> +     plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
> +                DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
>                  (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
>  
>       if (!intel_crtc_active(crtc)) {
> @@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc 
> *crtc)
>       /* Primary plane Drain Latency */
>       pixel_size = crtc->primary->fb->bits_per_pixel / 8;     /* BPP */
>       if (vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, 
> &drain_latency)) {
> -             plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> -                                        DDL_PLANE_PRECISION_64 :
> -                                        DDL_PLANE_PRECISION_32;
> +             plane_prec = (prec_mult == high_precision) ?
> +                                        DDL_PLANE_PRECISION_HIGH :
> +                                        DDL_PLANE_PRECISION_LOW;
>               plane_dl |= plane_prec | drain_latency;
>       }
>  
> @@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct drm_crtc 
> *crtc)
>       /* Program cursor DL only if it is enabled */
>       if (intel_crtc->cursor_base &&
>           vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, 
> &drain_latency)) {
> -             plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> -                                        DDL_CURSOR_PRECISION_64 :
> -                                        DDL_CURSOR_PRECISION_32;
> +             plane_prec = (prec_mult == high_precision) ?
> +                                        DDL_CURSOR_PRECISION_HIGH :
> +                                        DDL_CURSOR_PRECISION_LOW;
>               plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
>       }
>  
> @@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct 
> drm_plane *plane,
>       int plane_prec;
>       int sprite_dl;
>       int prec_mult;
> +     int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> +                                               DRAIN_LATENCY_PRECISION_64;

Could also be const.

Otherwise looks good, so:
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

>  
> -     sprite_dl = I915_READ(VLV_DDL(pipe)) & 
> ~(DDL_SPRITE_PRECISION_64(sprite) |
> +     sprite_dl = I915_READ(VLV_DDL(pipe)) & 
> ~(DDL_SPRITE_PRECISION_HIGH(sprite) |
>                   (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
>  
>       if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
>                                                &drain_latency)) {
> -             plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> -                                        DDL_SPRITE_PRECISION_64(sprite) :
> -                                        DDL_SPRITE_PRECISION_32(sprite);
> +             plane_prec = (prec_mult == high_precision) ?
> +                                        DDL_SPRITE_PRECISION_HIGH(sprite) :
> +                                        DDL_SPRITE_PRECISION_LOW(sprite);
>               sprite_dl |= plane_prec |
>                            (drain_latency << DDL_SPRITE_SHIFT(sprite));
>       }
> -- 
> 1.9.3

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to