On Mon, 25 Feb 2019, Lucas De Marchi <[email protected]> wrote: > On Mon, Feb 25, 2019 at 09:28:06PM +0200, Ville Syrjälä wrote: >>On Fri, Feb 22, 2019 at 04:34:48PM -0800, Lucas De Marchi wrote: >>> No change in behavior. Just removing the unused bits since it makes it >>> easier to compare them on new platforms and one of them was wrong >>> (PP_SEQUENCE_STATE_ON_S1_0 vs the supposedly correct name >>> PP_SEQUENCE_STATE_ON_S1_1) >>> >>> Cc: Rodrigo Vivi <[email protected]> >>> Signed-off-by: Lucas De Marchi <[email protected]> >>> --- >>> drivers/gpu/drm/i915/i915_reg.h | 12 +++--------- >>> 1 file changed, 3 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index 730bb1917fd1..e855dae978db 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -4717,15 +4717,9 @@ enum { >>> #define PP_SEQUENCE_SHIFT 28 >>> #define PP_CYCLE_DELAY_ACTIVE (1 << 27) >>> #define PP_SEQUENCE_STATE_MASK 0x0000000f >>> -#define PP_SEQUENCE_STATE_OFF_IDLE (0x0 << 0) >>> -#define PP_SEQUENCE_STATE_OFF_S0_1 (0x1 << 0) >>> -#define PP_SEQUENCE_STATE_OFF_S0_2 (0x2 << 0) >>> -#define PP_SEQUENCE_STATE_OFF_S0_3 (0x3 << 0) >>> -#define PP_SEQUENCE_STATE_ON_IDLE (0x8 << 0) >>> -#define PP_SEQUENCE_STATE_ON_S1_0 (0x9 << 0) >>> -#define PP_SEQUENCE_STATE_ON_S1_2 (0xa << 0) >>> -#define PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0) >>> -#define PP_SEQUENCE_STATE_RESET (0xf << 0) >>> +#define PP_SEQUENCE_STATE_OFF_IDLE 0x0 >>> +#define PP_SEQUENCE_STATE_ON_IDLE 0x8 >>> +#define PP_SEQUENCE_STATE_RESET 0xf >> >>But how am I supposed to remember what the register values mean? > > We only care for a small subset of those and the name should already be > enough, no? We don't need to bring in all the possible bits from spec, > even worse when they are misnamed. If we keep defining what we don't use > it actually makes our life harder to crosscheck with the spec if > everything is correct. Makes sense?
Dunno, my guideline has always been to add macros for the entire register contents if you're adding anything. BR, Jani. > > Lucas De Marchi > >> >>> >>> #define _PP_CONTROL 0x61204 >>> #define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL) >>> -- >>> 2.20.0 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >>-- >>Ville Syrjälä >>Intel > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
