Re: [PATCH 1/6] drm/display: hdmi: add generic mode_valid helper

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]

On Fri, Oct 18, 2024 at 11:34:19PM +0300, Dmitry Baryshkov wrote:
> Add drm_hdmi_connector_mode_valid(), generic helper for HDMI connectors.
> It can be either used directly or as a part of the .mode_valid callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/display/drm_hdmi_helper.c | 25 +++++++++++++++++++++++++
>  include/drm/display/drm_hdmi_helper.h     |  4 ++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 74dd4d01dd9b..0ac5cb000ee2 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -256,3 +256,28 @@ drm_hdmi_compute_mode_clock(const struct drm_display_mode *mode,
>  	return DIV_ROUND_CLOSEST_ULL(clock * bpc, 8);
>  }
>  EXPORT_SYMBOL(drm_hdmi_compute_mode_clock);
> +
> +/**
> + * drm_hdmi_connector_mode_valid() - Check if mode is valid for HDMI connector
> + * @connector: DRM connector to validate the mode
> + * @mode: Display mode to validate
> + *
> + * Generic .mode_valid implementation for HDMI connectors.
> + */
> +enum drm_mode_status
> +drm_hdmi_connector_mode_valid(const struct drm_connector *connector,
> +			      const struct drm_display_mode *mode)
> +{
> +	const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
> +	unsigned long long rate;
> +
> +	rate = drm_hdmi_compute_mode_clock(mode, 8, HDMI_COLORSPACE_RGB);
> +	if (!rate)
> +		return MODE_ERROR;
> +
> +	if (!funcs || !funcs->tmds_char_rate_valid)
> +		return MODE_OK;
> +
> +	return funcs->tmds_char_rate_valid(connector, mode, rate);
> +}
> +EXPORT_SYMBOL(drm_hdmi_connector_mode_valid);

As discussed in the discussion that sparked that change, I believe that
we should use hdmi_clock_valid.

AFAIU, your concern was that max_tmds_clock might get stale, but then it
would not only prevent mode_valid from running but also the commit
entirely.

We don't have any evidence from that, so I'd rather try to keep
consistency between the two. And we can always try to address whatever
issue we might have if it turned out to be a bad idea :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]