[igt-dev] [PATCH i-g-t 1/6] lib/psr: Add support for toggling edp psr through debugfs, v5.
Souza, Jose
jose.souza at intel.com
Wed Sep 5 21:05:57 UTC 2018
On Wed, 2018-09-05 at 10:09 -0700, Pandiyan, Dhinakaran wrote:
> On Tue, 2018-09-04 at 14:59 -0700, Souza, Jose wrote:
> > On Wed, 2018-08-29 at 14:57 -0700, Dhinakaran Pandiyan wrote:
> > > From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > >
> > > It's harmful to write to enable_psr at runtime, and the patch
> > > that
> > > allows
> > > us to change i915_edp_psr_debug with the panel running will
> > > require
> > > us
> > > to abandon the module parameter. Hence the userspace change needs
> > > to
> > > be
> > > put in IGT first before we can change it at kernel time.
> > >
> > > Toggling it to debugfs will mean we can skip a modeset when
> > > changing
> > > our
> > > feature set.
> > >
> > > Changes since v1:
> > > - Rebase with the previous patches dropped.
> > > Changes since v2:
> > > - Rebase on top of new api in i915_edp_psr_debug.
> > > Changes since v3:
> > > - Enable IRQ debugging for extra logging.
> > > - Force PSR1 mode. (dhnkrn)
> > > - Move PSR enable/disable functions to lib/igt_psr. (dhnkrn)
> > > Changes since v4:
> > > - Redisable irqs right away when debugfs api doesn't work.
> > > (dhnkrn)
> > > - Use hex everywhere. (dhnkrn)
> > >
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Signed-off-by: Maarten Lankhorst <
> > > maarten.lankhorst at linux.intel.com
> > > >
> > >
> > > ---
> > > lib/igt_psr.c | 86
> > > +++++++++++++++++++++++++++++++++++++++-
> > > lib/igt_psr.h | 2 +
> > > tests/kms_frontbuffer_tracking.c | 6 +--
> > > 3 files changed, 89 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > > index c979b0b5..614caa6f 100644
> > > --- a/lib/igt_psr.c
> > > +++ b/lib/igt_psr.c
> > > @@ -21,7 +21,9 @@
> > > * IN THE SOFTWARE.
> > > */
> > >
> > > -#include "igt_psr.h"
> > > +#include "igt_psr.h"
> > > +#include "igt_sysfs.h"
> > > +#include <errno.h>
> > >
> > > bool psr_active(int fd, bool check_active)
> > > {
> > > @@ -38,3 +40,85 @@ bool psr_wait_entry(int fd)
> > > {
> > > return igt_wait(psr_active(fd, true), 500, 1);
> > > }
> > > +
> > > +static ssize_t psr_write(int fd, const char *buf)
> > > +{
> > > + return igt_sysfs_write(fd, "i915_edp_psr_debug", buf,
> > > strlen(buf));
> > > +}
> > > +
> > > +static int has_psr_debugfs(int fd)
> > > +{
> > > + int ret;
> > > +
> > > + /*
> > > + * Check if new PSR debugfs api is usable by writing an
> > > invalid
> > > value.
> > > + * Legacy mode will return OK here, debugfs api will
> > > return
> > > -EINVAL.
> > > + * -ENODEV is returned.
> > > + */
> > > + ret = psr_write(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(fd, "0");
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static bool psr_modparam_set(int val)
> > > +{
> > > + static int oldval = -1;
> >
> > This looks wrong, why not read the module parameter? it will
> > guaratee
> > that the returned value is correct here if something else left the
> > parameter with another value.
> >
>
> This patch was merged as part of a different series, send a fix and
> Cc
> Maarten?
Okay, I did the patch and sent it to ML.
Thanks for the feedbak.
> >
> > > +
> > > + igt_set_module_param_int("enable_psr", val);
> > > +
> > > + if (val == oldval)
> > > + return false;
> > > +
> > > + oldval = val;
> > > + return true;
> > > +}
> > > +
> > > +static int psr_restore_debugfs_fd = -1;
> >
> > Maybe move this to the top of the file.
> >
> > Other than that LGTM.
> >
> > > +
> > > +static void restore_psr_debugfs(int sig)
> > > +{
> > > + psr_write(psr_restore_debugfs_fd, "0");
> > > +}
> > > +
> > > +static bool psr_set(int fd, bool enable)
> > > +{
> > > + int ret;
> > > +
> > > + ret = has_psr_debugfs(fd);
> > > + if (ret == -ENODEV) {
> > > + igt_skip_on_f(enable, "PSR not available\n");
> > > + return false;
> > > + }
> > > +
> > > + if (ret == -EINVAL) {
> > > + ret = psr_modparam_set(enable);
> > > + } else {
> > > + ret = psr_write(fd, enable ? "0x3" : "0x1");
> > > + igt_assert(ret > 0);
> > > + }
> > > +
> > > + /* Restore original value on exit */
> > > + if (psr_restore_debugfs_fd == -1) {
> > > + psr_restore_debugfs_fd = dup(fd);
> > > + igt_assert(psr_restore_debugfs_fd >= 0);
> > > + igt_install_exit_handler(restore_psr_debugfs);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +bool psr_enable(int fd)
> > > +{
> > > + return psr_set(fd, true);
> > > +}
> > > +
> > > +bool psr_disable(int fd)
> > > +{
> > > + return psr_set(fd, false);
> > > +}
> > > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > > index 980f85e0..0ef22c3d 100644
> > > --- a/lib/igt_psr.h
> > > +++ b/lib/igt_psr.h
> > > @@ -30,5 +30,7 @@
> > >
> > > bool psr_wait_entry(int fd);
> > > bool psr_active(int fd, bool check_active);
> > > +bool psr_enable(int fd);
> > > +bool psr_disable(int fd);
> > >
> > > #endif
> > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > b/tests/kms_frontbuffer_tracking.c
> > > index 1dfd7c1c..7ea2f697 100644
> > > --- a/tests/kms_frontbuffer_tracking.c
> > > +++ b/tests/kms_frontbuffer_tracking.c
> > > @@ -941,8 +941,6 @@ static bool
> > > drrs_wait_until_rr_switch_to_low(void)
> > >
> > > #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
> > > #define fbc_disable() igt_set_module_param_int("enable_fbc", 0)
> > > -#define psr_enable() igt_set_module_param_int("enable_psr", 1)
> > > -#define psr_disable() igt_set_module_param_int("enable_psr", 0)
> > > #define drrs_enable() drrs_set(1)
> > > #define drrs_disable() drrs_set(0)
> > >
> > > @@ -1137,7 +1135,7 @@ static void disable_features(const struct
> > > test_mode *t)
> > > return;
> > >
> > > fbc_disable();
> > > - psr_disable();
> > > + psr_disable(drm.debugfs);
> > > drrs_disable();
> > > }
> > >
> > > @@ -1719,7 +1717,7 @@ static void enable_features_for_test(const
> > > struct test_mode *t)
> > > if (t->feature & FEATURE_FBC)
> > > fbc_enable();
> > > if (t->feature & FEATURE_PSR)
> > > - psr_enable();
> > > + psr_enable(drm.debugfs);
> > > if (t->feature & FEATURE_DRRS)
> > > drrs_enable();
> > > }
More information about the igt-dev
mailing list