Re: [PATCH v5 14/44] drm/connector: hdmi: Add custom hook to filter TMDS character rate

2023-12-14 Thread Dave Stevenson
On Thu, 7 Dec 2023 at 15:50, Maxime Ripard  wrote:
>
> Most of the HDMI controllers have an upper TMDS character rate limit
> they can't exceed. On "embedded"-grade display controllers, it will
> typically be lower than what high-grade monitors can provide these days,
> so drivers will filter the TMDS character rate based on the controller
> capabilities.
>
> To make that easier to handle for drivers, let's provide an optional
> hook to be implemented by drivers so they can tell the HDMI controller
> helpers if a given TMDS character rate is reachable for them or not.
>
> This will then be useful to figure out the best format and bpc count for
> a given mode.
>
> Signed-off-by: Maxime Ripard 

Reviewed-by: Dave Stevenson 

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c  |  9 +++
>  drivers/gpu/drm/drm_connector.c|  4 ++
>  .../gpu/drm/tests/drm_atomic_state_helper_test.c   | 69 
> ++
>  drivers/gpu/drm/tests/drm_connector_test.c | 15 +
>  include/drm/drm_connector.h| 30 ++
>  5 files changed, 127 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 74bc3cc53c2d..a36edda590f8 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -687,11 +687,20 @@ hdmi_clock_valid(const struct drm_connector *connector,
>  const struct drm_display_mode *mode,
>  unsigned long long clock)
>  {
> +   const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
> const struct drm_display_info *info = >display_info;
>
> if (info->max_tmds_clock && clock > info->max_tmds_clock * 1000)
> return MODE_CLOCK_HIGH;
>
> +   if (funcs && funcs->tmds_char_rate_valid) {
> +   enum drm_mode_status status;
> +
> +   status = funcs->tmds_char_rate_valid(connector, mode, clock);
> +   if (status != MODE_OK)
> +   return status;
> +   }
> +
> return MODE_OK;
>  }
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 667326b09acc..9f314fee26ce 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -457,6 +457,7 @@ EXPORT_SYMBOL(drmm_connector_init);
>   * @dev: DRM device
>   * @connector: A pointer to the HDMI connector to init
>   * @funcs: callbacks for this connector
> + * @hdmi_funcs: HDMI-related callbacks for this connector
>   * @connector_type: user visible type of the connector
>   * @ddc: optional pointer to the associated ddc adapter
>   * @supported_formats: Bitmask of @hdmi_colorspace listing supported output 
> formats
> @@ -476,6 +477,7 @@ EXPORT_SYMBOL(drmm_connector_init);
>  int drmm_connector_hdmi_init(struct drm_device *dev,
>  struct drm_connector *connector,
>  const struct drm_connector_funcs *funcs,
> +const struct drm_connector_hdmi_funcs 
> *hdmi_funcs,
>  int connector_type,
>  struct i2c_adapter *ddc,
>  unsigned long supported_formats,
> @@ -512,6 +514,8 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> if (max_bpc > 8)
> drm_connector_attach_hdr_output_metadata_property(connector);
>
> +   connector->hdmi.funcs = hdmi_funcs;
> +
> return 0;
>  }
>  EXPORT_SYMBOL(drmm_connector_hdmi_init);
> diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c 
> b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
> index d76fafb91025..e7dbdd4a4e7f 100644
> --- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
> @@ -110,6 +110,21 @@ static int set_connector_edid(struct kunit *test, struct 
> drm_connector *connecto
> return 0;
>  }
>
> +static const struct drm_connector_hdmi_funcs dummy_connector_hdmi_funcs = {
> +};
> +
> +static enum drm_mode_status
> +reject_connector_tmds_char_rate_valid(const struct drm_connector *connector,
> +  const struct drm_display_mode *mode,
> +  unsigned long long tmds_rate)
> +{
> +   return MODE_BAD;
> +}
> +
> +static const struct drm_connector_hdmi_funcs reject_connector_hdmi_funcs = {
> +   .tmds_char_rate_valid   = reject_connector_tmds_char_rate_valid,
> +};
> +
>  static int dummy_connector_get_modes(struct drm_connector *connector)
>  {
> struct drm_atomic_helper_connector_hdmi_priv *priv =
> @@ -192,6 +207,7 @@ drm_atomic_helper_connector_hdmi_init(struct kunit *test,
> conn = >connector;
> ret = drmm_connector_hdmi_init(drm, conn,
>_connector_funcs,
> +  

[PATCH v5 14/44] drm/connector: hdmi: Add custom hook to filter TMDS character rate

2023-12-07 Thread Maxime Ripard
Most of the HDMI controllers have an upper TMDS character rate limit
they can't exceed. On "embedded"-grade display controllers, it will
typically be lower than what high-grade monitors can provide these days,
so drivers will filter the TMDS character rate based on the controller
capabilities.

To make that easier to handle for drivers, let's provide an optional
hook to be implemented by drivers so they can tell the HDMI controller
helpers if a given TMDS character rate is reachable for them or not.

This will then be useful to figure out the best format and bpc count for
a given mode.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_atomic_state_helper.c  |  9 +++
 drivers/gpu/drm/drm_connector.c|  4 ++
 .../gpu/drm/tests/drm_atomic_state_helper_test.c   | 69 ++
 drivers/gpu/drm/tests/drm_connector_test.c | 15 +
 include/drm/drm_connector.h| 30 ++
 5 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 74bc3cc53c2d..a36edda590f8 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -687,11 +687,20 @@ hdmi_clock_valid(const struct drm_connector *connector,
 const struct drm_display_mode *mode,
 unsigned long long clock)
 {
+   const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
const struct drm_display_info *info = >display_info;
 
if (info->max_tmds_clock && clock > info->max_tmds_clock * 1000)
return MODE_CLOCK_HIGH;
 
+   if (funcs && funcs->tmds_char_rate_valid) {
+   enum drm_mode_status status;
+
+   status = funcs->tmds_char_rate_valid(connector, mode, clock);
+   if (status != MODE_OK)
+   return status;
+   }
+
return MODE_OK;
 }
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 667326b09acc..9f314fee26ce 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -457,6 +457,7 @@ EXPORT_SYMBOL(drmm_connector_init);
  * @dev: DRM device
  * @connector: A pointer to the HDMI connector to init
  * @funcs: callbacks for this connector
+ * @hdmi_funcs: HDMI-related callbacks for this connector
  * @connector_type: user visible type of the connector
  * @ddc: optional pointer to the associated ddc adapter
  * @supported_formats: Bitmask of @hdmi_colorspace listing supported output 
formats
@@ -476,6 +477,7 @@ EXPORT_SYMBOL(drmm_connector_init);
 int drmm_connector_hdmi_init(struct drm_device *dev,
 struct drm_connector *connector,
 const struct drm_connector_funcs *funcs,
+const struct drm_connector_hdmi_funcs *hdmi_funcs,
 int connector_type,
 struct i2c_adapter *ddc,
 unsigned long supported_formats,
@@ -512,6 +514,8 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
if (max_bpc > 8)
drm_connector_attach_hdr_output_metadata_property(connector);
 
+   connector->hdmi.funcs = hdmi_funcs;
+
return 0;
 }
 EXPORT_SYMBOL(drmm_connector_hdmi_init);
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c 
b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index d76fafb91025..e7dbdd4a4e7f 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -110,6 +110,21 @@ static int set_connector_edid(struct kunit *test, struct 
drm_connector *connecto
return 0;
 }
 
+static const struct drm_connector_hdmi_funcs dummy_connector_hdmi_funcs = {
+};
+
+static enum drm_mode_status
+reject_connector_tmds_char_rate_valid(const struct drm_connector *connector,
+  const struct drm_display_mode *mode,
+  unsigned long long tmds_rate)
+{
+   return MODE_BAD;
+}
+
+static const struct drm_connector_hdmi_funcs reject_connector_hdmi_funcs = {
+   .tmds_char_rate_valid   = reject_connector_tmds_char_rate_valid,
+};
+
 static int dummy_connector_get_modes(struct drm_connector *connector)
 {
struct drm_atomic_helper_connector_hdmi_priv *priv =
@@ -192,6 +207,7 @@ drm_atomic_helper_connector_hdmi_init(struct kunit *test,
conn = >connector;
ret = drmm_connector_hdmi_init(drm, conn,
   _connector_funcs,
+  _connector_hdmi_funcs,
   DRM_MODE_CONNECTOR_HDMIA,
   NULL,
   formats,
@@ -956,6 +972,58 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct 
kunit *test)
KUNIT_EXPECT_EQ(test,