On Tue, Jul 13, 2021 at 07:44:12PM +0200, Daniel Vetter wrote: > On Tue, Jul 13, 2021 at 7:14 PM Grace An <gracan@xxxxxxxxxxxxxx> wrote: > > When CONFIG_PROVE_LOCKING is defined, the kernel randomly injects > > -EDEADLK errors for all the ww_mutex. This results in > > drm_atomic_get_private_obj_state randomly returning -EDEADLK. > > However, the mode_fixup functions do not propagate these error > > codes and return false, causing the atomic commit to fail with > > -EINVAL instead of retrying. > > > > Change encoder, crtc, and bridge mode_fixup functions to return > > an int instead of a boolean to indicate success or failure. If > > any of these functions fail, the mode_fixup function now returns > > the provided integer error code instead of -EINVAL. > > > > This change needs modifications across drivers, but before submitting > > the entire change, we want to get feedback on this RFC. > > > > Signed-off-by: Grace An <gracan@xxxxxxxxxxxxxx> > > Why don't you just use the various atomic_check hooks we have for > this? There you get passed the state and everything, have a full int > return value, and things actually work. > > ->mode_fixup is for compatibility with legacy crtc modeset helpers > from the pre-atomic times. If the kerneldoc isn't clear yet, please do > a patch to fix that up so that @mode_fixup points at the relevant > @atomic_check as the recommended function. Agreed, and we need to document this better. I have posted the following patch to make it more obvious that mode_fixup is deprecated. https://lore.kernel.org/dri-devel/20210713193257.958852-1-sam@xxxxxxxxxxxx/T/#u Sam
