[Intel-gfx] [RFC 2/3] drm/i915: Prefer IS_GEN<n> check with bitmask.
Rodrigo Vivi
rodrigo.vivi at intel.com
Fri Oct 26 19:40:56 UTC 2018
On Thu, Oct 25, 2018 at 12:11:57PM +0100, Julia Lawall wrote:
>
>
> On Thu, 25 Oct 2018, Ville Syrjälä wrote:
>
> > On Wed, Oct 24, 2018 at 04:41:06PM -0700, Rodrigo Vivi wrote:
> > > On Wed, Oct 24, 2018 at 01:22:57PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Oct 23, 2018 at 04:36:19PM -0700, Rodrigo Vivi wrote:
> > > > > Whenever possible we should stick with IS_GEN<n> checks.
> > > > >
> > > > > Bitmaks has been introduced on commit ae7617f0ef18 ("drm/i915:
> > > > > Allow optimized platform checks") for efficiency.
> > > > >
> > > > > Let's stick with it whenever possible.
> > > > >
> > > > > This patch was generated with coccinelle:
> > > > >
> > > > > spatch -sp_file is_gen.cocci *{c,h} --in-place
> > > > >
> > > > > is_gen.cocci:
> > > > > @gen2@ expression e; @@
> > > > > -INTEL_GEN(e) == 2
> > > > > +IS_GEN2(e)
> > > > > @gen3@ expression e; @@
> > > > > -INTEL_GEN(e) == 3
> > > > > +IS_GEN3(e)
> > > > > @gen4@ expression e; @@
> > > > > -INTEL_GEN(e) == 4
> > > > > +IS_GEN4(e)
> > > > > @gen5@ expression e; @@
> > > > > -INTEL_GEN(e) == 5
> > > > > +IS_GEN5(e)
> > > > > @gen6@ expression e; @@
> > > > > -INTEL_GEN(e) == 6
> > > > > +IS_GEN6(e)
> > > > > @gen7@ expression e; @@
> > > > > -INTEL_GEN(e) == 7
> > > > > +IS_GEN7(e)
> > > > > @gen8@ expression e; @@
> > > > > -INTEL_GEN(e) == 8
> > > > > +IS_GEN8(e)
> > > > > @gen9@ expression e; @@
> > > > > -INTEL_GEN(e) == 9
> > > > > +IS_GEN9(e)
> > > > > @gen10@ expression e; @@
> > > > > -INTEL_GEN(e) == 10
> > > > > +IS_GEN10(e)
> > > > > @gen11@ expression e; @@
> > > > > -INTEL_GEN(e) == 11
> > > > > +IS_GEN11(e)
> > > >
> > > > Slightly less repetitive version.
> > >
> > > Thanks. I'm supper newbie on coccinelle and it was
> > > easier to duplicate then trying to learn all this new lang.
> > >
> > > But I really appreciate you sharing this. I'd like
> > > to go with this so the better rule keeps on commit message.
> > >
> > > > Sadly not super neat on
> > > > account of having to use the python stuff.
> > >
> > > however it seems python version didn't work so well here:
> > >
> > > init_defs_builtins: /usr/lib64/coccinelle/standard.h
> > > Python error: No module named coccilib.elems
> > >
> > > do you have any idea about it?
> >
> > Never seen it. On gentoo it just works when I have the python
> > use flag enabled.
> >
> > Maybe your distro doesn't include the python support in the
> > build, or maybe some kind of python version issue?
>
> Thanks for the report. I will have someone look into it. It's an
> internal Coccinelle thing. Perhaps python is not enabled. Do you have
> the version of Coccinelle from github?
I just tried with coccinelle from github but got same result.
But with this commit before the bump to automake1.16:
commit 4db95998768557c66d6e337be2253032b5d810bc (HEAD -> master)
Author: Thierry Martinez <thierry.martinez at inria.fr>
Date: Wed Sep 12 14:43:41 2018 +0200
Compute depend by calling make recursilevy
The set of files in $(source_files) may change when the tarball is
unpacked.
----
$ python --version
Python 2.7.15
$ python3 --version
Python 3.6.6
----
I believe I'm moving this patch with the repetitive bad
rule for now that I'm sure it works in most of the distros.
Thanks,
Rodrigo.
>
> julia
>
> >
> > >
> > > I couldn't find anything on google or on pip repositories
> > > for that.
> > >
> > > Thanks,
> > > Rodrigo.
> > >
> > > >
> > > > @find@
> > > > identifier old =~ "^INTEL_GEN$";
> > > > expression exp;
> > > > constant gen;
> > > > @@
> > > > old(exp) == gen
> > > >
> > > > @script:python rename@
> > > > old << find.old;
> > > > gen << find.gen;
> > > > new;
> > > > @@
> > > > def do_rename(old, gen):
> > > > return "IS_GEN" + gen
> > > > coccinelle.new = cocci.make_ident(do_rename(old, gen))
> > > > print coccinelle.new
> > > >
> > > > @@
> > > > identifier find.old;
> > > > expression find.exp;
> > > > constant find.gen;
> > > > identifier rename.new;
> > > > @@
> > > > - old(exp) == gen
> > > > + new(exp)
> > > >
> > > >
> > > > I wish it would look something more like this:
> > > >
> > > > @@
> > > > identifier old =~ "^INTEL_GEN$";
> > > > expression exp;
> > > > constant gen;
> > > > fresh identifier new = "IS_GEN" ## gen;
> > > > @@
> > > > - old(exp) == gen
> > > > + new(exp)
> > > >
> > > > But coccinelle doesn't seem to accept the constant in the fresh
> > > > identifier thing.
> > > >
> > > >
> > > > Patch looks fine anyways
> > > > Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > >
> > > > >
> > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_atomic.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_device_info.c | 6 +++---
> > > > > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++--
> > > > > drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > > > > 11 files changed, 15 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > > index baac35f698f9..d755e84fed07 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > @@ -1329,7 +1329,7 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
> > > > > /* Need to calculate bandwidth only for Gen9 */
> > > > > if (IS_BROXTON(dev_priv))
> > > > > ret = bxt_get_dram_info(dev_priv);
> > > > > - else if (INTEL_GEN(dev_priv) == 9)
> > > > > + else if (IS_GEN9(dev_priv))
> > > > > ret = skl_get_dram_info(dev_priv);
> > > > > else
> > > > > ret = skl_dram_get_channels_info(dev_priv);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > index 760758ad21c1..3526f6d9c1ad 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > @@ -232,7 +232,7 @@ static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_sta
> > > > > if (plane_state && plane_state->base.fb &&
> > > > > plane_state->base.fb->format->is_yuv &&
> > > > > plane_state->base.fb->format->num_planes > 1) {
> > > > > - if (INTEL_GEN(dev_priv) == 9 &&
> > > > > + if (IS_GEN9(dev_priv) &&
> > > > > !IS_GEMINILAKE(dev_priv))
> > > > > mode = SKL_PS_SCALER_MODE_NV12;
> > > > > else
> > > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > > > > index 03df4e33763d..561ad0d4a53d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > > > @@ -744,7 +744,7 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
> > > > > if (INTEL_GEN(dev_priv) >= 10) {
> > > > > for_each_pipe(dev_priv, pipe)
> > > > > info->num_scalers[pipe] = 2;
> > > > > - } else if (INTEL_GEN(dev_priv) == 9) {
> > > > > + } else if (IS_GEN9(dev_priv)) {
> > > > > info->num_scalers[PIPE_A] = 2;
> > > > > info->num_scalers[PIPE_B] = 2;
> > > > > info->num_scalers[PIPE_C] = 1;
> > > > > @@ -843,9 +843,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
> > > > > cherryview_sseu_info_init(dev_priv);
> > > > > else if (IS_BROADWELL(dev_priv))
> > > > > broadwell_sseu_info_init(dev_priv);
> > > > > - else if (INTEL_GEN(dev_priv) == 9)
> > > > > + else if (IS_GEN9(dev_priv))
> > > > > gen9_sseu_info_init(dev_priv);
> > > > > - else if (INTEL_GEN(dev_priv) == 10)
> > > > > + else if (IS_GEN10(dev_priv))
> > > > > gen10_sseu_info_init(dev_priv);
> > > > > else if (INTEL_GEN(dev_priv) >= 11)
> > > > > gen11_sseu_info_init(dev_priv);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index fc7e3b0bd95c..a6869c547f3c 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -5268,7 +5268,7 @@ static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
> > > > > if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > > > return false;
> > > > >
> > > > > - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> > > > > + if ((IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) ||
> > > > > IS_CANNONLAKE(dev_priv))
> > > > > return true;
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 8c38efef77a1..761447c456c7 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -455,7 +455,7 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
> > > > > if (INTEL_GEN(dev_priv) >= 10) {
> > > > > source_rates = cnl_rates;
> > > > > size = ARRAY_SIZE(cnl_rates);
> > > > > - if (INTEL_GEN(dev_priv) == 10)
> > > > > + if (IS_GEN10(dev_priv))
> > > > > max_rate = cnl_max_source_rate(intel_dp);
> > > > > else
> > > > > max_rate = icl_max_source_rate(intel_dp);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > index 65f6c9bc10cf..a4b1d82c89da 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > @@ -812,7 +812,7 @@ u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv)
> > > > > u32 slice = fls(sseu->slice_mask);
> > > > > u32 subslice = fls(sseu->subslice_mask[slice]);
> > > > >
> > > > > - if (INTEL_GEN(dev_priv) == 10)
> > > > > + if (IS_GEN10(dev_priv))
> > > > > mcr_s_ss_select = GEN8_MCR_SLICE(slice) |
> > > > > GEN8_MCR_SUBSLICE(subslice);
> > > > > else if (INTEL_GEN(dev_priv) >= 11)
> > > > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > > > index c90954cdfb15..96f70d896c10 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > > > @@ -84,7 +84,7 @@ static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
> > > > > int lines;
> > > > >
> > > > > intel_fbc_get_plane_source_size(cache, NULL, &lines);
> > > > > - if (INTEL_GEN(dev_priv) == 7)
> > > > > + if (IS_GEN7(dev_priv))
> > > > > lines = min(lines, 2048);
> > > > > else if (INTEL_GEN(dev_priv) >= 8)
> > > > > lines = min(lines, 2560);
> > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > > index 67a4d0735291..3793866a9ee7 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > > @@ -4676,13 +4676,13 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> > > > > selected_result = method2;
> > > > > } else if (ddb_allocation >=
> > > > > fixed16_to_u32_round_up(wp->plane_blocks_per_line)) {
> > > > > - if (INTEL_GEN(dev_priv) == 9 &&
> > > > > + if (IS_GEN9(dev_priv) &&
> > > > > !IS_GEMINILAKE(dev_priv))
> > > > > selected_result = min_fixed16(method1, method2);
> > > > > else
> > > > > selected_result = method2;
> > > > > } else if (latency >= wp->linetime_us) {
> > > > > - if (INTEL_GEN(dev_priv) == 9 &&
> > > > > + if (IS_GEN9(dev_priv) &&
> > > > > !IS_GEMINILAKE(dev_priv))
> > > > > selected_result = min_fixed16(method1, method2);
> > > > > else
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index 423cdf84059c..bc2d88313ed0 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -574,7 +574,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > > > > if (dev_priv->psr.psr2_enabled) {
> > > > > u32 chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > > >
> > > > > - if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv))
> > > > > + if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> > > > > chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> > > > > | PSR2_ADD_VERTICAL_LINE_COUNT);
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > > > index f6ec48a75a69..d3a08d0f02fe 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > > > @@ -93,11 +93,11 @@ hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
> > > > > #define I915_MAX_SUBSLICES 8
> > > > >
> > > > > #define instdone_slice_mask(dev_priv__) \
> > > > > - (INTEL_GEN(dev_priv__) == 7 ? \
> > > > > + (IS_GEN7(dev_priv__) ? \
> > > > > 1 : INTEL_INFO(dev_priv__)->sseu.slice_mask)
> > > > >
> > > > > #define instdone_subslice_mask(dev_priv__) \
> > > > > - (INTEL_GEN(dev_priv__) == 7 ? \
> > > > > + (IS_GEN7(dev_priv__) ? \
> > > > > 1 : INTEL_INFO(dev_priv__)->sseu.subslice_mask[0])
> > > > >
> > > > > #define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
> > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > index 7cd59eee5cad..4c9f0b7138b3 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > > @@ -1798,7 +1798,7 @@ static bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> > > > > if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > > > return false;
> > > > >
> > > > > - if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv) && pipe == PIPE_C)
> > > > > + if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv) && pipe == PIPE_C)
> > > > > return false;
> > > > >
> > > > > if (plane_id != PLANE_PRIMARY && plane_id != PLANE_SPRITE0)
> > > > > --
> > > > > 2.19.1
> > > > >
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> >
> > --
> > Ville Syrjälä
> > Intel
> >
More information about the Intel-gfx
mailing list