[Intel-gfx] [PATCH 2/3] lib: Optionally initialize mmio register access without forcewake
Jani Nikula
jani.nikula at linux.intel.com
Fri Aug 28 08:12:02 PDT 2015
On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala at linux.intel.com> wrote:
> Sometimes it is beneficial to debug the forcewake registers
> themselves or registers that don't need or are interfered by
> forcewake. Add parameter to intel_register_access_init()
> to optionally avoid forcewake dance around register access.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
> debugger/debug_rdata.c | 2 +-
> debugger/eudb.c | 2 +-
> lib/intel_io.h | 2 +-
> lib/intel_mmio.c | 9 +++++++--
> tests/gem_workarounds.c | 2 +-
> tests/pm_lpsp.c | 2 +-
> tools/intel_display_poller.c | 2 +-
> tools/intel_forcewaked.c | 4 ++--
> tools/intel_gpu_top.c | 2 +-
> tools/intel_infoframes.c | 2 +-
> tools/intel_l3_parity.c | 2 +-
> tools/intel_panel_fitter.c | 2 +-
> tools/intel_perf_counters.c | 2 +-
> tools/intel_reg.c | 6 +++---
> tools/intel_watermark.c | 14 +++++++-------
> tools/quick_dump/chipset.i | 4 ++--
> 16 files changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/debugger/debug_rdata.c b/debugger/debug_rdata.c
> index 61d82d9..2643581 100644
> --- a/debugger/debug_rdata.c
> +++ b/debugger/debug_rdata.c
> @@ -135,7 +135,7 @@ int main(int argc, char *argv[]) {
> struct pci_device *pci_dev;
> pci_dev = intel_get_pci_device();
>
> - intel_register_access_init(pci_dev, 1);
> + intel_register_access_init(pci_dev, 1, 0);
> find_stuck_threads();
> // collect_rdata(atoi(argv[1]), atoi(argv[2]));
> return 0;
> diff --git a/debugger/eudb.c b/debugger/eudb.c
> index 39c5cca..1d6da48 100644
> --- a/debugger/eudb.c
> +++ b/debugger/eudb.c
> @@ -540,7 +540,7 @@ int main(int argc, char* argv[]) {
> abort();
> }
>
> - assert(intel_register_access_init(pci_dev, 1) == 0);
> + assert(intel_register_access_init(pci_dev, 1, 0) == 0);
>
> memset(bits, -1, sizeof(bits));
> /*
> diff --git a/lib/intel_io.h b/lib/intel_io.h
> index e2d6b47..3ce9474 100644
> --- a/lib/intel_io.h
> +++ b/lib/intel_io.h
> @@ -36,7 +36,7 @@ extern void *igt_global_mmio;
> void intel_mmio_use_pci_bar(struct pci_device *pci_dev);
> void intel_mmio_use_dump_file(char *file);
>
> -int intel_register_access_init(struct pci_device *pci_dev, int safe);
> +int intel_register_access_init(struct pci_device *pci_dev, int safe, int nofw);
Friday night nitpicking, in the long run adding boolean parameters to a
function makes it hard to use. (Especially so if they are ints used as
boolean! ;) I'd rather you either turned the two parameters into one
mode parameter, so you'd call it with init(dev, SAFE | NOFW), or added
an init_nofw() variant.
Just sayin'. I'm not insisting though.
BR,
Jani.
> void intel_register_access_fini(void);
> uint32_t intel_register_read(uint32_t reg);
> void intel_register_write(uint32_t reg, uint32_t val);
> diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c
> index e5e23c3..641fb4c 100644
> --- a/lib/intel_mmio.c
> +++ b/lib/intel_mmio.c
> @@ -155,6 +155,7 @@ release_forcewake_lock(int fd)
> * intel_register_access_init:
> * @pci_dev: intel graphics pci device
> * @safe: use safe register access tables
> + * @nofw: don't take forcewake lock during register access
> *
> * This initializes the new register access library, which supports forcewake
> * handling and also allows register access to be checked with an explicit
> @@ -165,7 +166,7 @@ release_forcewake_lock(int fd)
> * @pci_dev can be obtained from intel_get_pci_device().
> */
> int
> -intel_register_access_init(struct pci_device *pci_dev, int safe)
> +intel_register_access_init(struct pci_device *pci_dev, int safe, int nofw)
> {
> int ret;
>
> @@ -187,7 +188,11 @@ intel_register_access_init(struct pci_device *pci_dev, int safe)
> /* Find where the forcewake lock is. Forcewake doesn't exist
> * gen < 6, but the debugfs should do the right things for us.
> */
> - ret = igt_open_forcewake_handle();
> + if (nofw)
> + ret = -1;
> + else
> + ret = igt_open_forcewake_handle();
> +
> if (ret == -1)
> mmio_data.key = FAKEKEY;
> else
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index 3759c23..81d67dd 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -117,7 +117,7 @@ static int workaround_fail_count(void)
> {
> int i, fail_count = 0;
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> /* There is a small delay after coming ot of rc6 to the correct
> render context values will get loaded by hardware (bdw,chv).
> diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
> index 43444d8..faf20a2 100644
> --- a/tests/pm_lpsp.c
> +++ b/tests/pm_lpsp.c
> @@ -239,7 +239,7 @@ igt_main
>
> igt_require(supports_lpsp(devid));
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> kmstest_set_vt_graphics_mode();
> }
> diff --git a/tools/intel_display_poller.c b/tools/intel_display_poller.c
> index 6d6ea21..4c2bde7 100644
> --- a/tools/intel_display_poller.c
> +++ b/tools/intel_display_poller.c
> @@ -1350,7 +1350,7 @@ int main(int argc, char *argv[])
> break;
> }
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> printf("%s?\n", test_name(test, pipe, bit, test_pixelcount));
>
> diff --git a/tools/intel_forcewaked.c b/tools/intel_forcewaked.c
> index 01ca025..95e173d 100644
> --- a/tools/intel_forcewaked.c
> +++ b/tools/intel_forcewaked.c
> @@ -79,7 +79,7 @@ int main(int argc, char *argv[])
> INFO_PRINT("started daemon");
> }
>
> - ret = intel_register_access_init(intel_get_pci_device(), 1);
> + ret = intel_register_access_init(intel_get_pci_device(), 1, 0);
> if (ret) {
> INFO_PRINT("Couldn't init register access\n");
> exit(1);
> @@ -90,7 +90,7 @@ int main(int argc, char *argv[])
> if (!is_alive()) {
> INFO_PRINT("gpu reset? restarting daemon\n");
> intel_register_access_fini();
> - ret = intel_register_access_init(intel_get_pci_device(), 1);
> + ret = intel_register_access_init(intel_get_pci_device(), 1, 0);
> if (ret)
> INFO_PRINT("Reg access init fail\n");
> }
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index b5cfda0..6629dcc 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -511,7 +511,7 @@ int main(int argc, char **argv)
> }
>
> /* Grab access to the registers */
> - intel_register_access_init(pci_dev, 0);
> + intel_register_access_init(pci_dev, 0, 0);
>
> ring_init(&render_ring);
> if (IS_GEN4(devid) || IS_GEN5(devid))
> diff --git a/tools/intel_infoframes.c b/tools/intel_infoframes.c
> index e03cb2c..63eb10b 100644
> --- a/tools/intel_infoframes.c
> +++ b/tools/intel_infoframes.c
> @@ -1108,7 +1108,7 @@ int main(int argc, char *argv[])
> " perfectly: the Kernel might undo our changes.\n");
>
> pci_dev = intel_get_pci_device();
> - intel_register_access_init(pci_dev, 0);
> + intel_register_access_init(pci_dev, 0, 0);
> intel_check_pch();
>
> if (IS_GEN4(pci_dev->device_id))
> diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
> index a4b7d73..ef38177 100644
> --- a/tools/intel_l3_parity.c
> +++ b/tools/intel_l3_parity.c
> @@ -196,7 +196,7 @@ int main(int argc, char *argv[])
> if (intel_gen(devid) < 7 || IS_VALLEYVIEW(devid))
> exit(EXIT_SUCCESS);
>
> - assert(intel_register_access_init(intel_get_pci_device(), 0) == 0);
> + assert(intel_register_access_init(intel_get_pci_device(), 0, 0) == 0);
>
> ret = asprintf(&path[0], "/sys/class/drm/card%d/l3_parity", device);
> assert(ret != -1);
> diff --git a/tools/intel_panel_fitter.c b/tools/intel_panel_fitter.c
> index 5519361..31b9f0e 100644
> --- a/tools/intel_panel_fitter.c
> +++ b/tools/intel_panel_fitter.c
> @@ -279,7 +279,7 @@ int main (int argc, char *argv[])
> "solution that may or may not work. Use it at your own risk.\n");
>
> pci_dev = intel_get_pci_device();
> - intel_register_access_init(pci_dev, 0);
> + intel_register_access_init(pci_dev, 0, 0);
> devid = pci_dev->device_id;
>
> if (!HAS_PCH_SPLIT(devid)) {
> diff --git a/tools/intel_perf_counters.c b/tools/intel_perf_counters.c
> index 739f926..d26bba3 100644
> --- a/tools/intel_perf_counters.c
> +++ b/tools/intel_perf_counters.c
> @@ -483,7 +483,7 @@ main(int argc, char **argv)
>
> if (oacontrol) {
> /* Forcewake */
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> /* Enable performance counters */
> intel_register_write(OACONTROL,
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 2b60a83..b082555 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -409,7 +409,7 @@ static int intel_reg_read(struct config *config, int argc, char *argv[])
> if (config->mmiofile)
> intel_mmio_use_dump_file(config->mmiofile);
> else
> - intel_register_access_init(config->pci_dev, 0);
> + intel_register_access_init(config->pci_dev, 0, 0);
>
> for (i = 1; i < argc; i++) {
> struct reg reg;
> @@ -439,7 +439,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
> return EXIT_FAILURE;
> }
>
> - intel_register_access_init(config->pci_dev, 0);
> + intel_register_access_init(config->pci_dev, 0, 0);
>
> for (i = 1; i < argc; i += 2) {
> struct reg reg;
> @@ -477,7 +477,7 @@ static int intel_reg_dump(struct config *config, int argc, char *argv[])
> if (config->mmiofile)
> intel_mmio_use_dump_file(config->mmiofile);
> else
> - intel_register_access_init(config->pci_dev, 0);
> + intel_register_access_init(config->pci_dev, 0, 0);
>
> for (i = 0; i < config->regcount; i++) {
> reg = &config->regs[i];
> diff --git a/tools/intel_watermark.c b/tools/intel_watermark.c
> index 0b7c5e5..f399e96 100644
> --- a/tools/intel_watermark.c
> +++ b/tools/intel_watermark.c
> @@ -139,7 +139,7 @@ static void ilk_wm_dump(void)
> int num_pipes = is_gen7_plus(devid) ? 3 : 2;
> struct ilk_wm wm = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> for (i = 0; i < num_pipes; i++) {
> dspcntr[i] = read_reg(0x70180 + i * 0x1000);
> @@ -265,7 +265,7 @@ static void vlv_wm_dump(void)
> uint32_t dsp_ss_pm, ddr_setup2;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dsparb = read_reg(0x70030);
> dsparb2 = read_reg(0x70060);
> @@ -481,7 +481,7 @@ static void g4x_wm_dump(void)
> uint32_t mi_arb_state;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dspacntr = read_reg(0x70180);
> dspbcntr = read_reg(0x71180);
> @@ -567,7 +567,7 @@ static void gen4_wm_dump(void)
> uint32_t mi_arb_state;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dsparb = read_reg(0x70030);
> fw1 = read_reg(0x70034);
> @@ -638,7 +638,7 @@ static void pnv_wm_dump(void)
> uint32_t cbr;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dsparb = read_reg(0x70030);
> fw1 = read_reg(0x70034);
> @@ -728,7 +728,7 @@ static void gen3_wm_dump(void)
> uint32_t mi_arb_state;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dsparb = read_reg(0x70030);
> instpm = read_reg(0x20c0);
> @@ -797,7 +797,7 @@ static void gen2_wm_dump(void)
> uint32_t mi_state;
> struct gmch_wm wms[MAX_PLANE] = {};
>
> - intel_register_access_init(intel_get_pci_device(), 0);
> + intel_register_access_init(intel_get_pci_device(), 0, 0);
>
> dsparb = read_reg(0x70030);
> mem_mode = read_reg(0x20cc);
> diff --git a/tools/quick_dump/chipset.i b/tools/quick_dump/chipset.i
> index 90db40e..e6921cf 100644
> --- a/tools/quick_dump/chipset.i
> +++ b/tools/quick_dump/chipset.i
> @@ -13,7 +13,7 @@ extern int is_haswell(unsigned short pciid);
> extern int is_broadwell(unsigned short pciid);
> extern int is_skylake(unsigned short pciid);
> extern struct pci_device *intel_get_pci_device();
> -extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
> +extern int intel_register_access_init(struct pci_device *pci_dev, int safe, int nofw);
> extern uint32_t intel_register_read(uint32_t reg);
> extern void intel_register_write(uint32_t reg, uint32_t val);
> extern void intel_register_access_fini();
> @@ -31,7 +31,7 @@ extern int is_haswell(unsigned short pciid);
> extern int is_broadwell(unsigned short pciid);
> extern int is_skylake(unsigned short pciid);
> extern struct pci_device *intel_get_pci_device();
> -extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
> +extern int intel_register_access_init(struct pci_device *pci_dev, int safe, int nofw);
> extern uint32_t intel_register_read(uint32_t reg);
> extern void intel_register_write(uint32_t reg, uint32_t val);
> extern void intel_register_access_fini();
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list