2014-06-27 20:04 GMT-03:00  <ville.syrj...@linux.intel.com>:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> The VLV/CHV DDL registers are uniform, and neatly enough the register
> offsets are sane so we can easily unify them to a single set of defines
> and just pass the pipe as the parameter to compute the register offset.

What the commit message doesn't tell is that now we will call
vlv_compute_drain_latency() for pipe C on CHV since I see CHV is
defined with num_pipes=3. I think this is quite an important detail,
since it's the only way this patch changes the behavior of the code.

If that is intentional and correct, then I suggest amending the commit
message, even maybe the patch title. Then you can add: Reviewed-by:
Paulo Zanoni <paulo.r.zan...@intel.com>.

>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 54 
> ++++++++++-------------------------------
>  drivers/gpu/drm/i915/intel_pm.c | 52 ++++++++++++++++++---------------------
>  2 files changed, 36 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9fab647..60dd19c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4018,47 +4018,19 @@ enum punit_power_well {
>  /* drain latency register values*/
>  #define DRAIN_LATENCY_PRECISION_32     32
>  #define DRAIN_LATENCY_PRECISION_64     64
> -#define VLV_DDL1                       (VLV_DISPLAY_BASE + 0x70050)
> -#define DDL_CURSORA_PRECISION_64       (1<<31)
> -#define DDL_CURSORA_PRECISION_32       (0<<31)
> -#define DDL_CURSORA_SHIFT              24
> -#define DDL_SPRITEB_PRECISION_64       (1<<23)
> -#define DDL_SPRITEB_PRECISION_32       (0<<23)
> -#define DDL_SPRITEB_SHIFT              16
> -#define DDL_SPRITEA_PRECISION_64       (1<<15)
> -#define DDL_SPRITEA_PRECISION_32       (0<<15)
> -#define DDL_SPRITEA_SHIFT              8
> -#define DDL_PLANEA_PRECISION_64                (1<<7)
> -#define DDL_PLANEA_PRECISION_32                (0<<7)
> -#define DDL_PLANEA_SHIFT               0
> -
> -#define VLV_DDL2                       (VLV_DISPLAY_BASE + 0x70054)
> -#define DDL_CURSORB_PRECISION_64       (1<<31)
> -#define DDL_CURSORB_PRECISION_32       (0<<31)
> -#define DDL_CURSORB_SHIFT              24
> -#define DDL_SPRITED_PRECISION_64       (1<<23)
> -#define DDL_SPRITED_PRECISION_32       (0<<23)
> -#define DDL_SPRITED_SHIFT              16
> -#define DDL_SPRITEC_PRECISION_64       (1<<15)
> -#define DDL_SPRITEC_PRECISION_32       (0<<15)
> -#define DDL_SPRITEC_SHIFT              8
> -#define DDL_PLANEB_PRECISION_64                (1<<7)
> -#define DDL_PLANEB_PRECISION_32                (0<<7)
> -#define DDL_PLANEB_SHIFT               0
> -
> -#define VLV_DDL3                       (VLV_DISPLAY_BASE + 0x70058)
> -#define DDL_CURSORC_PRECISION_64       (1<<31)
> -#define DDL_CURSORC_PRECISION_32       (0<<31)
> -#define DDL_CURSORC_SHIFT              24
> -#define DDL_SPRITEF_PRECISION_64       (1<<23)
> -#define DDL_SPRITEF_PRECISION_32       (0<<23)
> -#define DDL_SPRITEF_SHIFT              16
> -#define DDL_SPRITEE_PRECISION_64       (1<<15)
> -#define DDL_SPRITEE_PRECISION_32       (0<<15)
> -#define DDL_SPRITEE_SHIFT              8
> -#define DDL_PLANEC_PRECISION_64                (1<<7)
> -#define DDL_PLANEC_PRECISION_32                (0<<7)
> -#define DDL_PLANEC_SHIFT               0
> +#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_SHIFT               24
> +#define DDL_SPRITE1_PRECISION_64       (1<<23)
> +#define DDL_SPRITE1_PRECISION_32       (0<<23)
> +#define DDL_SPRITE1_SHIFT              16
> +#define DDL_SPRITE0_PRECISION_64       (1<<15)
> +#define DDL_SPRITE0_PRECISION_32       (0<<15)
> +#define DDL_SPRITE0_SHIFT              8
> +#define DDL_PLANE_PRECISION_64         (1<<7)
> +#define DDL_PLANE_PRECISION_32         (0<<7)
> +#define DDL_PLANE_SHIFT                        0
>
>  /* FIFO watermark sizes etc */
>  #define G4X_FIFO_LINE_SIZE     64
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index dc858b5..f0516a7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1275,35 +1275,29 @@ static bool vlv_compute_drain_latency(struct 
> drm_device *dev,
>  static void vlv_update_drain_latency(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int planea_prec, planea_dl, planeb_prec, planeb_dl;
> -       int cursora_prec, cursora_dl, cursorb_prec, cursorb_dl;
> -       int plane_prec_mult, cursor_prec_mult; /* Precision multiplier is
> -                                                       either 16 or 32 */
> -
> -       /* For plane A, Cursor A */
> -       if (vlv_compute_drain_latency(dev, 0, &plane_prec_mult, &planea_dl,
> -                                     &cursor_prec_mult, &cursora_dl)) {
> -               cursora_prec = (cursor_prec_mult == 
> DRAIN_LATENCY_PRECISION_32) ?
> -                       DDL_CURSORA_PRECISION_32 : DDL_CURSORA_PRECISION_64;
> -               planea_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_32) 
> ?
> -                       DDL_PLANEA_PRECISION_32 : DDL_PLANEA_PRECISION_64;
> -
> -               I915_WRITE(VLV_DDL1, cursora_prec |
> -                               (cursora_dl << DDL_CURSORA_SHIFT) |
> -                               planea_prec | planea_dl);
> -       }
> -
> -       /* For plane B, Cursor B */
> -       if (vlv_compute_drain_latency(dev, 1, &plane_prec_mult, &planeb_dl,
> -                                     &cursor_prec_mult, &cursorb_dl)) {
> -               cursorb_prec = (cursor_prec_mult == 
> DRAIN_LATENCY_PRECISION_32) ?
> -                       DDL_CURSORB_PRECISION_32 : DDL_CURSORB_PRECISION_64;
> -               planeb_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_32) 
> ?
> -                       DDL_PLANEB_PRECISION_32 : DDL_PLANEB_PRECISION_64;
> -
> -               I915_WRITE(VLV_DDL2, cursorb_prec |
> -                               (cursorb_dl << DDL_CURSORB_SHIFT) |
> -                               planeb_prec | planeb_dl);
> +       enum pipe pipe;
> +
> +       for_each_pipe(pipe) {
> +               int plane_prec, plane_dl;
> +               int cursor_prec, cursor_dl;
> +               int plane_prec_mult, cursor_prec_mult;
> +
> +               if (!vlv_compute_drain_latency(dev, pipe, &plane_prec_mult, 
> &plane_dl,
> +                                              &cursor_prec_mult, &cursor_dl))
> +                       continue;
> +
> +               /*
> +                * FIXME CHV spec still lists 16 and 32 as the precision
> +                * values. Need to figure out if spec is outdated or what.
> +                */
> +               cursor_prec = (cursor_prec_mult == 
> DRAIN_LATENCY_PRECISION_64) ?
> +                       DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32;
> +               plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> +                       DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32;
> +
> +               I915_WRITE(VLV_DDL(pipe), cursor_prec |
> +                          (cursor_dl << DDL_CURSOR_SHIFT) |
> +                          plane_prec | (plane_dl << DDL_PLANE_SHIFT));
>         }
>  }
>
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Reply via email to