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

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
@ 2019-02-23  0:34 Lucas De Marchi
  2019-02-23  0:34 ` [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS Lucas De Marchi
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Lucas De Marchi @ 2019-02-23  0:34 UTC (permalink / raw)
  To: intel-gfx

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@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@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
 
 #define _PP_CONTROL			0x61204
 #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
-- 
2.20.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS
  2019-02-23  0:34 [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State Lucas De Marchi
@ 2019-02-23  0:34 ` Lucas De Marchi
  2019-02-25 19:31   ` Ville Syrjälä
  2019-02-23  2:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Lucas De Marchi @ 2019-02-23  0:34 UTC (permalink / raw)
  To: intel-gfx

Instead of checking the bits that give the internal machine state we can
simply rely on the information from the other bits: 1) on or off,
2) transitioning or not.

Bit 31 has the "Panel Power On Status"
Bits 29:28 has the "Power Sequence Progress"

So, wait_panel_on() only needs to wait for bit 31 to indicate it's on
and bits 29:28 to indicate there's no transition in progress.

From my limited test that includes the cycle delay, so we are safe with
only checking those bits, like we do in wait_panel_off().

Admittedly this needs more test, so let CI go through more platforms.

Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e1a051c0fbfe..9c16b69043cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2315,8 +2315,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 	}
 }
 
-#define IDLE_ON_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | PP_SEQUENCE_STATE_MASK)
-#define IDLE_ON_VALUE   	(PP_ON | PP_SEQUENCE_NONE | 0                     | PP_SEQUENCE_STATE_ON_IDLE)
+#define IDLE_ON_MASK		(PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | 0)
+#define IDLE_ON_VALUE		(PP_ON | PP_SEQUENCE_NONE | 0			  | 0)
 
 #define IDLE_OFF_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | 0)
 #define IDLE_OFF_VALUE		(0     | PP_SEQUENCE_NONE | 0                     | 0)
-- 
2.20.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS
  2019-02-23  0:34 ` [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS Lucas De Marchi
@ 2019-02-25 19:31   ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2019-02-25 19:31 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

On Fri, Feb 22, 2019 at 04:34:49PM -0800, Lucas De Marchi wrote:
> Instead of checking the bits that give the internal machine state we can
> simply rely on the information from the other bits: 1) on or off,
> 2) transitioning or not.
> 
> Bit 31 has the "Panel Power On Status"
> Bits 29:28 has the "Power Sequence Progress"
> 
> So, wait_panel_on() only needs to wait for bit 31 to indicate it's on
> and bits 29:28 to indicate there's no transition in progress.
> 
> From my limited test that includes the cycle delay, so we are safe with
> only checking those bits, like we do in wait_panel_off().

Is there a specific benefit to not checking the state?

> 
> Admittedly this needs more test, so let CI go through more platforms.

CI is probably not going to tell us if we break this and start angering
the panels.

The details of the state machine escape me right now, so can't remember
if there's some state which would somehow not be indicated quite right
by the other bits.

> 
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e1a051c0fbfe..9c16b69043cc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2315,8 +2315,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  	}
>  }
>  
> -#define IDLE_ON_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | PP_SEQUENCE_STATE_MASK)
> -#define IDLE_ON_VALUE   	(PP_ON | PP_SEQUENCE_NONE | 0                     | PP_SEQUENCE_STATE_ON_IDLE)
> +#define IDLE_ON_MASK		(PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | 0)
> +#define IDLE_ON_VALUE		(PP_ON | PP_SEQUENCE_NONE | 0			  | 0)
>  
>  #define IDLE_OFF_MASK		(PP_ON | PP_SEQUENCE_MASK | 0                     | 0)
>  #define IDLE_OFF_VALUE		(0     | PP_SEQUENCE_NONE | 0                     | 0)
> -- 
> 2.20.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-23  0:34 [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State Lucas De Marchi
  2019-02-23  0:34 ` [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS Lucas De Marchi
@ 2019-02-23  2:20 ` Patchwork
  2019-02-23 10:55 ` ✓ Fi.CI.IGT: " Patchwork
  2019-02-25 19:28 ` [PATCH 1/2] " Ville Syrjälä
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-02-23  2:20 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State
URL   : https://patchwork.freedesktop.org/series/57122/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5655 -> Patchwork_12293
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57122/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12293 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       PASS -> DMESG-WARN [fdo#108965]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-7560u:       PASS -> INCOMPLETE [fdo#108044] / [fdo#108744]

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       DMESG-WARN [fdo#105602] / [fdo#108529] -> PASS +1

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       SKIP [fdo#109271] -> PASS

  * igt@i915_pm_rpm@basic-rte:
    - fi-bsw-kefka:       FAIL [fdo#108800] -> PASS

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7567u:       DMESG-WARN [fdo#108529] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] -> WARN [fdo#109380]

  
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380


Participating hosts (45 -> 39)
------------------------------

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5655 -> Patchwork_12293

  CI_DRM_5655: a40729237602fa7454aaf3355ad3058cad5c6ee9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4853: 8afdfd8fa9ce17043d9105dedca46ad4555fdcdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12293: 4d73919911f45471d1c338ddf5a8bbd69407c154 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4d73919911f4 drm/i915: don't check internal state in PP_STATUS
1639b1dcfdc7 drm/i915: remove unused bits from Panel Power Sequence State

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12293/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-23  0:34 [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State Lucas De Marchi
  2019-02-23  0:34 ` [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS Lucas De Marchi
  2019-02-23  2:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State Patchwork
@ 2019-02-23 10:55 ` Patchwork
  2019-02-25 19:28 ` [PATCH 1/2] " Ville Syrjälä
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-02-23 10:55 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State
URL   : https://patchwork.freedesktop.org/series/57122/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5655_full -> Patchwork_12293_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12293_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@extended-parallel-bsd1:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +7

  * igt@gem_busy@extended-semaphore-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109275] +3

  * igt@gem_busy@extended-semaphore-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109275] / [fdo#109276] +1

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109281] +17

  * igt@gem_ctx_isolation@vcs1-reset:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] / [fdo#109281] +5

  * igt@gem_ctx_param@invalid-param-get:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109559]

  * igt@gem_ctx_param@invalid-param-set:
    - shard-kbl:          NOTRUN -> FAIL [fdo#109674]
    - shard-iclb:         NOTRUN -> FAIL [fdo#109674]

  * igt@gem_ctx_param@set-priority-not-supported:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109314]

  * igt@gem_eio@reset-stress:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540] / [fdo#109482]

  * igt@gem_exec_flush@basic-batch-kernel-default-cmd:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109313]

  * igt@gem_exec_params@no-vebox:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109283] +3

  * igt@gem_exec_parse@basic-rejected:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289] +19

  * igt@gem_exec_schedule@preempt-other-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +117

  * igt@gem_mmap_gtt@coherency:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109292] +1

  * igt@gem_mocs_settings@mocs-reset-ctx-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109287] +16

  * igt@gem_mocs_settings@mocs-settings-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] / [fdo#109287] +5

  * igt@gem_pwrite@huge-gtt-backwards:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290] +8

  * igt@gem_softpin@evict-snoop:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109312] +1

  * igt@gem_softpin@noreloc-s3:
    - shard-iclb:         NOTRUN -> INCOMPLETE [fdo#107713]

  * igt@gem_stolen@stolen-clear:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277] +13

  * igt@i915_missed_irq:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109503]

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107847]

  * igt@i915_pm_lpsp@edp-native:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109301] +2

  * igt@i915_pm_rpm@dpms-mode-unset-non-lpsp:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109308] +3

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107724] +8

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         NOTRUN -> DMESG-FAIL [fdo#107724]

  * igt@i915_pm_rpm@modeset-pc8-residency-stress:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109293] +1

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#108654]

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         NOTRUN -> FAIL [fdo#102250]

  * igt@i915_pm_rps@reset:
    - shard-iclb:         NOTRUN -> FAIL [fdo#102250] / [fdo#108059] +1

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109303]

  * igt@i915_query@query-topology-unsupported:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109302]

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_suspend@fence-restore-untiled:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-iclb:         NOTRUN -> FAIL [fdo#106641]

  * igt@kms_busy@basic-flip-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@basic-modeset-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956] +7

  * igt@kms_busy@extended-modeset-hang-newfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +48

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107725] +8

  * igt@kms_chamelium@dp-frame-dump:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +40

  * igt@kms_color@pipe-b-gamma:
    - shard-iclb:         NOTRUN -> FAIL [fdo#104782] +8

  * igt@kms_color@pipe-c-ctm-max:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108147] +2

  * igt@kms_color@pipe-c-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_content_protection@legacy:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109300] / [fdo#109527] +1

  * igt@kms_crtc_background_color:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109305]

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232] +22

  * igt@kms_cursor_crc@cursor-512x512-rapid-movement:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109279] +9

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109349] +1

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103833] +1

  * igt@kms_flip@2x-flip-vs-rmfb-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +64

  * igt@kms_force_connector_basic@force-edid:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109285] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-apl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167] +9

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-blt:
    - shard-snb:          PASS -> SKIP [fdo#109271] +7

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-shrfb-fliptrack:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +198

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +41

  * igt@kms_hdmi_inject@inject-audio:
    - shard-iclb:         NOTRUN -> FAIL [fdo#102370]

  * igt@kms_invalid_dotclock:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109310]

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103166] +7

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108948]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_scaling@2x-scaler-multi-pipe:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] / [fdo#109278]

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441] +19

  * igt@kms_setmode@basic:
    - shard-iclb:         NOTRUN -> FAIL [fdo#99912]

  * igt@kms_tv_load_detect@load-detect:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109309]

  * igt@kms_vblank@pipe-b-query-idle-hang:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@kms_vrr@flip-basic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109502] +2

  * igt@prime_nv_pcopy@test2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291] +26

  * igt@prime_vgem@basic-fence-flip:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109294]

  * igt@prime_vgem@fence-write-hang:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109295] +1

  * igt@testdisplay:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@tools_test@sysfs_l3_parity:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109307]

  * igt@v3d_get_param@get-bad-flags:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109315] +5

  
#### Possible fixes ####

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#102370]: https://bugs.freedesktop.org/show_bug.cgi?id=102370
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109275]: https://bugs.freedesktop.org/show_bug.cgi?id=109275
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109292]: https://bugs.freedesktop.org/show_bug.cgi?id=109292
  [fdo#109293]: https://bugs.freedesktop.org/show_bug.cgi?id=109293
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109301]: https://bugs.freedesktop.org/show_bug.cgi?id=109301
  [fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109305]: https://bugs.freedesktop.org/show_bug.cgi?id=109305
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109310]: https://bugs.freedesktop.org/show_bug.cgi?id=109310
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109314]: https://bugs.freedesktop.org/show_bug.cgi?id=109314
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109482]: https://bugs.freedesktop.org/show_bug.cgi?id=109482
  [fdo#109502]: https://bugs.freedesktop.org/show_bug.cgi?id=109502
  [fdo#109503]: https://bugs.freedesktop.org/show_bug.cgi?id=109503
  [fdo#109527]: https://bugs.freedesktop.org/show_bug.cgi?id=109527
  [fdo#109559]: https://bugs.freedesktop.org/show_bug.cgi?id=109559
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109674]: https://bugs.freedesktop.org/show_bug.cgi?id=109674
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (6 -> 6)
------------------------------

  Additional (1): shard-iclb 
  Missing    (1): shard-skl 


Build changes
-------------

    * Linux: CI_DRM_5655 -> Patchwork_12293

  CI_DRM_5655: a40729237602fa7454aaf3355ad3058cad5c6ee9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4853: 8afdfd8fa9ce17043d9105dedca46ad4555fdcdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12293: 4d73919911f45471d1c338ddf5a8bbd69407c154 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12293/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-23  0:34 [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State Lucas De Marchi
                   ` (2 preceding siblings ...)
  2019-02-23 10:55 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-02-25 19:28 ` Ville Syrjälä
  2019-02-25 21:54   ` Lucas De Marchi
  3 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2019-02-25 19:28 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

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@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@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?

>  
>  #define _PP_CONTROL			0x61204
>  #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
> -- 
> 2.20.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-25 19:28 ` [PATCH 1/2] " Ville Syrjälä
@ 2019-02-25 21:54   ` Lucas De Marchi
  2019-02-26 10:10     ` Jani Nikula
  2019-02-26 13:49     ` Ville Syrjälä
  0 siblings, 2 replies; 10+ messages in thread
From: Lucas De Marchi @ 2019-02-25 21:54 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

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@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-25 21:54   ` Lucas De Marchi
@ 2019-02-26 10:10     ` Jani Nikula
  2019-02-26 19:20       ` Lucas De Marchi
  2019-02-26 13:49     ` Ville Syrjälä
  1 sibling, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2019-02-26 10:10 UTC (permalink / raw)
  To: Lucas De Marchi, Ville Syrjälä; +Cc: intel-gfx

On Mon, 25 Feb 2019, Lucas De Marchi <lucas.demarchi@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@intel.com>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@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.

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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>-- 
>>Ville Syrjälä
>>Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-26 10:10     ` Jani Nikula
@ 2019-02-26 19:20       ` Lucas De Marchi
  0 siblings, 0 replies; 10+ messages in thread
From: Lucas De Marchi @ 2019-02-26 19:20 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Feb 26, 2019 at 12:10:29PM +0200, Jani Nikula wrote:
>On Mon, 25 Feb 2019, Lucas De Marchi <lucas.demarchi@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@intel.com>
>>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@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@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>>--
>>>Ville Syrjälä
>>>Intel
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State
  2019-02-25 21:54   ` Lucas De Marchi
  2019-02-26 10:10     ` Jani Nikula
@ 2019-02-26 13:49     ` Ville Syrjälä
  1 sibling, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2019-02-26 13:49 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

On Mon, Feb 25, 2019 at 01:54:16PM -0800, Lucas De Marchi 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@intel.com>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@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?

I have in the past looked at logs where I had to decode the state
bits to figure out where it was going.

> 
> 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@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >-- 
> >Ville Syrjälä
> >Intel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-02-26 19:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-23  0:34 [PATCH 1/2] drm/i915: remove unused bits from Panel Power Sequence State Lucas De Marchi
2019-02-23  0:34 ` [PATCH 2/2] drm/i915: don't check internal state in PP_STATUS Lucas De Marchi
2019-02-25 19:31   ` Ville Syrjälä
2019-02-23  2:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: remove unused bits from Panel Power Sequence State Patchwork
2019-02-23 10:55 ` ✓ Fi.CI.IGT: " Patchwork
2019-02-25 19:28 ` [PATCH 1/2] " Ville Syrjälä
2019-02-25 21:54   ` Lucas De Marchi
2019-02-26 10:10     ` Jani Nikula
2019-02-26 19:20       ` Lucas De Marchi
2019-02-26 13:49     ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.