[Intel-gfx] [PATCH 2/2] drm/i915/tgl: Move and restrict Wa_1408615072

Nick Desaulniers ndesaulniers at google.com
Thu Mar 5 19:15:38 UTC 2020


> Hello Jose,
> So this is triggering a very subtle bug in 2 separate CI systems:
> - KernelCI: https://storage.kernelci.org/next/master/next-20200305/x86_64/x86_64_defconfig/clang-9/build.log
> - our CI: https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/293826383
> 
> commit 50148a2 ("drm/i915/tgl: Move and restrict Wa_1408615072")
> 
> for Clang builds.  It seems that Clang is inlining `wa_masked_en` into
> `rcs_engine_wa_init`, which has implications for `__builtin_constant_p`
> evaluation.
> 
> The relevant call chain looks like:
> 
> rcs_engine_wa_init (drivers/gpu/drm/i915/gt/intel_workarounds.c) ->
>   wa_masked_en (drivers/gpu/drm/i915/gt/intel_workarounds.c) ->
>     _MASKED_BIT_ENABLE (drivers/gpu/drm/i915/i915_reg.h) ->
>       _MASKED_FIELD (drivers/gpu/drm/i915/i915_reg.h)
> 
> So it looks like there's build time validation that the masks and values have
> their bottom 2 bytes unset (mask & 0xffff0000, little endian).  (There doesn't
> seem to be equivalent runtime checks that the masks & values obey this
> invariant when they're not "integer constant expressions").
> 
> This will break should GCC ever decide to inline `wa_masked_en` into
> `rcs_engine_wa_init`.
> 
> VSUNIT_CLKGATE_DIS_TGL is defined as `REG_BIT(19)` which evaluates to
> 0x00080000, which would fail the check against
> 0xffff0000.
> 
> Can you please validate that VSUNIT_CLKGATE_DIS_TGL has the correct value, or
> if the invariant that the bottom two bytes of values passed to
> `_MASKED_BIT_ENABLE` must be set?

(Sorry, didn't quite finish that sentence, I meant to say):

Can you please validate that VSUNIT_CLKGATE_DIS_TGL has the correct value, or
if the invariant that the bottom two bytes of values passed to
`_MASKED_BIT_ENABLE` must not be set should still hold, otherwise please either
revert or drop your patch if you cannot resolve quickly (otherwise these two CI
systems will be red).

> 
> I would expect one of those two to be incorrect.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/918


More information about the Intel-gfx mailing list