Re: [PATCH i-g-t 4/7] lib/igt_chamelium: Add support for configurable DUT suspend/resume delay

[Martin Peres <martin.peres@linux.intel.com>]

On 26/06/17 17:33, Paul Kocialkowski wrote:
> On Mon, 2017-06-26 at 17:25 +0300, Martin Peres wrote:
>> On 26/06/17 16:59, Paul Kocialkowski wrote:
>>> This adds support for reading a SuspendResumeDelay property (under
>>> [DUT]) in the IGT configuration (igtrc) and exposing it through a
>>> chamelium_get_suspend_resume_delay function.
>>>
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
>>> ---
>>>    lib/igt_chamelium.c | 31 ++++++++++++++++++++++++++++++-
>>>    lib/igt_chamelium.h |  1 +
>>>    2 files changed, 31 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
>>> index 225f98c3..a1aaf405 100644
>>> --- a/lib/igt_chamelium.c
>>> +++ b/lib/igt_chamelium.c
>>> @@ -58,7 +58,7 @@
>>>     *	[Chamelium]
>>>     *	URL=http://chameleon:9992 # The URL used for connecting to the
>>> Chamelium's RPC server
>>>     *
>>> - *	# The rest of the sections are used for defining connector
>>> mappings.
>>> + *	# The following sections are used for defining connector
>>> mappings.
>>>     *	# This is required so any tests using the Chamelium know which
>>> connector
>>>     *	# on the test machine should be connected to each Chamelium
>>> port.
>>>     *	#
>>> @@ -70,12 +70,19 @@
>>>     *
>>>     *	[Chamelium:HDMI-A-1]
>>>     *	ChameliumPortID=3
>>> + *
>>> + *	# The following section is used for configuring the Device Under
>>> Test.
>>> + *	# It is not mandatory and allows overriding default values.
>>> + *	[DUT]
>>> + *	SuspendResumeDelay=10
>>>     * ]|
>>>     *
>>>     * By default, this file is expected to exist in ~/.igtrc . The directory
>>> for
>>>     * this can be overriden by setting the environment variable
>>> %IGT_CONFIG_PATH.
>>>     */
>>>    
>>> +#define SUSPEND_RESUME_DELAY_DEFAULT 20 /* seconds */
>>> +
>>>    struct chamelium_edid {
>>>    	int id;
>>>    	struct igt_list link;
>>> @@ -100,6 +107,7 @@ struct chamelium {
>>>    	xmlrpc_env env;
>>>    	xmlrpc_client *client;
>>>    	char *url;
>>> +	int suspend_resume_delay;
>>>    
>>>    	/* Indicates the last port to have been used for capturing video
>>> */
>>>    	struct chamelium_port *capturing_port;
>>> @@ -114,6 +122,20 @@ struct chamelium {
>>>    static struct chamelium *cleanup_instance;
>>
>> Why do you make this part of the chamelium configuration? This should be
>> common to all the suspend tests.
> 
> Fair enough, but at this point, only lib/igt_chamelium.c is using igtrc and all
> the parsing is implemented there.
> 
> So one course of action here would be to move igtrc parsing outside of
> igt_chamelium (maybe to igt_core?) and use some global variable for storing this
> value. What do you think?
> 

That seems like a plan, except you can just call a function that will do 
the parsing for you and return a structure with all the parameters. If 
it is called a second time, just return the cached value.

What do you think, Lyude?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx