[PATCH 3/4] drm/ast: astdp: Store mode index in connector state

[Jocelyn Falempe]

On 04/02/2025 14:26, Thomas Zimmermann wrote:
> Look up the mode index for the astdp transmitter ship in the encoder's
> atomic check and report an error if the display mode is not supported.
> 
> The lookup uses the DRM display mode instead of the driver's internal
> VBIOS mode. Both are be equivalent. The modesetting code later reads
Typo: Both are equivalent

> the calculated index from the connector state to avoid recalculating it.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe at redhat.com>
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
>   drivers/gpu/drm/ast/ast_dp.c | 37 ++++++++++++++++++++++++++++++------
>   1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index e7b00153c37e..056991afde7f 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -5,6 +5,7 @@
>   #include <linux/firmware.h>
>   #include <linux/delay.h>
>   
> +#include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_state_helper.h>
>   #include <drm/drm_edid.h>
>   #include <drm/drm_modeset_helper_vtables.h>
> @@ -44,6 +45,8 @@ static const struct ast_astdp_mode_index_table_entry ast_astdp_mode_index_table[
>   
>   struct ast_astdp_connector_state {
>   	struct drm_connector_state base;
> +
> +	int mode_index;
>   };
>   
>   static struct ast_astdp_connector_state *
> @@ -305,14 +308,12 @@ static void ast_astdp_encoder_helper_atomic_mode_set(struct drm_encoder *encoder
>   	struct ast_device *ast = to_ast_device(dev);
>   	struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
>   	const struct ast_vbios_enhtable *vmode = ast_crtc_state->vmode;
> -	int mode_index;
> +	struct ast_astdp_connector_state *astdp_conn_state =
> +		to_ast_astdp_connector_state(conn_state);
> +	int mode_index = astdp_conn_state->mode_index;
>   	u8 refresh_rate_index;
>   	u8 vgacre0, vgacre1, vgacre2;
>   
> -	mode_index = ast_astdp_get_mode_index(vmode->hde, vmode->vde);
> -	if (drm_WARN_ON(dev, mode_index < 0))
> -		return;
> -
>   	if (drm_WARN_ON(dev, vmode->refresh_rate_index < 1 || vmode->refresh_rate_index > 255))
>   		return;
>   	refresh_rate_index = vmode->refresh_rate_index - 1;
> @@ -368,10 +369,30 @@ static void ast_astdp_encoder_helper_atomic_disable(struct drm_encoder *encoder,
>   	ast_dp_set_phy_sleep(ast, true);
>   }
>   
> +static int ast_astdp_encoder_helper_atomic_check(struct drm_encoder *encoder,
> +						 struct drm_crtc_state *crtc_state,
> +						 struct drm_connector_state *conn_state)
> +{
> +	const struct drm_display_mode *mode = &crtc_state->mode;
> +	struct ast_astdp_connector_state *astdp_conn_state =
> +		to_ast_astdp_connector_state(conn_state);
> +	int res;
> +
> +	if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> +		res = ast_astdp_get_mode_index(mode->hdisplay, mode->vdisplay);
> +		if (res < 0)
> +			return res;
> +		astdp_conn_state->mode_index = res;
> +	}
> +
> +	return 0;
> +}
> +
>   static const struct drm_encoder_helper_funcs ast_astdp_encoder_helper_funcs = {
>   	.atomic_mode_set = ast_astdp_encoder_helper_atomic_mode_set,
>   	.atomic_enable = ast_astdp_encoder_helper_atomic_enable,
>   	.atomic_disable = ast_astdp_encoder_helper_atomic_disable,
> +	.atomic_check = ast_astdp_encoder_helper_atomic_check,
>   };
>   
>   /*
> @@ -459,7 +480,7 @@ static void ast_astdp_connector_reset(struct drm_connector *connector)
>   static struct drm_connector_state *
>   ast_astdp_connector_atomic_duplicate_state(struct drm_connector *connector)
>   {
> -	struct ast_astdp_connector_state *new_astdp_state;
> +	struct ast_astdp_connector_state *new_astdp_state, *astdp_state;
>   	struct drm_device *dev = connector->dev;
>   
>   	if (drm_WARN_ON(dev, !connector->state))
> @@ -470,6 +491,10 @@ ast_astdp_connector_atomic_duplicate_state(struct drm_connector *connector)
>   		return NULL;
>   	__drm_atomic_helper_connector_duplicate_state(connector, &new_astdp_state->base);
>   
> +	astdp_state = to_ast_astdp_connector_state(connector->state);
> +
> +	new_astdp_state->mode_index = astdp_state->mode_index;
> +
>   	return &new_astdp_state->base;
>   }
>