Am Freitag, dem 18.02.2022 um 02:00 +0100 schrieb Marek Vasut:
> Implement .atomic_check callback which prevents user space from setting
> unsupported mode. The tc_edp_common_atomic_check() variant is already
> prepared for DSI-to-DPI mode addition, which has different frequency
> limits.
> 
> Signed-off-by: Marek Vasut <ma...@denx.de>
> Cc: Jonas Karlman <jo...@kwiboo.se>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Maxime Ripard <max...@cerno.tech>
> Cc: Neil Armstrong <narmstr...@baylibre.com>
> Cc: Sam Ravnborg <s...@ravnborg.org>
> ---
> V2: - New patch
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> b/drivers/gpu/drm/bridge/tc358767.c
> index 522c2c4d8514f..01d11adee6c74 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge 
> *bridge,
>       return true;
>  }
>  
> +static int tc_edp_common_atomic_check(struct drm_bridge *bridge,

Drop the edp in the name here? Later in the series you call this
function from the DPI code, so this breaks the nice clean naming
separation from patch 1.

> +                                   struct drm_bridge_state *bridge_state,
> +                                   struct drm_crtc_state *crtc_state,
> +                                   struct drm_connector_state *conn_state,
> +                                   const unsigned int max_khz)
> +{
> +     tc_bridge_mode_fixup(bridge, &crtc_state->mode,
> +                          &crtc_state->adjusted_mode);
> +
> +     if (crtc_state->adjusted_mode.clock > max_khz)
> +             crtc_state->adjusted_mode.clock = max_khz;

I don't think this is correct. The adjusted most is just for minor
adjustments if the bridge can not fully match the mode. If userspace
supplies a invalid high modeclock I think it would be better to fail
the atomic check -> return -EINVAL

Regards,
Lucas

> +
> +     return 0;
> +}
> +
> +static int tc_edp_atomic_check(struct drm_bridge *bridge,
> +                            struct drm_bridge_state *bridge_state,
> +                            struct drm_crtc_state *crtc_state,
> +                            struct drm_connector_state *conn_state)
> +{
> +     /* DPI->(e)DP interface clock limitation: upto 154 MHz */
> +     return tc_edp_common_atomic_check(bridge, bridge_state, crtc_state,
> +                                       conn_state, 154000);
> +}
> +
>  static enum drm_mode_status
>  tc_edp_mode_valid(struct drm_bridge *bridge,
>                 const struct drm_display_info *info,
> @@ -1463,6 +1488,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
>       .detach = tc_edp_bridge_detach,
>       .mode_valid = tc_edp_mode_valid,
>       .mode_set = tc_bridge_mode_set,
> +     .atomic_check = tc_edp_atomic_check,
>       .atomic_enable = tc_edp_bridge_atomic_enable,
>       .atomic_disable = tc_edp_bridge_atomic_disable,
>       .mode_fixup = tc_bridge_mode_fixup,


Reply via email to