[PATCH 2/2] drm/doc: Clarify mode_fixup vs. atomic_check a bit more

[Jose Abreu]

Hi Daniel,


On 15-05-2017 10:11, Daniel Vetter wrote:
> Brought up by both Laurent and Andrzej when reviewing the new
> ->mode_valid hooks. Since mode_fixup is just a simpler version of the
> much more generic atomic_check we can't really unify it with
> mode_valid. Most drivers should probably switch their current
> mode_fixup code to either the new mode_valid or the atomic_check
> hooks, but e.g. that doesn't exist yet for bridges, and for CRTCs the
> situation is a bit more complicated. Hence there's no clear
> equivalence between mode_fixup and mode_valid, even if it looks like
> that at first glance.
>
> Cc: Andrzej Hajda <a.hajda at samsung.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Jose Abreu <Jose.Abreu at synopsys.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

Reviewed-by: Jose Abreu <joabreu at synopsys.com>

Best regards,
Jose Miguel Abreu

> ---
>  include/drm/drm_modeset_helper_vtables.h | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index c72fca544a41..613b2a602b77 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -156,7 +156,11 @@ struct drm_crtc_helper_funcs {
>  	 * allowed.
>  	 *
>  	 * Atomic drivers which need to inspect and adjust more state should
> -	 * instead use the @atomic_check callback.
> +	 * instead use the @atomic_check callback, but note that they're not
> +	 * perfectly equivalent: @mode_valid is called from
> +	 * drm_atomic_helper_check_modeset(), but @atomic_check is called from
> +	 * drm_atomic_helper_check_planes(), because originally it was meant for
> +	 * plane update checks only..
>  	 *
>  	 * Also beware that userspace can request its own custom modes, neither
>  	 * core nor helpers filter modes to the list of probe modes reported by
> @@ -529,7 +533,9 @@ struct drm_encoder_helper_funcs {
>  	 * allowed.
>  	 *
>  	 * Atomic drivers which need to inspect and adjust more state should
> -	 * instead use the @atomic_check callback.
> +	 * instead use the @atomic_check callback. If @atomic_check is used,
> +	 * this hook isn't called since @atomic_check allows a strict superset
> +	 * of the functionality of @mode_fixup.
>  	 *
>  	 * Also beware that userspace can request its own custom modes, neither
>  	 * core nor helpers filter modes to the list of probe modes reported by
> @@ -716,6 +722,11 @@ struct drm_encoder_helper_funcs {
>  	 * update the CRTC to match what the encoder needs for the requested
>  	 * connector.
>  	 *
> +	 * Since this provides a strict superset of the functionality of
> +	 * @mode_fixup (the requested and adjusted modes are both available
> +	 * through the passed in &struct drm_crtc_state) @mode_fixup is not
> +	 * called when @atomic_check is implemented.
> +	 *
>  	 * This function is used by the atomic helpers, but it is optional.
>  	 *
>  	 * NOTE: