From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 07/10] intel-gpu-tools: register range handling for forcewake hooks Date: Wed, 13 Jul 2011 22:15:42 +0100 Message-ID: References: <1310590312-21669-1-git-send-email-ben@bwidawsk.net> <1310590312-21669-8-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 36163A08BE for ; Wed, 13 Jul 2011 14:15:45 -0700 (PDT) In-Reply-To: <1310590312-21669-8-git-send-email-ben@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org Cc: Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Wed, 13 Jul 2011 13:51:49 -0700, Ben Widawsky 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 > --- > 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 > + * Ben Widawsky > * > */ > > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -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