Re: [Intel-gfx] [PATCH 13/19] drm/i915: Make IS_BROXTON only take dev_priv
On Wed, Oct 12, 2016 at 01:06:51PM +0100, Tvrtko Ursulin wrote: > > On 12/10/2016 12:52, David Weinehall wrote: > > On Tue, Oct 11, 2016 at 02:21:46PM +0100, Tvrtko Ursulin wrote: > > > From: Tvrtko Ursulin > > > > > > Saves 1392 bytes of .rodata strings. > > > > > > v2: Add parantheses around dev_priv. (Ville Syrjala) > > > > > > Signed-off-by: Tvrtko Ursulin > > This patch does quite a bit more than just change IS_BROXTON to use > > dev_priv... > > Some cascade effects on function prototypes here and there - if you find it > objectionable I can try to eliminate or at least minimise? I don't find the changes objectionable -- they are, as you say, cascade effects. It might, however, be worth explaining in the patch description that the patch does a bit more than just more than IS_BROXTON(). Doing so for just minor differences is overkill, but in this case it feels justified. > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > > drivers/gpu/drm/i915/i915_drv.h | 5 +++-- > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 40 > > > + > > > drivers/gpu/drm/i915/i915_irq.c | 2 +- > > > drivers/gpu/drm/i915/intel_ddi.c| 4 ++-- > > > drivers/gpu/drm/i915/intel_display.c| 31 ++--- > > > drivers/gpu/drm/i915/intel_dp.c | 16 ++--- > > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 +- > > > drivers/gpu/drm/i915/intel_dsi.c| 27 +++--- > > > drivers/gpu/drm/i915/intel_dsi_pll.c| 26 ++--- > > > drivers/gpu/drm/i915/intel_guc_loader.c | 8 +++ > > > drivers/gpu/drm/i915/intel_hdmi.c | 6 ++--- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- > > > 13 files changed, 89 insertions(+), 82 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index d854ea4a7e92..18af6d1ccec9 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -2437,7 +2437,7 @@ static int intel_runtime_resume(struct device *kdev) > > > if (IS_GEN6(dev_priv)) > > > intel_init_pch_refclk(dev); > > > - if (IS_BROXTON(dev)) { > > > + if (IS_BROXTON(dev_priv)) { > > > bxt_disable_dc9(dev_priv); > > > bxt_display_core_init(dev_priv, true); > > > if (dev_priv->csr.dmc_payload && > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 9784e61400e5..ad9299196d13 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -2664,7 +2664,7 @@ struct drm_i915_cmd_table { > > > #define IS_HASWELL(dev_priv)((dev_priv)->info.is_haswell) > > > #define IS_BROADWELL(dev_priv) ((dev_priv)->info.is_broadwell) > > > #define IS_SKYLAKE(dev_priv)((dev_priv)->info.is_skylake) > > > -#define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton) > > > +#define IS_BROXTON(dev_priv) ((dev_priv)->info.is_broxton) > > > #define IS_KABYLAKE(dev_priv) ((dev_priv)->info.is_kabylake) > > > #define IS_MOBILE(dev) (INTEL_INFO(dev)->is_mobile) > > > #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ > > > @@ -2724,7 +2724,8 @@ struct drm_i915_cmd_table { > > > #define BXT_REVID_B00x3 > > > #define BXT_REVID_C00x9 > > > -#define IS_BXT_REVID(p, since, until) (IS_BROXTON(p) && IS_REVID(p, > > > since, until)) > > > +#define IS_BXT_REVID(dev_priv, since, until) \ > > > + (IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until)) > > > #define KBL_REVID_A00x0 > > > #define KBL_REVID_B00x1 > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > index cf43a5632961..e628691fe97e 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > @@ -373,27 +373,29 @@ static void *kmap_page_dma(struct i915_page_dma *p) > > > /* We use the flushing unmap only with ppgtt structures: > > >* page directories, page tables and scratch pages. > > >*/ > > > -static void kunmap_page_dma(struct drm_device *dev, void *vaddr) > > > +static void kunmap_page_dma(struct drm_i915_private *dev_priv, void > > > *vaddr) > > > { > > > /* There are only few exceptions for gen >=6. chv and bxt. > > >* And we are not sure about the latter so play safe for now. > > >*/ > > > - if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) > > > + if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv)) > > > drm_clflush_virt_range(vaddr, PAGE_SIZE); > > > kunmap_atomic(vaddr); > > > } > > > #define kmap_px(px) kmap_page_dma(px_base(px)) > > > -#define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, > > > (vaddr)) > > > +#define kunmap_px(ppgtt, vaddr) \ > > > + kunmap_page
Re: [Intel-gfx] [PATCH 13/19] drm/i915: Make IS_BROXTON only take dev_priv
On 12/10/2016 12:52, David Weinehall wrote: On Tue, Oct 11, 2016 at 02:21:46PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Saves 1392 bytes of .rodata strings. v2: Add parantheses around dev_priv. (Ville Syrjala) Signed-off-by: Tvrtko Ursulin This patch does quite a bit more than just change IS_BROXTON to use dev_priv... Some cascade effects on function prototypes here and there - if you find it objectionable I can try to eliminate or at least minimise? Regards, Tvrtko --- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 5 +++-- drivers/gpu/drm/i915/i915_gem_gtt.c | 40 + drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_ddi.c| 4 ++-- drivers/gpu/drm/i915/intel_display.c| 31 ++--- drivers/gpu/drm/i915/intel_dp.c | 16 ++--- drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 +- drivers/gpu/drm/i915/intel_dsi.c| 27 +++--- drivers/gpu/drm/i915/intel_dsi_pll.c| 26 ++--- drivers/gpu/drm/i915/intel_guc_loader.c | 8 +++ drivers/gpu/drm/i915/intel_hdmi.c | 6 ++--- drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- 13 files changed, 89 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d854ea4a7e92..18af6d1ccec9 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2437,7 +2437,7 @@ static int intel_runtime_resume(struct device *kdev) if (IS_GEN6(dev_priv)) intel_init_pch_refclk(dev); - if (IS_BROXTON(dev)) { + if (IS_BROXTON(dev_priv)) { bxt_disable_dc9(dev_priv); bxt_display_core_init(dev_priv, true); if (dev_priv->csr.dmc_payload && diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9784e61400e5..ad9299196d13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2664,7 +2664,7 @@ struct drm_i915_cmd_table { #define IS_HASWELL(dev_priv) ((dev_priv)->info.is_haswell) #define IS_BROADWELL(dev_priv)((dev_priv)->info.is_broadwell) #define IS_SKYLAKE(dev_priv) ((dev_priv)->info.is_skylake) -#define IS_BROXTON(dev)(INTEL_INFO(dev)->is_broxton) +#define IS_BROXTON(dev_priv) ((dev_priv)->info.is_broxton) #define IS_KABYLAKE(dev_priv) ((dev_priv)->info.is_kabylake) #define IS_MOBILE(dev)(INTEL_INFO(dev)->is_mobile) #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ @@ -2724,7 +2724,8 @@ struct drm_i915_cmd_table { #define BXT_REVID_B0 0x3 #define BXT_REVID_C0 0x9 -#define IS_BXT_REVID(p, since, until) (IS_BROXTON(p) && IS_REVID(p, since, until)) +#define IS_BXT_REVID(dev_priv, since, until) \ + (IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until)) #define KBL_REVID_A0 0x0 #define KBL_REVID_B0 0x1 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index cf43a5632961..e628691fe97e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -373,27 +373,29 @@ static void *kmap_page_dma(struct i915_page_dma *p) /* We use the flushing unmap only with ppgtt structures: * page directories, page tables and scratch pages. */ -static void kunmap_page_dma(struct drm_device *dev, void *vaddr) +static void kunmap_page_dma(struct drm_i915_private *dev_priv, void *vaddr) { /* There are only few exceptions for gen >=6. chv and bxt. * And we are not sure about the latter so play safe for now. */ - if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) + if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv)) drm_clflush_virt_range(vaddr, PAGE_SIZE); kunmap_atomic(vaddr); } #define kmap_px(px) kmap_page_dma(px_base(px)) -#define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, (vaddr)) +#define kunmap_px(ppgtt, vaddr) \ + kunmap_page_dma(to_i915((ppgtt)->base.dev), (vaddr)) #define setup_px(dev, px) setup_page_dma((dev), px_base(px)) #define cleanup_px(dev, px) cleanup_page_dma((dev), px_base(px)) -#define fill_px(dev, px, v) fill_page_dma((dev), px_base(px), (v)) -#define fill32_px(dev, px, v) fill_page_dma_32((dev), px_base(px), (v)) +#define fill_px(dev_priv, px, v) fill_page_dma((dev_priv), px_base(px), (v)) +#define fill32_px(dev_priv, px, v) \ + fill_page_dma_32((dev_priv), px_base(px), (v)) -static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p, - const uint64_t val) +static void fill_page_dma(struct drm_i915_private *dev_priv, + struct i915_page_dma *p, const uint64_t val) { int i; uint64_t * const vaddr = kmap_page_dma(p); @@ -401,17 +403,17 @@ static void f
Re: [Intel-gfx] [PATCH 13/19] drm/i915: Make IS_BROXTON only take dev_priv
On Tue, Oct 11, 2016 at 02:21:46PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Saves 1392 bytes of .rodata strings. > > v2: Add parantheses around dev_priv. (Ville Syrjala) > > Signed-off-by: Tvrtko Ursulin This patch does quite a bit more than just change IS_BROXTON to use dev_priv... > --- > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 5 +++-- > drivers/gpu/drm/i915/i915_gem_gtt.c | 40 > + > drivers/gpu/drm/i915/i915_irq.c | 2 +- > drivers/gpu/drm/i915/intel_ddi.c| 4 ++-- > drivers/gpu/drm/i915/intel_display.c| 31 ++--- > drivers/gpu/drm/i915/intel_dp.c | 16 ++--- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 +- > drivers/gpu/drm/i915/intel_dsi.c| 27 +++--- > drivers/gpu/drm/i915/intel_dsi_pll.c| 26 ++--- > drivers/gpu/drm/i915/intel_guc_loader.c | 8 +++ > drivers/gpu/drm/i915/intel_hdmi.c | 6 ++--- > drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- > 13 files changed, 89 insertions(+), 82 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index d854ea4a7e92..18af6d1ccec9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2437,7 +2437,7 @@ static int intel_runtime_resume(struct device *kdev) > if (IS_GEN6(dev_priv)) > intel_init_pch_refclk(dev); > > - if (IS_BROXTON(dev)) { > + if (IS_BROXTON(dev_priv)) { > bxt_disable_dc9(dev_priv); > bxt_display_core_init(dev_priv, true); > if (dev_priv->csr.dmc_payload && > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9784e61400e5..ad9299196d13 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2664,7 +2664,7 @@ struct drm_i915_cmd_table { > #define IS_HASWELL(dev_priv) ((dev_priv)->info.is_haswell) > #define IS_BROADWELL(dev_priv) ((dev_priv)->info.is_broadwell) > #define IS_SKYLAKE(dev_priv) ((dev_priv)->info.is_skylake) > -#define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton) > +#define IS_BROXTON(dev_priv) ((dev_priv)->info.is_broxton) > #define IS_KABYLAKE(dev_priv)((dev_priv)->info.is_kabylake) > #define IS_MOBILE(dev) (INTEL_INFO(dev)->is_mobile) > #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ > @@ -2724,7 +2724,8 @@ struct drm_i915_cmd_table { > #define BXT_REVID_B0 0x3 > #define BXT_REVID_C0 0x9 > > -#define IS_BXT_REVID(p, since, until) (IS_BROXTON(p) && IS_REVID(p, since, > until)) > +#define IS_BXT_REVID(dev_priv, since, until) \ > + (IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until)) > > #define KBL_REVID_A0 0x0 > #define KBL_REVID_B0 0x1 > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index cf43a5632961..e628691fe97e 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -373,27 +373,29 @@ static void *kmap_page_dma(struct i915_page_dma *p) > /* We use the flushing unmap only with ppgtt structures: > * page directories, page tables and scratch pages. > */ > -static void kunmap_page_dma(struct drm_device *dev, void *vaddr) > +static void kunmap_page_dma(struct drm_i915_private *dev_priv, void *vaddr) > { > /* There are only few exceptions for gen >=6. chv and bxt. >* And we are not sure about the latter so play safe for now. >*/ > - if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) > + if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv)) > drm_clflush_virt_range(vaddr, PAGE_SIZE); > > kunmap_atomic(vaddr); > } > > #define kmap_px(px) kmap_page_dma(px_base(px)) > -#define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, (vaddr)) > +#define kunmap_px(ppgtt, vaddr) \ > + kunmap_page_dma(to_i915((ppgtt)->base.dev), (vaddr)) > > #define setup_px(dev, px) setup_page_dma((dev), px_base(px)) > #define cleanup_px(dev, px) cleanup_page_dma((dev), px_base(px)) > -#define fill_px(dev, px, v) fill_page_dma((dev), px_base(px), (v)) > -#define fill32_px(dev, px, v) fill_page_dma_32((dev), px_base(px), (v)) > +#define fill_px(dev_priv, px, v) fill_page_dma((dev_priv), px_base(px), (v)) > +#define fill32_px(dev_priv, px, v) \ > + fill_page_dma_32((dev_priv), px_base(px), (v)) > > -static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p, > - const uint64_t val) > +static void fill_page_dma(struct drm_i915_private *dev_priv, > + struct i915_page_dma *p, const uint64_t val) > { > int i; > uint64_t * const vaddr = kmap_page_dma(p); > @@ -401,17 +403,17 @@ static void fill_page_dma(struct drm_device *dev, > struct i915_page_dma *p, >