mbox series

[v3,0/2] drm/i915: CTS fixes

Message ID 20190625105832.19064-1-lionel.g.landwerlin@intel.com (mailing list archive)
Headers show
Series drm/i915: CTS fixes | expand

Message

Lionel Landwerlin June 25, 2019, 10:58 a.m. UTC 
From experimentation writing/reading those out those registers seems
to be read-only. Contrary to the description in the documentation...

So whitelist them as read only.

Cheers,

Lionel Landwerlin (2):
  drm/i915: whitelist PS_(DEPTH|INVOCATION)_COUNT
  drm/i915/icl: whitelist PS_(DEPTH|INVOCATION)_COUNT

 drivers/gpu/drm/i915/gt/intel_workarounds.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

--
2.21.0.392.gf8f6787159e

Comments

Lionel Landwerlin June 25, 2019, 1:09 p.m. UTC | #1 
Anybody knows whether the selftests are dealing with read only 
whitelisted registers?
I'm not quite sure what can be tested with those (unless you can 
exercise the 3d pipeline in this case).

-Lionel

On 25/06/2019 15:57, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: CTS fixes (rev3)
> URL   : https://patchwork.freedesktop.org/series/62437/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_6342 -> Patchwork_13413
> ====================================================
>
> Summary
> -------
>
>    **FAILURE**
>
>    Serious unknown changes coming with Patchwork_13413 absolutely need to be
>    verified manually.
>    
>    If you think the reported changes have nothing to do with the changes
>    introduced in Patchwork_13413, please notify your bug team to allow them
>    to document this new failure mode, which will reduce false positives in CI.
>
>    External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/
>
> Possible new issues
> -------------------
>
>    Here are the unknown changes that may have been introduced in Patchwork_13413:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
>    * igt@i915_selftest@live_workarounds:
>      - fi-cfl-guc:         [PASS][1] -> [DMESG-FAIL][2]
>     [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cfl-guc/igt@i915_selftest@live_workarounds.html
>     [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cfl-guc/igt@i915_selftest@live_workarounds.html
>      - fi-cml-u:           [PASS][3] -> [DMESG-FAIL][4]
>     [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cml-u/igt@i915_selftest@live_workarounds.html
>     [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cml-u/igt@i915_selftest@live_workarounds.html
>      - fi-cfl-8109u:       [PASS][5] -> [DMESG-FAIL][6]
>     [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cfl-8109u/igt@i915_selftest@live_workarounds.html
>     [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cfl-8109u/igt@i915_selftest@live_workarounds.html
>      - fi-whl-u:           [PASS][7] -> [DMESG-FAIL][8]
>     [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-whl-u/igt@i915_selftest@live_workarounds.html
>     [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-whl-u/igt@i915_selftest@live_workarounds.html
>      - fi-icl-dsi:         [PASS][9] -> [DMESG-FAIL][10]
>     [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-dsi/igt@i915_selftest@live_workarounds.html
>     [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-dsi/igt@i915_selftest@live_workarounds.html
>      - fi-icl-u3:          [PASS][11] -> [DMESG-FAIL][12]
>     [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-u3/igt@i915_selftest@live_workarounds.html
>     [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-u3/igt@i915_selftest@live_workarounds.html
>      - fi-cfl-8700k:       [PASS][13] -> [DMESG-FAIL][14]
>     [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cfl-8700k/igt@i915_selftest@live_workarounds.html
>     [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cfl-8700k/igt@i915_selftest@live_workarounds.html
>      - fi-icl-u2:          [PASS][15] -> [DMESG-FAIL][16]
>     [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-u2/igt@i915_selftest@live_workarounds.html
>     [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-u2/igt@i915_selftest@live_workarounds.html
>      - fi-cml-u2:          [PASS][17] -> [DMESG-FAIL][18]
>     [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cml-u2/igt@i915_selftest@live_workarounds.html
>     [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cml-u2/igt@i915_selftest@live_workarounds.html
>
>    
> Known issues
> ------------
>
>    Here are the changes found in Patchwork_13413 that come from known issues:
>
> ### IGT changes ###
>
> #### Issues hit ####
>
>    * igt@gem_exec_suspend@basic-s4-devices:
>      - fi-icl-u3:          [PASS][19] -> [DMESG-WARN][20] ([fdo#107724])
>     [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html
>     [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html
>
>    * igt@i915_pm_rpm@module-reload:
>      - fi-skl-6770hq:      [PASS][21] -> [FAIL][22] ([fdo#108511])
>     [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>     [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>
>    * igt@kms_chamelium@dp-crc-fast:
>      - fi-cml-u2:          [PASS][23] -> [FAIL][24] ([fdo#110627])
>     [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
>     [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
>
>    * igt@kms_frontbuffer_tracking@basic:
>      - fi-icl-u2:          [PASS][25] -> [FAIL][26] ([fdo#103167])
>     [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
>     [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
>
>    
> #### Possible fixes ####
>
>    * igt@i915_selftest@live_blt:
>      - fi-skl-iommu:       [INCOMPLETE][27] ([fdo#108602]) -> [PASS][28]
>     [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-skl-iommu/igt@i915_selftest@live_blt.html
>     [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-skl-iommu/igt@i915_selftest@live_blt.html
>
>    * igt@i915_selftest@live_sanitycheck:
>      - fi-icl-u3:          [DMESG-WARN][29] ([fdo#107724]) -> [PASS][30] +1 similar issue
>     [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6342/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html
>     [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html
>
>    
>    [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>    [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
>    [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
>    [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
>    [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
>
>
> Participating hosts (53 -> 45)
> ------------------------------
>
>    Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-icl-y fi-byt-clapper fi-bdw-samus
>
>
> Build changes
> -------------
>
>    * Linux: CI_DRM_6342 -> Patchwork_13413
>
>    CI_DRM_6342: 6eef272b254b34200129af8f2ec1e4cfe1ca6bff @ git://anongit.freedesktop.org/gfx-ci/linux
>    IGT_5067: 5eafa33dbdb1d3c190ac5060161c45152e9a298e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>    Patchwork_13413: 5e4d6c80907ce04521f6a3ea58b7eca2dd8193bc @ git://anongit.freedesktop.org/gfx-ci/linux
>
>
> == Linux commits ==
>
> 5e4d6c80907c drm/i915/icl: whitelist PS_(DEPTH|INVOCATION)_COUNT
> 73c970b28af1 drm/i915: whitelist PS_(DEPTH|INVOCATION)_COUNT
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13413/
>
Chris Wilson June 25, 2019, 1:14 p.m. UTC | #2 
Quoting Lionel Landwerlin (2019-06-25 14:09:57)
> Anybody knows whether the selftests are dealing with read only 
> whitelisted registers?
> I'm not quite sure what can be tested with those (unless you can 
> exercise the 3d pipeline in this case).

John added a hack:

static bool ro_register(u32 reg)
{
        if (reg & RING_FORCE_TO_NONPRIV_RD)
                return true;

        return false;
}

to ignore them. It is not clear why that did not take.
-Chris
Chris Wilson June 25, 2019, 1:16 p.m. UTC | #3 
Quoting Lionel Landwerlin (2019-06-25 14:09:57)
> I'm not quite sure what can be tested with those (unless you can 
> exercise the 3d pipeline in this case).

Sure, it's just setting up the GPU with a full 3D state after all ;)

Making sure that CI does include a functional test for these would be
useful. I thought we have piglit on icl (or maybe we don't have enough
icl for that yet?), but do we need crucible as well?
-Chris
Lionel Landwerlin June 25, 2019, 1:19 p.m. UTC | #4 
On 25/06/2019 16:16, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2019-06-25 14:09:57)
>> I'm not quite sure what can be tested with those (unless you can
>> exercise the 3d pipeline in this case).
> Sure, it's just setting up the GPU with a full 3D state after all ;)
>
> Making sure that CI does include a functional test for these would be
> useful. I thought we have piglit on icl (or maybe we don't have enough
> icl for that yet?), but do we need crucible as well?
> -Chris
>
There is one piglit test that catches this and quite a few Vulkan CTS tests.

I don't think we have anything in crucible.

Probably not run in CI yet.


-Lionel
Lionel Landwerlin June 25, 2019, 1:24 p.m. UTC | #5 
On 25/06/2019 16:14, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2019-06-25 14:09:57)
>> Anybody knows whether the selftests are dealing with read only
>> whitelisted registers?
>> I'm not quite sure what can be tested with those (unless you can
>> exercise the 3d pipeline in this case).
> John added a hack:
>
> static bool ro_register(u32 reg)
> {
>          if (reg & RING_FORCE_TO_NONPRIV_RD)
>                  return true;
>
>          return false;
> }
>
> to ignore them. It is not clear why that did not take.
> -Chris
>
Oh...

That seems broken for a read only register and a default value of 0 :


static bool result_neq(struct intel_engine_cs *engine,
                        u32 a, u32 b, i915_reg_t reg)
{
         if (a == b && !writeonly_reg(engine->i915, reg)) {
                 pr_err("Whitelist register 0x%4x:%08x was unwritable\n",
                        i915_mmio_reg_offset(reg), a);
                 return false;
         }

         return true;
}
Chris Wilson June 25, 2019, 1:28 p.m. UTC | #6 
Quoting Lionel Landwerlin (2019-06-25 14:24:42)
> On 25/06/2019 16:14, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2019-06-25 14:09:57)
> >> Anybody knows whether the selftests are dealing with read only
> >> whitelisted registers?
> >> I'm not quite sure what can be tested with those (unless you can
> >> exercise the 3d pipeline in this case).
> > John added a hack:
> >
> > static bool ro_register(u32 reg)
> > {
> >          if (reg & RING_FORCE_TO_NONPRIV_RD)
> >                  return true;
> >
> >          return false;
> > }
> >
> > to ignore them. It is not clear why that did not take.
> > -Chris
> >
> Oh...
> 
> That seems broken for a read only register and a default value of 0 :

True. Tvrtko proved right that he should have held out for an actual
selftest rather than letting it through with a hack.
-Chris