Re: [PATCH i-g-t 4/4] scripts/trace.pl: Simplify 'end' & 'notify' generation

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 20/01/2018 00:24, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> Delay the auto-generation of end/notify values until the point where
> everything is known. As opposed to potentially generating them
> multiple times with differing values.
>
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
>   scripts/trace.pl | 31 ++++++++++++++++++-------------
>   1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/scripts/trace.pl b/scripts/trace.pl
> index c5d822aa..fbf4b92e 100755
> --- a/scripts/trace.pl
> +++ b/scripts/trace.pl
> @@ -467,10 +467,11 @@ while (<>) {
>   }
>   
>   # Sanitation pass to fixup up out of order notify and context complete, and to
> -# fine the largest seqno to be used for timeline sorting purposes.
> +# find the largest seqno to be used for timeline sorting purposes.
>   my $max_seqno = 0;
>   foreach my $key (keys %db) {
>   	my $gkey = global_key($db{$key}->{'ring'}, $db{$key}->{'global'});
> +	my $notify = $notify{$gkey};
>   
>   	die unless exists $db{$key}->{'start'};
>   
> @@ -478,23 +479,21 @@ foreach my $key (keys %db) {
>   
>   	unless (exists $db{$key}->{'end'}) {
>   		# Context complete not received.
> -		if (exists $notify{$gkey}) {
> +		$db{$key}->{'no-end'} = 1;
> +
> +		if (defined($notify)) {
>   			# No context complete due req merging - use notify.
> -			$db{$key}->{'notify'} = $notify{$gkey};
> -			$db{$key}->{'end'} = $db{$key}->{'notify'};
> -			$db{$key}->{'no-end'} = 1;
> +			$db{$key}->{'notify'} = $notify;
> +			$db{$key}->{'end'} = $notify;
>   		} else {
> -			# No notify and no context complete - mark it.
> -			$db{$key}->{'no-end'} = 1;
> -			$db{$key}->{'end'} = $db{$key}->{'start'} + 999;
> -			$db{$key}->{'notify'} = $db{$key}->{'end'};
> +			# No notify and no context complete - give up for now.
>   			$db{$key}->{'incomplete'} = 1;
>   		}
>   	} else {
>   		# Notify arrived after context complete.
> -		if (exists $db{$key}->{'no-notify'} and exists $notify{$gkey}) {
> +		if (exists $db{$key}->{'no-notify'} and defined($notify)) {
>   			delete $db{$key}->{'no-notify'};
> -			$db{$key}->{'notify'} = $notify{$gkey};
> +			$db{$key}->{'notify'} = $notify;
>   		}
>   	}
>   }
> @@ -511,6 +510,7 @@ foreach my $key (@keys) {
>   	my $seqno = $db{$key}->{'seqno'};
>   	my $next_key;
>   	my $i = 1;
> +	my $end;
>   
>   	do {
>   		$next_key = db_key($ring, $ctx, $seqno + $i);
> @@ -519,9 +519,14 @@ foreach my $key (@keys) {
>   		 or $i > $keyCount);  # ugly stop hack
>   
>   	if (exists $db{$next_key}) {
> -		$db{$key}->{'notify'} = $db{$next_key}->{'end'};
> -		$db{$key}->{'end'} = $db{$key}->{'notify'};
> +		$end = $db{$next_key}->{'end'};
> +	} else {
> +		# No info at all, fake it:
> +		$end = $db{$key}->{'start'} + 999;
>   	}
> +
> +	$db{$key}->{'notify'} = $end;
> +	$db{$key}->{'end'} = $end;
>   }
>   
>   # GPU time accounting

Looks cleaner and still correct. Just please explain in the commit that 
the description applies only for requests marked as incomplete. AFICS 
those were the only ones it was setting the end and notify times 
potentially two times.

Regards,

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