[PATCH 1/3] tools/intel_reg: Use pci device from config on write

[PATCH 1/3] tools/intel_reg: Use pci device from config on write

[Mika Kuoppala]

Use the pre configured pci device from config also
in write path.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tools/intel_reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 190aa5b..2b60a83 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -439,7 +439,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
-	intel_register_access_init(intel_get_pci_device(), 0);
+	intel_register_access_init(config->pci_dev, 0);
 
 	for (i = 1; i < argc; i += 2) {
 		struct reg reg;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] lib: Optionally initialize mmio register access without forcewake

[Mika Kuoppala]

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@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);
 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] lib: Optionally initialize mmio register access without forcewake

[Jani Nikula]

On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] lib: Optionally initialize mmio register access without forcewake

[Ville Syrjälä]

On Fri, Aug 28, 2015 at 05:51:46PM +0300, Mika Kuoppala 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@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
<snip>  
> @@ -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();

IGT_NO_FORCEWAKE is handled in igt_open_forcewake_handle(). Maybe pull
it out so that all ways to disable forcewake are handled in the same
place?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] tools/intel_reg: Add option to skip forcewake

[Mika Kuoppala]

Sometimes we want to inspect the hardware as it is,
without doing the forcewake before access. Add option to skip
the forcewake dance.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tools/intel_reg.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index b082555..c73b9ef 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -70,12 +70,16 @@ struct config {
 	/* spread out bits for convenience */
 	bool binary;
 
+	/* dont take forcewake on register access */
+	int nofw;
+
 	/* register spec */
 	char *specfile;
 	struct reg *regs;
 	ssize_t regcount;
 
 	int verbosity;
+
 };
 
 /* port desc must have been set */
@@ -409,7 +413,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, 0);
+		intel_register_access_init(config->pci_dev, 0, config->nofw);
 
 	for (i = 1; i < argc; i++) {
 		struct reg reg;
@@ -439,7 +443,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
-	intel_register_access_init(config->pci_dev, 0, 0);
+	intel_register_access_init(config->pci_dev, 0, config->nofw);
 
 	for (i = 1; i < argc; i += 2) {
 		struct reg reg;
@@ -477,7 +481,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, 0);
+		intel_register_access_init(config->pci_dev, 0, config->nofw);
 
 	for (i = 0; i < config->regcount; i++) {
 		reg = &config->regs[i];
@@ -637,6 +641,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 	printf(" --all          Decode registers for all known platforms\n");
 	printf(" --binary       Binary dump registers\n");
 	printf(" --verbose      Increase verbosity\n");
+	printf(" --nofw         No forcewake on register access (DANGEROUS)\n");
 	printf(" --quiet        Reduce verbosity\n");
 
 	printf("\n");
@@ -769,6 +774,7 @@ enum opt {
 	OPT_BINARY,
 	OPT_SPEC,
 	OPT_VERBOSE,
+	OPT_NOFW,
 	OPT_QUIET,
 	OPT_HELP,
 };
@@ -781,6 +787,7 @@ int main(int argc, char *argv[])
 	const struct command *command = NULL;
 	struct config config = {
 		.count = 1,
+		.nofw = 0,
 	};
 	bool help = false;
 
@@ -800,6 +807,7 @@ int main(int argc, char *argv[])
 		/* options specific to read, dump and decode */
 		{ "all",	no_argument,		NULL,	OPT_ALL },
 		{ "binary",	no_argument,		NULL,	OPT_BINARY },
+		{ "nofw",	no_argument,		NULL,	OPT_NOFW },
 		{ 0 }
 	};
 
@@ -846,6 +854,9 @@ int main(int argc, char *argv[])
 		case OPT_BINARY:
 			config.binary = true;
 			break;
+		case OPT_NOFW:
+			config.nofw = 1;
+			break;
 		case OPT_VERBOSE:
 			config.verbosity++;
 			break;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] tools/intel_reg: Add option to skip forcewake

[Jani Nikula]

On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Sometimes we want to inspect the hardware as it is,
> without doing the forcewake before access. Add option to skip
> the forcewake dance.

This needs a manual page update.

Some trivial comments below, otherwise lgtm.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  tools/intel_reg.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index b082555..c73b9ef 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -70,12 +70,16 @@ struct config {
>  	/* spread out bits for convenience */
>  	bool binary;
>  
> +	/* dont take forcewake on register access */
> +	int nofw;

IMO should be a bool.


> +
>  	/* register spec */
>  	char *specfile;
>  	struct reg *regs;
>  	ssize_t regcount;
>  
>  	int verbosity;
> +

Superfluous whitespace change.

>  };
>  
>  /* port desc must have been set */
> @@ -409,7 +413,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, 0);
> +		intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 1; i < argc; i++) {
>  		struct reg reg;
> @@ -439,7 +443,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
>  		return EXIT_FAILURE;
>  	}
>  
> -	intel_register_access_init(config->pci_dev, 0, 0);
> +	intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 1; i < argc; i += 2) {
>  		struct reg reg;
> @@ -477,7 +481,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, 0);
> +		intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 0; i < config->regcount; i++) {
>  		reg = &config->regs[i];
> @@ -637,6 +641,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>  	printf(" --all          Decode registers for all known platforms\n");
>  	printf(" --binary       Binary dump registers\n");
>  	printf(" --verbose      Increase verbosity\n");
> +	printf(" --nofw         No forcewake on register access (DANGEROUS)\n");
>  	printf(" --quiet        Reduce verbosity\n");
>  
>  	printf("\n");
> @@ -769,6 +774,7 @@ enum opt {
>  	OPT_BINARY,
>  	OPT_SPEC,
>  	OPT_VERBOSE,
> +	OPT_NOFW,
>  	OPT_QUIET,
>  	OPT_HELP,
>  };
> @@ -781,6 +787,7 @@ int main(int argc, char *argv[])
>  	const struct command *command = NULL;
>  	struct config config = {
>  		.count = 1,
> +		.nofw = 0,

No need to initialize to zero.

>  	};
>  	bool help = false;
>  
> @@ -800,6 +807,7 @@ int main(int argc, char *argv[])
>  		/* options specific to read, dump and decode */
>  		{ "all",	no_argument,		NULL,	OPT_ALL },
>  		{ "binary",	no_argument,		NULL,	OPT_BINARY },
> +		{ "nofw",	no_argument,		NULL,	OPT_NOFW },
>  		{ 0 }
>  	};
>  
> @@ -846,6 +854,9 @@ int main(int argc, char *argv[])
>  		case OPT_BINARY:
>  			config.binary = true;
>  			break;
> +		case OPT_NOFW:
> +			config.nofw = 1;

= true;

> +			break;
>  		case OPT_VERBOSE:
>  			config.verbosity++;
>  			break;
> -- 
> 2.1.4
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] tools/intel_reg: Use pci device from config on write

[Jani Nikula]

On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Use the pre configured pci device from config also
> in write path.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  tools/intel_reg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 190aa5b..2b60a83 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -439,7 +439,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
>  		return EXIT_FAILURE;
>  	}
>  
> -	intel_register_access_init(intel_get_pci_device(), 0);
> +	intel_register_access_init(config->pci_dev, 0);

Whoops, my bad.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  
>  	for (i = 1; i < argc; i += 2) {
>  		struct reg reg;
> -- 
> 2.1.4
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] tools/intel_reg: Use pci device from config on write

[Thomas Wood]

On 28 August 2015 at 16:16, Jani Nikula <jani.nikula@intel.com> wrote:
> On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
>> Use the pre configured pci device from config also
>> in write path.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>>  tools/intel_reg.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
>> index 190aa5b..2b60a83 100644
>> --- a/tools/intel_reg.c
>> +++ b/tools/intel_reg.c
>> @@ -439,7 +439,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
>>               return EXIT_FAILURE;
>>       }
>>
>> -     intel_register_access_init(intel_get_pci_device(), 0);
>> +     intel_register_access_init(config->pci_dev, 0);
>
> Whoops, my bad.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Thanks, I've pushed this patch. The other two patches in this series
can be pushed once any of the review comments have been addressed.


>
>
>>
>>       for (i = 1; i < argc; i += 2) {
>>               struct reg reg;
>> --
>> 2.1.4
>>
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx