On Tue, Mar 23, 2021 at 07:40:58PM +0200, Ville Syrjälä wrote: > On Tue, Mar 23, 2021 at 10:27:34AM -0700, Matt Roper wrote: > > On Tue, Mar 23, 2021 at 07:25:57PM +0200, Ville Syrjälä wrote: > > > On Mon, Mar 22, 2021 at 04:38:40PM -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) > > > > | > > > > - (IS_DISPLAY_VER(dev_priv, 10) || IS_GEMINILAKE(dev_priv)) > > > > + IS_DISPLAY_VER(dev_priv, 10) > > > > | > > > > - (IS_GEMINILAKE(dev_priv) || IS_DISPLAY_VER(dev_priv, 10)) > > > > + IS_DISPLAY_VER(dev_priv, 10) > > > > ) > > > > > > > > @@ expression dev_priv; @@ > > > > - (IS_DISPLAY_VER(dev_priv, 9) && !IS_GEMINILAKE(dev_priv)) > > > > + IS_DISPLAY_VER(dev_priv, 9) > > > > > > > > @@ expression dev_priv; @@ > > > > ( > > > > - !(DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10)) > > > > + DISPLAY_VER(dev_priv) < 10 > > > > | > > > > - (DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10)) > > > > + DISPLAY_VER(dev_priv) >= 10 > > > > ) > > > > > > > > @@ expression dev_priv, E; @@ > > > > - E || DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10) > > > > + E || DISPLAY_VER(dev_priv) >= 10 > > > > > > > > @@ expression dev_priv, E; @@ > > > > - (IS_DISPLAY_RANGE(dev_priv, 11, E) || IS_DISPLAY_VER(dev_priv, 10)) > > > > + IS_DISPLAY_RANGE(dev_priv, 10, E) > > > > > > > > @@ expression dev_priv; @@ > > > > ( > > > > - DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv) || IS_GEN9_LP(dev_priv) > > > > + DISPLAY_VER(dev_priv) >= 10 || IS_GEN9_LP(dev_priv) > > > > | > > > > - IS_GEN9_LP(dev_priv) || DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv) > > > > + IS_GEN9_LP(dev_priv) || DISPLAY_VER(dev_priv) >= 10 > > > > ) > > > > > > > > @@ expression dev_priv, E; @@ > > > > - !(DISPLAY_VER(dev_priv) >= E) > > > > + DISPLAY_VER(dev_priv) < E > > > > > > > > v2: > > > > - Convert gen10 conditions that don't include GLK into CNL conditions. > > > > (Ville) > > > > > > > > v3: > > > > - Rework coccinelle rules so that "ver>=10" turns into "ver>=11||is_cnl." (Ville) > > > > > > > > v3.1: > > > > - Manually re-add the ".display.version = 10" to glk_info after > > > > regenerating patch via Coccinelle. > > > > > > > > v4: > > > > - Also apply cocci rules to intel_pm.c and i915_irq.c! (CI) > > > > > > Ugh. One thing that occurred to me when looking at i915_irq.c is that > > > IS_GEN9_LP() is now maybe broken on glk? So seems to me all uses of > > > IS_GEN9_LP() need to be reviewed and potentially changed. > > > > Broken how? That macro still uses the gen/gt version instead of the > > display number, so I think it still behaves the same as before? > > Oh you're not changng it to to use display ver? I guess it still kinda > works then. But it's going to be pretty confusing to use that for > display stuff now. Ie. we should probably stop using it. That macro is used throughout the driver (including non-display code), so I don't think we'd want to re-define it. But you're right that we probably should eliminate the places where it gets used in the display code (since the graphics gen shouldn't matter); I'll do that as a follow-up series. Matt > > -- > Ville Syrjälä > Intel -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
