Re: [igt-dev] [PATCH i-g-t 1/1] lib/igt_aux : Skip SUSPEND_STATE_S3 if mem_sleep deep state is not supported

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Aug 10, 2022 at 07:51:07AM -0400, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Tauro, Riana <riana.tauro@intel.com>
> > Sent: Wednesday, August 10, 2022 3:10 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> > <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Subject: [PATCH i-g-t 1/1] lib/igt_aux : Skip SUSPEND_STATE_S3 if mem_sleep
> > deep state is not supported
> > 
> > Forcing s3 by setting mem_sleep to deep fails if s3 is not supported by platform
> > 
> > Skip the test by checking if s3 is not one of the supported mem_sleep states
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6531
> > Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> > ---
> >  lib/igt_aux.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c index d80a1935..edb53425 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -882,6 +882,23 @@ static void set_mem_sleep(int power_dir, enum
> > igt_mem_sleep sleep)
> >  				 mem_sleep_name[sleep]));
> >  }
> > 
> > +static bool is_mem_sleep_state_supported(int power_dir, enum
> > +igt_mem_sleep state) {
> > +	const char *str;
> > +	char *mem_sleep_states;
> > +
> > +	igt_assert((mem_sleep_states = igt_sysfs_get(power_dir,
> > +"mem_sleep")));
> > +
> > +	str = strstr(mem_sleep_states, mem_sleep_name[state]);
> > +
> > +	if (!str)
> > +		igt_info("mem_sleep state %s not supported.\nSupported
> > mem_sleep states: %s\n",
> > +			 mem_sleep_name[state], mem_sleep_states);
> Nitpicking 
> With this It may print with []: 
> mem_sleep state deep not supported
> Supported mem_sleep states: [s2idle] shallow
> But as this could be an extra piece of information of current mem_sleep state as well.
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>

pushed. thanks for fixing this.

> 
> > +
> > +	free(mem_sleep_states);
> > +	return str;
> > +}
> > +
> >  /**
> >   * igt_system_suspend_autoresume:
> >   * @state: an #igt_suspend_state, the target suspend state @@ -923,6 +940,8
> > @@ void igt_system_suspend_autoresume(enum igt_suspend_state state,
> > 
> >  	if (state == SUSPEND_STATE_S3) {
> >  		orig_mem_sleep = get_mem_sleep();
> > +		igt_skip_on_f(!is_mem_sleep_state_supported(power_dir,
> > MEM_SLEEP_DEEP),
> > +			      "S3 not supported in this system.\n");
> >  		set_mem_sleep(power_dir, MEM_SLEEP_DEEP);
> >  		igt_skip_on_f(get_mem_sleep() != MEM_SLEEP_DEEP,
> >  			      "S3 not possible in this system.\n");
> > --
> > 2.25.1
>