[Intel-gfx] [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State

[Lucas De Marchi]

On Tue, Feb 26, 2019 at 12:10:29PM +0200, Jani Nikula wrote:
>On Mon, 25 Feb 2019, Lucas De Marchi <lucas.demarchi at intel.com> 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 at intel.com>
>>>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>>> ---
>>>>  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.

Ok. I disagree because it's a pain when unrelated bits change in the
spec and we have to check if we need to do anything.

But it seems I'm alone here, so I will withdraw this patch and replace
it with the typo fix.

Lucas De Marchi

>
>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 at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>>--
>>>Ville Syrjälä
>>>Intel
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center