[Intel-gfx] [PATCH i-g-t v2 03/14] lib/igt_kms: Rework pipe properties to be more atomic, v7.

[Maarten Lankhorst]

Op 06-03-18 om 14:41 schreef Daniel Vetter:
> On Mon, Mar 05, 2018 at 03:37:30PM +0100, Maxime Ripard wrote:
>> Hi,
>>
>> On Thu, Oct 12, 2017 at 01:54:24PM +0200, Maarten Lankhorst wrote:
>>> In the future I want to allow tests to commit more properties,
>>> but for this to work I have to fix all properties to work better
>>> with atomic commit. Instead of special casing each
>>> property make a bitmask for all property changed flags, and try to
>>> commit all properties.
>>>
>>> This has been the most involved one, since legacy pipe commit still
>>> handles a lot of the properties differently from the rest.
>>>
>>> Changes since v1:
>>> - Dump all changed properties on commit.
>>> - Fix bug in igt_pipe_refresh().
>>> Changes since v2:
>>> - Set pipe ACTIVE property changed flag on init.
>>> Changes since v3:
>>> - Add a missing igt_pipe_refresh() to kms_atomic_interruptible.
>>> Changes since v4:
>>> - Perform error handling when setting custom crtc properties.
>>> Changes since v5:
>>> - Only attempt to commit changes properties.
>>> Changes since v6:
>>> - Clear OUT_FENCE_PTR on succesful commit.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> I'm a bit late to the party on this one, but this commit broke the
>> chamelium tests on vc4, with every kernel since at least 4.12.
>>
>> This is the error message:
>> http://code.bulix.org/32fw1l-293842
>>
>> From the stack trace, it looks like the atomic commit was failing, and
>> indeed it fails here:
>> https://elixir.bootlin.com/linux/v4.16-rc4/source/drivers/gpu/drm/drm_atomic.c#L2319
>>
>> with prop_id being 0 for some reason.
>>
>> I had a look at that patch, but I can't see anything wrong with it. Do
>> you have any ideas?
> No idea tbh, I guess we need to start tracing where the igt library tries
> to set property 0. Would probably be really good to catch that in the
> libdrm atomic support (same with trying to set a prop on obj 0, neither
> makes any sense at all).
> -Daniel

Sorry, I cc'd the original posters on it but already have a fix:

https://patchwork.freedesktop.org/patch/208058/