Re: [Intel-gfx] [PATCH v2.1 6/6] drm/i915/display: Simplify GLK display version tests

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	"Ville Syrjälä" <ville.syrjala@intel.com>
Subject: Re: [Intel-gfx] [PATCH v2.1 6/6] drm/i915/display: Simplify GLK display version tests
Date: Mon, 22 Mar 2021 23:34:37 +0200	[thread overview]
Message-ID: <YFkNbfTKXnvnpLrW@intel.com> (raw)
In-Reply-To: <20210322212512.GR3422723@mdroper-desk1.amr.corp.intel.com>

On Mon, Mar 22, 2021 at 02:25:12PM -0700, Matt Roper wrote:
> On Mon, Mar 22, 2021 at 11:16:28PM +0200, Ville Syrjälä wrote:
> > On Mon, Mar 22, 2021 at 12:50:17PM -0700, Matt Roper wrote:
> > > GLK has always been a bit of a special case since it reports INTEL_GEN()
> > > as 9, but has version 10 display IP.  Now we can properly represent the
> > > display version as 10 and simplify the display generation tests
> > > throughout the display code.
> > > 
> > > Aside from manually adding the version to the glk_info structure, the
> > > rest of this patch is generated with a Coccinelle semantic patch.  Note
> > > that we also need to switch any code that matches gen10 today but *not*
> > > GLK to be CNL-specific:
> > > 
> > >         @@ expression dev_priv; @@
> > >         - DISPLAY_VER(dev_priv) > 9
> > >         + DISPLAY_VER(dev_priv) >= 10
> > > 
> > >         @@ expression dev_priv, E; @@
> > >         (
> > >         - DISPLAY_VER(dev_priv) >= 10 && E
> > >         + (DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv)) && E
> > >         |
> > >         - DISPLAY_VER(dev_priv) >= 10
> > >         + DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv)
> > >         |
> > >         - IS_DISPLAY_RANGE(dev_priv, 10, E)
> > >         + IS_DISPLAY_RANGE(dev_priv, 11, E) || IS_CANNONLAKE(dev_priv)
> > >         )
> > > 
> > >         @@ expression dev_priv, E, E2; @@
> > >         (
> > >         - (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > >         + IS_DISPLAY_VER(dev_priv, 10)
> > >         |
> > >         - E || IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)
> > >         + E || IS_DISPLAY_VER(dev_priv, 10)
> > >         |
> > >         - (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv))
> > >         + IS_DISPLAY_VER(dev_priv, 10)
> > >         |
> > >         - IS_GEMINILAKE(dev_priv) || E || IS_CANNONLAKE(dev_priv)
> > >         + E || IS_DISPLAY_VER(dev_priv, 10)
> > >         |
> > >         - E || IS_GEMINILAKE(dev_priv) || E2 || IS_CANNONLAKE(dev_priv)
> > >         + E || E2 || IS_DISPLAY_VER(dev_priv, 10)
> > 
> > Sometimes I really wish cocci would have a way to say "these things can
> > go in any order" :/
> 
> Coccinelle has support for user-defined isomorphisms that I think are
> supposed to be able to do this.  I tried to create some isomorphisms
> like:
> 
>         Expression
>         @ dv_no_sideeffects @
>         expression dev_priv;
>         int i;
>         binary operator OP;
>         @@
>         DISPLAY_VER(dev_priv) OP i || E <=> E || DISPLAY_VER(dev_priv) OP i
> 
> for the various IS_* and DISPLAY_VER() macros so that it would allow
> them in any order (since we know these macros have no side effects), but
> I must have been doing it wrong, or passing the iso file to coccinelle
> incorrectly, since they didn't seem to be working.

Ah, thanks for pointing this out. Need to give that a try
at some point.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx