Re: [Intel-gfx] [PATCH 26/40] drm/i915: Parametrize VLV_DDL registers

2014-07-31 Thread Ville Syrjälä
On Wed, Jul 30, 2014 at 05:43:10PM -0300, Paulo Zanoni wrote:
 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.

One of the following patches will add a proper cherryview_update_wm()
function which also fills out the actual watermarks for pipe C. Ideally
I probably should have reordered these patches. But I'll add a note
of some sort here to avoid bigger reordering pains now.

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


Re: [Intel-gfx] [PATCH 26/40] drm/i915: Parametrize VLV_DDL registers

2014-07-30 Thread Paulo Zanoni
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   (131)
 -#define DDL_CURSORA_PRECISION_32   (031)
 -#define DDL_CURSORA_SHIFT  24
 -#define DDL_SPRITEB_PRECISION_64   (123)
 -#define DDL_SPRITEB_PRECISION_32   (023)
 -#define DDL_SPRITEB_SHIFT  16
 -#define DDL_SPRITEA_PRECISION_64   (115)
 -#define DDL_SPRITEA_PRECISION_32   (015)
 -#define DDL_SPRITEA_SHIFT  8
 -#define DDL_PLANEA_PRECISION_64(17)
 -#define DDL_PLANEA_PRECISION_32(07)
 -#define DDL_PLANEA_SHIFT   0
 -
 -#define VLV_DDL2   (VLV_DISPLAY_BASE + 0x70054)
 -#define DDL_CURSORB_PRECISION_64   (131)
 -#define DDL_CURSORB_PRECISION_32   (031)
 -#define DDL_CURSORB_SHIFT  24
 -#define DDL_SPRITED_PRECISION_64   (123)
 -#define DDL_SPRITED_PRECISION_32   (023)
 -#define DDL_SPRITED_SHIFT  16
 -#define DDL_SPRITEC_PRECISION_64   (115)
 -#define DDL_SPRITEC_PRECISION_32   (015)
 -#define DDL_SPRITEC_SHIFT  8
 -#define DDL_PLANEB_PRECISION_64(17)
 -#define DDL_PLANEB_PRECISION_32(07)
 -#define DDL_PLANEB_SHIFT   0
 -
 -#define VLV_DDL3   (VLV_DISPLAY_BASE + 0x70058)
 -#define DDL_CURSORC_PRECISION_64   (131)
 -#define DDL_CURSORC_PRECISION_32   (031)
 -#define DDL_CURSORC_SHIFT  24
 -#define DDL_SPRITEF_PRECISION_64   (123)
 -#define DDL_SPRITEF_PRECISION_32   (023)
 -#define DDL_SPRITEF_SHIFT  16
 -#define DDL_SPRITEE_PRECISION_64   (115)
 -#define DDL_SPRITEE_PRECISION_32   (015)
 -#define DDL_SPRITEE_SHIFT  8
 -#define DDL_PLANEC_PRECISION_64(17)
 -#define DDL_PLANEC_PRECISION_32(07)
 -#define DDL_PLANEC_SHIFT   0
 +#define VLV_DDL(pipe)  (VLV_DISPLAY_BASE + 0x70050 + 4 * 
 (pipe))
 +#define DDL_CURSOR_PRECISION_64(131)
 +#define DDL_CURSOR_PRECISION_32(031)
 +#define DDL_CURSOR_SHIFT   24
 +#define DDL_SPRITE1_PRECISION_64   (123)
 +#define DDL_SPRITE1_PRECISION_32   (023)
 +#define DDL_SPRITE1_SHIFT  16
 +#define DDL_SPRITE0_PRECISION_64   (115)
 +#define DDL_SPRITE0_PRECISION_32   (015)
 +#define DDL_SPRITE0_SHIFT  8
 +#define DDL_PLANE_PRECISION_64 (17)
 +#define DDL_PLANE_PRECISION_32 (07)
 +#define DDL_PLANE_SHIFT0

  /* 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 ==