On Tue, Dec 19, 2017 at 03:33:57PM +0200, Laurent Pinchart wrote: > On Tuesday, 19 December 2017 12:03:55 EET Daniel Vetter wrote: > > On Mon, Dec 18, 2017 at 06:13:46PM +0200, Laurent Pinchart wrote: > > > On Thursday, 14 December 2017 22:30:53 EET Daniel Vetter wrote: > > >> + * Drivers are recommend to wrap these for each type of driver private > > >> state > > >> + * object they have, filtering on &drm_private_obj.funcs using > > >> for_each_if(), at > > >> + * least if they want to iterate over all objects of a given type. > > >> + * > > >> + * An earlier way to handle driver private state was by subclassing > > >> struct > > >> + * &drm_atomic_state. But since that encourages non-standard ways to > > >> implement > > >> + * the check/commit split atomic requires (by using e.g. "check and > > >> rollback or > > >> + * commit instead" of "duplicate state, check, then either commit or > > >> release > > >> + * duplicated state) it is deprecated in favour of using > > >> &drm_private_state. > > > > > > This I still don't agree with. I think it still makes sense to subclass > > > the global state object when you have true global state data. How about > > > starting by making it a recommendation instead, moving state data related > > > to driver- specific objects to the new framework, and keeping global data > > > in the drm_atomic_state subclass ? > > > > Converting all the existing drivers over is somewhere on my todo. I'm also > > not really clear what you mean with global data compared to > > driver-specific objects ... > > I'll take an example related to the rcar-du driver. The hardware groups CRTCs > by two and share resources (such as planes) between CRTCs in a group. This is > something I currently implement in a convoluted way, and using private objects > to handle groups (I already have a group object in my driver) will likely help > to model the group state. > > On the other hand, if the hardware didn't have groups but shared planes > between all CRTCs, the shared resources would be global, and it would make > sense to store them in the global state. Yeah the private stuff should probably get a hole lot better for singleton objects. I still think one global thing overall (and with a state handling pattern that's different from everything else) is not a good idea. Now it would be fairly easy to generate all the silly boilerplate with a macro, but that tends to wreak havoc with cscope and friends, so I'm not sure it's a great idea really. I'll probably have better ideas once the i915 conversion exists ... -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