Re: [PATCH v3 drm-dp 06/11] drm/hisilicon/hibmc: add dp mode valid check

[Yongbang Shi]

On Fri, Jul 18, 2025 at 02:51:20PM +0800, Yongbang Shi wrote:

From: Baihan Li <libai...@huawei.com>

If DP is connected, add mode check and BW check in mode_valid_ctx() to
ensure DP's cfg is usable.

For example: Some DP's mode(1920x1080 60HZ 138.5 MHz) gives to drm,
however, the GPU is configured for 148MHz in display_ctrl_adjust() by
looking up the table of struct hibmc_pll_table{}. This difference will
cause the DP to not display because the clock the DP gets via drm frame
is 138MHz.

Fixes: f9698f802e50 ("drm/hisilicon/hibmc: Restructuring the header dp_reg.h")
Signed-off-by: Baihan Li <libai...@huawei.com>
Signed-off-by: Yongbang Shi <shiyongb...@huawei.com>
---
  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 10 ++++
  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  6 +++
  .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 51 +++++++++++++++++++
  3 files changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c 
b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index b4d612047f36..7fbd51ffd778 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -276,6 +276,16 @@ void hibmc_dp_reset_link(struct hibmc_dp *dp)
        dp->dp_dev->link.status.channel_equalized = false;
  }

+u8 hibmc_dp_get_link_rate(struct hibmc_dp *dp)

+{
+       return dp->dp_dev->link.cap.link_rate;
+}
+
+u8 hibmc_dp_get_lanes(struct hibmc_dp *dp)
+{
+       return dp->dp_dev->link.cap.lanes;
+}
+
  static const struct hibmc_dp_color_raw g_rgb_raw[] = {
        {CBAR_COLOR_BAR, 0x000, 0x000, 0x000},
        {CBAR_WHITE,     0xfff, 0xfff, 0xfff},
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h 
b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 9b45e88e47e4..0059a2648a38 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -12,6 +12,10 @@
  #include <drm/drm_print.h>
  #include <drm/display/drm_dp_helper.h>

+/* 27 * 10000000 * 80% = 216000000 */

+#define DP_MODE_VALI_CAL       216000000
+#define BPP_24                         24
+
  struct hibmc_dp_dev;

enum hibmc_dp_cbar_pattern {

@@ -62,5 +66,7 @@ void hibmc_dp_reset_link(struct hibmc_dp *dp);
  void hibmc_dp_hpd_cfg(struct hibmc_dp *dp);
  void hibmc_dp_enable_int(struct hibmc_dp *dp);
  void hibmc_dp_disable_int(struct hibmc_dp *dp);
+u8 hibmc_dp_get_link_rate(struct hibmc_dp *dp);
+u8 hibmc_dp_get_lanes(struct hibmc_dp *dp);

#endif

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 354e18bb2998..99b30a6c7e06 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -15,6 +15,28 @@

#define DP_MASKED_SINK_HPD_PLUG_INT BIT(2) +struct hibmc_dp_disp_clk {

+       u16 hdisplay;
+       u16 vdisplay;
+       u32 clock;
+};
+
+static const struct hibmc_dp_disp_clk hibmc_dp_clk_table[] = {
+       {640, 480, 25175}, /* 25175 khz */
+       {800, 600, 40000}, /* 40000 khz */
+       {1024, 768, 65000}, /* 65000 khz */
+       {1152, 864, 80000}, /* 80000 khz */
+       {1280, 768, 79500}, /* 79500 khz */
+       {1280, 720, 74250}, /* 74250 khz */
+       {1280, 960, 108000}, /* 108000 khz */
+       {1280, 1024, 108000}, /* 108000 khz */
+       {1440, 900, 106500}, /* 106500 khz */
+       {1600, 900, 108000}, /* 108000 khz */
+       {1600, 1200, 162000}, /* 162000 khz */
+       {1920, 1080, 148500}, /* 148500 khz */
+       {1920, 1200, 193250}, /* 193250 khz */
+};

I generally think that these kind of tables are too limiting. Please try
to achieve the same by checking the clock with the PLL code instead of
checking the modes against the table.

Okay!

+
  static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
  {
        const struct drm_edid *drm_edid;
@@ -42,9 +64,38 @@ static int hibmc_dp_detect(struct drm_connector *connector,
        return connector_status_disconnected;
  }

+static int hibmc_dp_mode_valid(struct drm_connector *connector,

+                              const struct drm_display_mode *mode,
+                              struct drm_modeset_acquire_ctx *ctx,
+                              enum drm_mode_status *status)
+{
+       struct hibmc_dp *dp = to_hibmc_dp(connector);
+       u64 cur_val, max_val;
+
+       /* check DP link BW */
+       cur_val = (u64)mode->htotal * mode->vtotal * drm_mode_vrefresh(mode) * 
BPP_24;
+       max_val = (u64)hibmc_dp_get_link_rate(dp) * DP_MODE_VALI_CAL * 
hibmc_dp_get_lanes(dp);
+
+       *status = cur_val > max_val ? MODE_CLOCK_HIGH : MODE_OK;
+
+       /* check the clock */
+       for (size_t i = 0; i < ARRAY_SIZE(hibmc_dp_clk_table); i++) {
+               if (hibmc_dp_clk_table[i].hdisplay == mode->hdisplay &&
+                   hibmc_dp_clk_table[i].vdisplay == mode->vdisplay) {
+                       if (hibmc_dp_clk_table[i].clock != mode->clock) {
+                               *status = MODE_CLOCK_RANGE;
+                               return 0;
+                       }
+               }
+       }
+
+       return 0;
+}
+
  static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
        .get_modes = hibmc_dp_connector_get_modes,
        .detect_ctx = hibmc_dp_detect,
+       .mode_valid_ctx = hibmc_dp_mode_valid,
  };

static int hibmc_dp_late_register(struct drm_connector *connector)

--
2.33.0