Re: [PATCH i-g-t] tests/kms_setmode: increase MAX_CRTCS to 6

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2017-05-31 09:32 AM, Harry Wentland wrote:
On 2017-05-31 05:37 AM, Daniel Vetter wrote:
On Tue, May 30, 2017 at 04:01:40PM -0400, Harry Wentland wrote:
AMD GPUs can have 6 CRTCs.

This requires us to allocate the combinations on the heap.

Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>

I think just dynamically allocating stuff directly and dropping the
#define would be even neater ... GetResources can tell us how much of each
exists.
-Daniel


Agreed. I'll send out a v3 later.


About to send v3 but this code is quite majorly wrong as-is. In particular I see these two issues:

1) In iterate_combinations we assign a stack object to our list of
   combinations which is then popped off the stack as soon as
   get_combinations returns. Later on in test_combinations we then try
   to access it with
	connector_idxs = &connector_combs.items[i].elems[0];
	...
	int *crtc_idxs = &crtc_combs.items[j].elems[0];

2) This for loop in iterate_combinations will simply override
   comb->elems[depth] with new values:

	for (v = base; v < n; v++) {
		comb->elems[depth] = v;
		[...]
	}

It looks like whoever wrote the code tried to do some k choose n algorithm to find all possible sets of crtcs and connectors but then only ended up picking one crtc and connector out of each list anyways (after the item popped from the stack).

If I find time I might rewrite the test_combinations function with a simple nested for loop that tries all crtcs with all connectors one-to-one as this seems to be currently the intention of this function.

Harry

Harry

---
  tests/kms_setmode.c | 25 +++++++++++++++----------
  1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 430568a1c24e..847ad650d27f 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -35,11 +35,13 @@
  #include "intel_bufmgr.h"
  #define MAX_CONNECTORS  10
-#define MAX_CRTCS       3
+#define MAX_CRTCS       6
  /* max combinations with repetitions */
+/* MAX_CONNECTORS ^ MAX_CRTCS */
+/* TODO should really be MAX_CONNECTORS ^ MAX_CONNECTORS ??? */
  #define MAX_COMBINATION_COUNT   \
-    (MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS)
+ (MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS)
  #define MAX_COMBINATION_ELEMS   MAX_CRTCS
  static int drm_fd;
@@ -743,22 +745,25 @@ static void get_combinations(int n, int k, bool allow_repetitions,
  static void test_combinations(const struct test_config *tconf,
                    int connector_count)
  {
-    struct combination_set connector_combs;
-    struct combination_set crtc_combs;
+    struct combination_set *connector_combs;
+    struct combination_set *crtc_combs;
      struct connector_config *cconfs;
      int i;
      if (connector_count > 2 && (tconf->flags & TEST_STEALING))
          return;
+    connector_combs = malloc(sizeof(*connector_combs));
+    crtc_combs = malloc(sizeof(*crtc_combs));
+
get_combinations(tconf->resources->count_connectors, connector_count,
-             false, &connector_combs);
+             false, connector_combs);
      get_combinations(tconf->resources->count_crtcs, connector_count,
-             true, &crtc_combs);
+             true, crtc_combs);
      igt_info("Testing: %s %d connector combinations\n", tconf->name,
           connector_count);
-    for (i = 0; i < connector_combs.count; i++) {
+    for (i = 0; i < connector_combs->count; i++) {
          int *connector_idxs;
          int ret;
          int j;
@@ -766,14 +771,14 @@ static void test_combinations(const struct test_config *tconf,
          cconfs = malloc(sizeof(*cconfs) * connector_count);
          igt_assert(cconfs);
-        connector_idxs = &connector_combs.items[i].elems[0];
+        connector_idxs = &connector_combs->items[i].elems[0];
          ret = get_connectors(tconf->resources, connector_idxs,
                       connector_count, cconfs);
          if (ret < 0)
              goto free_cconfs;
-        for (j = 0; j < crtc_combs.count; j++) {
-            int *crtc_idxs = &crtc_combs.items[j].elems[0];
+        for (j = 0; j < crtc_combs->count; j++) {
+            int *crtc_idxs = &crtc_combs->items[j].elems[0];
              ret = assign_crtc_to_connectors(tconf, crtc_idxs,
                              connector_count,
                                  cconfs);
--
2.11.0

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

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux