On Mon, 25 Feb 2019, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> 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 <rodrigo.vivi@xxxxxxxxx>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
>>> ---
>>> 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
>>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>--
>>Ville Syrjälä
>>Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
