igt-dev.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] igt/kms_frontbuffer_tracking: Restore modparams around test
Date: Mon, 06 Aug 2018 13:56:15 -0700	[thread overview]
Message-ID: <1533588975.2640.2.camel@intel.com> (raw)
In-Reply-To: <20180806202239.15522-1-chris@chris-wilson.co.uk>

Em Seg, 2018-08-06 às 21:22 +0100, Chris Wilson escreveu:
> Save the module parameters from setup and restore them on teardown,
> so
> that we leave the system in the same state as we found it.

Currently kms_fbt uses igt_set_module_param_int() which uses
igt_save_module_param(), which installs an exit handler, which is
already supposed to accomplish what's described in the sentence above.
What's missing from the current exit handler? Why is the new strategy
better?

Thanks,
Paulo

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/kms_frontbuffer_tracking.c | 43 +++++++++++++++++++++++++++++-
> --
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 1dfd7c1ce..503a6fb6d 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -155,6 +155,7 @@ struct rect {
>  struct {
>  	int fd;
>  	int debugfs;
> +	int modparams;
>  	igt_display_t display;
>  
>  	drm_intel_bufmgr *bufmgr;
> @@ -939,10 +940,10 @@ static bool
> drrs_wait_until_rr_switch_to_low(void)
>  	return igt_wait(is_drrs_low(), 5000, 1);
>  }
>  
> -#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 fbc_enable() igt_sysfs_set(drm.modparams, "enable_fbc", "1")
> +#define fbc_disable() igt_sysfs_set(drm.modparams, "enable_fbc",
> "0")
> +#define psr_enable() igt_sysfs_set(drm.modparams, "enable_psr", "1")
> +#define psr_disable() igt_sysfs_set(drm.modparams, "enable_psr",
> "0")
>  #define drrs_enable()	drrs_set(1)
>  #define drrs_disable()	drrs_set(0)
>  
> @@ -1286,6 +1287,7 @@ static void setup_drm(void)
>  {
>  	drm.fd = drm_open_driver_master(DRIVER_INTEL);
>  	drm.debugfs = igt_debugfs_dir(drm.fd);
> +	drm.modparams = igt_sysfs_open_parameters(drm.fd);
>  
>  	kmstest_set_vt_graphics_mode();
>  	igt_display_init(&drm.display, drm.fd);
> @@ -1299,6 +1301,7 @@ static void teardown_drm(void)
>  {
>  	drm_intel_bufmgr_destroy(drm.bufmgr);
>  	igt_display_fini(&drm.display);
> +	close(drm.modparams);
>  	close(drm.fd);
>  }
>  
> @@ -1467,11 +1470,40 @@ static void setup_drrs(void)
>  	drrs.can_test = true;
>  }
>  
> +static struct {
> +	const char *param;
> +	char buf[12];
> +	int len;
> +} modparams[] = {
> +	{ "enable_psr" },
> +	{ "enable_fbc" },
> +	{ }
> +};
> +
> +static void save_modparams(void)
> +{
> +	for (typeof(*modparams) *p = modparams; p->param; p++)
> +		p->len = igt_sysfs_read(drm.modparams,
> +					p->param, p->buf, sizeof(p-
> >buf));
> +}
> +
> +static void restore_modparams(void)
> +{
> +	for (typeof(*modparams) *p = modparams; p->param; p++) {
> +		if (p->len <= 0)
> +			continue;
> +
> +		igt_sysfs_write(drm.modparams, p->param, p->buf, p-
> >len);
> +	}
> +}
> +
>  static void setup_environment(void)
>  {
>  	setup_drm();
>  	setup_modeset();
>  
> +	save_modparams();
> +
>  	setup_fbc();
>  	setup_psr();
>  	setup_drrs();
> @@ -1486,6 +1518,9 @@ static void teardown_environment(void)
>  	teardown_crcs();
>  	teardown_psr();
>  	teardown_fbc();
> +
> +	restore_modparams();
> +
>  	teardown_modeset();
>  	teardown_drm();
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2018-08-06 20:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-06 20:22 [igt-dev] [PATCH i-g-t] igt/kms_frontbuffer_tracking: Restore modparams around test Chris Wilson
2018-08-06 20:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2018-08-06 20:56 ` Paulo Zanoni [this message]
2018-08-06 21:07   ` [igt-dev] [PATCH i-g-t] " Chris Wilson
2018-08-06 21:17     ` [Intel-gfx] " Paulo Zanoni
2018-08-07  1:13 ` [igt-dev] ✓ Fi.CI.IGT: success for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1533588975.2640.2.camel@intel.com \
    --to=paulo.r.zanoni@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).