Re: [igt-dev] [PATCH i-g-t 07/10] lib/psr: Drop support to old kernels without new PSR debugfs interface

["Souza, Jose" <jose.souza@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3557 bytes --]

On Wed, 2019-01-16 at 18:16 -0800, Dhinakaran Pandiyan wrote:
> On Wed, 2019-01-16 at 18:07 -0800, Souza, Jose wrote:
> > On Tue, 2019-01-15 at 21:21 -0800, Dhinakaran Pandiyan wrote:
> > > On Fri, 2019-01-11 at 17:46 -0800, José Roberto de Souza wrote:
> > > > The nexts patches will add PSR2 tests and for that we need the
> > > > new
> > > > PSR debugfs interface that was released in kernel 4.20 so it
> > > > will
> > > > not
> > > > break for any updated system and we can drop support to the old
> > > > debugsfs and the support to kernels without even a debugfs
> > > > interface.
> > > 
> > > Can't we have psr_set(mode == PSR_MODE_2) skip if the kernel does
> > > not
> > > support the new interface?
> > 
> > We could but why keep supporting old kernels? My experience of
> > running
> > a newer IGT over a old kernel or on the contrary was always
> > problematic.
> 
> Downstream OS'es can package a newer IGT with an older kernel. In
> this
> case, it's not much of an effort to continue support for PSR1 tests
> from what I can tell. 

Other problem here it that with the enable_psr parameter we don't
control what PSR version is going to be enabled, if possible PSR2 will
be enabled causing tests to fail in the first psr_wait_entry().

But okay, I will drop this patch.



> > 	
> > 
> > > 
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  lib/igt_psr.c | 35 +++++++----------------------------
> > > >  1 file changed, 7 insertions(+), 28 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > > > index c6638c2c..5cc0fbc2 100644
> > > > --- a/lib/igt_psr.c
> > > > +++ b/lib/igt_psr.c
> > > > @@ -73,27 +73,7 @@ static int has_psr_debugfs(int debugfs_fd)
> > > >  	 * -ENODEV is returned when PSR is unavailable.
> > > >  	 */
> > > >  	ret = psr_write(debugfs_fd, "0xf");
> > > > -	if (ret == -EINVAL)
> > > > -		return 0;
> > > > -	else if (ret < 0)
> > > > -		return ret;
> > > > -
> > > > -	/* legacy debugfs api, we enabled irqs by writing,
> > > > disable
> > > > them. */
> > > > -	psr_write(debugfs_fd, "0");
> > > > -	return -EINVAL;
> > > > -}
> > > > -
> > > > -static bool psr_modparam_set(int val)
> > > > -{
> > > > -	static int oldval = -1;
> > > > -
> > > > -	igt_set_module_param_int("enable_psr", val);
> > > > -
> > > > -	if (val == oldval)
> > > > -		return false;
> > > > -
> > > > -	oldval = val;
> > > > -	return true;
> > > > +	return ret == -EINVAL ? 0 : ret;
> > > >  }
> > > >  
> > > >  static int psr_restore_debugfs_fd = -1;
> > > > @@ -109,16 +89,15 @@ static bool psr_set(int debugfs_fd, bool
> > > > enable)
> > > >  
> > > >  	ret = has_psr_debugfs(debugfs_fd);
> > > >  	if (ret == -ENODEV) {
> > > > -		igt_skip_on_f(enable, "PSR not available\n");
> > > > +		igt_skip("PSR not available\n");
> > > > +		return false;
> > > > +	} else if (ret) {
> > > > +		igt_skip("PSR debugfs interface not
> > > > available\n");
> > > >  		return false;
> > > >  	}
> > > >  
> > > > -	if (ret == -EINVAL) {
> > > > -		ret = psr_modparam_set(enable);
> > > > -	} else {
> > > > -		ret = psr_write(debugfs_fd, enable ? "0x3" :
> > > > "0x1");
> > > > -		igt_assert(ret > 0);
> > > > -	}
> > > > +	ret = psr_write(debugfs_fd, enable ? "0x3" : "0x1");
> > > > +	igt_assert(ret > 0);
> > > >  
> > > >  	/* Restore original value on exit */
> > > >  	if (psr_restore_debugfs_fd == -1) {

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev