Hi Daniel, On Tuesday, 19 December 2017 16:00:36 EET Daniel Vetter wrote: > 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 ... So how about splitting this in two steps then, first deprecating subclassing drm_atomic_state to store private object state, and only in a second step also deprecating subclassing the structure for global state ? -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx