| Message ID | 1355855487-20424-6-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote: > The iomapping of the register region has historically been a uint32_t > for the obvious reason that our PTE size was always 4b. In the future > however, we cannot make this assumption. > > By making the type void, it makes the upcoming pointer math we will do > much easier, and hopefully gives the compiler opportunities to warn us > when we do stupid things. > > v2: Cast to __iomem, caught by Ville > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 49f465a..90dfbd5 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -800,7 +800,7 @@ typedef struct drm_i915_private { > unsigned long stolen_base; /* limited to low memory (32-bit) */ > > /** "Graphics Stolen Memory" holds the global PTEs */ > - uint32_t __iomem *gsm; > + void __iomem *gsm; > > struct io_mapping *gtt_mapping; > phys_addr_t gtt_base_addr; > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index b4c1e34..a52e784 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev) > return; > > > - pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); > + pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); This cast is still missing __iomem. > for (i = 0; i < ppgtt->num_pd_entries; i++) { > dma_addr_t pt_addr; > > @@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj, > struct scatterlist *sg = st->sgl; > const int first_entry = obj->gtt_space->start >> PAGE_SHIFT; > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; > - gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry; > + gtt_pte_t __iomem *gtt_entries = > + (gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry; > int unused, i = 0; > unsigned int len, m = 0; > dma_addr_t addr; > -- > 1.8.0.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 19, 2012 at 01:45:23PM +0200, Ville Syrjälä wrote: > On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote: > > The iomapping of the register region has historically been a uint32_t > > for the obvious reason that our PTE size was always 4b. In the future > > however, we cannot make this assumption. > > > > By making the type void, it makes the upcoming pointer math we will do > > much easier, and hopefully gives the compiler opportunities to warn us > > when we do stupid things. > > > > v2: Cast to __iomem, caught by Ville > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++-- > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 49f465a..90dfbd5 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -800,7 +800,7 @@ typedef struct drm_i915_private { > > unsigned long stolen_base; /* limited to low memory (32-bit) */ > > > > /** "Graphics Stolen Memory" holds the global PTEs */ > > - uint32_t __iomem *gsm; > > + void __iomem *gsm; > > > > struct io_mapping *gtt_mapping; > > phys_addr_t gtt_base_addr; > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index b4c1e34..a52e784 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev) > > return; > > > > > > - pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); > > + pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); > > This cast is still missing __iomem. That looks to be correct. But it wasn't introduced by me, right? Ie. separate patch to fix it before this one? > > > for (i = 0; i < ppgtt->num_pd_entries; i++) { > > dma_addr_t pt_addr; > > > > @@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj, > > struct scatterlist *sg = st->sgl; > > const int first_entry = obj->gtt_space->start >> PAGE_SHIFT; > > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; > > - gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry; > > + gtt_pte_t __iomem *gtt_entries = > > + (gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry; > > int unused, i = 0; > > unsigned int len, m = 0; > > dma_addr_t addr; > > -- > > 1.8.0.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Wed, Dec 19, 2012 at 11:20:15AM -0800, Ben Widawsky wrote: > On Wed, Dec 19, 2012 at 01:45:23PM +0200, Ville Syrjälä wrote: > > On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote: > > > The iomapping of the register region has historically been a uint32_t > > > for the obvious reason that our PTE size was always 4b. In the future > > > however, we cannot make this assumption. > > > > > > By making the type void, it makes the upcoming pointer math we will do > > > much easier, and hopefully gives the compiler opportunities to warn us > > > when we do stupid things. > > > > > > v2: Cast to __iomem, caught by Ville > > > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++-- > > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 49f465a..90dfbd5 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -800,7 +800,7 @@ typedef struct drm_i915_private { > > > unsigned long stolen_base; /* limited to low memory (32-bit) */ > > > > > > /** "Graphics Stolen Memory" holds the global PTEs */ > > > - uint32_t __iomem *gsm; > > > + void __iomem *gsm; > > > > > > struct io_mapping *gtt_mapping; > > > phys_addr_t gtt_base_addr; > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > index b4c1e34..a52e784 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev) > > > return; > > > > > > > > > - pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); > > > + pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); > > > > This cast is still missing __iomem. > > That looks to be correct. But it wasn't introduced by me, right? Ie. > separate patch to fix it before this one? Very much, and right in this patch - pd_addr and mm.gsm are both __iomem, so you need to keep that annotation in the cast from void* to gtt_pte_t *. Fixed up and the remaining three patches applied. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 49f465a..90dfbd5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -800,7 +800,7 @@ typedef struct drm_i915_private { unsigned long stolen_base; /* limited to low memory (32-bit) */ /** "Graphics Stolen Memory" holds the global PTEs */ - uint32_t __iomem *gsm; + void __iomem *gsm; struct io_mapping *gtt_mapping; phys_addr_t gtt_base_addr; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b4c1e34..a52e784 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev) return; - pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); + pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t); for (i = 0; i < ppgtt->num_pd_entries; i++) { dma_addr_t pt_addr; @@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj, struct scatterlist *sg = st->sgl; const int first_entry = obj->gtt_space->start >> PAGE_SHIFT; const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; - gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry; + gtt_pte_t __iomem *gtt_entries = + (gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry; int unused, i = 0; unsigned int len, m = 0; dma_addr_t addr;
The iomapping of the register region has historically been a uint32_t for the obvious reason that our PTE size was always 4b. In the future however, we cannot make this assumption. By making the type void, it makes the upcoming pointer math we will do much easier, and hopefully gives the compiler opportunities to warn us when we do stupid things. v2: Cast to __iomem, caught by Ville Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)