Re: [PATCH 44/89] drm/i915/skl: Register definitions and macros for SKL Watermark regs

Damien Lespiau
2014-09-16T14:12:10Z

On Wed, Sep 10, 2014 at 09:04:04PM +0300, Ville Syrjälä wrote:
> > +#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.

While I'm generally for simple things, there's some pleasure to be had
in abusing the preprocessor a bit. Sometimes it has to be about fun to
keep us interested :)

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

Fair enough, will spin a new version for this one.

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

--
Damien