[PATCH for drm-misc-fixes v6 0/4] Fix some bugs in the hibmc driver

[PATCH for drm-misc-fixes v6 0/4] Fix some bugs in the hibmc driver

[Yongbang Shi]

From: Lin He <helin52@huawei.com>

There are some bugfix for hibmc-drm driver.

---
ChangeLog:
v5 -> v6:
  - Rename 'phys_state' to 'phys_status'
  - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
    and adjust the timing of the call.
  - Split "return vdac->phys_state = state;" into separate statements.
  - Change `get_mode` to return 0 when no modes are available, allowing DRM to 
    install a default resolution.
  - Change the initial phys_status of vdac from
    'connector_status_connected' to 'connector_status_disconnected'
  - Removed invalid bit configurations from `hibmc_display_ctrl`.
  - Modify the return values of `hibmc_get_best_clock_idx` and
    `hibmc_crtc_mode_valid`
v4 -> v5:
  - The 'epoch_counter' of the vdac connector is incremented when the
	physical status changes.
  - The polling mechanism for the KMS helper is enabled.
v3 -> v4:
  - fix incorrect cover-letter subject prefix in v3.
v2 -> v3:
  - remove unused macro CLOCK_TOLERANCE.
v1 -> v2:
  - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
  - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
  - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
    be given in public.
  - add 'drm-misc-fixes' in subject prefix.
---

Lin He (4):
  drm/hisilicon/hibmc: add updating link cap in DP detect()
  drm/hisilicon/hibmc: fix no showing when no connectors connected
  drm/hisilicon/hibmc: move display contrl config to hibmc_probe()
  drm/hisilicon/hibmc: use clock to look up the PLL value

 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  1 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  2 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 +++++---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 14 ++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 75 +++++++++--------
 8 files changed, 124 insertions(+), 85 deletions(-)

-- 
2.33.0

[PATCH for drm-misc-fixes v6 1/4] drm/hisilicon/hibmc: add updating link cap in DP detect()

[Yongbang Shi]

From: Lin He <helin52@huawei.com>

In the past, the link cap is updated in link training at encoder enable
stage, but the hibmc_dp_mode_valid() is called before it, which will use
DP link's rate and lanes. So add the hibmc_dp_update_caps() in
hibmc_dp_update_caps() to avoid some potential risks.

Fixes: 607805abfb74 ("drm/hisilicon/hibmc: add dp mode valid check")
Signed-off-by: Lin He <helin52@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v1 -> v2:
  - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
  - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
    be given in public.
  - add 'drm-misc-fixes' in subject prefix.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h   | 1 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c   | 2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
index f9ee7ebfec55..f53dac256ee0 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -69,5 +69,6 @@ int hibmc_dp_link_training(struct hibmc_dp_dev *dp);
 int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp);
 int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp);
 int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX]);
+void hibmc_dp_update_caps(struct hibmc_dp_dev *dp);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
index 0726cb5b736e..8c53f16db516 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
@@ -325,7 +325,7 @@ static int hibmc_dp_link_downgrade_training_eq(struct hibmc_dp_dev *dp)
 	return hibmc_dp_link_reduce_rate(dp);
 }
 
-static void hibmc_dp_update_caps(struct hibmc_dp_dev *dp)
+void hibmc_dp_update_caps(struct hibmc_dp_dev *dp)
 {
 	dp->link.cap.link_rate = dp->dpcd[DP_MAX_LINK_RATE];
 	if (dp->link.cap.link_rate > DP_LINK_BW_8_1 || !dp->link.cap.link_rate)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 616821e3c933..35dff7bfbf76 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -41,6 +41,8 @@ static bool hibmc_dp_get_dpcd(struct hibmc_dp_dev *dp_dev)
 	if (ret)
 		return false;
 
+	hibmc_dp_update_caps(dp_dev);
+
 	dp_dev->is_branch = drm_dp_is_branch(dp_dev->dpcd);
 
 	ret = drm_dp_read_desc(dp_dev->aux, &dp_dev->desc, dp_dev->is_branch);
-- 
2.33.0

Re: [PATCH for drm-misc-fixes v6 1/4] drm/hisilicon/hibmc: add updating link cap in DP detect()

[Thomas Zimmermann]

Am 09.05.26 um 05:22 schrieb Yongbang Shi:
> From: Lin He <helin52@huawei.com>
>
> In the past, the link cap is updated in link training at encoder enable
> stage, but the hibmc_dp_mode_valid() is called before it, which will use
> DP link's rate and lanes. So add the hibmc_dp_update_caps() in
> hibmc_dp_update_caps() to avoid some potential risks.
>
> Fixes: 607805abfb74 ("drm/hisilicon/hibmc: add dp mode valid check")
> Signed-off-by: Lin He <helin52@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> ChangeLog:
> v1 -> v2:
>    - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>      be given in public.
>    - add 'drm-misc-fixes' in subject prefix.
> ---
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h   | 1 +
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c   | 2 +-
>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 2 ++
>   3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> index f9ee7ebfec55..f53dac256ee0 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> @@ -69,5 +69,6 @@ int hibmc_dp_link_training(struct hibmc_dp_dev *dp);
>   int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp);
>   int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp);
>   int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX]);
> +void hibmc_dp_update_caps(struct hibmc_dp_dev *dp);
>   
>   #endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
> index 0726cb5b736e..8c53f16db516 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
> @@ -325,7 +325,7 @@ static int hibmc_dp_link_downgrade_training_eq(struct hibmc_dp_dev *dp)
>   	return hibmc_dp_link_reduce_rate(dp);
>   }
>   
> -static void hibmc_dp_update_caps(struct hibmc_dp_dev *dp)
> +void hibmc_dp_update_caps(struct hibmc_dp_dev *dp)
>   {
>   	dp->link.cap.link_rate = dp->dpcd[DP_MAX_LINK_RATE];
>   	if (dp->link.cap.link_rate > DP_LINK_BW_8_1 || !dp->link.cap.link_rate)
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> index 616821e3c933..35dff7bfbf76 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> @@ -41,6 +41,8 @@ static bool hibmc_dp_get_dpcd(struct hibmc_dp_dev *dp_dev)
>   	if (ret)
>   		return false;
>   
> +	hibmc_dp_update_caps(dp_dev);
> +
>   	dp_dev->is_branch = drm_dp_is_branch(dp_dev->dpcd);
>   
>   	ret = drm_dp_read_desc(dp_dev->aux, &dp_dev->desc, dp_dev->is_branch);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

[PATCH for drm-misc-fixes v6 2/4] drm/hisilicon/hibmc: fix no showing when no connectors connected

[Yongbang Shi]

From: Lin He <helin52@huawei.com>

Our chip support KVM over IP feature, so hibmc driver need to support
displaying without any connectors plugged in. If no connectors are
connected, the vdac connector status should be set to 'connected' to
ensure proper KVM display functionality. Additionally, for
previous-generation products that may lack hardware link support and
thus cannot detect the monitor, the same approach should be applied
to ensure VGA display functionality.

* Add phys_state in the struct of dp and vdac to check physical outputs.

* The 'epoch_counter' of the vdac connector is incremented when the
physical status changes.

For get_modes: using BMC modes for connector if no display is attached to
phys VGA cable, otherwise use EDID modes by drm_connector_helper_get_modes,
because KVM doesn't provide EDID reads.

The polling mechanism for the KMS helper is enabled.

Fixes: 4c962bc929f1 ("drm/hisilicon/hibmc: Add vga connector detect functions")
Reported-by: Thomas Zimmermann <tzimmermann@suse.de>
Closes: https://lore.kernel.org/all/0eb5c509-2724-4c57-87ad-74e4270d5a5a@suse.de/
Signed-off-by: Lin He <helin52@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v5 -> v6:
  - Rename 'phys_state' to 'phys_status'
  - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
    and adjust the timing of the call.
  - Split "return vdac->phys_state = state;" into separate statements.
  - Change `get_modes` to return 0 when no modes are available, allowing DRM to 
    install a default resolution.
  - Change the initial phys_status of vdac from
    'connector_status_connected' to 'connector_status_disconnected'
v4 -> v5:
  - The 'epoch_counter' of the vdac connector is incremented when the physical
    status changes.
  - The polling mechanism for the KMS helper is enabled.
v1 -> v2:
  - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
  - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
    be given in public.
  - add 'drm-misc-fixes' in subject prefix.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 33 ++++++++----
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  3 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 53 +++++++++++++------
 5 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 31316fe1ea8d..0f3662d8737e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -55,6 +55,7 @@ struct hibmc_dp {
 	struct drm_dp_aux aux;
 	struct hibmc_dp_cbar_cfg cfg;
 	u32 irq_status;
+	int phys_status;
 };
 
 int hibmc_dp_hw_init(struct hibmc_dp *dp);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 35dff7bfbf76..596c5bfe32d8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -61,27 +61,38 @@ static int hibmc_dp_detect(struct drm_connector *connector,
 {
 	struct hibmc_dp *dp = to_hibmc_dp(connector);
 	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
-	int ret;
+	int ret = connector_status_disconnected;
 
 	if (dp->irq_status) {
-		if (dp_dev->hpd_status != HIBMC_HPD_IN)
-			return connector_status_disconnected;
+		if (dp_dev->hpd_status != HIBMC_HPD_IN) {
+			ret = connector_status_disconnected;
+			goto exit;
+		}
 	}
 
-	if (!hibmc_dp_get_dpcd(dp_dev))
-		return connector_status_disconnected;
+	if (!hibmc_dp_get_dpcd(dp_dev)) {
+		ret = connector_status_disconnected;
+		goto exit;
+	}
 
-	if (!dp_dev->is_branch)
-		return connector_status_connected;
+	if (!dp_dev->is_branch) {
+		ret = connector_status_connected;
+		goto exit;
+	}
 
 	if (drm_dp_read_sink_count_cap(connector, dp_dev->dpcd, &dp_dev->desc) &&
 	    dp_dev->downstream_ports[0] & DP_DS_PORT_HPD) {
 		ret = drm_dp_read_sink_count(dp_dev->aux);
-		if (ret > 0)
-			return connector_status_connected;
+		if (ret > 0) {
+			ret = connector_status_connected;
+			goto exit;
+		}
 	}
 
-	return connector_status_disconnected;
+exit:
+	dp->phys_status = ret;
+
+	return ret;
 }
 
 static int hibmc_dp_mode_valid(struct drm_connector *connector,
@@ -243,5 +254,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
 
 	connector->polled = DRM_CONNECTOR_POLL_HPD;
 
+	dp->phys_status = connector_status_disconnected;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index a0ecf82b576f..e3b40c6a6250 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -25,6 +25,7 @@
 #include <drm/drm_managed.h>
 #include <drm/drm_module.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_probe_helper.h>
 
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
@@ -385,6 +386,8 @@ static int hibmc_load(struct drm_device *dev)
 	/* reset all the states of crtc/plane/encoder/connector */
 	drm_mode_config_reset(dev);
 
+	drmm_kms_helper_poll_init(dev);
+
 	return 0;
 
 err:
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 4e212af6143d..dce8572bf63e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -31,6 +31,7 @@ struct hibmc_vdac {
 	struct drm_connector connector;
 	struct i2c_adapter adapter;
 	struct i2c_algo_bit_data bit_data;
+	int phys_status;
 };
 
 struct hibmc_drm_private {
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 841e81f47b68..2fcfa3246fd1 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -24,28 +24,21 @@
 
 static int hibmc_connector_get_modes(struct drm_connector *connector)
 {
+	struct drm_mode_config *mode_config = &connector->dev->mode_config;
 	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
-	const struct drm_edid *drm_edid;
 	int count;
 
-	drm_edid = drm_edid_read_ddc(connector, &vdac->adapter);
-
-	drm_edid_connector_update(connector, drm_edid);
-
-	if (drm_edid) {
-		count = drm_edid_connector_add_modes(connector);
+	if (vdac->phys_status == connector_status_connected) {
+		count = drm_connector_helper_get_modes(connector);
+	} else {
+		drm_edid_connector_update(connector, NULL);
+		count = drm_add_modes_noedid(connector,
+					     mode_config->max_width,
+					     mode_config->max_height);
 		if (count)
-			goto out;
+			drm_set_preferred_mode(connector, 1024, 768);
 	}
 
-	count = drm_add_modes_noedid(connector,
-				     connector->dev->mode_config.max_width,
-				     connector->dev->mode_config.max_height);
-	drm_set_preferred_mode(connector, 1024, 768);
-
-out:
-	drm_edid_free(drm_edid);
-
 	return count;
 }
 
@@ -57,10 +50,34 @@ static void hibmc_connector_destroy(struct drm_connector *connector)
 	drm_connector_cleanup(connector);
 }
 
+static int hibmc_vdac_detect(struct drm_connector *connector,
+			     struct drm_modeset_acquire_ctx *ctx,
+			     bool force)
+{
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(connector->dev);
+	int status = drm_connector_helper_detect_from_ddc(connector, ctx,
+							 force);
+	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
+
+	if (priv->dp.phys_status == connector_status_connected) {
+		vdac->phys_status = status;
+		return status;
+	}
+
+	if (status != vdac->phys_status)
+		++connector->epoch_counter;
+	vdac->phys_status = status;
+
+	/* When both the DP and VDAC physical status are disconnected,
+	 * the "connected" status is returned to support KVM display.
+	 */
+	return connector_status_connected;
+}
+
 static const struct drm_connector_helper_funcs
 	hibmc_connector_helper_funcs = {
 	.get_modes = hibmc_connector_get_modes,
-	.detect_ctx = drm_connector_helper_detect_from_ddc,
+	.detect_ctx = hibmc_vdac_detect,
 };
 
 static const struct drm_connector_funcs hibmc_connector_funcs = {
@@ -130,6 +147,8 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 
 	connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
 
+	vdac->phys_status = connector_status_disconnected;
+
 	return 0;
 
 err:
-- 
2.33.0

Re: [PATCH for drm-misc-fixes v6 2/4] drm/hisilicon/hibmc: fix no showing when no connectors connected

[Thomas Zimmermann]

Am 09.05.26 um 05:23 schrieb Yongbang Shi:
> From: Lin He <helin52@huawei.com>
>
> Our chip support KVM over IP feature, so hibmc driver need to support
> displaying without any connectors plugged in. If no connectors are
> connected, the vdac connector status should be set to 'connected' to
> ensure proper KVM display functionality. Additionally, for
> previous-generation products that may lack hardware link support and
> thus cannot detect the monitor, the same approach should be applied
> to ensure VGA display functionality.
>
> * Add phys_state in the struct of dp and vdac to check physical outputs.
>
> * The 'epoch_counter' of the vdac connector is incremented when the
> physical status changes.
>
> For get_modes: using BMC modes for connector if no display is attached to
> phys VGA cable, otherwise use EDID modes by drm_connector_helper_get_modes,
> because KVM doesn't provide EDID reads.
>
> The polling mechanism for the KMS helper is enabled.
>
> Fixes: 4c962bc929f1 ("drm/hisilicon/hibmc: Add vga connector detect functions")
> Reported-by: Thomas Zimmermann <tzimmermann@suse.de>
> Closes: https://lore.kernel.org/all/0eb5c509-2724-4c57-87ad-74e4270d5a5a@suse.de/
> Signed-off-by: Lin He <helin52@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

This patch fixes the problem with my test device. I now get the default 
VGA resolutions, which seems like the correct solution to me.

Tested-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> ChangeLog:
> v5 -> v6:
>    - Rename 'phys_state' to 'phys_status'
>    - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
>      and adjust the timing of the call.
>    - Split "return vdac->phys_state = state;" into separate statements.
>    - Change `get_modes` to return 0 when no modes are available, allowing DRM to
>      install a default resolution.
>    - Change the initial phys_status of vdac from
>      'connector_status_connected' to 'connector_status_disconnected'
> v4 -> v5:
>    - The 'epoch_counter' of the vdac connector is incremented when the physical
>      status changes.
>    - The polling mechanism for the KMS helper is enabled.
> v1 -> v2:
>    - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>      be given in public.
>    - add 'drm-misc-fixes' in subject prefix.
> ---
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 33 ++++++++----
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  3 ++
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 53 +++++++++++++------
>   5 files changed, 64 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> index 31316fe1ea8d..0f3662d8737e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> @@ -55,6 +55,7 @@ struct hibmc_dp {
>   	struct drm_dp_aux aux;
>   	struct hibmc_dp_cbar_cfg cfg;
>   	u32 irq_status;
> +	int phys_status;
>   };
>   
>   int hibmc_dp_hw_init(struct hibmc_dp *dp);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> index 35dff7bfbf76..596c5bfe32d8 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> @@ -61,27 +61,38 @@ static int hibmc_dp_detect(struct drm_connector *connector,
>   {
>   	struct hibmc_dp *dp = to_hibmc_dp(connector);
>   	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
> -	int ret;
> +	int ret = connector_status_disconnected;
>   
>   	if (dp->irq_status) {
> -		if (dp_dev->hpd_status != HIBMC_HPD_IN)
> -			return connector_status_disconnected;
> +		if (dp_dev->hpd_status != HIBMC_HPD_IN) {
> +			ret = connector_status_disconnected;
> +			goto exit;
> +		}
>   	}
>   
> -	if (!hibmc_dp_get_dpcd(dp_dev))
> -		return connector_status_disconnected;
> +	if (!hibmc_dp_get_dpcd(dp_dev)) {
> +		ret = connector_status_disconnected;
> +		goto exit;
> +	}
>   
> -	if (!dp_dev->is_branch)
> -		return connector_status_connected;
> +	if (!dp_dev->is_branch) {
> +		ret = connector_status_connected;
> +		goto exit;
> +	}
>   
>   	if (drm_dp_read_sink_count_cap(connector, dp_dev->dpcd, &dp_dev->desc) &&
>   	    dp_dev->downstream_ports[0] & DP_DS_PORT_HPD) {
>   		ret = drm_dp_read_sink_count(dp_dev->aux);
> -		if (ret > 0)
> -			return connector_status_connected;
> +		if (ret > 0) {
> +			ret = connector_status_connected;
> +			goto exit;
> +		}
>   	}
>   
> -	return connector_status_disconnected;
> +exit:
> +	dp->phys_status = ret;
> +
> +	return ret;
>   }
>   
>   static int hibmc_dp_mode_valid(struct drm_connector *connector,
> @@ -243,5 +254,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
>   
>   	connector->polled = DRM_CONNECTOR_POLL_HPD;
>   
> +	dp->phys_status = connector_status_disconnected;
> +
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index a0ecf82b576f..e3b40c6a6250 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -25,6 +25,7 @@
>   #include <drm/drm_managed.h>
>   #include <drm/drm_module.h>
>   #include <drm/drm_vblank.h>
> +#include <drm/drm_probe_helper.h>
>   
>   #include "hibmc_drm_drv.h"
>   #include "hibmc_drm_regs.h"
> @@ -385,6 +386,8 @@ static int hibmc_load(struct drm_device *dev)
>   	/* reset all the states of crtc/plane/encoder/connector */
>   	drm_mode_config_reset(dev);
>   
> +	drmm_kms_helper_poll_init(dev);
> +
>   	return 0;
>   
>   err:
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 4e212af6143d..dce8572bf63e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -31,6 +31,7 @@ struct hibmc_vdac {
>   	struct drm_connector connector;
>   	struct i2c_adapter adapter;
>   	struct i2c_algo_bit_data bit_data;
> +	int phys_status;
>   };
>   
>   struct hibmc_drm_private {
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index 841e81f47b68..2fcfa3246fd1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -24,28 +24,21 @@
>   
>   static int hibmc_connector_get_modes(struct drm_connector *connector)
>   {
> +	struct drm_mode_config *mode_config = &connector->dev->mode_config;
>   	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
> -	const struct drm_edid *drm_edid;
>   	int count;
>   
> -	drm_edid = drm_edid_read_ddc(connector, &vdac->adapter);
> -
> -	drm_edid_connector_update(connector, drm_edid);
> -
> -	if (drm_edid) {
> -		count = drm_edid_connector_add_modes(connector);
> +	if (vdac->phys_status == connector_status_connected) {
> +		count = drm_connector_helper_get_modes(connector);
> +	} else {
> +		drm_edid_connector_update(connector, NULL);
> +		count = drm_add_modes_noedid(connector,
> +					     mode_config->max_width,
> +					     mode_config->max_height);
>   		if (count)
> -			goto out;
> +			drm_set_preferred_mode(connector, 1024, 768);
>   	}
>   
> -	count = drm_add_modes_noedid(connector,
> -				     connector->dev->mode_config.max_width,
> -				     connector->dev->mode_config.max_height);
> -	drm_set_preferred_mode(connector, 1024, 768);
> -
> -out:
> -	drm_edid_free(drm_edid);
> -
>   	return count;
>   }
>   
> @@ -57,10 +50,34 @@ static void hibmc_connector_destroy(struct drm_connector *connector)
>   	drm_connector_cleanup(connector);
>   }
>   
> +static int hibmc_vdac_detect(struct drm_connector *connector,
> +			     struct drm_modeset_acquire_ctx *ctx,
> +			     bool force)
> +{
> +	struct hibmc_drm_private *priv = to_hibmc_drm_private(connector->dev);
> +	int status = drm_connector_helper_detect_from_ddc(connector, ctx,
> +							 force);
> +	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
> +
> +	if (priv->dp.phys_status == connector_status_connected) {
> +		vdac->phys_status = status;
> +		return status;
> +	}
> +
> +	if (status != vdac->phys_status)
> +		++connector->epoch_counter;
> +	vdac->phys_status = status;
> +
> +	/* When both the DP and VDAC physical status are disconnected,
> +	 * the "connected" status is returned to support KVM display.
> +	 */
> +	return connector_status_connected;
> +}
> +
>   static const struct drm_connector_helper_funcs
>   	hibmc_connector_helper_funcs = {
>   	.get_modes = hibmc_connector_get_modes,
> -	.detect_ctx = drm_connector_helper_detect_from_ddc,
> +	.detect_ctx = hibmc_vdac_detect,
>   };
>   
>   static const struct drm_connector_funcs hibmc_connector_funcs = {
> @@ -130,6 +147,8 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>   
>   	connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
>   
> +	vdac->phys_status = connector_status_disconnected;
> +
>   	return 0;
>   
>   err:

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

[PATCH for drm-misc-fixes v6 3/4] drm/hisilicon/hibmc: move display contrl config to hibmc_probe()

[Yongbang Shi]

From: Lin He <helin52@huawei.com>

If there's no VGA output, this encoder modeset won't be called, which
will cause displaying data from GPU being cut off. It's actually a
common display config for DP and VGA, so move the vdac encoder modeset
to driver load stage.

Removed invalid bit configurations from `hibmc_display_ctrl`

Fixes: 5294967f4ae4 ("drm/hisilicon/hibmc: Add support for VDAC")
Signed-off-by: Lin He <helin52@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v5 -> v6:
  - Removed invalid bit configurations from `hibmc_display_ctrl`.
v1 -> v2:
  - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
    be given in public.
  - add 'drm-misc-fixes' in subject prefix.
---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 11 ++++++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 22 -------------------
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index e3b40c6a6250..19d3193e2f76 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -241,6 +241,15 @@ void hibmc_set_current_gate(struct hibmc_drm_private *priv, unsigned int gate)
 	writel(gate, mmio + gate_reg);
 }
 
+static void hibmc_display_ctrl(struct hibmc_drm_private *priv)
+{
+	u32 reg;
+
+	reg = readl(priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
+	reg |= HIBMC_DISPLAY_CONTROL_PANELDATE(1);
+	writel(reg, priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
+}
+
 static void hibmc_hw_config(struct hibmc_drm_private *priv)
 {
 	u32 reg;
@@ -272,6 +281,8 @@ static void hibmc_hw_config(struct hibmc_drm_private *priv)
 	reg |= HIBMC_MSCCTL_LOCALMEM_RESET(1);
 
 	writel(reg, priv->mmio + HIBMC_MISC_CTRL);
+
+	hibmc_display_ctrl(priv);
 }
 
 static int hibmc_hw_map(struct hibmc_drm_private *priv)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 2fcfa3246fd1..b9bd6d33fb0f 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -88,26 +88,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
-				   struct drm_display_mode *mode,
-				   struct drm_display_mode *adj_mode)
-{
-	u32 reg;
-	struct drm_device *dev = encoder->dev;
-	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
-
-	reg = readl(priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
-	reg |= HIBMC_DISPLAY_CONTROL_FPVDDEN(1);
-	reg |= HIBMC_DISPLAY_CONTROL_PANELDATE(1);
-	reg |= HIBMC_DISPLAY_CONTROL_FPEN(1);
-	reg |= HIBMC_DISPLAY_CONTROL_VBIASEN(1);
-	writel(reg, priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
-}
-
-static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
-	.mode_set = hibmc_encoder_mode_set,
-};
-
 int hibmc_vdac_init(struct hibmc_drm_private *priv)
 {
 	struct drm_device *dev = &priv->dev;
@@ -130,8 +110,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 		goto err;
 	}
 
-	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
-
 	ret = drm_connector_init_with_ddc(dev, connector,
 					  &hibmc_connector_funcs,
 					  DRM_MODE_CONNECTOR_VGA,
-- 
2.33.0

Re: [PATCH for drm-misc-fixes v6 3/4] drm/hisilicon/hibmc: move display contrl config to hibmc_probe()

[Thomas Zimmermann]

Am 09.05.26 um 05:23 schrieb Yongbang Shi:
> From: Lin He <helin52@huawei.com>
>
> If there's no VGA output, this encoder modeset won't be called, which
> will cause displaying data from GPU being cut off. It's actually a
> common display config for DP and VGA, so move the vdac encoder modeset
> to driver load stage.
>
> Removed invalid bit configurations from `hibmc_display_ctrl`
>
> Fixes: 5294967f4ae4 ("drm/hisilicon/hibmc: Add support for VDAC")
> Signed-off-by: Lin He <helin52@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> ChangeLog:
> v5 -> v6:
>    - Removed invalid bit configurations from `hibmc_display_ctrl`.
> v1 -> v2:
>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>      be given in public.
>    - add 'drm-misc-fixes' in subject prefix.
> ---
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 11 ++++++++++
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 22 -------------------
>   2 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index e3b40c6a6250..19d3193e2f76 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -241,6 +241,15 @@ void hibmc_set_current_gate(struct hibmc_drm_private *priv, unsigned int gate)
>   	writel(gate, mmio + gate_reg);
>   }
>   
> +static void hibmc_display_ctrl(struct hibmc_drm_private *priv)
> +{
> +	u32 reg;
> +
> +	reg = readl(priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
> +	reg |= HIBMC_DISPLAY_CONTROL_PANELDATE(1);
> +	writel(reg, priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
> +}
> +
>   static void hibmc_hw_config(struct hibmc_drm_private *priv)
>   {
>   	u32 reg;
> @@ -272,6 +281,8 @@ static void hibmc_hw_config(struct hibmc_drm_private *priv)
>   	reg |= HIBMC_MSCCTL_LOCALMEM_RESET(1);
>   
>   	writel(reg, priv->mmio + HIBMC_MISC_CTRL);
> +
> +	hibmc_display_ctrl(priv);
>   }
>   
>   static int hibmc_hw_map(struct hibmc_drm_private *priv)
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index 2fcfa3246fd1..b9bd6d33fb0f 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -88,26 +88,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>   
> -static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
> -				   struct drm_display_mode *mode,
> -				   struct drm_display_mode *adj_mode)
> -{
> -	u32 reg;
> -	struct drm_device *dev = encoder->dev;
> -	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
> -
> -	reg = readl(priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
> -	reg |= HIBMC_DISPLAY_CONTROL_FPVDDEN(1);
> -	reg |= HIBMC_DISPLAY_CONTROL_PANELDATE(1);
> -	reg |= HIBMC_DISPLAY_CONTROL_FPEN(1);
> -	reg |= HIBMC_DISPLAY_CONTROL_VBIASEN(1);
> -	writel(reg, priv->mmio + HIBMC_DISPLAY_CONTROL_HISILE);
> -}
> -
> -static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
> -	.mode_set = hibmc_encoder_mode_set,
> -};
> -
>   int hibmc_vdac_init(struct hibmc_drm_private *priv)
>   {
>   	struct drm_device *dev = &priv->dev;
> @@ -130,8 +110,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>   		goto err;
>   	}
>   
> -	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> -
>   	ret = drm_connector_init_with_ddc(dev, connector,
>   					  &hibmc_connector_funcs,
>   					  DRM_MODE_CONNECTOR_VGA,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

[PATCH for drm-misc-fixes v6 4/4] drm/hisilicon/hibmc: use clock to look up the PLL value

[Yongbang Shi]

From: Lin He <helin52@huawei.com>

In the past, we use width and height to look up our PLL value.
But actually the actual clock check is also necessnary. There are
some resolutions that width and height same, but its clock different.
Add the clock check when using pll_table to determine the PLL value.

Fixes: da52605eea8f ("drm/hisilicon/hibmc: Add support for display engine")
Signed-off-by: Lin He <helin52@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v5 -> v6:
  - Modify the return values of `hibmc_get_best_clock_idx` and
    `hibmc_crtc_mode_valid`
v2 -> v3:
  - remove unused macro CLOCK_TOLERANCE.
v1 -> v2:
  - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
    be given in public.
  - add 'drm-misc-fixes' in subject prefix.
---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index fd3f05ba62df..583670c7b60e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -34,26 +34,43 @@ struct hibmc_display_panel_pll {
 struct hibmc_dislay_pll_config {
 	u64 hdisplay;
 	u64 vdisplay;
+	int clock;
 	u32 pll1_config_value;
 	u32 pll2_config_value;
 };
 
 static const struct hibmc_dislay_pll_config hibmc_pll_table[] = {
-	{640, 480, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ},
-	{800, 600, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ},
-	{1024, 768, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ},
-	{1152, 864, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ},
-	{1280, 768, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ},
-	{1280, 720, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ},
-	{1280, 960, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
-	{1280, 1024, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
-	{1440, 900, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ},
-	{1600, 900, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
-	{1600, 1200, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ},
-	{1920, 1080, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ},
-	{1920, 1200, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ},
+	{640, 480, 25000, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ},
+	{800, 600, 40000, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ},
+	{1024, 768, 65000, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ},
+	{1152, 864, 78750, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ},
+	{1280, 768, 80000, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ},
+	{1280, 720, 74375, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ},
+	{1280, 960, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
+	{1280, 1024, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
+	{1440, 900, 105952, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ},
+	{1600, 900, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
+	{1600, 1200, 162500, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ},
+	{1920, 1080, 148750, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ},
+	{1920, 1200, 193750, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ},
 };
 
+static int hibmc_get_best_clock_idx(const struct drm_display_mode *mode)
+{
+	int i, diff;
+
+	for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) {
+		if (hibmc_pll_table[i].hdisplay == mode->hdisplay &&
+		    hibmc_pll_table[i].vdisplay == mode->vdisplay) {
+			diff = abs(mode->clock - hibmc_pll_table[i].clock);
+			if (diff < mode->clock / 100) /* tolerance 1/100 */
+				return i;
+		}
+	}
+
+	return -MODE_CLOCK_RANGE;
+}
+
 static int hibmc_plane_atomic_check(struct drm_plane *plane,
 				    struct drm_atomic_state *state)
 {
@@ -213,19 +230,15 @@ static enum drm_mode_status
 hibmc_crtc_mode_valid(struct drm_crtc *crtc,
 		      const struct drm_display_mode *mode)
 {
-	size_t i = 0;
 	int vrefresh = drm_mode_vrefresh(mode);
 
 	if (vrefresh < 59 || vrefresh > 61)
 		return MODE_NOCLOCK;
 
-	for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) {
-		if (hibmc_pll_table[i].hdisplay == mode->hdisplay &&
-		    hibmc_pll_table[i].vdisplay == mode->vdisplay)
-			return MODE_OK;
-	}
+	if (hibmc_get_best_clock_idx(mode) >= 0)
+		return MODE_OK;
 
-	return MODE_BAD;
+	return MODE_CLOCK_RANGE;
 }
 
 static u32 format_pll_reg(void)
@@ -280,23 +293,20 @@ static void set_vclock_hisilicon(struct drm_device *dev, u64 pll)
 	writel(val, priv->mmio + CRT_PLL1_HS);
 }
 
-static void get_pll_config(u64 x, u64 y, u32 *pll1, u32 *pll2)
+static void get_pll_config(struct drm_display_mode *mode, u32 *pll1, u32 *pll2)
 {
-	size_t i;
-	size_t count = ARRAY_SIZE(hibmc_pll_table);
-
-	for (i = 0; i < count; i++) {
-		if (hibmc_pll_table[i].hdisplay == x &&
-		    hibmc_pll_table[i].vdisplay == y) {
-			*pll1 = hibmc_pll_table[i].pll1_config_value;
-			*pll2 = hibmc_pll_table[i].pll2_config_value;
-			return;
-		}
+	int idx;
+
+	idx = hibmc_get_best_clock_idx(mode);
+	if (idx < 0) {
+		/* if found none, we use default value */
+		*pll1 = CRT_PLL1_HS_25MHZ;
+		*pll2 = CRT_PLL2_HS_25MHZ;
+		return;
 	}
 
-	/* if found none, we use default value */
-	*pll1 = CRT_PLL1_HS_25MHZ;
-	*pll2 = CRT_PLL2_HS_25MHZ;
+	*pll1 = hibmc_pll_table[idx].pll1_config_value;
+	*pll2 = hibmc_pll_table[idx].pll2_config_value;
 }
 
 /*
@@ -318,7 +328,7 @@ static u32 display_ctrl_adjust(struct drm_device *dev,
 	x = mode->hdisplay;
 	y = mode->vdisplay;
 
-	get_pll_config(x, y, &pll1, &pll2);
+	get_pll_config(mode, &pll1, &pll2);
 	writel(pll2, priv->mmio + CRT_PLL2_HS);
 	set_vclock_hisilicon(dev, pll1);
 
-- 
2.33.0

Re: [PATCH for drm-misc-fixes v6 4/4] drm/hisilicon/hibmc: use clock to look up the PLL value

[Thomas Zimmermann]

Am 09.05.26 um 05:23 schrieb Yongbang Shi:
> From: Lin He <helin52@huawei.com>
>
> In the past, we use width and height to look up our PLL value.
> But actually the actual clock check is also necessnary. There are
> some resolutions that width and height same, but its clock different.
> Add the clock check when using pll_table to determine the PLL value.
>
> Fixes: da52605eea8f ("drm/hisilicon/hibmc: Add support for display engine")
> Signed-off-by: Lin He <helin52@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> ChangeLog:
> v5 -> v6:
>    - Modify the return values of `hibmc_get_best_clock_idx` and
>      `hibmc_crtc_mode_valid`
> v2 -> v3:
>    - remove unused macro CLOCK_TOLERANCE.
> v1 -> v2:
>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>      be given in public.
>    - add 'drm-misc-fixes' in subject prefix.
> ---
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
>   1 file changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index fd3f05ba62df..583670c7b60e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -34,26 +34,43 @@ struct hibmc_display_panel_pll {
>   struct hibmc_dislay_pll_config {
>   	u64 hdisplay;
>   	u64 vdisplay;
> +	int clock;
>   	u32 pll1_config_value;
>   	u32 pll2_config_value;
>   };
>   
>   static const struct hibmc_dislay_pll_config hibmc_pll_table[] = {
> -	{640, 480, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ},
> -	{800, 600, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ},
> -	{1024, 768, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ},
> -	{1152, 864, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ},
> -	{1280, 768, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ},
> -	{1280, 720, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ},
> -	{1280, 960, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> -	{1280, 1024, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> -	{1440, 900, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ},
> -	{1600, 900, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> -	{1600, 1200, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ},
> -	{1920, 1080, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ},
> -	{1920, 1200, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ},
> +	{640, 480, 25000, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ},
> +	{800, 600, 40000, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ},
> +	{1024, 768, 65000, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ},
> +	{1152, 864, 78750, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ},
> +	{1280, 768, 80000, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ},
> +	{1280, 720, 74375, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ},
> +	{1280, 960, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> +	{1280, 1024, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> +	{1440, 900, 105952, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ},
> +	{1600, 900, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ},
> +	{1600, 1200, 162500, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ},
> +	{1920, 1080, 148750, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ},
> +	{1920, 1200, 193750, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ},
>   };
>   
> +static int hibmc_get_best_clock_idx(const struct drm_display_mode *mode)
> +{
> +	int i, diff;
> +
> +	for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) {
> +		if (hibmc_pll_table[i].hdisplay == mode->hdisplay &&
> +		    hibmc_pll_table[i].vdisplay == mode->vdisplay) {
> +			diff = abs(mode->clock - hibmc_pll_table[i].clock);
> +			if (diff < mode->clock / 100) /* tolerance 1/100 */
> +				return i;
> +		}
> +	}
> +
> +	return -MODE_CLOCK_RANGE;
> +}
> +
>   static int hibmc_plane_atomic_check(struct drm_plane *plane,
>   				    struct drm_atomic_state *state)
>   {
> @@ -213,19 +230,15 @@ static enum drm_mode_status
>   hibmc_crtc_mode_valid(struct drm_crtc *crtc,
>   		      const struct drm_display_mode *mode)
>   {
> -	size_t i = 0;
>   	int vrefresh = drm_mode_vrefresh(mode);
>   
>   	if (vrefresh < 59 || vrefresh > 61)
>   		return MODE_NOCLOCK;
>   
> -	for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) {
> -		if (hibmc_pll_table[i].hdisplay == mode->hdisplay &&
> -		    hibmc_pll_table[i].vdisplay == mode->vdisplay)
> -			return MODE_OK;
> -	}
> +	if (hibmc_get_best_clock_idx(mode) >= 0)
> +		return MODE_OK;
>   
> -	return MODE_BAD;
> +	return MODE_CLOCK_RANGE;
>   }
>   
>   static u32 format_pll_reg(void)
> @@ -280,23 +293,20 @@ static void set_vclock_hisilicon(struct drm_device *dev, u64 pll)
>   	writel(val, priv->mmio + CRT_PLL1_HS);
>   }
>   
> -static void get_pll_config(u64 x, u64 y, u32 *pll1, u32 *pll2)
> +static void get_pll_config(struct drm_display_mode *mode, u32 *pll1, u32 *pll2)
>   {
> -	size_t i;
> -	size_t count = ARRAY_SIZE(hibmc_pll_table);
> -
> -	for (i = 0; i < count; i++) {
> -		if (hibmc_pll_table[i].hdisplay == x &&
> -		    hibmc_pll_table[i].vdisplay == y) {
> -			*pll1 = hibmc_pll_table[i].pll1_config_value;
> -			*pll2 = hibmc_pll_table[i].pll2_config_value;
> -			return;
> -		}
> +	int idx;
> +
> +	idx = hibmc_get_best_clock_idx(mode);
> +	if (idx < 0) {
> +		/* if found none, we use default value */
> +		*pll1 = CRT_PLL1_HS_25MHZ;
> +		*pll2 = CRT_PLL2_HS_25MHZ;
> +		return;
>   	}
>   
> -	/* if found none, we use default value */
> -	*pll1 = CRT_PLL1_HS_25MHZ;
> -	*pll2 = CRT_PLL2_HS_25MHZ;
> +	*pll1 = hibmc_pll_table[idx].pll1_config_value;
> +	*pll2 = hibmc_pll_table[idx].pll2_config_value;
>   }
>   
>   /*
> @@ -318,7 +328,7 @@ static u32 display_ctrl_adjust(struct drm_device *dev,
>   	x = mode->hdisplay;
>   	y = mode->vdisplay;
>   
> -	get_pll_config(x, y, &pll1, &pll2);
> +	get_pll_config(mode, &pll1, &pll2);
>   	writel(pll2, priv->mmio + CRT_PLL2_HS);
>   	set_vclock_hisilicon(dev, pll1);
>   

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH for drm-misc-fixes v6 0/4] Fix some bugs in the hibmc driver

[Thomas Zimmermann]

Hi,

I've gone through the series and reviewed the patches. Let me know when 
they are ready to get merged.

Best regards
Thomas


Am 09.05.26 um 05:22 schrieb Yongbang Shi:
> From: Lin He <helin52@huawei.com>
>
> There are some bugfix for hibmc-drm driver.
>
> ---
> ChangeLog:
> v5 -> v6:
>    - Rename 'phys_state' to 'phys_status'
>    - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
>      and adjust the timing of the call.
>    - Split "return vdac->phys_state = state;" into separate statements.
>    - Change `get_mode` to return 0 when no modes are available, allowing DRM to
>      install a default resolution.
>    - Change the initial phys_status of vdac from
>      'connector_status_connected' to 'connector_status_disconnected'
>    - Removed invalid bit configurations from `hibmc_display_ctrl`.
>    - Modify the return values of `hibmc_get_best_clock_idx` and
>      `hibmc_crtc_mode_valid`
> v4 -> v5:
>    - The 'epoch_counter' of the vdac connector is incremented when the
> 	physical status changes.
>    - The polling mechanism for the KMS helper is enabled.
> v3 -> v4:
>    - fix incorrect cover-letter subject prefix in v3.
> v2 -> v3:
>    - remove unused macro CLOCK_TOLERANCE.
> v1 -> v2:
>    - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
>    - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>      be given in public.
>    - add 'drm-misc-fixes' in subject prefix.
> ---
>
> Lin He (4):
>    drm/hisilicon/hibmc: add updating link cap in DP detect()
>    drm/hisilicon/hibmc: fix no showing when no connectors connected
>    drm/hisilicon/hibmc: move display contrl config to hibmc_probe()
>    drm/hisilicon/hibmc: use clock to look up the PLL value
>
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  1 +
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  2 +-
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 +++++---
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 14 ++++
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 75 +++++++++--------
>   8 files changed, 124 insertions(+), 85 deletions(-)
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH for drm-misc-fixes v6 0/4] Fix some bugs in the hibmc driver

[Yongbang Shi]

> Hi,
> 
> I've gone through the series and reviewed the patches. Let me know when they are ready to get merged.
>

Thanks for the review. The series is ready for merging now.

Thanks,
Yongbang.


> Best regards
> Thomas
> 
> 
> Am 09.05.26 um 05:22 schrieb Yongbang Shi:
>> From: Lin He <helin52@huawei.com>
>>
>> There are some bugfix for hibmc-drm driver.
>>
>> ---
>> ChangeLog:
>> v5 -> v6:
>>    - Rename 'phys_state' to 'phys_status'
>>    - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
>>      and adjust the timing of the call.
>>    - Split "return vdac->phys_state = state;" into separate statements.
>>    - Change `get_mode` to return 0 when no modes are available, allowing DRM to
>>      install a default resolution.
>>    - Change the initial phys_status of vdac from
>>      'connector_status_connected' to 'connector_status_disconnected'
>>    - Removed invalid bit configurations from `hibmc_display_ctrl`.
>>    - Modify the return values of `hibmc_get_best_clock_idx` and
>>      `hibmc_crtc_mode_valid`
>> v4 -> v5:
>>    - The 'epoch_counter' of the vdac connector is incremented when the
>>     physical status changes.
>>    - The polling mechanism for the KMS helper is enabled.
>> v3 -> v4:
>>    - fix incorrect cover-letter subject prefix in v3.
>> v2 -> v3:
>>    - remove unused macro CLOCK_TOLERANCE.
>> v1 -> v2:
>>    - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
>>    - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
>>    - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>>      be given in public.
>>    - add 'drm-misc-fixes' in subject prefix.
>> ---
>>
>> Lin He (4):
>>    drm/hisilicon/hibmc: add updating link cap in DP detect()
>>    drm/hisilicon/hibmc: fix no showing when no connectors connected
>>    drm/hisilicon/hibmc: move display contrl config to hibmc_probe()
>>    drm/hisilicon/hibmc: use clock to look up the PLL value
>>
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  1 +
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  2 +-
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 +++++---
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 14 ++++
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 75 +++++++++--------
>>   8 files changed, 124 insertions(+), 85 deletions(-)
>>
> 

Re: [PATCH for drm-misc-fixes v6 0/4] Fix some bugs in the hibmc driver

[Thomas Zimmermann]

Hi

Am 12.05.26 um 14:12 schrieb Yongbang Shi:
>> Hi,
>>
>> I've gone through the series and reviewed the patches. Let me know when they are ready to get merged.
>>
> Thanks for the review. The series is ready for merging now.

Thanks for sticking with it. The series is now merged into drm-misc-next.

Best regards
Thomas

>
> Thanks,
> Yongbang.
>
>
>> Best regards
>> Thomas
>>
>>
>> Am 09.05.26 um 05:22 schrieb Yongbang Shi:
>>> From: Lin He <helin52@huawei.com>
>>>
>>> There are some bugfix for hibmc-drm driver.
>>>
>>> ---
>>> ChangeLog:
>>> v5 -> v6:
>>>     - Rename 'phys_state' to 'phys_status'
>>>     - Replace `drm_kms_helper_poll_init` with `drmm_kms_helper_poll_init`
>>>       and adjust the timing of the call.
>>>     - Split "return vdac->phys_state = state;" into separate statements.
>>>     - Change `get_mode` to return 0 when no modes are available, allowing DRM to
>>>       install a default resolution.
>>>     - Change the initial phys_status of vdac from
>>>       'connector_status_connected' to 'connector_status_disconnected'
>>>     - Removed invalid bit configurations from `hibmc_display_ctrl`.
>>>     - Modify the return values of `hibmc_get_best_clock_idx` and
>>>       `hibmc_crtc_mode_valid`
>>> v4 -> v5:
>>>     - The 'epoch_counter' of the vdac connector is incremented when the
>>>      physical status changes.
>>>     - The polling mechanism for the KMS helper is enabled.
>>> v3 -> v4:
>>>     - fix incorrect cover-letter subject prefix in v3.
>>> v2 -> v3:
>>>     - remove unused macro CLOCK_TOLERANCE.
>>> v1 -> v2:
>>>     - fix the checkpatch.pl warning "Unknown commit ID 'e6c7c59da494'".
>>>     - fix the checkpatch.pl warning "line length of 83/85 exceeds 80 columns".
>>>     - remove tag "Reviewed-by: Tao Tian <tiantao6@hisilicon.com>", witch will
>>>       be given in public.
>>>     - add 'drm-misc-fixes' in subject prefix.
>>> ---
>>>
>>> Lin He (4):
>>>     drm/hisilicon/hibmc: add updating link cap in DP detect()
>>>     drm/hisilicon/hibmc: fix no showing when no connectors connected
>>>     drm/hisilicon/hibmc: move display contrl config to hibmc_probe()
>>>     drm/hisilicon/hibmc: use clock to look up the PLL value
>>>
>>>    drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  1 +
>>>    drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  1 +
>>>    drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  2 +-
>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++--------
>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 +++++---
>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 14 ++++
>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  1 +
>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 75 +++++++++--------
>>>    8 files changed, 124 insertions(+), 85 deletions(-)
>>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)