Hi Sam,
Thank you for the patch.
On Sun, Jul 17, 2022 at 07:58:00PM +0200, Sam Ravnborg wrote:
> All users are converted over to drm_bridge_funcs.atomic_check()
> so it is safe to drop the mode_fixup support.
>
> Update the comment for atomic_check with relevant parts from mode_fixup.
>
> Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: David Airlie <airl...@linux.ie>
> Cc: Daniel Vetter <dan...@ffwll.ch>
Reviewed-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_bridge.c | 7 +----
> include/drm/drm_bridge.h | 60 ++++++++++--------------------------
> 2 files changed, 17 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index b6f56d8f3547..3f5acb19957c 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -685,10 +685,6 @@ static int drm_atomic_bridge_check(struct drm_bridge
> *bridge,
> crtc_state, conn_state);
> if (ret)
> return ret;
> - } else if (bridge->funcs->mode_fixup) {
> - if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode,
> - &crtc_state->adjusted_mode))
> - return -EINVAL;
> }
>
> return 0;
> @@ -934,8 +930,7 @@ drm_atomic_bridge_propagate_bus_flags(struct drm_bridge
> *bridge,
> * @conn_state: new connector state
> *
> * First trigger a bus format negotiation before calling
> - * &drm_bridge_funcs.atomic_check() (falls back on
> - * &drm_bridge_funcs.mode_fixup()) op for all the bridges in the encoder
> chain,
> + * &drm_bridge_funcs.atomic_check() op for all the bridges in the encoder
> chain,
> * starting from the last bridge to the first. These are called before
> calling
> * &drm_encoder_helper_funcs.atomic_check()
> *
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 7496f41535b1..8c93369bcc74 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -106,7 +106,7 @@ struct drm_bridge_funcs {
> * to look at anything else but the passed-in mode, and validate it
> * against configuration-invariant hardward constraints. Any further
> * limits which depend upon the configuration can only be checked in
> - * @mode_fixup.
> + * @atomic_check.
> *
> * RETURNS:
> *
> @@ -116,46 +116,6 @@ struct drm_bridge_funcs {
> const struct drm_display_info *info,
> const struct drm_display_mode *mode);
>
> - /**
> - * @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 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.
> - *
> - * This is the only hook that allows a bridge to reject a modeset. If
> - * this function passes all other callbacks must succeed for this
> - * configuration.
> - *
> - * The mode_fixup callback is optional. &drm_bridge_funcs.mode_fixup()
> - * is not called when &drm_bridge_funcs.atomic_check() is implemented,
> - * so only one of them should be provided.
> - *
> - * NOTE:
> - *
> - * This function is called in the check phase of atomic modesets, which
> - * can be aborted for any reason (including on userspace's request to
> - * just check whether a configuration would be possible). Drivers MUST
> - * NOT touch any persistent state (hardware or software) or data
> - * structures except the passed in @state parameter.
> - *
> - * Also beware that userspace can request its own custom modes, neither
> - * core nor helpers filter modes to the list of probe modes reported by
> - * the GETCONNECTOR IOCTL and stored in &drm_connector.modes. To ensure
> - * that modes are filtered consistently put any bridge constraints and
> - * limits checks into @mode_valid.
> - *
> - * RETURNS:
> - *
> - * True if an acceptable configuration is possible, false if the modeset
> - * operation should be rejected.
> - */
> - bool (*mode_fixup)(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode);
> /**
> * @disable:
> *
> @@ -466,9 +426,7 @@ struct drm_bridge_funcs {
> * &drm_bridge_funcs.atomic_check() hooks are called in reverse
> * order (from the last to the first bridge).
> *
> - * This method is optional. &drm_bridge_funcs.mode_fixup() is not
> - * called when &drm_bridge_funcs.atomic_check() is implemented, so only
> - * one of them should be provided.
> + * This method is optional.
> *
> * If drivers need to tweak &drm_bridge_state.input_bus_cfg.flags or
> * &drm_bridge_state.output_bus_cfg.flags it should happen in
> @@ -478,6 +436,20 @@ struct drm_bridge_funcs {
> * &drm_connector.display_info.bus_flags if the bridge is the last
> * element in the chain.
> *
> + * NOTE:
> + *
> + * This function is called in the check phase of atomic modesets, which
> + * can be aborted for any reason (including on userspace's request to
> + * just check whether a configuration would be possible). Drivers MUST
> + * NOT touch any persistent state (hardware or software) or data
> + * structures except the passed in @state parameter.
> + *
> + * Also beware that userspace can request its own custom modes, neither
> + * core nor helpers filter modes to the list of probe modes reported by
> + * the GETCONNECTOR IOCTL and stored in &drm_connector.modes. To ensure
> + * that modes are filtered consistently put any bridge constraints and
> + * limits checks into @mode_valid.
> + *
> * RETURNS:
> * zero if the check passed, a negative error code otherwise.
> */
--
Regards,
Laurent Pinchart
