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

Lucas De Marchi lucas.demarchi at intel.com
Mon Feb 25 21:54:16 UTC 2019


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?

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


More information about the Intel-gfx mailing list