Re: [Intel-gfx] [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
On Thu, Oct 22, 2015 at 03:32:11PM +0200, Daniel Vetter wrote: > On Thu, Oct 22, 2015 at 03:34:56PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > There's no need for __raw_i915_read8() & co. bot be macros, so make them > > s/bot/be Actually s/bot/to/ :) > > > inline funcitons. To avoid typo mistakes generate the inline functions > > s/funcitons/functions/ > > > using preprocessor templates. > > > > We have a few users of the raw register acces functions outside > > intel_uncore.c, so let's also move the functions into intel_drv.h. > > > > While doing that switch I915_READ_FW() & co. to use the > > __raw_i915_read() functions, and use the _FW macros everywhere > > outside intel_uncore.c where we want to read registers without > > grabbing forcewake and whatnot. The only exception is > > i915_check_vgpu() which itself gets called from intel_uncore.c, > > so using the __raw_i915_read stuff there seems appropriate. > > > > v2: Squash in the intel_uncore.c->i915_drv.h move > > Convert I915_READ_FW() to use __raw_i915_read(), and use > > I915_READ_FW() outside of intel_uncore.c (Chris) > > > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Daniel Vetter > > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 30 -- > > drivers/gpu/drm/i915/i915_irq.c | 10 -- > > drivers/gpu/drm/i915/i915_vgpu.c| 6 +++--- > > drivers/gpu/drm/i915/intel_uncore.c | 14 +- > > 5 files changed, 37 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index eca94d0..89ba549 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m) > > seq_printf(m, "RC information accurate: %s\n", yesno(count < > > 51)); > > } > > > > - gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS); > > + gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS); > > trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true); > > > > rpmodectl1 = I915_READ(GEN6_RP_CONTROL); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 9a15ebe..01fdc70 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private > > *dev_priv, int val); > > #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) > > #define POSTING_READ16(reg)(void)I915_READ16_NOTRACE(reg) > > > > +#define __raw_read(x, s) \ > > +static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private > > *dev_priv, \ > > +uint32_t reg) \ > > +{ \ > > + return read##s(dev_priv->regs + reg); \ > > +} > > + > > +#define __raw_write(x, s) \ > > +static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \ > > + uint32_t reg, uint##x##_t val) \ > > +{ \ > > + write##s(val, dev_priv->regs + reg); \ > > +} > > +__raw_read(8, b) > > +__raw_read(16, w) > > +__raw_read(32, l) > > +__raw_read(64, q) > > + > > +__raw_write(8, b) > > +__raw_write(16, w) > > +__raw_write(32, l) > > +__raw_write(64, q) > > + > > +#undef __raw_read > > +#undef __raw_write > > + > > /* These are untraced mmio-accessors that are only valid to be used inside > > * criticial sections inside IRQ handlers where forcewake is explicitly > > * controlled. > > @@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private > > *dev_priv, int val); > > * Note: Should only be used between intel_uncore_forcewake_irqlock() and > > * intel_uncore_forcewake_irqunlock(). > > */ > > -#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__)) > > -#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__)) > > +#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__)) > > +#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), > > (val__)) > > #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__) > > > > /* "Broadcast RGB" property */ > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 9caf22c..a0ed58d 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device > > *dev, unsigned int pipe) > > return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); > > } > > > > -/* raw reads, only for fast reads of display block, no need for forcewake > > etc. */ > > -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + > > (reg__)) > > - > > +/* I915_READ_FW, only for fast reads of display block, no need for > > forcewake etc. */ > > static int __intel_get_crtc_scanline(struct intel_crtc *crtc) > >
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
On Thu, Oct 22, 2015 at 03:34:56PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > There's no need for __raw_i915_read8() & co. bot be macros, so make them s/bot/be > inline funcitons. To avoid typo mistakes generate the inline functions s/funcitons/functions/ > using preprocessor templates. > > We have a few users of the raw register acces functions outside > intel_uncore.c, so let's also move the functions into intel_drv.h. > > While doing that switch I915_READ_FW() & co. to use the > __raw_i915_read() functions, and use the _FW macros everywhere > outside intel_uncore.c where we want to read registers without > grabbing forcewake and whatnot. The only exception is > i915_check_vgpu() which itself gets called from intel_uncore.c, > so using the __raw_i915_read stuff there seems appropriate. > > v2: Squash in the intel_uncore.c->i915_drv.h move > Convert I915_READ_FW() to use __raw_i915_read(), and use > I915_READ_FW() outside of intel_uncore.c (Chris) > > Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 30 -- > drivers/gpu/drm/i915/i915_irq.c | 10 -- > drivers/gpu/drm/i915/i915_vgpu.c| 6 +++--- > drivers/gpu/drm/i915/intel_uncore.c | 14 +- > 5 files changed, 37 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index eca94d0..89ba549 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m) > seq_printf(m, "RC information accurate: %s\n", yesno(count < > 51)); > } > > - gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS); > + gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS); > trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true); > > rpmodectl1 = I915_READ(GEN6_RP_CONTROL); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9a15ebe..01fdc70 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private > *dev_priv, int val); > #define POSTING_READ(reg)(void)I915_READ_NOTRACE(reg) > #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) > > +#define __raw_read(x, s) \ > +static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private > *dev_priv, \ > + uint32_t reg) \ > +{ \ > + return read##s(dev_priv->regs + reg); \ > +} > + > +#define __raw_write(x, s) \ > +static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \ > +uint32_t reg, uint##x##_t val) \ > +{ \ > + write##s(val, dev_priv->regs + reg); \ > +} > +__raw_read(8, b) > +__raw_read(16, w) > +__raw_read(32, l) > +__raw_read(64, q) > + > +__raw_write(8, b) > +__raw_write(16, w) > +__raw_write(32, l) > +__raw_write(64, q) > + > +#undef __raw_read > +#undef __raw_write > + > /* These are untraced mmio-accessors that are only valid to be used inside > * criticial sections inside IRQ handlers where forcewake is explicitly > * controlled. > @@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private > *dev_priv, int val); > * Note: Should only be used between intel_uncore_forcewake_irqlock() and > * intel_uncore_forcewake_irqunlock(). > */ > -#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__)) > -#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__)) > +#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__)) > +#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), > (val__)) > #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__) > > /* "Broadcast RGB" property */ > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 9caf22c..a0ed58d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device *dev, > unsigned int pipe) > return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); > } > > -/* raw reads, only for fast reads of display block, no need for forcewake > etc. */ > -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + > (reg__)) > - > +/* I915_READ_FW, only for fast reads of display block, no need for forcewake > etc. */ > static int __intel_get_crtc_scanline(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -733,9 +731,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc > *crtc) > vtotal /= 2; > > if (IS_GEN2(dev)) > - position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & > DSL_LINEMASK_GEN2; > + position = I915_REA
[Intel-gfx] [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
From: Ville Syrjälä There's no need for __raw_i915_read8() & co. bot be macros, so make them inline funcitons. To avoid typo mistakes generate the inline functions using preprocessor templates. We have a few users of the raw register acces functions outside intel_uncore.c, so let's also move the functions into intel_drv.h. While doing that switch I915_READ_FW() & co. to use the __raw_i915_read() functions, and use the _FW macros everywhere outside intel_uncore.c where we want to read registers without grabbing forcewake and whatnot. The only exception is i915_check_vgpu() which itself gets called from intel_uncore.c, so using the __raw_i915_read stuff there seems appropriate. v2: Squash in the intel_uncore.c->i915_drv.h move Convert I915_READ_FW() to use __raw_i915_read(), and use I915_READ_FW() outside of intel_uncore.c (Chris) Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 30 -- drivers/gpu/drm/i915/i915_irq.c | 10 -- drivers/gpu/drm/i915/i915_vgpu.c| 6 +++--- drivers/gpu/drm/i915/intel_uncore.c | 14 +- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index eca94d0..89ba549 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m) seq_printf(m, "RC information accurate: %s\n", yesno(count < 51)); } - gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS); + gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS); trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true); rpmodectl1 = I915_READ(GEN6_RP_CONTROL); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9a15ebe..01fdc70 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) #define POSTING_READ16(reg)(void)I915_READ16_NOTRACE(reg) +#define __raw_read(x, s) \ +static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \ +uint32_t reg) \ +{ \ + return read##s(dev_priv->regs + reg); \ +} + +#define __raw_write(x, s) \ +static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \ + uint32_t reg, uint##x##_t val) \ +{ \ + write##s(val, dev_priv->regs + reg); \ +} +__raw_read(8, b) +__raw_read(16, w) +__raw_read(32, l) +__raw_read(64, q) + +__raw_write(8, b) +__raw_write(16, w) +__raw_write(32, l) +__raw_write(64, q) + +#undef __raw_read +#undef __raw_write + /* These are untraced mmio-accessors that are only valid to be used inside * criticial sections inside IRQ handlers where forcewake is explicitly * controlled. @@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); * Note: Should only be used between intel_uncore_forcewake_irqlock() and * intel_uncore_forcewake_irqunlock(). */ -#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__)) -#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__)) +#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__)) +#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__)) #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__) /* "Broadcast RGB" property */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9caf22c..a0ed58d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe) return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); } -/* raw reads, only for fast reads of display block, no need for forcewake etc. */ -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__)) - +/* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -733,9 +731,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) vtotal /= 2; if (IS_GEN2(dev)) - position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN2; + position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2; else - position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3; + position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3; /* * On HSW, the DSL reg (0x7) appears to return 0 if we @@ -827,7 +825,7 @@ static int i915_get_crtc_scanoutp