From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marius Vlad Subject: Re: [PATCH i-g-t v4 1/1] lib/igt_pm: Lib for power management Date: Thu, 18 Feb 2016 15:32:33 +0200 Message-ID: <20160218133232.GA10907@mcvlad-wk.rb.intel.com> References: <1455793726-25141-1-git-send-email-david.weinehall@linux.intel.com> <1455793726-25141-2-git-send-email-david.weinehall@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0933914787==" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id DCCE56E34F for ; Thu, 18 Feb 2016 13:34:35 +0000 (UTC) In-Reply-To: <1455793726-25141-2-git-send-email-david.weinehall@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: David Weinehall Cc: intel-gfx@lists.freedesktop.org, David Weinehall List-Id: intel-gfx@lists.freedesktop.org --===============0933914787== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BXVAT5kNtrzKuDFl" Content-Disposition: inline --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 18, 2016 at 01:08:46PM +0200, David Weinehall wrote: > Move power management related code to a separate library. > Initially this is done only for workarounds that apply to external > components. Modify the users of such workarounds accordingly. > This currently involves HD audio and SATA link power management. > For SATA link PM there's also code to save the previous settings, > to allow for resetting the values after we've finished testing. >=20 > Signed-off-by: David Weinehall Reviewed-by: Marius Vlad > --- > .../intel-gpu-tools/intel-gpu-tools-docs.xml | 1 + > lib/Makefile.sources | 2 + > lib/igt.h | 1 + > lib/igt_aux.c | 15 +- > lib/igt_pm.c | 233 +++++++++++++++= ++++++ > lib/igt_pm.h | 31 +++ > tests/pm_lpsp.c | 25 +-- > tests/pm_rpm.c | 40 +--- > 8 files changed, 281 insertions(+), 67 deletions(-) > create mode 100644 lib/igt_pm.c > create mode 100644 lib/igt_pm.h >=20 > diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml b/do= cs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > index 618dc5fd7076..3ea3563a029a 100644 > --- a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > @@ -24,6 +24,7 @@ > > > > + > > > > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > index e33861e73755..1316fd21e040 100644 > --- a/lib/Makefile.sources > +++ b/lib/Makefile.sources > @@ -60,6 +60,8 @@ libintel_tools_la_SOURCES =3D \ > igt_core.h \ > igt_draw.c \ > igt_draw.h \ > + igt_pm.c \ > + igt_pm.h \ > uwildmat/uwildmat.h \ > uwildmat/uwildmat.c \ > $(NULL) > diff --git a/lib/igt.h b/lib/igt.h > index 3be25511bb77..d751f2439de2 100644 > --- a/lib/igt.h > +++ b/lib/igt.h > @@ -35,6 +35,7 @@ > #include "igt_fb.h" > #include "igt_gt.h" > #include "igt_kms.h" > +#include "igt_pm.h" > #include "igt_stats.h" > #include "instdone.h" > #include "intel_batchbuffer.h" > diff --git a/lib/igt_aux.c b/lib/igt_aux.c > index ebee119c411d..7d35666eb7f3 100644 > --- a/lib/igt_aux.c > +++ b/lib/igt_aux.c > @@ -59,6 +59,7 @@ > #include "intel_reg.h" > #include "ioctl_wrappers.h" > #include "igt_kms.h" > +#include "igt_pm.h" > =20 > /** > * SECTION:igt_aux > @@ -544,19 +545,7 @@ bool igt_setup_runtime_pm(void) > if (pm_status_fd >=3D 0) > return true; > =20 > - /* The Audio driver can get runtime PM references, so we need to make > - * sure its runtime PM is enabled, so it can release the refs and > - * actually enable us to runtime suspend. */ > - fd =3D open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY= ); > - if (fd >=3D 0) { > - igt_assert(write(fd, "1\n", 2) =3D=3D 2); > - close(fd); > - } > - fd =3D open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY= ); > - if (fd >=3D 0) { > - igt_assert(write(fd, "auto\n", 5) =3D=3D 5); > - close(fd); > - } > + igt_pm_enable_audio_runtime_pm(); > =20 > /* Our implementation uses autosuspend. Try to set it to 0ms so the test > * suite goes faster and we have a higher probability of triggering race > diff --git a/lib/igt_pm.c b/lib/igt_pm.c > new file mode 100644 > index 000000000000..b1b5503c4016 > --- /dev/null > +++ b/lib/igt_pm.c > @@ -0,0 +1,233 @@ > +/* > + * Copyright =A9 2013, 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the = next > + * paragraph) shall be included in all copies or substantial portions of= the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER D= EALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Paulo Zanoni > + * David Weinehall > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "drmtest.h" > +#include "igt_pm.h" > + > +enum { > + POLICY_UNKNOWN =3D -1, > + POLICY_MAX_PERFORMANCE =3D 0, > + POLICY_MEDIUM_POWER =3D 1, > + POLICY_MIN_POWER =3D 2 > +}; > + > +#define MAX_PERFORMANCE_STR "max_performance\n" > +#define MEDIUM_POWER_STR "medium_power\n" > +#define MIN_POWER_STR "min_power\n" > +/* Remember to fix this if adding longer strings */ > +#define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR) > + > +/** > + * SECTION:igt_pm > + * @short_description: Power Management related helpers > + * @title: Power Management > + * @include: igt.h > + * > + * This library provides various helpers to enable power management for, > + * and in some cases subsequently allow restoring the old behaviour of, > + * various external components that by default are set up in a way > + * that interferes with the testing of our power management functionalit= y. > + */ > +/** > + * igt_pm_enable_audio_runtime_pm: > + * > + * We know that if we don't enable audio runtime PM, snd_hda_intel will = never > + * release its power well refcount, and we'll never reach the LPSP state. > + * There's no guarantee that it will release the power well if we enable > + * runtime PM, but at least we can try. > + * > + * We don't have any assertions on open since the user may not even have > + * snd_hda_intel loaded, which is not a problem. > + */ > +void igt_pm_enable_audio_runtime_pm(void) > +{ > + int fd; > + > + fd =3D open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY= ); > + if (fd >=3D 0) { > + igt_assert_eq(write(fd, "1\n", 2), 2); > + close(fd); > + } > + fd =3D open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY= ); > + if (fd >=3D 0) { > + igt_assert_eq(write(fd, "auto\n", 5), 5); > + close(fd); > + } > + /* Give some time for it to react. */ > + sleep(1); > +} > + > +/** > + * igt_pm_enable_sata_link_power_management: > + * > + * Enable the min_power policy for SATA link power management. > + * Without this we cannot reach deep runtime power states. > + * > + * We don't have any assertions on open since the system might not have > + * a SATA host. > + * > + * Returns: > + * An opaque pointer to the data needed to restore the default values > + * after the test has terminated, or NULL if SATA link power management > + * is not supported. This pointer should be freed when no longer used > + * (typically after having called restore_sata_link_power_management()). > + */ > +int8_t *igt_pm_enable_sata_link_power_management(void) > +{ > + int fd, i; > + ssize_t len; > + char *buf; > + char *file_name; > + int8_t *link_pm_policies =3D NULL; > + > + file_name =3D malloc(PATH_MAX); > + buf =3D malloc(MAX_POLICY_STRLEN + 1); > + > + for (i =3D 0; ; i++) { > + int8_t policy; > + > + snprintf(file_name, PATH_MAX, > + "/sys/class/scsi_host/host%d/link_power_management_policy", > + i); > + > + fd =3D open(file_name, O_RDWR); > + if (fd < 0) > + break; > + > + len =3D read(fd, buf, MAX_POLICY_STRLEN); > + buf[len] =3D '\0'; > + > + if (!strncmp(MAX_PERFORMANCE_STR, buf, > + strlen(MAX_PERFORMANCE_STR))) > + policy =3D POLICY_MAX_PERFORMANCE; > + else if (!strncmp(MEDIUM_POWER_STR, buf, > + strlen(MEDIUM_POWER_STR))) > + policy =3D POLICY_MEDIUM_POWER; > + else if (!strncmp(MIN_POWER_STR, buf, > + strlen(MIN_POWER_STR))) > + policy =3D POLICY_MIN_POWER; > + else > + policy =3D POLICY_UNKNOWN; > + > + if (!(i % 256)) > + link_pm_policies =3D realloc(link_pm_policies, > + (i / 256 + 1) * 256 + 1); > + > + link_pm_policies[i] =3D policy; > + link_pm_policies[i + 1] =3D 0; > + > + /* If the policy is something we don't know about, > + * don't touch it, since we might potentially break things. > + * And we obviously don't need to touch anything if the > + * setting is already correct... > + */ > + if (policy !=3D POLICY_UNKNOWN && > + policy !=3D POLICY_MIN_POWER) { > + lseek(fd, 0, SEEK_SET); > + igt_assert_eq(write(fd, MIN_POWER_STR, > + strlen(MIN_POWER_STR)), > + strlen(MIN_POWER_STR)); > + } > + close(fd); > + } > + free(buf); > + free(file_name); > + > + return link_pm_policies; > +} > + > +/** > + * igt_pm_restore_sata_link_power_management: > + * @pm_data: An opaque pointer with saved link PM policies; > + * If NULL is passed we force enable the "max_performance" pol= icy. > + * > + * Restore the link power management policies to the values > + * prior to enabling min_power. > + * > + * Caveat: If the system supports hotplugging and hotplugging takes > + * place during our testing so that the hosts change numbers > + * we might restore the settings to the wrong hosts. > + */ > +void igt_pm_restore_sata_link_power_management(int8_t *pm_data) > +{ > + int fd, i; > + char *file_name; > + > + /* Disk runtime PM policies. */ > + file_name =3D malloc(PATH_MAX); > + for (i =3D 0; ; i++) { > + int8_t policy; > + > + if (!pm_data) > + policy =3D POLICY_MAX_PERFORMANCE; > + else if (pm_data[i] =3D=3D POLICY_UNKNOWN) > + continue; > + else > + policy =3D pm_data[i]; > + > + snprintf(file_name, PATH_MAX, > + "/sys/class/scsi_host/host%d/link_power_management_policy", > + i); > + > + fd =3D open(file_name, O_WRONLY); > + if (fd < 0) > + break; > + > + switch (policy) { > + default: > + case POLICY_MAX_PERFORMANCE: > + igt_assert_eq(write(fd, MAX_PERFORMANCE_STR, > + strlen(MAX_PERFORMANCE_STR)), > + strlen(MAX_PERFORMANCE_STR)); > + break; > + > + case POLICY_MEDIUM_POWER: > + igt_assert_eq(write(fd, MEDIUM_POWER_STR, > + strlen(MEDIUM_POWER_STR)), > + strlen(MEDIUM_POWER_STR)); > + break; > + > + case POLICY_MIN_POWER: > + igt_assert_eq(write(fd, MIN_POWER_STR, > + strlen(MIN_POWER_STR)), > + strlen(MIN_POWER_STR)); > + break; > + } > + > + close(fd); > + } > + free(file_name); > +} > diff --git a/lib/igt_pm.h b/lib/igt_pm.h > new file mode 100644 > index 000000000000..c14ff1f7a0ef > --- /dev/null > +++ b/lib/igt_pm.h > @@ -0,0 +1,31 @@ > +/* > + * Copyright =A9 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the = next > + * paragraph) shall be included in all copies or substantial portions of= the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER D= EALINGS > + * IN THE SOFTWARE. > + */ > + > +#ifndef IGT_PM_H > +#define IGT_PM_H > + > +void igt_pm_enable_audio_runtime_pm(void); > +int8_t *igt_pm_enable_sata_link_power_management(void); > +void igt_pm_restore_sata_link_power_management(int8_t *pm_data); > + > +#endif /* IGT_PM_H */ > diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c > index a82420bf06de..4cedefffb545 100644 > --- a/tests/pm_lpsp.c > +++ b/tests/pm_lpsp.c > @@ -31,29 +31,6 @@ > #include > =20 > =20 > -/* We know that if we don't enable audio runtime PM, snd_hda_intel will = never > - * release its power well refcount, and we'll never reach the LPSP sate.= OTOH > - * there's no guarantee that it will release the power well if we enable= runtime > - * PM, but at least we can try. We don't have any assertions since the = user may > - * not even have snd_hda_intel loaded, which is not a problem. */ > -static void disable_audio_runtime_pm(void) > -{ > - int fd; > - > - fd =3D open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY= ); > - if (fd >=3D 0) { > - igt_assert_eq(write(fd, "1\n", 2), 2); > - close(fd); > - } > - fd =3D open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY= ); > - if (fd >=3D 0) { > - igt_assert_eq(write(fd, "auto\n", 5), 5); > - close(fd); > - } > - /* Give some time for it to react. */ > - sleep(1); > -} > - > static bool supports_lpsp(uint32_t devid) > { > return IS_HASWELL(devid) || IS_BROADWELL(devid); > @@ -227,7 +204,7 @@ igt_main > drm_connectors[i] =3D drmModeGetConnectorCurrent(drm_fd, > drm_res->connectors[i]); > =20 > - disable_audio_runtime_pm(); > + igt_pm_enable_audio_runtime_pm(); > =20 > igt_require(supports_lpsp(devid)); > =20 > diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c > index 22dc2b4d6de4..2aa6c1018aa2 100644 > --- a/tests/pm_rpm.c > +++ b/tests/pm_rpm.c > @@ -1,5 +1,5 @@ > /* > - * Copyright =A9 2013 Intel Corporation > + * Copyright =A9 2013, 2015 Intel Corporation > * > * Permission is hereby granted, free of charge, to any person obtaining= a > * copy of this software and associated documentation files (the "Softwa= re"), > @@ -109,6 +109,8 @@ struct modeset_params lpsp_mode_params; > struct modeset_params non_lpsp_mode_params; > struct modeset_params *default_mode_params; > =20 > +static int8_t *pm_data =3D NULL; > + > /* If the read fails, then the machine doesn't support PC8+ residencies.= */ > static bool supports_pc8_plus_residencies(void) > { > @@ -685,41 +687,13 @@ static void setup_pc8(void) > has_pc8 =3D true; > } > =20 > -/* If we want to actually reach PC8+ states, we need to properly configu= re all > - * the devices on the system to allow this. This function will try to se= tup the > - * things we know we need, but won't scream in case anything fails: we d= on't > - * know which devices are present on your machine, so we can't really ex= pect > - * anything, just try to help with the more common problems. */ > -static void setup_non_graphics_runtime_pm(void) > -{ > - int fd, i; > - char *file_name; > - > - /* Disk runtime PM policies. */ > - file_name =3D malloc(PATH_MAX); > - for (i =3D 0; ; i++) { > - > - snprintf(file_name, PATH_MAX, > - "/sys/class/scsi_host/host%d/link_power_management_policy", > - i); > - > - fd =3D open(file_name, O_WRONLY); > - if (fd < 0) > - break; > - > - igt_assert(write(fd, "min_power\n", 10) =3D=3D 10); > - close(fd); > - } > - free(file_name); > -} > - > static void setup_environment(void) > { > drm_fd =3D drm_open_driver_master(DRIVER_INTEL); > =20 > init_mode_set_data(&ms_data); > =20 > - setup_non_graphics_runtime_pm(); > + pm_data =3D igt_pm_enable_sata_link_power_management(); > =20 > has_runtime_pm =3D igt_setup_runtime_pm(); > setup_pc8(); > @@ -728,11 +702,17 @@ static void setup_environment(void) > igt_info("PC8 residency support: %d\n", has_pc8); > =20 > igt_require(has_runtime_pm); > +} > =20 > +static void restore_environment(void) > +{ > + igt_pm_restore_sata_link_power_management(pm_data); > + free(pm_data); > } > =20 > static void teardown_environment(void) > { > + restore_environment(); > fini_mode_set_data(&ms_data); > drmClose(drm_fd); > close(msr_fd); > --=20 > 2.7.0 >=20 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx --BXVAT5kNtrzKuDFl Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWxcfwAAoJELmLWIAQzyE+qO4IAIKPTQEzxfBJ7PmZwoQXRn3o GRgCzSv9u2JNVTyeIW7lF8ljY2cTMaDktQbaSFms2/vzRsSUkL3X/f6S/tCEdVbC YW0Mo10fHexwxvMmXbmdsmpQGHnawogvp54dlDyOf2BDL5VbTxCdO82biZVI5M+Y 5MXD8ta8tkDVMANtp5wDo9Bld7qt7wSpxIP+Fp+s6mtnar4LNcPBAxZ79xssL/17 IE+aku/OKPdE4fOV/tBbhWGld2ojEvUaMYiW8bq5WHKUNcldMdJzMy3leXSqJpcZ 1ECWlklyx+nL/XjFPcorz/9KHy2q6mDYlA+ogCnSBdocT8nmjQS/LyvrJgrckZ0= =edvY -----END PGP SIGNATURE----- --BXVAT5kNtrzKuDFl-- --===============0933914787== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0933914787==--