Message ID | 20190409003729.20857-1-paulo.r.zanoni@intel.com (mailing list archive) |
---|---|
Headers | show |
Series | IRQ initialization debloat and conversion to uncore | expand |
Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu: > == Series Details == > > Series: IRQ initialization debloat and conversion to uncore > URL : https://patchwork.freedesktop.org/series/59202/ > State : warning > > == Summary == > > $ dim checkpatch origin/drm-tip > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > +#define GEN8_IRQ_RESET_NDX(type, which) \ > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > + imr_val, ier_val) > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \ > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \ > GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \ > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \ > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > imr_val, ier_val) The whiches are not really a regression, but OK we can deal with them to make the robots happy. > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')' > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228: > + GEN2_IRQ_RESET(uncore, ); > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252: > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > ^ > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')' > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397: > + GEN3_IRQ_RESET(uncore, ); > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430: > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > ^ > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')' > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508: > + GEN3_IRQ_RESET(uncore, ); > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552: > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); For these ones I really think the spaces help. I would love to read some opinions. Perhaps some comment like /* paste token here */ would help make the code more readable and could help silence checkpatch. Opinions? > ^ > > total: 6 errors, 0 warnings, 2 checks, 432 lines checked > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote: > Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu: > > == Series Details == > > > > Series: IRQ initialization debloat and conversion to uncore > > URL : https://patchwork.freedesktop.org/series/59202/ > > State : warning > > > > == Summary == > > > > $ dim checkpatch origin/drm-tip > > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros > > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > +#define GEN8_IRQ_RESET_NDX(type, which) \ > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > + imr_val, ier_val) > > > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked > > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore > > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore > > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \ > > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \ > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \ > > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \ > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > imr_val, ier_val) > > The whiches are not really a regression, but OK we can deal with them > to make the robots happy. > > > > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')' > > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228: > > + GEN2_IRQ_RESET(uncore, ); > > > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252: > > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > ^ > > > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')' > > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397: > > + GEN3_IRQ_RESET(uncore, ); > > > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430: > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > ^ > > > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')' > > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508: > > + GEN3_IRQ_RESET(uncore, ); > > > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552: > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > For these ones I really think the spaces help. I would love to read > some opinions. Perhaps some comment like /* paste token here */ would > help make the code more readable and could help silence checkpatch. > Opinions? Or maybe rename the registers to eg. I9XX_IIR? > > > ^ > > > > total: 6 errors, 0 warnings, 2 checks, 432 lines checked > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em ter, 2019-04-09 às 21:20 +0300, Ville Syrjälä escreveu: > On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote: > > Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu: > > > == Series Details == > > > > > > Series: IRQ initialization debloat and conversion to uncore > > > URL : https://patchwork.freedesktop.org/series/59202/ > > > State : warning > > > > > > == Summary == > > > > > > $ dim checkpatch origin/drm-tip > > > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros > > > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > > +#define GEN8_IRQ_RESET_NDX(type, which) \ > > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > > + imr_val, ier_val) > > > > > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked > > > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore > > > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore > > > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \ > > > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \ > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \ > > > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \ > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > > imr_val, ier_val) > > > > The whiches are not really a regression, but OK we can deal with them > > to make the robots happy. > > > > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228: > > > + GEN2_IRQ_RESET(uncore, ); > > > > > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252: > > > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > ^ > > > > > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397: > > > + GEN3_IRQ_RESET(uncore, ); > > > > > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430: > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > ^ > > > > > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508: > > > + GEN3_IRQ_RESET(uncore, ); > > > > > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552: > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > > For these ones I really think the spaces help. I would love to read > > some opinions. Perhaps some comment like /* paste token here */ would > > help make the code more readable and could help silence checkpatch. > > Opinions? > > Or maybe rename the registers to eg. I9XX_IIR? That makes more sense. We use these regs on gen2 too, so I suppose I8XX_IIR (or GEN2_IIR) would make more sense. OTOH it would break our current naming rule. I'll submit v2 soon. Thanks. > > > > ^ > > > > > > total: 6 errors, 0 warnings, 2 checks, 432 lines checked > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Wed, Apr 10, 2019 at 03:53:05PM -0700, Paulo Zanoni wrote: > Em ter, 2019-04-09 às 21:20 +0300, Ville Syrjälä escreveu: > > On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote: > > > Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu: > > > > == Series Details == > > > > > > > > Series: IRQ initialization debloat and conversion to uncore > > > > URL : https://patchwork.freedesktop.org/series/59202/ > > > > State : warning > > > > > > > > == Summary == > > > > > > > > $ dim checkpatch origin/drm-tip > > > > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros > > > > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > > > +#define GEN8_IRQ_RESET_NDX(type, which) \ > > > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > > > > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > > > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > > > + imr_val, ier_val) > > > > > > > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked > > > > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore > > > > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore > > > > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169: > > > > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \ > > > > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \ > > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > > > > > > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects? > > > > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236: > > > > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \ > > > > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \ > > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > > > imr_val, ier_val) > > > > > > The whiches are not really a regression, but OK we can deal with them > > > to make the robots happy. > > > > > > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228: > > > > + GEN2_IRQ_RESET(uncore, ); > > > > > > > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252: > > > > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > > ^ > > > > > > > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397: > > > > + GEN3_IRQ_RESET(uncore, ); > > > > > > > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430: > > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > > ^ > > > > > > > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')' > > > > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508: > > > > + GEN3_IRQ_RESET(uncore, ); > > > > > > > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW) > > > > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552: > > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask); > > > > > > For these ones I really think the spaces help. I would love to read > > > some opinions. Perhaps some comment like /* paste token here */ would > > > help make the code more readable and could help silence checkpatch. > > > Opinions? > > > > Or maybe rename the registers to eg. I9XX_IIR? > > That makes more sense. We use these regs on gen2 too, so I suppose > I8XX_IIR (or GEN2_IIR) would make more sense. OTOH it would break our > current naming rule. I tend to use i9xx to indicate anything gmch, and sometimes it even means pre-skl :/ Not the best naming scheme perhaps but I've not been able to come up with anything better.