BUG: mode_fixup() documentation misleading?

[Russell King - ARM Linux]

The mode fixup code has a nice, logical explanation of what it should
do, but unfortunately it doesn't reflect reality.

For bridges:
         * @mode_fixup:
         *
         * This callback is used to validate and adjust a mode. The paramater
         * mode is the display mode that should be fed to the next element in
         * the display chain, either the final &drm_connector or the next
         * &drm_bridge. The parameter adjusted_mode is the input mode the bridge
         * requires. It can be modified by this callback and does not need to
         * match mode. See also &drm_crtc_state.adjusted_mode for more details.

but the bridge code actually does:

bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
                        const struct drm_display_mode *mode,
                        struct drm_display_mode *adjusted_mode)
{
...
        if (bridge->funcs->mode_fixup)
                ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode);

        ret = ret && drm_bridge_mode_fixup(bridge->next, mode, adjusted_mode);
}

which means 'mode' and 'adjusted_mode' parameters are the same for
each bridge in the chain.  Moreover, the encoder doc says similar:

         * @mode_fixup:
         *
         * This callback is used to validate and adjust a mode. The parameter
         * mode is the display mode that should be fed to the next element in
         * the display chain, either the final &drm_connector or a &drm_bridge.
         * The parameter adjusted_mode is the input mode the encoder requires.

but the code in mode_fixup() in the atomic helper does:

                ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
                                &new_crtc_state->adjusted_mode);
...
                        ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
                                                &new_crtc_state->adjusted_mode);

In other words, 'mode' is _always_ the requested mode, and
'adjusted_mode' is the currently adjusted mode in fixup order.

Now, as to the fixup order, it is not as these comments suggest.

The first to be called is the bridge allegedly attached to the output
of the encoder.  Then the bridge attached to the first bridge, etc.
Finally, the encoder gets called.  So the order for a whole chain is:

crtc -> encoder -> bridge 1 -> bridge 2 -> bridge 3 -> connector
 5         4         1           2           3

What this means is that bridge 1 gets first dibs on applying
adjustments to 'adjusted_mode', then bridge 2, then bridge 3, then
the encoder, and finally the crtc... which is quite an odd ordering.

I don't have a real issue here, just an observation that the comments
do not match the code.

Someone who knows how this is supposed to work may wish to consider
either fixing the code to do what the comments say, or fixing the
comments to reflect what the code actually does.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up