[igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Madhumitha Tolakanahalli Pradeep]

The tile property parser parses the connector tile property obtained
from connector's Display ID block and set per connector.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com>
---
 lib/igt_kms.c | 26 ++++++++++++++++++++++++++
 lib/igt_kms.h | 11 +++++++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 175e71c3..846314de 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -4511,3 +4511,29 @@ bool igt_display_has_format_mod(igt_display_t *display, uint32_t format,
 
 	return false;
 }
+
+/**
+ * igt_parse_connector_tile_blob:
+ * @blob: pointer to the connector's tile properties
+ * @tile: pointer to tile structure that is populated by the function
+ *
+ * Parses the connector tile blob to extract the tile information
+ *
+ */
+
+void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
+				igt_tile_info_t * tile)
+{
+	char * blob_data = blob->data;
+
+	if(blob) {
+		tile->tile_group_id = atoi(strtok(blob_data, ":"));
+		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
+		tile->num_h_tile = atoi(strtok(NULL, ":"));
+		tile->num_v_tile = atoi(strtok(NULL, ":"));
+		tile->tile_h_loc = atoi(strtok(NULL, ":"));
+		tile->tile_v_loc = atoi(strtok(NULL, ":"));
+		tile->tile_h_size = atoi(strtok(NULL, ":"));
+		tile->tile_v_size = atoi(strtok(NULL, ":"));
+	}
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 0486737b..08483505 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -387,6 +387,14 @@ struct igt_display {
 	int format_mod_count;
 };
 
+typedef struct {
+	int tile_group_id;
+	bool tile_is_single_monitor;
+	uint8_t num_h_tile, num_v_tile;
+	uint8_t tile_h_loc, tile_v_loc;
+	uint16_t tile_h_size, tile_v_size;
+} igt_tile_info_t;
+
 void igt_display_require(igt_display_t *display, int drm_fd);
 void igt_display_fini(igt_display_t *display);
 void igt_display_reset(igt_display_t *display);
@@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t a, uint32_t b)
 	return igt_vblank_after(b, a);
 }
 
+void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
+			igt_tile_info_t * tile);
+
 #endif /* __IGT_KMS_H__ */
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for lib/igt_kms.c igt_kms.h: tile property parser

[Patchwork]

== Series Details ==

Series: lib/igt_kms.c igt_kms.h: tile property parser
URL   : https://patchwork.freedesktop.org/series/63832/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6502 -> IGTPW_3274
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [DMESG-WARN][1] ([fdo#107724]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/fi-icl-u3/igt@gem_basic@bad-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/fi-icl-u3/igt@gem_basic@bad-close.html

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u3:          [INCOMPLETE][3] ([fdo#107713] / [fdo#109100]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/fi-icl-u3/igt@gem_ctx_create@basic-files.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/fi-icl-u3/igt@gem_ctx_create@basic-files.html

  * {igt@gem_ctx_switch@legacy-render}:
    - fi-icl-guc:         [INCOMPLETE][5] ([fdo#107713]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html

  * {igt@gem_ctx_switch@rcs0}:
    - fi-icl-u2:          [INCOMPLETE][7] ([fdo#107713]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/fi-icl-u2/igt@gem_ctx_switch@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/fi-icl-u2/igt@gem_ctx_switch@rcs0.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#109485]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (52 -> 47)
------------------------------

  Additional (1): fi-apl-guc 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * IGT: IGT_5102 -> IGTPW_3274

  CI_DRM_6502: 606a844d5d932fb07b2377b95c0fe7b08383e32a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3274: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/
  IGT_5102: 6038ace76016d8892f4d13aef5301f71ca1a6e2d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for lib/igt_kms.c igt_kms.h: tile property parser

[Patchwork]

== Series Details ==

Series: lib/igt_kms.c igt_kms.h: tile property parser
URL   : https://patchwork.freedesktop.org/series/63832/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6502_full -> IGTPW_3274_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([fdo#109661])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-apl6/igt@gem_eio@reset-stress.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-apl6/igt@gem_eio@reset-stress.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#110854])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb4/igt@gem_exec_balancer@smoke.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@i915_selftest@live_hangcheck:
    - shard-iclb:         [PASS][5] -> [INCOMPLETE][6] ([fdo#107713] / [fdo#108569])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb2/igt@i915_selftest@live_hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb3/igt@i915_selftest@live_hangcheck.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#104873])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-glk3/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-glk3/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#105363])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#108566]) +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103166])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109642] / [fdo#111068])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb4/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb8/igt@kms_psr@psr2_cursor_render.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries_display_off:
    - shard-iclb:         [DMESG-WARN][23] -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb5/igt@debugfs_test@read_all_entries_display_off.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb8/igt@debugfs_test@read_all_entries_display_off.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [DMESG-WARN][25] ([fdo#108686]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-glk5/igt@gem_tiled_swapping@non-threaded.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-glk5/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-apl3/igt@gem_workarounds@suspend-resume.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-apl1/igt@gem_workarounds@suspend-resume.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-iclb:         [SKIP][29] ([fdo#110933]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb8/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb8/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][31] ([fdo#105767]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [FAIL][33] ([fdo#105363]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-glk7/igt@kms_flip@flip-vs-expired-vblank.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-glk2/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-snb:          [INCOMPLETE][35] ([fdo#105411]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-snb1/igt@kms_flip@flip-vs-suspend-interruptible.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-snb2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +6 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-iclb:         [INCOMPLETE][39] ([fdo#107713] / [fdo#110036 ]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb7/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb5/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +5 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][43] ([fdo#99912]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-kbl7/igt@kms_setmode@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-kbl3/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@i915_pm_rc6_residency@media-rc6-accuracy:
    - shard-iclb:         [SKIP][45] ([fdo#109289]) -> [SKIP][46] ([fdo#110933])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-iclb4/igt@i915_pm_rc6_residency@media-rc6-accuracy.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-iclb1/igt@i915_pm_rc6_residency@media-rc6-accuracy.html

  * igt@kms_atomic_transition@3x-modeset-transitions-fencing:
    - shard-snb:          [SKIP][47] ([fdo#109271] / [fdo#109278]) -> [SKIP][48] ([fdo#109271])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6502/shard-snb5/igt@kms_atomic_transition@3x-modeset-transitions-fencing.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/shard-snb2/igt@kms_atomic_transition@3x-modeset-transitions-fencing.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110036 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110036 
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#110933]: https://bugs.freedesktop.org/show_bug.cgi?id=110933
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


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

  * IGT: IGT_5102 -> IGTPW_3274
  * Piglit: piglit_4509 -> None

  CI_DRM_6502: 606a844d5d932fb07b2377b95c0fe7b08383e32a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3274: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3274/
  IGT_5102: 6038ace76016d8892f4d13aef5301f71ca1a6e2d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Ser, Simon]

Hi,

Here are a few comments.

On Wed, 2019-07-17 at 11:10 -0700, Madhumitha Tolakanahalli Pradeep wrote:
> The tile property parser parses the connector tile property obtained
> from connector's Display ID block and set per connector.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com>
> ---
>  lib/igt_kms.c | 26 ++++++++++++++++++++++++++
>  lib/igt_kms.h | 11 +++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 175e71c3..846314de 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -4511,3 +4511,29 @@ bool igt_display_has_format_mod(igt_display_t *display, uint32_t format,
>  
>  	return false;
>  }
> +
> +/**
> + * igt_parse_connector_tile_blob:
> + * @blob: pointer to the connector's tile properties
> + * @tile: pointer to tile structure that is populated by the function
> + *
> + * Parses the connector tile blob to extract the tile information
> + *
> + */
> +
> +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> +				igt_tile_info_t * tile)

Style: align with as many tabs as possible, then fill the rest with
spaces.

> +{
> +	char * blob_data = blob->data;

Style: no space after star.

    char *thing = stuff;

Applies to the whole patch.

> +	if(blob) {

Style: space after if keyword.

> +		tile->tile_group_id = atoi(strtok(blob_data, ":"));

This segfaults if the field doesn't exist. It would be nicer to
igt_assert that strtok doesn't return NULL. Maybe with a wrapper
function? e.g.

    static int parse_next_tile_field(char *data)

Also this doesn't check for integer overflows, but not sure it's worth
the hassle.

> +		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
> +		tile->num_h_tile = atoi(strtok(NULL, ":"));
> +		tile->num_v_tile = atoi(strtok(NULL, ":"));
> +		tile->tile_h_loc = atoi(strtok(NULL, ":"));
> +		tile->tile_v_loc = atoi(strtok(NULL, ":"));
> +		tile->tile_h_size = atoi(strtok(NULL, ":"));
> +		tile->tile_v_size = atoi(strtok(NULL, ":"));
> +	}
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0486737b..08483505 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -387,6 +387,14 @@ struct igt_display {
>  	int format_mod_count;
>  };
>  
> +typedef struct {
> +	int tile_group_id;
> +	bool tile_is_single_monitor;
> +	uint8_t num_h_tile, num_v_tile;
> +	uint8_t tile_h_loc, tile_v_loc;
> +	uint16_t tile_h_size, tile_v_size;

I wouldn't repeat the tile_ prefix for all of these fields, it's
already clear that it's about the tile from the type name.

> +} igt_tile_info_t;

I'm personally not a fan of typedefs, but it seems like the rest of IGT
doesn't care, so it's probably fine.

>  void igt_display_require(igt_display_t *display, int drm_fd);
>  void igt_display_fini(igt_display_t *display);
>  void igt_display_reset(igt_display_t *display);
> @@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t a, uint32_t b)
>  	return igt_vblank_after(b, a);
>  }
>  
> +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> +			igt_tile_info_t * tile);
> +
>  #endif /* __IGT_KMS_H__ */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Manasi Navare]

On Wed, Jul 17, 2019 at 11:24:33PM -0700, Ser, Simon wrote:
> Hi,
> 
> Here are a few comments.
> 
> On Wed, 2019-07-17 at 11:10 -0700, Madhumitha Tolakanahalli Pradeep wrote:
> > The tile property parser parses the connector tile property obtained
> > from connector's Display ID block and set per connector.
> > 
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com>
> > ---
> >  lib/igt_kms.c | 26 ++++++++++++++++++++++++++
> >  lib/igt_kms.h | 11 +++++++++++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 175e71c3..846314de 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -4511,3 +4511,29 @@ bool igt_display_has_format_mod(igt_display_t *display, uint32_t format,
> >  
> >  	return false;
> >  }
> > +
> > +/**
> > + * igt_parse_connector_tile_blob:
> > + * @blob: pointer to the connector's tile properties
> > + * @tile: pointer to tile structure that is populated by the function
> > + *
> > + * Parses the connector tile blob to extract the tile information
> > + *
> > + */
> > +
> > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > +				igt_tile_info_t * tile)
> 
> Style: align with as many tabs as possible, then fill the rest with
> spaces.
> 
> > +{
> > +	char * blob_data = blob->data;
> 
> Style: no space after star.
> 
>     char *thing = stuff;
> 
> Applies to the whole patch.
> 
> > +	if(blob) {
> 
> Style: space after if keyword.
> 
> > +		tile->tile_group_id = atoi(strtok(blob_data, ":"));
> 
> This segfaults if the field doesn't exist. It would be nicer to
> igt_assert that strtok doesn't return NULL. Maybe with a wrapper
> function? e.g.
> 
>     static int parse_next_tile_field(char *data)
> 
> Also this doesn't check for integer overflows, but not sure it's worth
> the hassle.
> 
> > +		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
> > +		tile->num_h_tile = atoi(strtok(NULL, ":"));
> > +		tile->num_v_tile = atoi(strtok(NULL, ":"));
> > +		tile->tile_h_loc = atoi(strtok(NULL, ":"));
> > +		tile->tile_v_loc = atoi(strtok(NULL, ":"));
> > +		tile->tile_h_size = atoi(strtok(NULL, ":"));
> > +		tile->tile_v_size = atoi(strtok(NULL, ":"));
> > +	}
> > +}
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 0486737b..08483505 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -387,6 +387,14 @@ struct igt_display {
> >  	int format_mod_count;
> >  };
> >  
> > +typedef struct {
> > +	int tile_group_id;
> > +	bool tile_is_single_monitor;
> > +	uint8_t num_h_tile, num_v_tile;
> > +	uint8_t tile_h_loc, tile_v_loc;
> > +	uint16_t tile_h_size, tile_v_size;
> 
> I wouldn't repeat the tile_ prefix for all of these fields, it's
> already clear that it's about the tile from the type name.

The idea of using tile_ prefix is so that it aligns with the tile fields in the
kernel and these are the exact names that get printed in the dmesg so makes
it more intuitive if we stick to these names.

Manasi

> 
> > +} igt_tile_info_t;
> 
> I'm personally not a fan of typedefs, but it seems like the rest of IGT
> doesn't care, so it's probably fine.
> 
> >  void igt_display_require(igt_display_t *display, int drm_fd);
> >  void igt_display_fini(igt_display_t *display);
> >  void igt_display_reset(igt_display_t *display);
> > @@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t a, uint32_t b)
> >  	return igt_vblank_after(b, a);
> >  }
> >  
> > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > +			igt_tile_info_t * tile);
> > +
> >  #endif /* __IGT_KMS_H__ */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Tolakanahalli Pradeep, Madhumitha]

Thank you for your comments, Simon.

I have taken care of the comments regarding style changes.

On Thu, 2019-07-18 at 07:24 +0100, Ser, Simon wrote:
> Hi,
> 
> Here are a few comments.
> 
> On Wed, 2019-07-17 at 11:10 -0700, Madhumitha Tolakanahalli Pradeep
> wrote:
> > The tile property parser parses the connector tile property
> > obtained
> > from connector's Display ID block and set per connector.
> > 
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Madhumitha Tolakanahalli Pradeep <
> > madhumitha.tolakanahalli.pradeep@intel.com>
> > ---
> >  lib/igt_kms.c | 26 ++++++++++++++++++++++++++
> >  lib/igt_kms.h | 11 +++++++++++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 175e71c3..846314de 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -4511,3 +4511,29 @@ bool
> > igt_display_has_format_mod(igt_display_t *display, uint32_t format,
> >  
> >  	return false;
> >  }
> > +
> > +/**
> > + * igt_parse_connector_tile_blob:
> > + * @blob: pointer to the connector's tile properties
> > + * @tile: pointer to tile structure that is populated by the
> > function
> > + *
> > + * Parses the connector tile blob to extract the tile information
> > + *
> > + */
> > +
> > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > +				igt_tile_info_t * tile)
> 
> Style: align with as many tabs as possible, then fill the rest with
> spaces.
> 
> > +{
> > +	char * blob_data = blob->data;
> 
> Style: no space after star.
> 
>     char *thing = stuff;
> 
> Applies to the whole patch.
> 
> > +	if(blob) {
> 
> Style: space after if keyword.
> 
> > +		tile->tile_group_id = atoi(strtok(blob_data, ":"));
> 
> This segfaults if the field doesn't exist. It would be nicer to
> igt_assert that strtok doesn't return NULL. Maybe with a wrapper
> function? e.g.
> 
>     static int parse_next_tile_field(char *data)

This blob information is obtained from the kernel
(drm/drm_connector.c). It's format is defined in
drm_connector_set_tile_property() to be of the form of 8 integers using
':' as a separator. Hence, the field not existing wouldn't be a
possibility, which is why no NULL return checks are being performed.

> Also this doesn't check for integer overflows, but not sure it's
> worth
> the hassle.
> 
> > +		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
> > +		tile->num_h_tile = atoi(strtok(NULL, ":"));
> > +		tile->num_v_tile = atoi(strtok(NULL, ":"));
> > +		tile->tile_h_loc = atoi(strtok(NULL, ":"));
> > +		tile->tile_v_loc = atoi(strtok(NULL, ":"));
> > +		tile->tile_h_size = atoi(strtok(NULL, ":"));
> > +		tile->tile_v_size = atoi(strtok(NULL, ":"));
> > +	}
> > +}
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 0486737b..08483505 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -387,6 +387,14 @@ struct igt_display {
> >  	int format_mod_count;
> >  };
> >  
> > +typedef struct {
> > +	int tile_group_id;
> > +	bool tile_is_single_monitor;
> > +	uint8_t num_h_tile, num_v_tile;
> > +	uint8_t tile_h_loc, tile_v_loc;
> > +	uint16_t tile_h_size, tile_v_size;
> 
> I wouldn't repeat the tile_ prefix for all of these fields, it's
> already clear that it's about the tile from the type name.

The naming convention of the tile properties confroms to that in the
kernel to maintain uniformity.

> 
> > +} igt_tile_info_t;
> 
> I'm personally not a fan of typedefs, but it seems like the rest of
> IGT
> doesn't care, so it's probably fine.
> 
> >  void igt_display_require(igt_display_t *display, int drm_fd);
> >  void igt_display_fini(igt_display_t *display);
> >  void igt_display_reset(igt_display_t *display);
> > @@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t
> > a, uint32_t b)
> >  	return igt_vblank_after(b, a);
> >  }
> >  
> > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > +			igt_tile_info_t * tile);
> > +
> >  #endif /* __IGT_KMS_H__ */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Ser, Simon]

On Tue, 2019-07-30 at 12:24 -0700, Manasi Navare wrote:
> On Wed, Jul 17, 2019 at 11:24:33PM -0700, Ser, Simon wrote:
> > Hi,
> > 
> > Here are a few comments.
> > 
> > On Wed, 2019-07-17 at 11:10 -0700, Madhumitha Tolakanahalli Pradeep wrote:
> > > The tile property parser parses the connector tile property obtained
> > > from connector's Display ID block and set per connector.
> > > 
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com>
> > > ---
> > >  lib/igt_kms.c | 26 ++++++++++++++++++++++++++
> > >  lib/igt_kms.h | 11 +++++++++++
> > >  2 files changed, 37 insertions(+)
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index 175e71c3..846314de 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -4511,3 +4511,29 @@ bool igt_display_has_format_mod(igt_display_t *display, uint32_t format,
> > >  
> > >  	return false;
> > >  }
> > > +
> > > +/**
> > > + * igt_parse_connector_tile_blob:
> > > + * @blob: pointer to the connector's tile properties
> > > + * @tile: pointer to tile structure that is populated by the function
> > > + *
> > > + * Parses the connector tile blob to extract the tile information
> > > + *
> > > + */
> > > +
> > > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > > +				igt_tile_info_t * tile)
> > 
> > Style: align with as many tabs as possible, then fill the rest with
> > spaces.
> > 
> > > +{
> > > +	char * blob_data = blob->data;
> > 
> > Style: no space after star.
> > 
> >     char *thing = stuff;
> > 
> > Applies to the whole patch.
> > 
> > > +	if(blob) {
> > 
> > Style: space after if keyword.
> > 
> > > +		tile->tile_group_id = atoi(strtok(blob_data, ":"));
> > 
> > This segfaults if the field doesn't exist. It would be nicer to
> > igt_assert that strtok doesn't return NULL. Maybe with a wrapper
> > function? e.g.
> > 
> >     static int parse_next_tile_field(char *data)
> > 
> > Also this doesn't check for integer overflows, but not sure it's worth
> > the hassle.
> > 
> > > +		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
> > > +		tile->num_h_tile = atoi(strtok(NULL, ":"));
> > > +		tile->num_v_tile = atoi(strtok(NULL, ":"));
> > > +		tile->tile_h_loc = atoi(strtok(NULL, ":"));
> > > +		tile->tile_v_loc = atoi(strtok(NULL, ":"));
> > > +		tile->tile_h_size = atoi(strtok(NULL, ":"));
> > > +		tile->tile_v_size = atoi(strtok(NULL, ":"));
> > > +	}
> > > +}
> > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > index 0486737b..08483505 100644
> > > --- a/lib/igt_kms.h
> > > +++ b/lib/igt_kms.h
> > > @@ -387,6 +387,14 @@ struct igt_display {
> > >  	int format_mod_count;
> > >  };
> > >  
> > > +typedef struct {
> > > +	int tile_group_id;
> > > +	bool tile_is_single_monitor;
> > > +	uint8_t num_h_tile, num_v_tile;
> > > +	uint8_t tile_h_loc, tile_v_loc;
> > > +	uint16_t tile_h_size, tile_v_size;
> > 
> > I wouldn't repeat the tile_ prefix for all of these fields, it's
> > already clear that it's about the tile from the type name.
> 
> The idea of using tile_ prefix is so that it aligns with the tile fields in the
> kernel and these are the exact names that get printed in the dmesg so makes
> it more intuitive if we stick to these names.

I see, makes perfect sense.

> Manasi
> 
> > > +} igt_tile_info_t;
> > 
> > I'm personally not a fan of typedefs, but it seems like the rest of IGT
> > doesn't care, so it's probably fine.
> > 
> > >  void igt_display_require(igt_display_t *display, int drm_fd);
> > >  void igt_display_fini(igt_display_t *display);
> > >  void igt_display_reset(igt_display_t *display);
> > > @@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t a, uint32_t b)
> > >  	return igt_vblank_after(b, a);
> > >  }
> > >  
> > > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > > +			igt_tile_info_t * tile);
> > > +
> > >  #endif /* __IGT_KMS_H__ */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] lib/igt_kms.c igt_kms.h: tile property parser

[Ser, Simon]

On Tue, 2019-07-30 at 17:51 -0700, Tolakanahalli Pradeep, Madhumitha wrote:
> Thank you for your comments, Simon.
> 
> I have taken care of the comments regarding style changes.
> 
> On Thu, 2019-07-18 at 07:24 +0100, Ser, Simon wrote:
> > Hi,
> > 
> > Here are a few comments.
> > 
> > On Wed, 2019-07-17 at 11:10 -0700, Madhumitha Tolakanahalli Pradeep
> > wrote:
> > > The tile property parser parses the connector tile property
> > > obtained
> > > from connector's Display ID block and set per connector.
> > > 
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Signed-off-by: Madhumitha Tolakanahalli Pradeep <
> > > madhumitha.tolakanahalli.pradeep@intel.com>
> > > ---
> > >  lib/igt_kms.c | 26 ++++++++++++++++++++++++++
> > >  lib/igt_kms.h | 11 +++++++++++
> > >  2 files changed, 37 insertions(+)
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index 175e71c3..846314de 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -4511,3 +4511,29 @@ bool
> > > igt_display_has_format_mod(igt_display_t *display, uint32_t format,
> > >  
> > >  	return false;
> > >  }
> > > +
> > > +/**
> > > + * igt_parse_connector_tile_blob:
> > > + * @blob: pointer to the connector's tile properties
> > > + * @tile: pointer to tile structure that is populated by the
> > > function
> > > + *
> > > + * Parses the connector tile blob to extract the tile information
> > > + *
> > > + */
> > > +
> > > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > > +				igt_tile_info_t * tile)
> > 
> > Style: align with as many tabs as possible, then fill the rest with
> > spaces.
> > 
> > > +{
> > > +	char * blob_data = blob->data;
> > 
> > Style: no space after star.
> > 
> >     char *thing = stuff;
> > 
> > Applies to the whole patch.
> > 
> > > +	if(blob) {
> > 
> > Style: space after if keyword.
> > 
> > > +		tile->tile_group_id = atoi(strtok(blob_data, ":"));
> > 
> > This segfaults if the field doesn't exist. It would be nicer to
> > igt_assert that strtok doesn't return NULL. Maybe with a wrapper
> > function? e.g.
> > 
> >     static int parse_next_tile_field(char *data)
> 
> This blob information is obtained from the kernel
> (drm/drm_connector.c). It's format is defined in
> drm_connector_set_tile_property() to be of the form of 8 integers using
> ':' as a separator. Hence, the field not existing wouldn't be a
> possibility, which is why no NULL return checks are being performed.

Generally IGT tests assume the kernel could do anything. For instance,
a driver could roll its own logic instead of using
drm_connector_set_tile_property. Or the TILE blob format could be
extended in a kernel patch, breaking backwards compatibility for a
corner case.

I agree it's not that of a big deal for this specific case. I'm fine
either way.

> > Also this doesn't check for integer overflows, but not sure it's
> > worth
> > the hassle.
> > 
> > > +		tile->tile_is_single_monitor = atoi(strtok(NULL, ":"));
> > > +		tile->num_h_tile = atoi(strtok(NULL, ":"));
> > > +		tile->num_v_tile = atoi(strtok(NULL, ":"));
> > > +		tile->tile_h_loc = atoi(strtok(NULL, ":"));
> > > +		tile->tile_v_loc = atoi(strtok(NULL, ":"));
> > > +		tile->tile_h_size = atoi(strtok(NULL, ":"));
> > > +		tile->tile_v_size = atoi(strtok(NULL, ":"));
> > > +	}
> > > +}
> > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > index 0486737b..08483505 100644
> > > --- a/lib/igt_kms.h
> > > +++ b/lib/igt_kms.h
> > > @@ -387,6 +387,14 @@ struct igt_display {
> > >  	int format_mod_count;
> > >  };
> > >  
> > > +typedef struct {
> > > +	int tile_group_id;
> > > +	bool tile_is_single_monitor;
> > > +	uint8_t num_h_tile, num_v_tile;
> > > +	uint8_t tile_h_loc, tile_v_loc;
> > > +	uint16_t tile_h_size, tile_v_size;
> > 
> > I wouldn't repeat the tile_ prefix for all of these fields, it's
> > already clear that it's about the tile from the type name.
> 
> The naming convention of the tile properties confroms to that in the
> kernel to maintain uniformity.

Indeed, that makes sense. Better to keep it that way.

> > > +} igt_tile_info_t;
> > 
> > I'm personally not a fan of typedefs, but it seems like the rest of
> > IGT
> > doesn't care, so it's probably fine.
> > 
> > >  void igt_display_require(igt_display_t *display, int drm_fd);
> > >  void igt_display_fini(igt_display_t *display);
> > >  void igt_display_reset(igt_display_t *display);
> > > @@ -835,4 +843,7 @@ static inline bool igt_vblank_before(uint32_t
> > > a, uint32_t b)
> > >  	return igt_vblank_after(b, a);
> > >  }
> > >  
> > > +void igt_parse_connector_tile_blob(drmModePropertyBlobPtr blob,
> > > +			igt_tile_info_t * tile);
> > > +
> > >  #endif /* __IGT_KMS_H__ */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev