Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL

2019-03-06 Thread Ville Syrjälä 
On Tue, Mar 05, 2019 at 01:07:34PM -0800, Lucas De Marchi wrote:
> On Tue, Mar 05, 2019 at 03:23:48PM +0200, Jani Nikula wrote:
> >On Mon, 04 Mar 2019, Jani Nikula  wrote:
> >> On Mon, 04 Mar 2019, Ville Syrjälä  wrote:
> >>> On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote:
>  This register was placed in the middle of the PP_STATUS definition. Move
>  it down together with PP_CONTROL and fix the aligment of the bit
>  definition (as per documentation it should be 2 spaces instead of 1).
> 
>  Signed-off-by: Lucas De Marchi 
>  ---
>   drivers/gpu/drm/i915/i915_reg.h | 22 +++---
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>  b/drivers/gpu/drm/i915/i915_reg.h
>  index c9b868347481..bbbc0649a180 100644
>  --- a/drivers/gpu/drm/i915/i915_reg.h
>  +++ b/drivers/gpu/drm/i915/i915_reg.h
>  @@ -4692,17 +4692,6 @@ enum {
>   #define _PP_STATUS  0x61200
>   #define PP_STATUS(pps_idx)  _MMIO_PPS(pps_idx, _PP_STATUS)
>   #define   PP_ON (1 << 31)
>  -
>  -#define _PP_CONTROL_1   0xc7204
>  -#define _PP_CONTROL_2   0xc7304
>  -#define ICP_PP_CONTROL(x)   _MMIO(((x) == 1) ? 
>  _PP_CONTROL_1 : \
>  -  _PP_CONTROL_2)
>  -#define  POWER_CYCLE_DELAY_MASK (0x1f << 4)
>  -#define  POWER_CYCLE_DELAY_SHIFT4
>  -#define  VDD_OVERRIDE_FORCE (1 << 3)
>  -#define  BACKLIGHT_ENABLE   (1 << 2)
>  -#define  PWR_DOWN_ON_RESET  (1 << 1)
>  -#define  PWR_STATE_TARGET   (1 << 0)
>   /*
>    * Indicates that all dependencies of the panel are on:
>    *
>  @@ -4728,6 +4717,17 @@ enum {
>   #define   PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0)
>   #define   PP_SEQUENCE_STATE_RESET   (0xf << 0)
> 
>  +#define _PP_CONTROL_1   0xc7204
>  +#define _PP_CONTROL_2   0xc7304
>  +#define ICP_PP_CONTROL(x)   _MMIO(((x) == 1) ? 
>  _PP_CONTROL_1 : \
>  +  _PP_CONTROL_2)
>  +#define   POWER_CYCLE_DELAY_MASK(0x1f << 4)
>  +#define   POWER_CYCLE_DELAY_SHIFT   4
>  +#define   VDD_OVERRIDE_FORCE(1 << 3)
>  +#define   BACKLIGHT_ENABLE  (1 << 2)
>  +#define   PWR_DOWN_ON_RESET (1 << 1)
>  +#define   PWR_STATE_TARGET  (1 << 0)
> >>>
> >>> This entire register looks 100% redundant. Just nuke the whole thing?
> >>
> >> Needed in the future?
> >
> >D'oh, missed the PPS base thing. Nuke it.
> 
> But ICP_PP_CONTROL() is also unused. Should I nuke it as well?

Yes.

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL

2019-03-05 Thread Lucas De Marchi 

On Tue, Mar 05, 2019 at 03:23:48PM +0200, Jani Nikula wrote:

On Mon, 04 Mar 2019, Jani Nikula  wrote:

On Mon, 04 Mar 2019, Ville Syrjälä  wrote:

On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote:

This register was placed in the middle of the PP_STATUS definition. Move
it down together with PP_CONTROL and fix the aligment of the bit
definition (as per documentation it should be 2 spaces instead of 1).

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_reg.h | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c9b868347481..bbbc0649a180 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4692,17 +4692,6 @@ enum {
 #define _PP_STATUS 0x61200
 #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS)
 #define   PP_ON(1 << 31)
-
-#define _PP_CONTROL_1  0xc7204
-#define _PP_CONTROL_2  0xc7304
-#define ICP_PP_CONTROL(x)  _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
- _PP_CONTROL_2)
-#define  POWER_CYCLE_DELAY_MASK(0x1f << 4)
-#define  POWER_CYCLE_DELAY_SHIFT   4
-#define  VDD_OVERRIDE_FORCE(1 << 3)
-#define  BACKLIGHT_ENABLE  (1 << 2)
-#define  PWR_DOWN_ON_RESET (1 << 1)
-#define  PWR_STATE_TARGET  (1 << 0)
 /*
  * Indicates that all dependencies of the panel are on:
  *
@@ -4728,6 +4717,17 @@ enum {
 #define   PP_SEQUENCE_STATE_ON_S1_3(0xb << 0)
 #define   PP_SEQUENCE_STATE_RESET  (0xf << 0)

+#define _PP_CONTROL_1  0xc7204
+#define _PP_CONTROL_2  0xc7304
+#define ICP_PP_CONTROL(x)  _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
+ _PP_CONTROL_2)
+#define   POWER_CYCLE_DELAY_MASK   (0x1f << 4)
+#define   POWER_CYCLE_DELAY_SHIFT  4
+#define   VDD_OVERRIDE_FORCE   (1 << 3)
+#define   BACKLIGHT_ENABLE (1 << 2)
+#define   PWR_DOWN_ON_RESET(1 << 1)
+#define   PWR_STATE_TARGET (1 << 0)


This entire register looks 100% redundant. Just nuke the whole thing?


Needed in the future?


D'oh, missed the PPS base thing. Nuke it.


But ICP_PP_CONTROL() is also unused. Should I nuke it as well?

Lucas De Marchi



BR,
Jani.




BR,
Jani.




+
 #define _PP_CONTROL0x61204
 #define PP_CONTROL(pps_idx)_MMIO_PPS(pps_idx, _PP_CONTROL)
 #define  PANEL_UNLOCK_REGS (0xabcd << 16)
--
2.20.1


--
Jani Nikula, Intel Open Source Graphics Center

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL

2019-03-05 Thread Jani Nikula 
On Mon, 04 Mar 2019, Jani Nikula  wrote:
> On Mon, 04 Mar 2019, Ville Syrjälä  wrote:
>> On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote:
>>> This register was placed in the middle of the PP_STATUS definition. Move
>>> it down together with PP_CONTROL and fix the aligment of the bit
>>> definition (as per documentation it should be 2 spaces instead of 1).
>>> 
>>> Signed-off-by: Lucas De Marchi 
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h | 22 +++---
>>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index c9b868347481..bbbc0649a180 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -4692,17 +4692,6 @@ enum {
>>>  #define _PP_STATUS 0x61200
>>>  #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS)
>>>  #define   PP_ON(1 << 31)
>>> -
>>> -#define _PP_CONTROL_1  0xc7204
>>> -#define _PP_CONTROL_2  0xc7304
>>> -#define ICP_PP_CONTROL(x)  _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
>>> - _PP_CONTROL_2)
>>> -#define  POWER_CYCLE_DELAY_MASK(0x1f << 4)
>>> -#define  POWER_CYCLE_DELAY_SHIFT   4
>>> -#define  VDD_OVERRIDE_FORCE(1 << 3)
>>> -#define  BACKLIGHT_ENABLE  (1 << 2)
>>> -#define  PWR_DOWN_ON_RESET (1 << 1)
>>> -#define  PWR_STATE_TARGET  (1 << 0)
>>>  /*
>>>   * Indicates that all dependencies of the panel are on:
>>>   *
>>> @@ -4728,6 +4717,17 @@ enum {
>>>  #define   PP_SEQUENCE_STATE_ON_S1_3(0xb << 0)
>>>  #define   PP_SEQUENCE_STATE_RESET  (0xf << 0)
>>>  
>>> +#define _PP_CONTROL_1  0xc7204
>>> +#define _PP_CONTROL_2  0xc7304
>>> +#define ICP_PP_CONTROL(x)  _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
>>> + _PP_CONTROL_2)
>>> +#define   POWER_CYCLE_DELAY_MASK   (0x1f << 4)
>>> +#define   POWER_CYCLE_DELAY_SHIFT  4
>>> +#define   VDD_OVERRIDE_FORCE   (1 << 3)
>>> +#define   BACKLIGHT_ENABLE (1 << 2)
>>> +#define   PWR_DOWN_ON_RESET(1 << 1)
>>> +#define   PWR_STATE_TARGET (1 << 0)
>>
>> This entire register looks 100% redundant. Just nuke the whole thing?
>
> Needed in the future?

D'oh, missed the PPS base thing. Nuke it.

BR,
Jani.


>
> BR,
> Jani.
>
>>
>>> +
>>>  #define _PP_CONTROL0x61204
>>>  #define PP_CONTROL(pps_idx)_MMIO_PPS(pps_idx, _PP_CONTROL)
>>>  #define  PANEL_UNLOCK_REGS (0xabcd << 16)
>>> -- 
>>> 2.20.1

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL

2019-03-04 Thread Jani Nikula 
On Mon, 04 Mar 2019, Ville Syrjälä  wrote:
> On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote:
>> This register was placed in the middle of the PP_STATUS definition. Move
>> it down together with PP_CONTROL and fix the aligment of the bit
>> definition (as per documentation it should be 2 spaces instead of 1).
>> 
>> Signed-off-by: Lucas De Marchi 
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h | 22 +++---
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index c9b868347481..bbbc0649a180 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4692,17 +4692,6 @@ enum {
>>  #define _PP_STATUS  0x61200
>>  #define PP_STATUS(pps_idx)  _MMIO_PPS(pps_idx, _PP_STATUS)
>>  #define   PP_ON (1 << 31)
>> -
>> -#define _PP_CONTROL_1   0xc7204
>> -#define _PP_CONTROL_2   0xc7304
>> -#define ICP_PP_CONTROL(x)   _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
>> -  _PP_CONTROL_2)
>> -#define  POWER_CYCLE_DELAY_MASK (0x1f << 4)
>> -#define  POWER_CYCLE_DELAY_SHIFT4
>> -#define  VDD_OVERRIDE_FORCE (1 << 3)
>> -#define  BACKLIGHT_ENABLE   (1 << 2)
>> -#define  PWR_DOWN_ON_RESET  (1 << 1)
>> -#define  PWR_STATE_TARGET   (1 << 0)
>>  /*
>>   * Indicates that all dependencies of the panel are on:
>>   *
>> @@ -4728,6 +4717,17 @@ enum {
>>  #define   PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0)
>>  #define   PP_SEQUENCE_STATE_RESET   (0xf << 0)
>>  
>> +#define _PP_CONTROL_1   0xc7204
>> +#define _PP_CONTROL_2   0xc7304
>> +#define ICP_PP_CONTROL(x)   _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
>> +  _PP_CONTROL_2)
>> +#define   POWER_CYCLE_DELAY_MASK(0x1f << 4)
>> +#define   POWER_CYCLE_DELAY_SHIFT   4
>> +#define   VDD_OVERRIDE_FORCE(1 << 3)
>> +#define   BACKLIGHT_ENABLE  (1 << 2)
>> +#define   PWR_DOWN_ON_RESET (1 << 1)
>> +#define   PWR_STATE_TARGET  (1 << 0)
>
> This entire register looks 100% redundant. Just nuke the whole thing?

Needed in the future?

BR,
Jani.

>
>> +
>>  #define _PP_CONTROL 0x61204
>>  #define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL)
>>  #define  PANEL_UNLOCK_REGS  (0xabcd << 16)
>> -- 
>> 2.20.1

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL

2019-03-04 Thread Ville Syrjälä 
On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote:
> This register was placed in the middle of the PP_STATUS definition. Move
> it down together with PP_CONTROL and fix the aligment of the bit
> definition (as per documentation it should be 2 spaces instead of 1).
> 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c9b868347481..bbbc0649a180 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4692,17 +4692,6 @@ enum {
>  #define _PP_STATUS   0x61200
>  #define PP_STATUS(pps_idx)   _MMIO_PPS(pps_idx, _PP_STATUS)
>  #define   PP_ON  (1 << 31)
> -
> -#define _PP_CONTROL_10xc7204
> -#define _PP_CONTROL_20xc7304
> -#define ICP_PP_CONTROL(x)_MMIO(((x) == 1) ? _PP_CONTROL_1 : \
> -   _PP_CONTROL_2)
> -#define  POWER_CYCLE_DELAY_MASK  (0x1f << 4)
> -#define  POWER_CYCLE_DELAY_SHIFT 4
> -#define  VDD_OVERRIDE_FORCE  (1 << 3)
> -#define  BACKLIGHT_ENABLE(1 << 2)
> -#define  PWR_DOWN_ON_RESET   (1 << 1)
> -#define  PWR_STATE_TARGET(1 << 0)
>  /*
>   * Indicates that all dependencies of the panel are on:
>   *
> @@ -4728,6 +4717,17 @@ enum {
>  #define   PP_SEQUENCE_STATE_ON_S1_3  (0xb << 0)
>  #define   PP_SEQUENCE_STATE_RESET(0xf << 0)
>  
> +#define _PP_CONTROL_10xc7204
> +#define _PP_CONTROL_20xc7304
> +#define ICP_PP_CONTROL(x)_MMIO(((x) == 1) ? _PP_CONTROL_1 : \
> +   _PP_CONTROL_2)
> +#define   POWER_CYCLE_DELAY_MASK (0x1f << 4)
> +#define   POWER_CYCLE_DELAY_SHIFT4
> +#define   VDD_OVERRIDE_FORCE (1 << 3)
> +#define   BACKLIGHT_ENABLE   (1 << 2)
> +#define   PWR_DOWN_ON_RESET  (1 << 1)
> +#define   PWR_STATE_TARGET   (1 << 0)

This entire register looks 100% redundant. Just nuke the whole thing?

> +
>  #define _PP_CONTROL  0x61204
>  #define PP_CONTROL(pps_idx)  _MMIO_PPS(pps_idx, _PP_CONTROL)
>  #define  PANEL_UNLOCK_REGS   (0xabcd << 16)
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx