On Wed, 13 Jul 2011 13:51:49 -0700, Ben Widawsky <ben at bwidawsk.net> wrote:
Non-text part: multipart/mixed
> We can deprecate the old code by using the non-safe flag in the new API.
> The safe flag should allow the previous behavior to continue.
>
> The code also adds some range checking on register access. This code is
> gives hooks to prevent tools from doing bad things.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> lib/Makefile.am | 1 +
> lib/intel_chipset.h | 8 ++
> lib/intel_gpu_tools.h | 27 ++++++++
> lib/intel_mmio.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/intel_reg_map.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 388 insertions(+), 0 deletions(-)
>
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 0c9380d..4612cd5 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -8,6 +8,7 @@ libintel_tools_la_SOURCES = \
> intel_mmio.c \
> intel_pci.c \
> intel_reg.h \
> + intel_reg_map.c \
> instdone.c \
> instdone.h \
> drmtest.h
> diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
> index c3db3ab..a38f661 100755
> --- a/lib/intel_chipset.h
> +++ b/lib/intel_chipset.h
> @@ -168,3 +168,11 @@
>
> #define HAS_BLT_RING(devid) (IS_GEN6(devid) || \
> IS_GEN7(devid))
> +
> +#define IS_BROADWATER(devid) (devid == PCI_CHIP_I946_GZ || \
> + devid == PCI_CHIP_I965_G_1 || \
> + devid == PCI_CHIP_I965_Q || \
> + devid == PCI_CHIP_I965_G)
> +
> +#define IS_CRESTLINE(devid) (devid == PCI_CHIP_I965_GM || \
> + devid == PCI_CHIP_I965_GME)
> diff --git a/lib/intel_gpu_tools.h b/lib/intel_gpu_tools.h
> index acee657..a145fb9 100644
> --- a/lib/intel_gpu_tools.h
> +++ b/lib/intel_gpu_tools.h
> @@ -37,6 +37,33 @@
> extern void *mmio;
> void intel_get_mmio(struct pci_device *pci_dev);
>
> +/* New style register access API */
> +int intel_register_access_init(struct pci_device *pci_dev, int safe);
> +void intel_register_access_fini(void);
> +uint32_t intel_register_read(uint32_t reg);
> +void intel_register_write(uint32_t reg, uint32_t val);
> +
> +#define INTEL_RANGE_RSVD (0<<0) /* Shouldn't be read or written */
> +#define INTEL_RANGE_READ (1<<0)
> +#define INTEL_RANGE_WRITE (1<<1)
> +#define INTEL_RANGE_RW (INTEL_RANGE_READ | INTEL_RANGE_WRITE)
> +#define INTEL_RANGE_END (1<<31)
> +
> +struct intel_register_range {
> + uint32_t base;
> + uint32_t size;
> + uint32_t flags;
> +};
> +
> +struct intel_register_map {
> + struct intel_register_range *map;
> + uint32_t top;
> + uint32_t alignment_mask;
> +};
> +struct intel_register_map intel_get_register_map(uint32_t devid);
> +struct intel_register_range *intel_get_register_range(struct intel_register_map map, uint32_t offset, int mode);
> +
> +
> static inline uint32_t
> INREG(uint32_t reg)
> {
> diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c
> index 0228a87..05cde4f 100644
> --- a/lib/intel_mmio.c
> +++ b/lib/intel_mmio.c
> @@ -22,12 +22,15 @@
> *
> * Authors:
> * Eric Anholt <eric at anholt.net>
> + * Ben Widawsky <ben at bwidawsk.net>
> *
> */
>
> #include <unistd.h>
> #include <stdlib.h>
> #include <stdio.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> #include <string.h>
> #include <errno.h>
> #include <err.h>
> @@ -41,6 +44,16 @@
>
> void *mmio;
>
> +static struct _mmio_data {
> + int inited;
> + bool safe;
> + char debugfs_path[FILENAME_MAX];
> + char debugfs_forcewake_path[FILENAME_MAX];
> + uint32_t i915_devid;
> + struct intel_register_map map;
> + int key;
> +} mmio_data;
> +
> void
> intel_map_file(char *file)
> {
> @@ -89,3 +102,168 @@ intel_get_mmio(struct pci_device *pci_dev)
> }
> }
>
> +/*
> + * If successful, i915_debugfs_path and i915_debugfs_forcewake_path are both
> + * updated with the correct path.
> + */
> +static int
> +find_debugfs_path(char *dri_base)
> +{
> + int i;
> + char buf[FILENAME_MAX];
> + char name[256];
> + char pciid[256];
> + FILE *file;
> +
> + for (i = 0; i < 16; i++) {
> + int n;
> + snprintf(buf, FILENAME_MAX, "%s/%i/name", dri_base, i);
> +
> + file = fopen(buf, "r");
> + if (file == NULL)
> + continue;
> +
> + /*
> + *if (master->unique) {
> + * seq_printf(m, "%s %s %s\n",
> + * bus_name,
> + * dev_name(dev->dev), master->unique);
> + *} else {
> + * seq_printf(m, "%s %s\n",
> + * bus_name, dev_name(dev->dev));
> + *}
> + */
> + n = fscanf(file, "%s %s ", name, pciid);
> + fclose (file);
> +
> + if (n != 2)
> + continue;
> +
> + if (strncmp("0000:00:02.0", pciid, strlen("0000:00:02.0")))
> + continue;
We'd like to maintain the option of someday handling more than one
adaptor. We have the pciid, that should be enough to identify it as a
supported Intel gfx chipset?
> + snprintf(mmio_data.debugfs_path, FILENAME_MAX,
> + "%s/%i/", dri_base, i);
> + snprintf(mmio_data.debugfs_forcewake_path, FILENAME_MAX,
> + "%s/%i/i915_forcewake_user", dri_base, i);
> +
And this file won't even exist otherwise... You need a stat here to
confirm that it does.
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +/* force wake locking is stubbed out until accepted upstream */
> +static int
> +get_forcewake_lock(void)
> +{
> + return open(mmio_data.debugfs_forcewake_path, 0);
> +}
> +
> +static void
> +release_forcewake_lock(int fd)
> +{
> + close(fd);
> +}
> +
> +/*
> + * Initialize register access library.
> + *
> + * @pci_dev: pci device we're mucking with
> + * @safe: use safe register access tables
> + */
> +int
> +intel_register_access_init(struct pci_device *pci_dev, int safe)
> +{
> + int ret;
> +
> + /* after old API is deprecated, remove this */
> + if (mmio == NULL)
> + intel_get_mmio(pci_dev);
> +
> + assert(mmio != NULL);
> +
> + if (mmio_data.inited)
> + return -1;
> +
> + mmio_data.safe = safe != 0 ? true : false;
> +
> + /* Find where the forcewake lock is */
> + ret = find_debugfs_path("/sys/kernel/debug/dri");
Need to try /debug afterwards, just because that's the other convention.
> + if (ret) {
> + fprintf(stderr, "Couldn't find path to dri/debugfs entry\n");
> + return ret;
> + }
> +
> + mmio_data.i915_devid = pci_dev->device_id;
> + if (mmio_data.safe)
> + mmio_data.map = intel_get_register_map(mmio_data.i915_devid);
> +
> + mmio_data.key = get_forcewake_lock();
> + mmio_data.inited++;
> + return 0;
> +}
> +
> +void
> +intel_register_access_fini(void)
> +{
> + release_forcewake_lock(mmio_data.key);
> + mmio_data.inited--;
> +}
> +
> +uint32_t
> +intel_register_read(uint32_t reg)
> +{
> + struct intel_register_range *range;
> + uint32_t ret;
> +
> + assert(mmio_data.inited);
> +
> + if (IS_GEN6(mmio_data.i915_devid))
> + assert(mmio_data.key != -1);
> +
> + if (!mmio_data.safe)
> + goto read_out;
> +
> + range = intel_get_register_range(mmio_data.map,
> + reg,
> + INTEL_RANGE_READ);
> +
> + if(!range) {
if (!range) {
> +struct intel_register_map
> +intel_get_register_map(uint32_t devid)
> +{
> + struct intel_register_map map;
> +
> + if (IS_GEN6(devid)) {
> + map.map = gen6_gt_register_map;
> + map.top = 0x180000;
> + } else if (IS_BROADWATER(devid) || IS_CRESTLINE(devid)) {
> + map.map = gen_bwcl_register_map;
> + map.top = 0x80000;
> + } else if (IS_GEN4(devid) || IS_GEN5(devid)) {
> + map.map = gen4_register_map;
> + map.top = 0x80000;
> + } else {
> + abort();
Give us some warning that this function will die horribly on gen2/3!
-Chris
--
Chris Wilson, Intel Open Source Technology Centre