Re: [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully

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

 



Hi Tvrtko,
On 2023-10-10 at 12:07:14 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> 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@xxxxxxxxxxxxxxx>

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
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux