On Wed, May 25, 2016 at 08:57:49AM +0200, Maarten Lankhorst wrote: > Op 24-05-16 om 17:49 schreef Daniel Vetter: > > This reverts the following patches: > > > > d55dbd06bb5e1399aba9ab5227465339d1bbefff drm/i915: Allow nonblocking update of pageflips. > > 15c86bdb760185e871c7a0f559978328aa500971 drm/i915: Check for unpin correctness. > > 95c2ccdc82d520f59ae3b6fdc097b63c9b7082bb Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates" > > a6747b7304a9d66758a196d885dab8bbfa5e7d1f drm/i915: Make unpin async. > > 03f476e1fcb42fca88fc50b94b0d3adbdbe887f0 drm/i915: Prepare connectors for nonblocking checks. > > 2099deffef4404f949ba1b68d2b17e0608190bc2 drm/i915: Pass atomic states to fbc update functions. > > ee7171af72c39c18b7d7571419a4ac6ca30aea66 drm/i915: Remove reset_counter from intel_crtc. > > 2ee004f7c59b2e642f0bb2834f847d756f2dd7b7 drm/i915: Remove queue_flip pointer. > > b8d2afae557dbb9b9c7bc6f6ec4f5278f3c4c34e drm/i915: Remove use_mmio_flip kernel parameter. > > 8dd634d922615ec3a9af7976029110ec037f8b50 drm/i915: Remove cs based page flip support. > > 143f73b3bf48c089b40f58462dd7f7c199fd4f0f drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3. > > 84fc494b64e8c591be446a966b7447a9db519c88 drm/i915: Add the exclusive fence to plane_state. > > 6885843ae164e11f6c802209d06921e678a3f3f3 drm/i915: Convert flip_work to a list. > > aa420ddd8eeaa5df579894a412289e4d07c2fee9 drm/i915: Allow mmio updates on all platforms, v2. > > afee4d8707ab1f21b7668de995be3a5961e83582 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates" > > > > "drm/i915: Allow nonblocking update of pageflips" should have been > > split up, misses a proper commit message and seems to cause issues in > > the legacy page_flip path as demonstrated by kms_flip. > > > > "drm/i915: Make unpin async" doesn't handle the unthrottled cursor > > updates correctly, leading to an apparent pin count leak. This is > > caught by the WARN_ON in i915_gem_object_do_pin which screams if we > > have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins. > > > > Unfortuantely we can't just revert these two because this patch series > > came with a built-in bisect breakage in the form of temporarily > > removing the unthrottled cursor update hack for legacy cursor ioctl. > > Therefore there's no other option than to revert the entire pile :( > > > > There's one tiny conflict in intel_drv.h due to other patches, nothing > > serious. > > > > Normally I'd wait a bit longer with doing a maintainer revert, but > > since the minimal set of patches we need to revert (due to the bisect > > breakage) is so big, time is running out fast. And very soon > > (especially after a few attempts at fixing issues) it'll be really > > hard to revert things cleanly. > > > > Lessons learned: > > - Not a good idea to rush the review (done by someone fairly new to > > the area) and not make sure domain experts had a chance to read it. > > > > - Patches should be properly split up. I only looked at the two > > patches that should be reverted in detail, but both look like the > > mix up different things in one patch. > > > > - Patches really should have proper commit messages. Especially when > > doing more than one thing, and especially when touching critical and > > tricky core code. > > > > - Building a patch series and r-b stamping it when it has a built-in > > bisect breakage is not a good idea. > > > > - I also think we need to stop building up technical debt by > > postponing atomic igt testcases even longer. I think it's clear that > > there's enough corner cases in this beast that we really need to > > have the testcases _before_ the next step lands. > Acked-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> Ok applied&pushed with irc acks from Dave, Jani&Ville added. > It should be possible to make avoid stalling work without much issue after reworking intel_crtc_page_flip to be almost atomic, > but it would be better to edit the mmio updates on all platforms patch for lesson #4.. Yeah, this entire story would have been much less tricky without that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
