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

[Paul Kocialkowski <paul.kocialkowski@linux.intel.com>]

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?

-- 
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx