[igt-dev] [PATCH i-g-t 1/1] lib/igt_chamelium: Increase timeout for CaptureVideo

[Summers, Stuart]

On Tue, 2019-04-16 at 09:13 -0700, Neel Desai wrote:
> For some chamelium boards, we observed that the chamelium_rpc()
> call with method_name as CaptureVideo timed out because the display
> resolution read by the FPGA using the VideoDumper did not match the
> resolution in the RX chip. The resolution read by the FPGA from the
> memory was incorrect after 10 seconds and needed more time to
> stabilize
> to the correct value.
> 
> So, instead of checking the chamelium->env.fault_occured in the
> chamelium_rpc() funtion, we defer the check in the parent function
> chamelium_capture(). We iterate upto 10 times if chamelium_rpc() with
> method name as CaptureVideo fails thus effectively increasing the
> timeout to 100 seconds instead if 10 seconds. If after 10 iterations,
> chamelium->env.fault_occured is still set, we return an error.
> 
> This looks like an issue from Google's end in the FPGA code. This
> patch
> introduces a workaround till we get the issue resolved from Google's
> end.
> 
> Signed-off-by: Neel Desai <neel.desai at intel.com>
> ---
>  lib/igt_chamelium.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 02cc9b2c..497d4421 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -301,6 +301,25 @@ static xmlrpc_value *chamelium_rpc(struct
> chamelium *chamelium,
>  		igt_cleanup_hotplug(monitor_args.mon);
>  	}
>  
> +	/*
> +	 * For some chamelium boards, we observed that the
> +	 * chamelium_rpc() call with method_name as CaptureVideo timed
> +	 * out because the display resolution read by the FPGA using
> +	 * VideoDumper did not match the resolution in the rx chip. The
> +	 * resolution read by the FPGA from the memory was
> +	 * incorrect and needed more time to stabilize to the correct
> +	 * value. So, instead of checking the
> +	 * chamelium->env.fault_occured here, we defer checking in the
> +	 * parent (chamelium_capture()) call and retry for 10 times.
> +	 * This increases the timeout to 100 seconds instead of 10
> +	 * seconds for CaptureVideo. After 10 iterations of
> +	 * chamelium_rpc with method_name as CaptureVideo if the
> +	 * chamelium->env.fault_occured is still set, we return an
> +	 * error.
> +	 */
> +	if (!strcmp(method_name, "CaptureVideo"))
> +		return res;
> +

Why not make this a standard capability for all RPC interaction? What
if we hit this in one of the other calls?

-Stuart

>  	igt_assert_f(!chamelium->env.fault_occurred,
>  		     "Chamelium RPC call failed: %s\n",
>  		     chamelium->env.fault_string);
> @@ -838,9 +857,19 @@ void chamelium_stop_capture(struct chamelium
> *chamelium, int frame_count)
>  void chamelium_capture(struct chamelium *chamelium, struct
> chamelium_port *port,
>  		       int x, int y, int w, int h, int frame_count)
>  {
> -	xmlrpc_DECREF(chamelium_rpc(chamelium, port, "CaptureVideo",
> +	int iter = 0;
> +	xmlrpc_value *res;
> +	do {
> +		res = chamelium_rpc(chamelium, port, "CaptureVideo",
>  				    (w && h) ? "(iiiiii)" : "(iinnnn)",
> -				    port->id, frame_count, x, y, w,
> h));
> +				    port->id, frame_count, x, y, w, h);
> +		iter++;
> +	} while (chamelium->env.fault_occurred && iter < 10);
> +
> +	igt_assert_f(!chamelium->env.fault_occurred,
> +		     "Chamelium RPC call failed: %s\n",
> +		     chamelium->env.fault_string);
> +	xmlrpc_DECREF(res);
>  	chamelium->capturing_port = port;
>  }
>  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3270 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190416/8a7a801e/attachment.bin>