| Message ID | CAPM=9ty+Vyn8aSxNqWY+_KEnqj8nGZbp2PRJTvQLcV1iPhG7dA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [git,pull] drm fixes for 5.7-rc8/final | expand |
On Thu, May 28, 2020 at 5:21 PM Dave Airlie <airlied@gmail.com> wrote: > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes > and minor ingenic fixes. Dave, this doesn't even build. WTF? In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a engine_heartbeat_disable() function that takes two arguments, but the new "live_timeslice_nopreempt()" gives it only one. I'd blame a merge problem, since the failure is in new code, but the problem exists in your top-of-tree, not just my merge. In fact, it's not even your merge of the i915 tree that is the source of the problem (although the fact that you clearly didn't _test_ the end result most definitely is _part_ of the problem!). Because the problem exists in the commit that introduced that thing: commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into unpreemptable requests"). It's garbage, and never compiled. See here: git grep -1wh engine_heartbeat_disable 1f65efb62 \ drivers/gpu/drm/i915/gt/selftest_lrc.c and you'll see how the definition of that function looks like this: static void engine_heartbeat_disable(struct intel_engine_cs *engine, unsigned long *saved) but then in the middle of that grep, you'll find engine_heartbeat_disable(engine); WTF? That commit seems to be a cherry-pick of another commit, and maybe it worked in that other context (which I don't see), but it sure doesn't work in the context it was then cherry-picked into. So people took that thing, and it went through at least two different people WHO NEVER EVEN BOTHERED TO TEST IF IT BUILDS! Christ, people. This is why I absolutely DO NOT WANT TO SEE random rebases or cherry-picks and then sending the resulting untested crap on to me. Because it's exactly that: untested crap. It doesn't matter *how* well you have tested a commit in some original context: the moment you rebase it (or cherry-pick it, which is just another form of rebasing), it's a completely new commit in a completely new environment, and all your old testing is null and void. Guys, get your act together! I should not be getting these kinds of shit pull requests days before a release! And how the hell did this not get any build testing at any point before asking me to pull? Linus
On Fri, 29 May 2020 at 11:49, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Thu, May 28, 2020 at 5:21 PM Dave Airlie <airlied@gmail.com> wrote: > > > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes > > and minor ingenic fixes. > > Dave, this doesn't even build. WTF? > > In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a > engine_heartbeat_disable() function that takes two arguments, but the > new "live_timeslice_nopreempt()" gives it only one. > > I'd blame a merge problem, since the failure is in new code, but the > problem exists in your top-of-tree, not just my merge. > > In fact, it's not even your merge of the i915 tree that is the source > of the problem (although the fact that you clearly didn't _test_ the > end result most definitely is _part_ of the problem!). > > Because the problem exists in the commit that introduced that thing: > commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into > unpreemptable requests"). > > It's garbage, and never compiled. I thought I'd dropped the ball completely. but I see it's a selftest failure, I must not have selftests built in my config here, I don't do exhaustive builds randconfig This has also been built by Intel, but I'm assuming they missed their selftest bits as well. I'll revert that and resend. Sorry for the noise. I'll add self tests to my builds here for future ones. Dave.
On Fri, 29 May 2020 at 12:02, Dave Airlie <airlied@gmail.com> wrote: > > On Fri, 29 May 2020 at 11:49, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Thu, May 28, 2020 at 5:21 PM Dave Airlie <airlied@gmail.com> wrote: > > > > > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes > > > and minor ingenic fixes. > > > > Dave, this doesn't even build. WTF? > > > > In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a > > engine_heartbeat_disable() function that takes two arguments, but the > > new "live_timeslice_nopreempt()" gives it only one. > > > > I'd blame a merge problem, since the failure is in new code, but the > > problem exists in your top-of-tree, not just my merge. > > > > In fact, it's not even your merge of the i915 tree that is the source > > of the problem (although the fact that you clearly didn't _test_ the > > end result most definitely is _part_ of the problem!). > > > > Because the problem exists in the commit that introduced that thing: > > commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into > > unpreemptable requests"). > > > > It's garbage, and never compiled. > > I thought I'd dropped the ball completely. but I see it's a selftest > failure, I must not have selftests built in my config here, I don't do > exhaustive builds randconfig > > This has also been built by Intel, but I'm assuming they missed their > selftest bits as well. > > I'll revert that and resend. I did drop the ball in one way, I see sfr reported it broken this morning I normally expect stuff coming from Intel has been through their CI, even their fixes tree generally gets pushed through that system before I get it, and it usually catches these things. I might have to push back on intel fixes this late in the day, as maybe the land on next and cherry-pick to fixes model has made them a bit lax on how much stuff goes through CI. I've just drop the whole i915 fixes from the tree and will resend with it removed. Dave.
On Fri, May 29, 2020 at 12:15:27PM +1000, Dave Airlie wrote: > On Fri, 29 May 2020 at 12:02, Dave Airlie <airlied@gmail.com> wrote: > > > > On Fri, 29 May 2020 at 11:49, Linus Torvalds > > <torvalds@linux-foundation.org> wrote: > > > > > > On Thu, May 28, 2020 at 5:21 PM Dave Airlie <airlied@gmail.com> wrote: > > > > > > > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes > > > > and minor ingenic fixes. > > > > > > Dave, this doesn't even build. WTF? > > > > > > In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a > > > engine_heartbeat_disable() function that takes two arguments, but the > > > new "live_timeslice_nopreempt()" gives it only one. > > > > > > I'd blame a merge problem, since the failure is in new code, but the > > > problem exists in your top-of-tree, not just my merge. > > > > > > In fact, it's not even your merge of the i915 tree that is the source > > > of the problem (although the fact that you clearly didn't _test_ the > > > end result most definitely is _part_ of the problem!). > > > > > > Because the problem exists in the commit that introduced that thing: > > > commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into > > > unpreemptable requests"). > > > > > > It's garbage, and never compiled. > > > > I thought I'd dropped the ball completely. but I see it's a selftest > > failure, I must not have selftests built in my config here, I don't do > > exhaustive builds randconfig > > > > This has also been built by Intel, but I'm assuming they missed their > > selftest bits as well. > > > > I'll revert that and resend. > > I did drop the ball in one way, I see sfr reported it broken this morning > > I normally expect stuff coming from Intel has been through their CI, > even their fixes tree generally gets pushed through that system before > I get it, and it usually catches these things. > > I might have to push back on intel fixes this late in the day, as > maybe the land on next and cherry-pick to fixes model has made them a > bit lax on how much stuff goes through CI. > > I've just drop the whole i915 fixes from the tree and will resend with > it removed. Yes, that was my fault. I also didn't have the selftest on my drm-intel-fixes build and I confused the build run numbers when checking the https://intel-gfx-ci.01.org/tree/drm-intel-fixes/index.html? before sending... If we have another round next week I will make sure CI runs well before sending another pull request. Sorry, Rodrigo. > > Dave. > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hey Linus, Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes and minor ingenic fixes. Should be it until the merge window. Dave. drm-fixes-2020-05-29: drm fixes for 5.7 final i915: - gcc 9 compile warning fix - timeslicing fixes amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-05-29 for you to fetch changes up to d099f415d50c3980339479f56f124f8bfa6875bc: Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2020-05-29 09:25:23 +1000) ---------------------------------------------------------------- drm fixes for 5.7 final i915: - gcc 9 compile warning fix - timeslicing fixes amdgpu: - display atomic test fix - Fix soft hang in display vupdate code ingenic: - fix pointer cast - fix crtc atomic check callback ---------------------------------------------------------------- Aric Cyr (1): drm/amd/display: Fix potential integer wraparound resulting in a hang Arnd Bergmann (2): drm/i915/pmu: avoid an maybe-uninitialized warning drm/i915: work around false-positive maybe-uninitialized warning Chris Wilson (2): drm/i915/gt: Incorporate the virtual engine into timeslicing drm/i915/gt: Prevent timeslicing into unpreemptable requests Dave Airlie (3): Merge tag 'amd-drm-fixes-5.7-2020-05-27' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'drm-misc-fixes-2020-05-28' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Paul Cercueil (2): gpu/drm: ingenic: Fix bogus crtc_atomic_check callback gpu/drm: Ingenic: Fix opaque pointer casted to wrong type Simon Ser (1): drm/amd/display: drop cursor position check in atomic test drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 -- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 + drivers/gpu/drm/i915/gt/intel_lrc.c | 31 ++++-- drivers/gpu/drm/i915/gt/selftest_lrc.c | 118 ++++++++++++++++++++- drivers/gpu/drm/i915/gt/selftest_workarounds.c | 2 + drivers/gpu/drm/i915/i915_pmu.c | 84 +++++++-------- drivers/gpu/drm/i915/i915_priolist_types.h | 2 +- drivers/gpu/drm/ingenic/ingenic-drm.c | 6 +- 8 files changed, 192 insertions(+), 60 deletions(-)