Hi all, Maxime seems to have a need for a bit more than what the current drm_mode_config_reste can do, so here's a bunch of ideas inspired by i915. I think minimally what you need is a drm_atomic_state_helper_readout() functions, which instead of resetting, allocates all the obj->state pointers and fills them out. For that I think the simplest is to add atomic_readout_state functions to crtc, connector and plane (if you want to take over the firmware fb allocation too), which take as parameter the object, and return the read-out state. Important, they must _not_ touch anything persistent, otherwise the following stuff here doesn't work. Next up is the challenge of bridges and encoders. If the goal is to properly shut down encoders/bridges, they also need their state. And on most hw, they need a mix of the crtc and connector state, so best to pass both of those (plus bridge state for bridges) to them. You can do that if we assume that connector_helper_funcs->atomic_readout_state sets the drm_connector_state->crtc pointer correctly. So the drm_atomic_state_helper_readout function would first loop through connectors and crtcs, and then loop through encoders and bridges to fill in the gaps. Last you probably want to go through planes. Now even more fun hw will have trouble and might need to look up random other objects to set stuff, so we need a drm_atomic_state structure to tie all these things together. For reasons that will become obvious later on these read-out states should be stored in the old_ state pointers. Finally we need the actual helper which takes that read-out state and smashes it into the real obj->state pointers, essentially a swap_state but in reverse (since we need to write the old_ state pointers into obj->state). One thing i915 does, but I don't think is really needed: We read out the connector->crtc routing as a first step, and once we have that, we read out the connector/encoder/crtc steps. I think if you first read out (and hence allocate) crtrc states, and then connector, and then encoder/bridges that should work, and simplifies the flow a bit. So we need another drm_atomic_state_helper_reset_to_readout or whatever, which uses _readout and then does the reverse swap. Drivers call this instead of drm_mode_config_reset. Now the real issue: How do you make sure this actually works? Testing with different fw configurations is usually impossible, you cant easily tell the firmware to boot with different modes. Or at least it's cumbersome since manual testing and lots of reboots. Waiting for bug reports and then fixing them, while probably breaking something else is a game of whack-a-mole. So what i915 does is read out the hw state on every nonblocking modeset (the additional time spent doesn't matter), but _only_ for the objects touched in that modeset state. This is why you need to read out into old_ state pointers, since after a blocking modeset those are unused and available. Next item is to add a atomic_compare_state function to crtc/connector&plane and maybe bridges (i.e. all objects with state), which compares 2 state objects for equality. This needs to be a driver callback since each driver will only read out the state relevant from take-over from fw, not every possible feature, so there's lots you need to ignore. If any of these functions note a mismatch you splat with a warning and dump both the old and new states with the atomic_print driver hooks. I915 uses some #define so that these comparisons are one-liners (see PIPE_CONFIG_CHECK_X/I and so on, maybe we should have a few default ones with proper atomic naming, the names date back to the first somewhat-atomic modeset flow in i915). So for validation we need a drm_atomic_state_helper_check which uses _readout, and then the compare functions plus debug printouts if it goes wrong. I'd wire that directly into the default drm_atomic_helper_commit function. With these pieces you should have a state readout code that actually tends to work, and you can even test it (simply by doing a bunch of modesets). In i915 we have the _check code running unconditionally, blocking modesets are slow enough that it really doesn't matter. One more thing on the implementation: Since this is all helpers all the hooks should probably be in the respective helper function tables. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
