On Thu, Sep 04, 2014 at 12:27:10PM +0100, Damien Lespiau wrote:
> From: Pradeep Bhat <pradeep.b...@intel.com>
> 
> This patch defines SKL specific PLANE_WM Watermark registers. It also
> defines macros to get the addresses of different LP levels within a pipe.
> 
> v2: Reworked the register definitions and associated macros to make it more
>     generic and be able to use for_each_pipe in values computation.
>     Incorporated Damien's review comments and indentation.
> 
> v3: Added default values for lines and blocks. Provided mask for blocks.
> 
> Signed-off-by: Pradeep Bhat <pradeep.b...@intel.com>
> Signed-off-by: Damien Lespiau <damien.lesp...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bc55990..9fbce2c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4066,6 +4066,43 @@ enum punit_power_well {
>  #define I965_CURSOR_MAX_WM   32
>  #define I965_CURSOR_DFT_WM   8
>  
> +/* Watermark register definitions for SKL */
> +#define CUR_WM_A_0           0x70140
> +#define CUR_WM_B_0           0x71140
> +#define PLANE_WM_1_A_0               0x70240
> +#define PLANE_WM_1_B_0               0x71240
> +#define PLANE_WM_2_A_0               0x70340
> +#define PLANE_WM_2_B_0               0x71340
> +#define PLANE_WM_TRANS_1_A_0 0x70268
> +#define PLANE_WM_TRANS_1_B_0 0x71268
> +#define PLANE_WM_TRANS_2_A_0 0x70368
> +#define PLANE_WM_TRANS_2_B_0 0x71368
> +#define CUR_WM_TRANS_A_0     0x70168
> +#define CUR_WM_TRANS_B_0     0x71168
> +#define   PLANE_WM_EN                (1 << 31)
> +#define   PLANE_WM_LINES_SHIFT       14
> +#define   PLANE_WM_LINES_MASK        0x1f
> +#define   PLANE_WM_BLOCKS_MASK       0x3ff
> +#define   PLANE_WM_LINES_DEFAULT     0x1
> +#define   PLANE_WM_BLOCKS_DEFAULT    0x7
> +

These numbers seemed to make sense when I tried to compare with the
spec.

> +#define CUR_WM_0(pipe) _PIPE(pipe, CUR_WM_A_0, CUR_WM_B_0)
> +#define CUR_WM(pipe, level) (CUR_WM_0(pipe) + ((4) * (level)))
> +#define CUR_WM_TRANS(pipe) _PIPE(pipe, CUR_WM_TRANS_A_0, CUR_WM_TRANS_B_0)
> +
> +#define PLANE_WM_1(pipe) _PIPE(pipe, PLANE_WM_1_A_0, PLANE_WM_1_B_0)
> +#define PLANE_WM_2(pipe) _PIPE(pipe, PLANE_WM_2_A_0, PLANE_WM_2_B_0)
> +#define PLANE_WM_BASE(pipe, plane)   \
> +                     _PLANE(plane, PLANE_WM_1(pipe), PLANE_WM_2(pipe))
> +#define PLANE_WM(pipe, plane, level) \
> +                     (PLANE_WM_BASE(pipe, plane) + ((4) * (level)))
> +#define PLANE_WM_TRANS_1(pipe)               \
> +                     _PIPE(pipe, PLANE_WM_TRANS_1_A_0, PLANE_WM_TRANS_1_B_0)
> +#define PLANE_WM_TRANS_2(pipe)               \
> +                     _PIPE(pipe, PLANE_WM_TRANS_2_A_0, PLANE_WM_TRANS_2_B_0)
> +#define PLANE_WM_TRANS(pipe, plane)  \
> +             _PLANE(plane, PLANE_WM_TRANS_1(pipe), PLANE_WM_TRANS_2(pipe))

I must admit to my eyes glazing over when trying to parse these macros.
Not sure if a bit less redirection might help here. Anyway I plugged them
into a small test program and the resulting register offsets were all good.

Just one small nit: if the intermediate macros aren't supposed to be used
by anywhere outside this file then they might be prefixed with _ to make
it clear they're internal.

Either way:
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> +
>  /* define the Watermark register on Ironlake */
>  #define WM0_PIPEA_ILK                0x45100
>  #define  WM0_PIPE_PLANE_MASK (0xffff<<16)
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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