Re: [PATCH i-g-t] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()

Daniel Stone
2016-01-20T15:08:21Z

Hi,

On 20 January 2016 at 14:52, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 15-01-16 om 10:06 schreef Marius Vlad:
>> + /* populate plane req */
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> Set crtc_id and fb_id to 0 when disabling plane.
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
>> +
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
>> +
>> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> One drmModeAtomicCommit per igt_display_commit2 is enough. :)
>
> Patch is looking good otherwise, would be nice if we could get rid of the duplication with kms_atomic.c
> That will make it a lot easier to add more tests in the future.

I agree, it'd be really nice indeed. Some of kms_atomic can't sensibly
use it, since it's pushing tricky edge cases, but certainly a good
chunk of the boilerplate to populate requests, get properties, etc,
could be merged with this.

Cheers,
Daniel