[igt-dev] [PATCH i-g-t 4/8] lib/psr: Make psr_active() only cares about PSR1

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 11:47 -0800, Souza, Jose wrote:
> On Tue, 2018-12-11 at 16:16 -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > > PSR2 will have it own function to detect if is active so we can
> > > drop
> > > the sleep state search and also to make sure that PSR1 is active
> > > lets
> > > search for "PSR1 enabled".
> > 
> > How do you plan to handle PSR2 in kms_frontbuffer_tracking or
> > fbt_fbcon? Have new PSR2 sub tests for them?
> 
> My plan is have a PSR and PSR2 function to make functions really
> simple
> and easy to read.
> 

We'll end up with 

if (psr2)
	/*test PSR2 exit */
else
	/* test PSR1 exit */

in every single sub-test.


Since, the general format for both PSR1 and PSR2 tests is
1) Wait and check for PSR entry
2) Do some operation
3) Wait and check for PSR exit


Isn't it better to use the psr_wait_entry() and psr_wait_exit()
interfaces in all tests and deal with PSR1 and PSR2 specific way of
testing inside them?

> > 
> > 
> > Would it be better to have some sort of PSR state maintained in the
> > psr
> > library so that the functions know whether to test for PSR1 or
> > PSR2?
> > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  lib/igt_psr.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > > index eecee459..c6d6390f 100644
> > > --- a/lib/igt_psr.c
> > > +++ b/lib/igt_psr.c
> > > @@ -37,7 +37,8 @@ static bool psr_active(int debugfs_fd, bool
> > > check_active)
> > >  
> > >  	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
> > >  		 strstr(buf, "Source PSR ctl: enabled");
> > > -	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > > +	active &= !!strstr(buf, "SRDENT");
> > > +	active &= !!strstr(buf, "Status: PSR1 enabled");
> > >  	return check_active ? active : !active;
> > >  }
> > >