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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jan 22, 2016 at 08:00:10AM -0800, Matt Roper wrote:
> On Thu, Jan 21, 2016 at 09:43:14PM -0800, Palleti, Avinash Reddy wrote:
> > Thanks Matt for pointing me to this.
> > 
> > Vlad,
> > As Matt mentioned, we are also working on this to get atomic support in i-g-t. Last week we finalized the design with Matt. I am putting the design we discussed here for reference,
> > 
> > - New commit style will be added as "COMMIT_NUCLEAR"
> > - If the commit style is nuclear we will use libdrm interface to build the input structure by taking each property(drmModeAtomicAddProperty)
> > - Above libdrm interface will create the list of properties and add each property to list whenever called
> > - Once all properties are added into list, call drmModeAtomicCommit() to do final commit.
> > - There will be configuration variable or environment variable which specifies commit style (e.g., IGT_COMMIT_STYLE) that will allow to override the commit style of existing IGT tests.
> > 
> > Matt,
> > As far as I know both nuclear and atomic are same, and they mean
> > commit per CRTC. Though userspace do commit once at top level for
> > multiple CRTC together, Kernel will internally commit once per CRTC.
> > So no need of two commit styles to be exposed in IGT. 
> 
> Atomic (as an interface) allows you to submit a single propertyset that
> updates multiple CRTC's.  Our Intel platforms don't have locked vblanks,
> so you don't have a guarantee that the changes will be visible at
> exactly the same time across the displays, but you do still get a
> guarantee that the commit as a whole completely succeeds or completely
> fails without leaving you in some kind of halfway limbo state.
> 
> "Nuclear pageflip" is a subset of that greater atomic modeset
> functionality where you're only submitting changes that affect a single
> CRTC.  This is interesting because it matches the behavior of most
> userspace compositors; a lot of compositors tend to handle each CRTC
> separately and submit new update transactions tied to the specific
> CRTC's vblanks.
> 
> Some of my initial work had two new IGT commit styles, NUCLEAR and
> ATOMIC, because we had nuclear pageflip support in i915 well before we
> finished implementing full atomic modeset.  These days (thanks to lots
> of work by Maarten and others) our kernel code supports both; the only
> thing we're really lacking from an interface perspective is support for
> non-blocking commits, and that's another issue altogether.  It's
> probably fine to just have a single new commit style in IGT now, but I'd
> suggest calling it 'ATOMIC' rather than 'NUCLEAR' and make sure that it
> allows properties for multple CRTC's to all be committed together.  If a
> specific IGT test wants to exercise the subset of functionality
> sometimes referred to as "nuclear pageflip" then that test can easily
> just make sure that it only updates the properties of a single CRTC
> before submitting the commit.

Yeah, COMMIT_ATOMIC should also set the ALLOW_MODESET flag imo. That would
reflect popular usage at least.

btw another consideration is how to expose TEST_ONLY. I think best option
is probably to have a new igt_commit_check (which again needs to take
commit flags becaue of the nuclear vs. atomic thing) for this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx