intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/i915/icl: replace check for combo phy
@ 2018-11-13  2:45 Lucas De Marchi
  2018-11-13  2:45 ` [PATCH v2 2/2] drm/i915/icl: reverse uninit order Lucas De Marchi
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Lucas De Marchi @ 2018-11-13  2:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

These are the only places that assume ports A and B are the ones with
combo phy.  Let's use intel_port_is_combophy() there to make sure
it checks for combo phy ports the same way everywhere.

v2: define for_each_combo_port() helper to check the ports

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---

v2 of https://patchwork.freedesktop.org/series/52269/

 drivers/gpu/drm/i915/intel_combo_phy.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index f7c16f6724f0..49f3a533860d 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -5,6 +5,10 @@
 
 #include "intel_drv.h"
 
+#define for_each_combo_port(__dev_priv, __port) \
+	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))
+
 enum {
 	PROCMON_0_85V_DOT_0,
 	PROCMON_0_95V_DOT_0,
@@ -199,7 +203,7 @@ void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	enum port port;
 
-	for (port = PORT_A; port <= PORT_B; port++) {
+	for_each_combo_port(dev_priv, port) {
 		u32 val;
 
 		if (icl_combo_phy_verify_state(dev_priv, port)) {
@@ -228,7 +232,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 {
 	enum port port;
 
-	for (port = PORT_A; port <= PORT_B; port++) {
+	for_each_combo_port(dev_priv, port) {
 		u32 val;
 
 		if (!icl_combo_phy_verify_state(dev_priv, port))
-- 
2.19.1.1.g56c4683e68

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

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

* [PATCH v2 2/2] drm/i915/icl: reverse uninit order
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
@ 2018-11-13  2:45 ` Lucas De Marchi
  2018-11-13 13:19   ` Imre Deak
  2018-11-13  3:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy Patchwork
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Lucas De Marchi @ 2018-11-13  2:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

Bspec 21257 says "DDIA PHY is the comp master, so it must
not be un-initialized if other combo PHYs are in use". Here
we are shutting down all phys, so it's not strictly required.
However let's be consistent on deinitializing things in the
reversed order we initialized them.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index 49f3a533860d..9c06be45b84e 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -9,6 +9,12 @@
 	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
 		for_each_if(intel_port_is_combophy(__dev_priv, __port))
 
+#define for_each_combo_port_rev(__dev_priv, __port) \
+	for ((__port) = I915_MAX_PORTS - 1; \
+	     (__port) >= PORT_A && (__port) < I915_MAX_PORTS; \
+	     (__port)--) \
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))
+
 enum {
 	PROCMON_0_85V_DOT_0,
 	PROCMON_0_95V_DOT_0,
@@ -232,7 +238,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 {
 	enum port port;
 
-	for_each_combo_port(dev_priv, port) {
+	for_each_combo_port_rev(dev_priv, port) {
 		u32 val;
 
 		if (!icl_combo_phy_verify_state(dev_priv, port))
-- 
2.19.1.1.g56c4683e68

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
  2018-11-13  2:45 ` [PATCH v2 2/2] drm/i915/icl: reverse uninit order Lucas De Marchi
@ 2018-11-13  3:09 ` Patchwork
  2018-11-13  3:35 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-13  3:09 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
URL   : https://patchwork.freedesktop.org/series/52400/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6b83938535de drm/i915/icl: replace check for combo phy
-:22: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__port' - possible side-effects?
#22: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:8:
+#define for_each_combo_port(__dev_priv, __port) \
+	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))

total: 0 errors, 0 warnings, 1 checks, 26 lines checked
082c96aa9390 drm/i915/icl: reverse uninit order
-:22: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__port' - possible side-effects?
#22: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:12:
+#define for_each_combo_port_rev(__dev_priv, __port) \
+	for ((__port) = I915_MAX_PORTS - 1; \
+	     (__port) >= PORT_A && (__port) < I915_MAX_PORTS; \
+	     (__port)--) \
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))

total: 0 errors, 0 warnings, 1 checks, 20 lines checked

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

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

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
  2018-11-13  2:45 ` [PATCH v2 2/2] drm/i915/icl: reverse uninit order Lucas De Marchi
  2018-11-13  3:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy Patchwork
@ 2018-11-13  3:35 ` Patchwork
  2018-11-13  5:05 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-13  3:35 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
URL   : https://patchwork.freedesktop.org/series/52400/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5127 -> Patchwork_10810 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_contexts:
      fi-icl-u:           NOTRUN -> DMESG-FAIL (fdo#108569)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-glk-dsi:         FAIL -> PASS +2

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-glk-dsi:         FAIL (fdo#103375) -> PASS +3
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@pm_rpm@module-reload:
      fi-glk-dsi:         INCOMPLETE (fdo#103359, k.org#198133) -> PASS +1

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (51 -> 45) ==

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u 


== Build changes ==

    * Linux: CI_DRM_5127 -> Patchwork_10810

  CI_DRM_5127: 31b5aaa2364ba86baf1293c69c5dcab446ef3a0e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10810: 082c96aa9390caa4b7536acf17a82efbb37bb289 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

082c96aa9390 drm/i915/icl: reverse uninit order
6b83938535de drm/i915/icl: replace check for combo phy

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
                   ` (2 preceding siblings ...)
  2018-11-13  3:35 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-13  5:05 ` Patchwork
  2018-11-13 13:44 ` [PATCH v2 1/2] " Imre Deak
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-13  5:05 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy
URL   : https://patchwork.freedesktop.org/series/52400/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5127_full -> Patchwork_10810_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drm_import_export@import-close-race-flink:
      shard-skl:          PASS -> TIMEOUT (fdo#108667)

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-hsw:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-kbl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-snb:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge:
      shard-skl:          PASS -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          PASS -> FAIL (fdo#103232, fdo#103191)

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-apl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@kms_plane@plane-position-covered-pipe-b-planes:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@pm_rpm@basic-rte:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807) +1

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries_display_off:
      shard-skl:          INCOMPLETE (fdo#104108) -> PASS

    igt@gem_cpu_reloc@full:
      shard-skl:          INCOMPLETE (fdo#108073) -> PASS

    igt@kms_cursor_crc@cursor-128x128-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
      shard-glk:          FAIL (fdo#108145) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    igt@perf_pmu@busy-start-vcs0:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +1

    igt@pm_rpm@gem-mmap-cpu:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS +1

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108667 https://bugs.freedesktop.org/show_bug.cgi?id=108667
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5127 -> Patchwork_10810

  CI_DRM_5127: 31b5aaa2364ba86baf1293c69c5dcab446ef3a0e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10810: 082c96aa9390caa4b7536acf17a82efbb37bb289 @ 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_10810/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 2/2] drm/i915/icl: reverse uninit order
  2018-11-13  2:45 ` [PATCH v2 2/2] drm/i915/icl: reverse uninit order Lucas De Marchi
@ 2018-11-13 13:19   ` Imre Deak
  2018-11-13 13:23     ` Imre Deak
  0 siblings, 1 reply; 16+ messages in thread
From: Imre Deak @ 2018-11-13 13:19 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Paulo Zanoni

On Mon, Nov 12, 2018 at 06:45:03PM -0800, Lucas De Marchi wrote:
> Bspec 21257 says "DDIA PHY is the comp master, so it must
> not be un-initialized if other combo PHYs are in use". Here
> we are shutting down all phys, so it's not strictly required.
> However let's be consistent on deinitializing things in the
> reversed order we initialized them.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 49f3a533860d..9c06be45b84e 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -9,6 +9,12 @@
>  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
>  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
>  
> +#define for_each_combo_port_rev(__dev_priv, __port) \
> +	for ((__port) = I915_MAX_PORTS - 1; \
> +	     (__port) >= PORT_A && (__port) < I915_MAX_PORTS; \
> +	     (__port)--) \

Heh, so 'enum port' is unsigned. Surely I would have get this right
only after seeing it fail with only the >= condition :) An
alternative:

	for ((__port) = I915_MAX_PORTS; (__port)-- > 0;)

or add a negative value, like INVALID_PORT=-1 to 'enum port', but we'd
need some user for that too. Either way:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> +
>  enum {
>  	PROCMON_0_85V_DOT_0,
>  	PROCMON_0_95V_DOT_0,
> @@ -232,7 +238,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
>  
> -	for_each_combo_port(dev_priv, port) {
> +	for_each_combo_port_rev(dev_priv, port) {
>  		u32 val;
>  
>  		if (!icl_combo_phy_verify_state(dev_priv, port))
> -- 
> 2.19.1.1.g56c4683e68
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 2/2] drm/i915/icl: reverse uninit order
  2018-11-13 13:19   ` Imre Deak
@ 2018-11-13 13:23     ` Imre Deak
  2018-11-13 17:40       ` Lucas De Marchi
  2018-11-13 18:56       ` [PATCH v2] " Lucas De Marchi
  0 siblings, 2 replies; 16+ messages in thread
From: Imre Deak @ 2018-11-13 13:23 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Paulo Zanoni

On Tue, Nov 13, 2018 at 03:19:06PM +0200, Imre Deak wrote:
> On Mon, Nov 12, 2018 at 06:45:03PM -0800, Lucas De Marchi wrote:
> > Bspec 21257 says "DDIA PHY is the comp master, so it must
> > not be un-initialized if other combo PHYs are in use". Here
> > we are shutting down all phys, so it's not strictly required.
> > However let's be consistent on deinitializing things in the
> > reversed order we initialized them.
> > 
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_combo_phy.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> > index 49f3a533860d..9c06be45b84e 100644
> > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -9,6 +9,12 @@
> >  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> >  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> >  
> > +#define for_each_combo_port_rev(__dev_priv, __port) \
> > +	for ((__port) = I915_MAX_PORTS - 1; \
> > +	     (__port) >= PORT_A && (__port) < I915_MAX_PORTS; \
> > +	     (__port)--) \
> 
> Heh, so 'enum port' is unsigned. Surely I would have get this right
> only after seeing it fail with only the >= condition :) An
> alternative:
> 
> 	for ((__port) = I915_MAX_PORTS; (__port)-- > 0;)
> 
> or add a negative value, like INVALID_PORT=-1 to 'enum port', but we'd

Hm, actually we do have PORT_NONE=-1, in which case 'enum port' is a
signed int. So why do we need (__port) < I915_MAX_PORTS ?

> need some user for that too. Either way:
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> > +
> >  enum {
> >  	PROCMON_0_85V_DOT_0,
> >  	PROCMON_0_95V_DOT_0,
> > @@ -232,7 +238,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> >  {
> >  	enum port port;
> >  
> > -	for_each_combo_port(dev_priv, port) {
> > +	for_each_combo_port_rev(dev_priv, port) {
> >  		u32 val;
> >  
> >  		if (!icl_combo_phy_verify_state(dev_priv, port))
> > -- 
> > 2.19.1.1.g56c4683e68
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 1/2] drm/i915/icl: replace check for combo phy
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
                   ` (3 preceding siblings ...)
  2018-11-13  5:05 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-11-13 13:44 ` Imre Deak
  2018-11-13 19:08 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Imre Deak @ 2018-11-13 13:44 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Paulo Zanoni

On Mon, Nov 12, 2018 at 06:45:02PM -0800, Lucas De Marchi wrote:
> These are the only places that assume ports A and B are the ones with
> combo phy.  Let's use intel_port_is_combophy() there to make sure
> it checks for combo phy ports the same way everywhere.
> 
> v2: define for_each_combo_port() helper to check the ports
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
> 
> v2 of https://patchwork.freedesktop.org/series/52269/
> 
>  drivers/gpu/drm/i915/intel_combo_phy.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> index f7c16f6724f0..49f3a533860d 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -5,6 +5,10 @@
>  
>  #include "intel_drv.h"
>  
> +#define for_each_combo_port(__dev_priv, __port) \
> +	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> +
>  enum {
>  	PROCMON_0_85V_DOT_0,
>  	PROCMON_0_95V_DOT_0,
> @@ -199,7 +203,7 @@ void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
>  
> -	for (port = PORT_A; port <= PORT_B; port++) {
> +	for_each_combo_port(dev_priv, port) {
>  		u32 val;
>  
>  		if (icl_combo_phy_verify_state(dev_priv, port)) {
> @@ -228,7 +232,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
>  
> -	for (port = PORT_A; port <= PORT_B; port++) {
> +	for_each_combo_port(dev_priv, port) {
>  		u32 val;
>  
>  		if (!icl_combo_phy_verify_state(dev_priv, port))
> -- 
> 2.19.1.1.g56c4683e68
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 2/2] drm/i915/icl: reverse uninit order
  2018-11-13 13:23     ` Imre Deak
@ 2018-11-13 17:40       ` Lucas De Marchi
  2018-11-13 18:56       ` [PATCH v2] " Lucas De Marchi
  1 sibling, 0 replies; 16+ messages in thread
From: Lucas De Marchi @ 2018-11-13 17:40 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, Paulo Zanoni

On Tue, Nov 13, 2018 at 03:23:07PM +0200, Imre Deak wrote:
> On Tue, Nov 13, 2018 at 03:19:06PM +0200, Imre Deak wrote:
> > On Mon, Nov 12, 2018 at 06:45:03PM -0800, Lucas De Marchi wrote:
> > > Bspec 21257 says "DDIA PHY is the comp master, so it must
> > > not be un-initialized if other combo PHYs are in use". Here
> > > we are shutting down all phys, so it's not strictly required.
> > > However let's be consistent on deinitializing things in the
> > > reversed order we initialized them.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_combo_phy.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> > > index 49f3a533860d..9c06be45b84e 100644
> > > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> > > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > > @@ -9,6 +9,12 @@
> > >  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> > >  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> > >  
> > > +#define for_each_combo_port_rev(__dev_priv, __port) \
> > > +	for ((__port) = I915_MAX_PORTS - 1; \
> > > +	     (__port) >= PORT_A && (__port) < I915_MAX_PORTS; \
> > > +	     (__port)--) \
> > 
> > Heh, so 'enum port' is unsigned. Surely I would have get this right
> > only after seeing it fail with only the >= condition :) An
> > alternative:
> > 
> > 	for ((__port) = I915_MAX_PORTS; (__port)-- > 0;)
> > 
> > or add a negative value, like INVALID_PORT=-1 to 'enum port', but we'd
> 
> Hm, actually we do have PORT_NONE=-1, in which case 'enum port' is a
> signed int. So why do we need (__port) < I915_MAX_PORTS ?

We don't actually *need*. I only thought the fact that the type is not explicit
but rather depends on the enum value, it would be to fragile and it would start
to fail silently if we ever change that. I like the alternative you gave since it's
shorter and works on both cases.

thanks
Lucas De Marchi

> 
> > need some user for that too. Either way:
> > 
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> > 
> > > +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> > > +
> > >  enum {
> > >  	PROCMON_0_85V_DOT_0,
> > >  	PROCMON_0_95V_DOT_0,
> > > @@ -232,7 +238,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> > >  {
> > >  	enum port port;
> > >  
> > > -	for_each_combo_port(dev_priv, port) {
> > > +	for_each_combo_port_rev(dev_priv, port) {
> > >  		u32 val;
> > >  
> > >  		if (!icl_combo_phy_verify_state(dev_priv, port))
> > > -- 
> > > 2.19.1.1.g56c4683e68
> > > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2] drm/i915/icl: reverse uninit order
  2018-11-13 13:23     ` Imre Deak
  2018-11-13 17:40       ` Lucas De Marchi
@ 2018-11-13 18:56       ` Lucas De Marchi
  2018-11-13 20:31         ` Jani Nikula
  1 sibling, 1 reply; 16+ messages in thread
From: Lucas De Marchi @ 2018-11-13 18:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

Bspec 21257 says "DDIA PHY is the comp master, so it must
not be un-initialized if other combo PHYs are in use". Here
we are shutting down all phys, so it's not strictly required.
However let's be consistent on deinitializing things in the
reversed order we initialized them.

v2: simplify protection for enum port being unsigned in future

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index 49f3a533860d..62939df048d7 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -9,6 +9,10 @@
 	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
 		for_each_if(intel_port_is_combophy(__dev_priv, __port))
 
+#define for_each_combo_port_rev(__dev_priv, __port) \
+	for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))
+
 enum {
 	PROCMON_0_85V_DOT_0,
 	PROCMON_0_95V_DOT_0,
@@ -232,7 +236,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 {
 	enum port port;
 
-	for_each_combo_port(dev_priv, port) {
+	for_each_combo_port_rev(dev_priv, port) {
 		u32 val;
 
 		if (!icl_combo_phy_verify_state(dev_priv, port))
-- 
2.19.1.1.g56c4683e68

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
                   ` (4 preceding siblings ...)
  2018-11-13 13:44 ` [PATCH v2 1/2] " Imre Deak
@ 2018-11-13 19:08 ` Patchwork
  2018-11-13 19:30 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-11-14 17:23 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-13 19:08 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
URL   : https://patchwork.freedesktop.org/series/52400/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
cf571a614561 drm/i915/icl: replace check for combo phy
-:23: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__port' - possible side-effects?
#23: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:8:
+#define for_each_combo_port(__dev_priv, __port) \
+	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))

total: 0 errors, 0 warnings, 1 checks, 26 lines checked
bf6d9fe15951 drm/i915/icl: reverse uninit order
-:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__port' - possible side-effects?
#24: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:12:
+#define for_each_combo_port_rev(__dev_priv, __port) \
+	for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \
+		for_each_if(intel_port_is_combophy(__dev_priv, __port))

total: 0 errors, 0 warnings, 1 checks, 18 lines checked

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

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

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
                   ` (5 preceding siblings ...)
  2018-11-13 19:08 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2) Patchwork
@ 2018-11-13 19:30 ` Patchwork
  2018-11-14 17:23 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-13 19:30 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
URL   : https://patchwork.freedesktop.org/series/52400/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5133 -> Patchwork_10818 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52400/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_create@basic-files:
      fi-bsw-kefka:       PASS -> FAIL (fdo#108656)

    igt@kms_chamelium@common-hpd-after-suspend:
      fi-skl-6700k2:      PASS -> WARN (fdo#108680)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-ilk-650:         PASS -> DMESG-WARN (fdo#106387)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-bwr-2160:        DMESG-FAIL (fdo#108735) -> PASS

    igt@gem_ctx_switch@basic-default:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
  fdo#108680 https://bugs.freedesktop.org/show_bug.cgi?id=108680
  fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735


== Participating hosts (53 -> 46) ==

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_5133 -> Patchwork_10818

  CI_DRM_5133: 5c71926a1834348a68951622a950de0355b73450 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10818: bf6d9fe159515700d2b86b87fa10304924af75aa @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bf6d9fe15951 drm/i915/icl: reverse uninit order
cf571a614561 drm/i915/icl: replace check for combo phy

== Logs ==

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

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

* Re: [PATCH v2] drm/i915/icl: reverse uninit order
  2018-11-13 18:56       ` [PATCH v2] " Lucas De Marchi
@ 2018-11-13 20:31         ` Jani Nikula
  2018-11-13 21:22           ` Lucas De Marchi
  0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2018-11-13 20:31 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Paulo Zanoni

On Tue, 13 Nov 2018, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Bspec 21257 says "DDIA PHY is the comp master, so it must
> not be un-initialized if other combo PHYs are in use". Here
> we are shutting down all phys, so it's not strictly required.
> However let's be consistent on deinitializing things in the
> reversed order we initialized them.
>
> v2: simplify protection for enum port being unsigned in future
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 49f3a533860d..62939df048d7 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -9,6 +9,10 @@
>  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
>  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
>  
> +#define for_each_combo_port_rev(__dev_priv, __port) \

The list.h list macros use _reverse suffix for reverse traversal. I
think we can afford to spend the extra letters here too.

BR,
Jani.

> +	for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \
> +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> +
>  enum {
>  	PROCMON_0_85V_DOT_0,
>  	PROCMON_0_95V_DOT_0,
> @@ -232,7 +236,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
>  
> -	for_each_combo_port(dev_priv, port) {
> +	for_each_combo_port_rev(dev_priv, port) {
>  		u32 val;
>  
>  		if (!icl_combo_phy_verify_state(dev_priv, port))

-- 
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] 16+ messages in thread

* Re: [PATCH v2] drm/i915/icl: reverse uninit order
  2018-11-13 20:31         ` Jani Nikula
@ 2018-11-13 21:22           ` Lucas De Marchi
  2018-11-13 21:57             ` Jani Nikula
  0 siblings, 1 reply; 16+ messages in thread
From: Lucas De Marchi @ 2018-11-13 21:22 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Paulo Zanoni

On Tue, Nov 13, 2018 at 10:31:25PM +0200, Jani Nikula wrote:
> On Tue, 13 Nov 2018, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > Bspec 21257 says "DDIA PHY is the comp master, so it must
> > not be un-initialized if other combo PHYs are in use". Here
> > we are shutting down all phys, so it's not strictly required.
> > However let's be consistent on deinitializing things in the
> > reversed order we initialized them.
> >
> > v2: simplify protection for enum port being unsigned in future
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_combo_phy.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> > index 49f3a533860d..62939df048d7 100644
> > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -9,6 +9,10 @@
> >  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> >  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> >  
> > +#define for_each_combo_port_rev(__dev_priv, __port) \
> 
> The list.h list macros use _reverse suffix for reverse traversal. I
> think we can afford to spend the extra letters here too.

I followed what we do on other for_each macro:
for_each_power_well_rev
for_each_power_domain_well_rev

Should those be reverse as well?

Lucas De Marchi

> 
> BR,
> Jani.
> 
> > +	for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \
> > +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
> > +
> >  enum {
> >  	PROCMON_0_85V_DOT_0,
> >  	PROCMON_0_95V_DOT_0,
> > @@ -232,7 +236,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> >  {
> >  	enum port port;
> >  
> > -	for_each_combo_port(dev_priv, port) {
> > +	for_each_combo_port_rev(dev_priv, port) {
> >  		u32 val;
> >  
> >  		if (!icl_combo_phy_verify_state(dev_priv, port))
> 
> -- 
> 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] 16+ messages in thread

* Re: [PATCH v2] drm/i915/icl: reverse uninit order
  2018-11-13 21:22           ` Lucas De Marchi
@ 2018-11-13 21:57             ` Jani Nikula
  0 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2018-11-13 21:57 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Paulo Zanoni

On Tue, 13 Nov 2018, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Nov 13, 2018 at 10:31:25PM +0200, Jani Nikula wrote:
>> On Tue, 13 Nov 2018, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> > Bspec 21257 says "DDIA PHY is the comp master, so it must
>> > not be un-initialized if other combo PHYs are in use". Here
>> > we are shutting down all phys, so it's not strictly required.
>> > However let's be consistent on deinitializing things in the
>> > reversed order we initialized them.
>> >
>> > v2: simplify protection for enum port being unsigned in future
>> >
>> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_combo_phy.c | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
>> > index 49f3a533860d..62939df048d7 100644
>> > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
>> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
>> > @@ -9,6 +9,10 @@
>> >  	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
>> >  		for_each_if(intel_port_is_combophy(__dev_priv, __port))
>> >  
>> > +#define for_each_combo_port_rev(__dev_priv, __port) \
>> 
>> The list.h list macros use _reverse suffix for reverse traversal. I
>> think we can afford to spend the extra letters here too.
>
> I followed what we do on other for_each macro:
> for_each_power_well_rev
> for_each_power_domain_well_rev
>
> Should those be reverse as well?

I think so yes.

BR,
Jani.


>
> Lucas De Marchi
>
>> 
>> BR,
>> Jani.
>> 
>> > +	for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \
>> > +		for_each_if(intel_port_is_combophy(__dev_priv, __port))
>> > +
>> >  enum {
>> >  	PROCMON_0_85V_DOT_0,
>> >  	PROCMON_0_95V_DOT_0,
>> > @@ -232,7 +236,7 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
>> >  {
>> >  	enum port port;
>> >  
>> > -	for_each_combo_port(dev_priv, port) {
>> > +	for_each_combo_port_rev(dev_priv, port) {
>> >  		u32 val;
>> >  
>> >  		if (!icl_combo_phy_verify_state(dev_priv, port))
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
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] 16+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
  2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
                   ` (6 preceding siblings ...)
  2018-11-13 19:30 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-14 17:23 ` Patchwork
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-11-14 17:23 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
URL   : https://patchwork.freedesktop.org/series/52400/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5133_full -> Patchwork_10818_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10818_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10818_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10818_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_atomic_interruptible@universal-setplane-cursor:
      shard-snb:          SKIP -> PASS +2

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106887, fdo#103665, fdo#106023)

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108074)

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_chv_cursor_fail@pipe-b-64x64-right-edge:
      shard-glk:          PASS -> DMESG-FAIL (fdo#104671, fdo#106538)

    igt@kms_cursor_crc@cursor-256x256-onscreen:
      shard-skl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          PASS -> FAIL (fdo#103191, fdo#103232)

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
      shard-hsw:          PASS -> FAIL (fdo#105767)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_fbcon_fbt@fbc-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@plain-flip-ts-check:
      shard-skl:          PASS -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_plane@plane-position-covered-pipe-b-planes:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#107815)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-skl:          INCOMPLETE (fdo#106886) -> PASS
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359, fdo#106886) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-skl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge:
      shard-skl:          FAIL (fdo#104671) -> PASS

    igt@kms_color@pipe-c-ctm-blue-to-red:
      shard-skl:          FAIL (fdo#107201) -> PASS

    igt@kms_cursor_crc@cursor-128x128-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-skl:          FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
      shard-glk:          FAIL (fdo#103167) -> PASS +2

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
  fdo#107201 https://bugs.freedesktop.org/show_bug.cgi?id=107201
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5133 -> Patchwork_10818

  CI_DRM_5133: 5c71926a1834348a68951622a950de0355b73450 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10818: bf6d9fe159515700d2b86b87fa10304924af75aa @ 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_10818/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-11-14 17:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-13  2:45 [PATCH v2 1/2] drm/i915/icl: replace check for combo phy Lucas De Marchi
2018-11-13  2:45 ` [PATCH v2 2/2] drm/i915/icl: reverse uninit order Lucas De Marchi
2018-11-13 13:19   ` Imre Deak
2018-11-13 13:23     ` Imre Deak
2018-11-13 17:40       ` Lucas De Marchi
2018-11-13 18:56       ` [PATCH v2] " Lucas De Marchi
2018-11-13 20:31         ` Jani Nikula
2018-11-13 21:22           ` Lucas De Marchi
2018-11-13 21:57             ` Jani Nikula
2018-11-13  3:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy Patchwork
2018-11-13  3:35 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-13  5:05 ` ✓ Fi.CI.IGT: " Patchwork
2018-11-13 13:44 ` [PATCH v2 1/2] " Imre Deak
2018-11-13 19:08 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2) Patchwork
2018-11-13 19:30 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-14 17:23 ` ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).