* [Intel-gfx] [PATCH i-g-t 1/4] tools/intel_gpu_top: Fix clients header width when no clients
@ 2023-10-11 8:38 Tvrtko Ursulin
2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 2/4] tools/intel_gpu_top: Fix client layout on first sample period Tvrtko Ursulin
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2023-10-11 8:38 UTC (permalink / raw)
To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Recent refactoring broke the clients header in cases when there are no
clients displayed. To fix it we need to account the width of the "NAME"
label.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
tools/intel_gpu_top.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 10601e66b18e..60fe06917531 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -1967,6 +1967,8 @@ print_clients_header(struct igt_drm_clients *clients, int lines,
int con_w, int con_h, int *class_w)
{
struct intel_clients *iclients = clients->private_data;
+ const int max_name_len = clients->max_name_len < 4 ?
+ 4 : clients->max_name_len; /* At least "NAME" */
if (output_mode == INTERACTIVE) {
unsigned int num_active = 0;
@@ -1990,9 +1992,8 @@ print_clients_header(struct igt_drm_clients *clients, int lines,
num_active++;
}
- *class_w = width =
- (con_w - len - clients->max_name_len - 1) /
- num_active;
+ *class_w = width = (con_w - len - max_name_len - 1) /
+ num_active;
for (i = 0; i <= iclients->classes.max_engine_id; i++) {
const char *name = iclients->classes.names[i];
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Intel-gfx] [PATCH i-g-t 2/4] tools/intel_gpu_top: Fix client layout on first sample period 2023-10-11 8:38 [Intel-gfx] [PATCH i-g-t 1/4] tools/intel_gpu_top: Fix clients header width when no clients Tvrtko Ursulin @ 2023-10-11 8:38 ` Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin 2 siblings, 0 replies; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-11 8:38 UTC (permalink / raw) To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> When I moved the client name to be last, I did not account for the fact current code skips showing engine utilisation until at least two sampling periods have passed. Consequence of this is that client name gets printed as the second field and not under the "NAME" column header. Fix it by emitting spaces instead of engine utilisation until two samples have been collected. v2: * Fix n_spaces return type to signed. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> --- tools/intel_gpu_top.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c index 60fe06917531..b6d1014f00b8 100644 --- a/tools/intel_gpu_top.c +++ b/tools/intel_gpu_top.c @@ -928,12 +928,14 @@ static void free_display_clients(struct igt_drm_clients *clients) static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" }; -static void n_spaces(const unsigned int n) +static int n_spaces(const int n) { - unsigned int i; + int i; for (i = 0; i < n; i++) putchar(' '); + + return n; } static void @@ -2043,14 +2045,17 @@ print_client(struct igt_drm_client *c, struct engines *engines, double t, int li len = printf("%*s ", clients->max_pid_len, c->pid_str); - for (i = 0; - c->samples > 1 && i <= iclients->classes.max_engine_id; - i++) { + for (i = 0; i <= iclients->classes.max_engine_id; i++) { double pct, max; if (!iclients->classes.capacity[i]) continue; + if (c->samples < 2) { + len += n_spaces(*class_w); + continue; + } + pct = (double)c->val[i] / period_us / 1e3 * 100; /* -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit 2023-10-11 8:38 [Intel-gfx] [PATCH i-g-t 1/4] tools/intel_gpu_top: Fix clients header width when no clients Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 2/4] tools/intel_gpu_top: Fix client layout on first sample period Tvrtko Ursulin @ 2023-10-11 8:38 ` Tvrtko Ursulin 2023-10-11 12:40 ` Kamil Konieczny 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin 2 siblings, 1 reply; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-11 8:38 UTC (permalink / raw) To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Padding the percentage bars and table columns with spaces happens quite a lot so lets do better than putchar at a time. Have a table of visually empty strings and build the required length out of those chunks. While at it, also move the percentage bar table into its function scope. v2: * Fix checkpatch and use ARRAY_SIZE. (Kamil) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> --- tools/intel_gpu_top.c | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c index b6d1014f00b8..006879c4ae67 100644 --- a/tools/intel_gpu_top.c +++ b/tools/intel_gpu_top.c @@ -926,14 +926,39 @@ static void free_display_clients(struct igt_drm_clients *clients) free(clients); } -static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" }; - static int n_spaces(const int n) { - int i; + static const char *spaces[] = { + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + " ", + }; + int i, r = n; - for (i = 0; i < n; i++) - putchar(' '); + while (r) { + if (r > ARRAY_SIZE(spaces)) + i = ARRAY_SIZE(spaces) - 1; + else + i = r - 1; + fputs(spaces[i], stdout); + r -= i + 1; + } return n; } @@ -941,6 +966,9 @@ static int n_spaces(const int n) static void print_percentage_bar(double percent, double max, int max_len, bool numeric) { + static const char *bars[] = { + " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" + }; int bar_len, i, len = max_len - 2; const int w = 8; -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit Tvrtko Ursulin @ 2023-10-11 12:40 ` Kamil Konieczny 2023-10-12 8:07 ` Tvrtko Ursulin 0 siblings, 1 reply; 10+ messages in thread From: Kamil Konieczny @ 2023-10-11 12:40 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin Hi Tvrtko, On 2023-10-11 at 09:38:44 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Padding the percentage bars and table columns with spaces happens quite a > lot so lets do better than putchar at a time. Have a table of visually > empty strings and build the required length out of those chunks. > > While at it, also move the percentage bar table into its function scope. > > v2: > * Fix checkpatch and use ARRAY_SIZE. (Kamil) > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> > --- > tools/intel_gpu_top.c | 38 +++++++++++++++++++++++++++++++++----- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c > index b6d1014f00b8..006879c4ae67 100644 > --- a/tools/intel_gpu_top.c > +++ b/tools/intel_gpu_top.c > @@ -926,14 +926,39 @@ static void free_display_clients(struct igt_drm_clients *clients) > free(clients); > } > > -static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" }; > - > static int n_spaces(const int n) > { > - int i; > + static const char *spaces[] = { > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + " ", > + }; > + int i, r = n; > > - for (i = 0; i < n; i++) > - putchar(' '); > + while (r) { > + if (r > ARRAY_SIZE(spaces)) > + i = ARRAY_SIZE(spaces) - 1; > + else > + i = r - 1; > + fputs(spaces[i], stdout); > + r -= i + 1; > + } > > return n; > } > @@ -941,6 +966,9 @@ static int n_spaces(const int n) > static void > print_percentage_bar(double percent, double max, int max_len, bool numeric) > { > + static const char *bars[] = { > + " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" > + }; > int bar_len, i, len = max_len - 2; > const int w = 8; > > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit 2023-10-11 12:40 ` Kamil Konieczny @ 2023-10-12 8:07 ` Tvrtko Ursulin 0 siblings, 0 replies; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-12 8:07 UTC (permalink / raw) To: Kamil Konieczny, igt-dev, Intel-gfx, Tvrtko Ursulin On 11/10/2023 13:40, Kamil Konieczny wrote: > Hi Tvrtko, > On 2023-10-11 at 09:38:44 +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Padding the percentage bars and table columns with spaces happens quite a >> lot so lets do better than putchar at a time. Have a table of visually >> empty strings and build the required length out of those chunks. >> >> While at it, also move the percentage bar table into its function scope. >> >> v2: >> * Fix checkpatch and use ARRAY_SIZE. (Kamil) >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> Thanks Kamil, now pushed! Regards, Tvrtko ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully 2023-10-11 8:38 [Intel-gfx] [PATCH i-g-t 1/4] tools/intel_gpu_top: Fix clients header width when no clients Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 2/4] tools/intel_gpu_top: Fix client layout on first sample period Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit Tvrtko Ursulin @ 2023-10-11 8:38 ` Tvrtko Ursulin 2 siblings, 0 replies; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-11 8:38 UTC (permalink / raw) To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Instead of asserting just skip trying to print columns when terminal is too narrow. At the same time fix some type confusion to fix calculations going huge. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> --- tools/intel_gpu_top.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c index 006879c4ae67..00506c63db4e 100644 --- a/tools/intel_gpu_top.c +++ b/tools/intel_gpu_top.c @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) int bar_len, i, len = max_len - 2; const int w = 8; - assert(max_len > 0); + if (len < 2) /* For edge lines '|' */ + return; bar_len = ceil(w * percent * len / max); if (bar_len > w * len) @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) printf("%s", bars[i]); len -= (bar_len + (w - 1)) / w; + if (len < 1) + return; n_spaces(len); putchar('|'); @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients *clients, int lines, 4 : clients->max_name_len; /* At least "NAME" */ if (output_mode == INTERACTIVE) { - unsigned int num_active = 0; - int len; + int len, num_active = 0; if (lines++ >= con_h) return lines; -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH i-g-t 0/4] Fix various intel_gpu_top UI layout issues @ 2023-10-10 11:07 Tvrtko Ursulin 2023-10-10 11:07 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin 0 siblings, 1 reply; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-10 11:07 UTC (permalink / raw) To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> A collection of small fixes around various edge case scenarios. Tvrtko Ursulin (4): tools/intel_gpu_top: Fix clients header width when no clients tools/intel_gpu_top: Fix client layout on first sample period tools/intel_gpu_top: Optimise interactive display a bit tools/intel_gpu_top: Handle narrow terminals more gracefully tools/intel_gpu_top.c | 66 +++++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 15 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully 2023-10-10 11:07 [Intel-gfx] [PATCH i-g-t 0/4] Fix various intel_gpu_top UI layout issues Tvrtko Ursulin @ 2023-10-10 11:07 ` Tvrtko Ursulin 2023-10-10 16:43 ` Kamil Konieczny 0 siblings, 1 reply; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-10 11:07 UTC (permalink / raw) To: igt-dev, Intel-gfx; +Cc: Tvrtko Ursulin From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Instead of asserting just skip trying to print columns when terminal is too narrow. At the same time fix some type confusion to fix calculations going huge. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 --- tools/intel_gpu_top.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c index 472ce3f13ba9..6d1397cb8214 100644 --- a/tools/intel_gpu_top.c +++ b/tools/intel_gpu_top.c @@ -926,7 +926,7 @@ static void free_display_clients(struct igt_drm_clients *clients) free(clients); } -static unsigned int n_spaces(const unsigned int n) +static int n_spaces(const int n) { static const char *spaces[] = { " ", @@ -950,7 +950,7 @@ static unsigned int n_spaces(const unsigned int n) " ", #define MAX_SPACES 19 }; - unsigned int i, r = n; + int i, r = n; while (r) { if (r > MAX_SPACES) @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) int bar_len, i, len = max_len - 2; const int w = 8; - assert(max_len > 0); + if (len < 2) /* For edge lines '|' */ + return; bar_len = ceil(w * percent * len / max); if (bar_len > w * len) @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) printf("%s", bars[i]); len -= (bar_len + (w - 1)) / w; + if (len < 1) + return; n_spaces(len); putchar('|'); @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients *clients, int lines, 4 : clients->max_name_len; /* At least "NAME" */ if (output_mode == INTERACTIVE) { - unsigned int num_active = 0; - int len; + int len, num_active = 0; if (lines++ >= con_h) return lines; -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully 2023-10-10 11:07 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin @ 2023-10-10 16:43 ` Kamil Konieczny 2023-10-11 8:22 ` Tvrtko Ursulin 0 siblings, 1 reply; 10+ messages in thread From: Kamil Konieczny @ 2023-10-10 16:43 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin Hi Tvrtko, On 2023-10-10 at 12:07:14 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Instead of asserting just skip trying to print columns when terminal is > too narrow. > > At the same time fix some type confusion to fix calculations going huge. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 Did you tested this in screensaver? I mean running intel_gpu_top in terminal windows under X (Gnome or other) and leaving desktop unattanded, entering screen saver mode (possible with screen turned off) and then re-enabling screen? > --- > tools/intel_gpu_top.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c > index 472ce3f13ba9..6d1397cb8214 100644 > --- a/tools/intel_gpu_top.c > +++ b/tools/intel_gpu_top.c > @@ -926,7 +926,7 @@ static void free_display_clients(struct igt_drm_clients *clients) > free(clients); > } > > -static unsigned int n_spaces(const unsigned int n) > +static int n_spaces(const int n) --------- ^^^ Could you make it int at your first patch touching this function? With or without this suggestion, Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> Regards, Kamil > { > static const char *spaces[] = { > " ", > @@ -950,7 +950,7 @@ static unsigned int n_spaces(const unsigned int n) > " ", > #define MAX_SPACES 19 > }; > - unsigned int i, r = n; > + int i, r = n; > > while (r) { > if (r > MAX_SPACES) > @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) > int bar_len, i, len = max_len - 2; > const int w = 8; > > - assert(max_len > 0); > + if (len < 2) /* For edge lines '|' */ > + return; > > bar_len = ceil(w * percent * len / max); > if (bar_len > w * len) > @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) > printf("%s", bars[i]); > > len -= (bar_len + (w - 1)) / w; > + if (len < 1) > + return; > n_spaces(len); > > putchar('|'); > @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients *clients, int lines, > 4 : clients->max_name_len; /* At least "NAME" */ > > if (output_mode == INTERACTIVE) { > - unsigned int num_active = 0; > - int len; > + int len, num_active = 0; > > if (lines++ >= con_h) > return lines; > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully 2023-10-10 16:43 ` Kamil Konieczny @ 2023-10-11 8:22 ` Tvrtko Ursulin 2023-10-11 8:31 ` Tvrtko Ursulin 0 siblings, 1 reply; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-11 8:22 UTC (permalink / raw) To: Kamil Konieczny, igt-dev, Intel-gfx, Tvrtko Ursulin On 10/10/2023 17:43, Kamil Konieczny wrote: > Hi Tvrtko, > On 2023-10-10 at 12:07:14 +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Instead of asserting just skip trying to print columns when terminal is >> too narrow. >> >> At the same time fix some type confusion to fix calculations going huge. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 > > Did you tested this in screensaver? I mean running intel_gpu_top > in terminal windows under X (Gnome or other) and leaving desktop > unattanded, entering screen saver mode (possible with screen > turned off) and then re-enabling screen? I tested it by resizing the terminal to crazy small dimensions and confirmed asserts and endless printing of spaces failure modes are fixed. Also under the screen lock. But no DPMS and no console screensavers. > >> --- >> tools/intel_gpu_top.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c >> index 472ce3f13ba9..6d1397cb8214 100644 >> --- a/tools/intel_gpu_top.c >> +++ b/tools/intel_gpu_top.c >> @@ -926,7 +926,7 @@ static void free_display_clients(struct igt_drm_clients *clients) >> free(clients); >> } >> >> -static unsigned int n_spaces(const unsigned int n) >> +static int n_spaces(const int n) > --------- ^^^ > Could you make it int at your first patch touching this function? Honestly no, can't be bothered to churn this too much. I think argument can be made that this patch is fixing type confusion in many places so hopefully you can accept it as is. Regards, Tvrtko > > With or without this suggestion, > Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > Regards, > Kamil > >> { >> static const char *spaces[] = { >> " ", >> @@ -950,7 +950,7 @@ static unsigned int n_spaces(const unsigned int n) >> " ", >> #define MAX_SPACES 19 >> }; >> - unsigned int i, r = n; >> + int i, r = n; >> >> while (r) { >> if (r > MAX_SPACES) >> @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) >> int bar_len, i, len = max_len - 2; >> const int w = 8; >> >> - assert(max_len > 0); >> + if (len < 2) /* For edge lines '|' */ >> + return; >> >> bar_len = ceil(w * percent * len / max); >> if (bar_len > w * len) >> @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, int max_len, bool numeric) >> printf("%s", bars[i]); >> >> len -= (bar_len + (w - 1)) / w; >> + if (len < 1) >> + return; >> n_spaces(len); >> >> putchar('|'); >> @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients *clients, int lines, >> 4 : clients->max_name_len; /* At least "NAME" */ >> >> if (output_mode == INTERACTIVE) { >> - unsigned int num_active = 0; >> - int len; >> + int len, num_active = 0; >> >> if (lines++ >= con_h) >> return lines; >> -- >> 2.39.2 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully 2023-10-11 8:22 ` Tvrtko Ursulin @ 2023-10-11 8:31 ` Tvrtko Ursulin 0 siblings, 0 replies; 10+ messages in thread From: Tvrtko Ursulin @ 2023-10-11 8:31 UTC (permalink / raw) To: Kamil Konieczny, igt-dev, Intel-gfx, Tvrtko Ursulin On 11/10/2023 09:22, Tvrtko Ursulin wrote: > > On 10/10/2023 17:43, Kamil Konieczny wrote: >> Hi Tvrtko, >> On 2023-10-10 at 12:07:14 +0100, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> >>> Instead of asserting just skip trying to print columns when terminal is >>> too narrow. >>> >>> At the same time fix some type confusion to fix calculations going huge. >>> >>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 >> >> Did you tested this in screensaver? I mean running intel_gpu_top >> in terminal windows under X (Gnome or other) and leaving desktop >> unattanded, entering screen saver mode (possible with screen >> turned off) and then re-enabling screen? > > I tested it by resizing the terminal to crazy small dimensions and > confirmed asserts and endless printing of spaces failure modes are > fixed. Also under the screen lock. > > But no DPMS and no console screensavers. > >> >>> --- >>> tools/intel_gpu_top.c | 12 +++++++----- >>> 1 file changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c >>> index 472ce3f13ba9..6d1397cb8214 100644 >>> --- a/tools/intel_gpu_top.c >>> +++ b/tools/intel_gpu_top.c >>> @@ -926,7 +926,7 @@ static void free_display_clients(struct >>> igt_drm_clients *clients) >>> free(clients); >>> } >>> -static unsigned int n_spaces(const unsigned int n) >>> +static int n_spaces(const int n) >> --------- ^^^ >> Could you make it int at your first patch touching this function? > > Honestly no, can't be bothered to churn this too much. I think argument > can be made that this patch is fixing type confusion in many places so > hopefully you can accept it as is. Ah sorry, I did make it unsigned in in a previous patch.. I will respin the whole series. Regards, Tvrtko > > Regards, > > Tvrtko > >> >> With or without this suggestion, >> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com> >> >> Regards, >> Kamil >> >>> { >>> static const char *spaces[] = { >>> " ", >>> @@ -950,7 +950,7 @@ static unsigned int n_spaces(const unsigned int n) >>> " ", >>> #define MAX_SPACES 19 >>> }; >>> - unsigned int i, r = n; >>> + int i, r = n; >>> while (r) { >>> if (r > MAX_SPACES) >>> @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, >>> int max_len, bool numeric) >>> int bar_len, i, len = max_len - 2; >>> const int w = 8; >>> - assert(max_len > 0); >>> + if (len < 2) /* For edge lines '|' */ >>> + return; >>> bar_len = ceil(w * percent * len / max); >>> if (bar_len > w * len) >>> @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, >>> int max_len, bool numeric) >>> printf("%s", bars[i]); >>> len -= (bar_len + (w - 1)) / w; >>> + if (len < 1) >>> + return; >>> n_spaces(len); >>> putchar('|'); >>> @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients >>> *clients, int lines, >>> 4 : clients->max_name_len; /* At least "NAME" */ >>> if (output_mode == INTERACTIVE) { >>> - unsigned int num_active = 0; >>> - int len; >>> + int len, num_active = 0; >>> if (lines++ >= con_h) >>> return lines; >>> -- >>> 2.39.2 >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-12 8:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-11 8:38 [Intel-gfx] [PATCH i-g-t 1/4] tools/intel_gpu_top: Fix clients header width when no clients Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 2/4] tools/intel_gpu_top: Fix client layout on first sample period Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 3/4] tools/intel_gpu_top: Optimise interactive display a bit Tvrtko Ursulin 2023-10-11 12:40 ` Kamil Konieczny 2023-10-12 8:07 ` Tvrtko Ursulin 2023-10-11 8:38 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin -- strict thread matches above, loose matches on Subject: below -- 2023-10-10 11:07 [Intel-gfx] [PATCH i-g-t 0/4] Fix various intel_gpu_top UI layout issues Tvrtko Ursulin 2023-10-10 11:07 ` [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully Tvrtko Ursulin 2023-10-10 16:43 ` Kamil Konieczny 2023-10-11 8:22 ` Tvrtko Ursulin 2023-10-11 8:31 ` Tvrtko Ursulin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox