[Intel-gfx] [PATCH v3 4/4] drm: Resurrect atomic rmfb code, v3

[Maarten Lankhorst]

Op 16-02-17 om 10:45 schreef Jani Nikula:
> On Wed, 15 Feb 2017, Sinclair Yeh <syeh at vmware.com> wrote:
>> On Wed, Feb 15, 2017 at 03:56:09PM +0200, Jani Nikula wrote:
>>> On Wed, 25 Jan 2017, Maarten Lankhorst <maarten.lankhorst at linux.intel.com> wrote:
>>>> This was somehow lost between v3 and the merged version in Maarten's
>>>> patch merged as:
>>>>
>>>> commit f2d580b9a8149735cbc4b59c4a8df60173658140
>>>> Author: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>> Date:   Wed May 4 14:38:26 2016 +0200
>>>>
>>>>     drm/core: Do not preserve framebuffer on rmfb, v4.
>>>>
>>>> This introduces a slight behavioral change to rmfb. Instead of
>>>> disabling a crtc when the primary plane is disabled, we try to
>>>> preserve it.
>>>>
>>>> Apart from old versions of the vmwgfx xorg driver, there is
>>>> nothing depending on rmfb disabling a crtc. Since vmwgfx is
>>>> a legacy driver we can safely only disable the plane with atomic.
>>>>
>>>> If this commit is rejected by the driver then we will still fall
>>>> back to the old behavior and turn off the crtc.
>>>>
>>>> v2:
>>>> - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
>>>> - Add WARN_ON when atomic_remove_fb fails.
>>>> - Always call drm_atomic_state_put.
>>>> v3:
>>>> - Use drm_drv_uses_atomic_modeset
>>>> - Handle the case where the first plane-disable-only commit fails
>>>>   with -EINVAL. Some drivers do not support this, fall back to
>>>>   disabling all crtc's in this case.
>>>>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> Pushed to drm-misc-next-fixes, and sent the pull request to Dave. Thanks
>>> for the patch.
>> I verified yesterday that this patch will cause a regression for vmwgfx
>> multi-mon.  Can we hold off on this?
> Turns out it fails some of our tests too. Maybe three weeks old CI
> results are not to be trusted, the base moves too fast. *shrug*.
>
> I've dropped the patch, and asked Dave not to pull. Let's go back to the
> drawing board...
Yeah it's unfortunate. We tend to trigger a lot of bugs in other parts of the code with this change. The reload tests are fixed by fixing drm_atomic_helper_disable_all.
I haven't seen the crc bugs before, would be interesting to know why other tests a're suddenly failing.

~Maarten