[PATCH 7/8] drm: Add old state pointer to CRTC .enable() helper function

[Daniel Vetter]

On Wed, Jun 28, 2017 at 12:16:20AM +0300, Laurent Pinchart wrote:
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 474a1029ec79..d74a2cafc3de 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -316,24 +316,6 @@ struct drm_crtc_helper_funcs {
>  	void (*disable)(struct drm_crtc *crtc);
>  
>  	/**
> -	 * @enable:
> -	 *
> -	 * This callback should be used to enable the CRTC. With the atomic
> -	 * drivers it is called before all encoders connected to this CRTC are
> -	 * enabled through the encoder's own &drm_encoder_helper_funcs.enable
> -	 * hook.  If that sequence is too simple drivers can just add their own
> -	 * hooks and call it from this CRTC callback here by looping over all
> -	 * encoders connected to it using for_each_encoder_on_crtc().
> -	 *
> -	 * This hook is used only by atomic helpers, for symmetry with @disable.
> -	 * Atomic drivers don't need to implement it if there's no need to
> -	 * enable anything at the CRTC level. To ensure that runtime PM handling
> -	 * (using either DPMS or the new "ACTIVE" property) works
> -	 * @enable must be the inverse of @disable for atomic drivers.
> -	 */
> -	void (*enable)(struct drm_crtc *crtc);
> -
> -	/**
>  	 * @atomic_check:
>  	 *
>  	 * Drivers should check plane-update related CRTC constraints in this
> @@ -433,6 +415,26 @@ struct drm_crtc_helper_funcs {
>  			     struct drm_crtc_state *old_crtc_state);
>  
>  	/**
> +	 * @atomic_enable:
> +	 *
> +	 * This callback should be used to enable the CRTC. With the atomic
> +	 * drivers it is called before all encoders connected to this CRTC are
> +	 * enabled through the encoder's own &drm_encoder_helper_funcs.enable
> +	 * hook.  If that sequence is too simple drivers can just add their own
> +	 * hooks and call it from this CRTC callback here by looping over all
> +	 * encoders connected to it using for_each_encoder_on_crtc().
> +	 *
> +	 * This hook is used only by atomic helpers, for symmetry with
> +	 * @atomic_disable. Atomic drivers don't need to implement it if there's
> +	 * no need to enable anything at the CRTC level. To ensure that runtime
> +	 * PM handling (using either DPMS or the new "ACTIVE" property) works
> +	 * @atomic_enable must be the inverse of @atomic_disable for atomic
> +	 * drivers.

A few lines explaing what @old_crtc_state is good for would be good here
(like we already have for @atomic_disable). Otherwise lgtm.
-Daniel

> +	void (*atomic_enable)(struct drm_crtc *crtc,
> +			      struct drm_crtc_state *old_crtc_state);
> +
> +	/**
>  	 * @atomic_disable:
>  	 *
>  	 * This callback should be used to disable the CRTC. With the atomic
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch