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

[Yongbang Shi <shiyongbang@huawei.com>]

> On Fri, Jun 20, 2025 at 05:31:00PM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@huawei.com>
>>
>> If DP is connected, add mode check and BW check in mode_valid_ctx() to
>> ensure DP's cfg is usable.
>>
>> Fixes: f9698f802e50 ("drm/hisilicon/hibmc: Restructuring the header dp_reg.h")
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>> ---
>> ChangeLog:
>> v1 -> v2:
>>    - delete if (!dp->is_connected) in hibmc_dp_mode_valid(), suggested by Dmitry Baryshkov.
>> ---
>>   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 98cc534ba794..5b1f943b601c 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>> @@ -273,6 +273,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 c0de796225b7..40f95880b278 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 */
>> +};
>> +
>>   static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>>   {
>>   	const struct drm_edid *drm_edid;
>> @@ -49,9 +71,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 */
> Why? Is it really fixed to the values in the table?

Right, our chip spec said that one resolution can only support one clock, and we only support 11 clocks
above in the struct hibmc_dp_disp_clk{}, which is the limited of our display block.

Thanks,
Baihan.


>> +	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
>>