On Sun, Jun 19, 2016 at 9:57 PM, Jyri Sarha jsarha@ti.com wrote:

Hi, The documentation of drm_atomic_helper_check_modeset() says:

"Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a plane update can't be done without a full modeset) _must_ call this function afterwards after that change. It is permitted to call this function multiple times for the same update, e.g. when the ->atomic_check functions depend upon the adjusted dotclock for fifo space allocation and watermark computation."

Then drm_crtc_helper_funcs mode_fixup() callback documentations says:

"Atomic drivers which need to inspect and adjust more state should instead use the @atomic_check callback."

Now an atomic driver that needs to do the both in the check phase: update ->mode_changed and inspect and adjust the mode, is in trouble. This is because any adjusted_mode modifications are reset by the afterwards call of drm_atomic_helper_check_modeset().

I can get the atomic tilcdc driver working either: by using the crtc_helper mode_fixup() calback, tuning the mode once more in mode_set_nofb(), or calling drm_atomic_helper_check_modeset() last in drm_mode_config_funcs atomic_check() callback.

What is the right thing to do? Any of these three approaches (and possibly updating the documentation), or some other option that I did not think of?

Tuning the mode once more in mode_set_nofb is no-go - that's in the commit phase, you're not allowed to change state objects then any more. Also note that the hint for mode_fixup is just a "should", and meant that way. You can split things up between mode_fixup and atomic_check callbacks. Pick whatever works best for you from the other 2 options.

-Daniel