linux-kernel - Re:[PATCH] drm/rockchip: cdn-dp: Remove redundant workarounds for firmware loading

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <109c6f19.2559.1907b817a99.Coremail.andyshrk@163.com>
Date: Thu, 4 Jul 2024 10:10:01 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Dragan Simic" <dsimic@...jaro.org>
Cc: linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org, 
	heiko@...ech.de, hjc@...k-chips.com, andy.yan@...k-chips.com, 
	maarten.lankhorst@...ux.intel.com, mripard@...nel.org, 
	tzimmermann@...e.de, airlied@...il.com, daniel@...ll.ch, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re:[PATCH] drm/rockchip: cdn-dp: Remove redundant workarounds for
 firmware loading


Hi Dragan，

At 2024-07-04 07:32:02, "Dragan Simic" <dsimic@...jaro.org> wrote:
>After the additional firmware-related module information was introduced by
>the commit c0677e41a47f ("drm/rockchip: cdn-dp-core: add MODULE_FIRMWARE
>macro"), there's no longer need for the firmware-loading workarounds whose
>sole purpose was to prevent the missing firmware blob in an initial ramdisk
>from causing driver initialization to fail.  Thus, delete the workarounds,
>which removes a sizable chunk of redundant code.

What would happen if there was no ramdisk? And the firmware is in rootfs ？

For example： A buildroot based tiny embedded system。


>
>Various utilities used by Linux distributions to generate initial ramdisks
>need to obey the firmware-related module information, so we can rely on the
>firmware blob being present in the generated initial ramdisks.
>
>Signed-off-by: Dragan Simic <dsimic@...jaro.org>
>---
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 53 +++++---------------------
> 1 file changed, 10 insertions(+), 43 deletions(-)
>
>diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>index bd7aa891b839..e1a7c6a1172b 100644
>--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>@@ -44,9 +44,9 @@ static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder)
> #define DPTX_HPD_DEL		(2 << 12)
> #define DPTX_HPD_SEL_MASK	(3 << 28)
> 
>-#define CDN_FW_TIMEOUT_MS	(64 * 1000)
> #define CDN_DPCD_TIMEOUT_MS	5000
> #define CDN_DP_FIRMWARE		"rockchip/dptx.bin"
>+
> MODULE_FIRMWARE(CDN_DP_FIRMWARE);
> 
> struct cdn_dp_data {
>@@ -909,61 +909,28 @@ static int cdn_dp_audio_codec_init(struct cdn_dp_device *dp,
> 	return PTR_ERR_OR_ZERO(dp->audio_pdev);
> }
> 
>-static int cdn_dp_request_firmware(struct cdn_dp_device *dp)
>-{
>-	int ret;
>-	unsigned long timeout = jiffies + msecs_to_jiffies(CDN_FW_TIMEOUT_MS);
>-	unsigned long sleep = 1000;
>-
>-	WARN_ON(!mutex_is_locked(&dp->lock));
>-
>-	if (dp->fw_loaded)
>-		return 0;
>-
>-	/* Drop the lock before getting the firmware to avoid blocking boot */
>-	mutex_unlock(&dp->lock);
>-
>-	while (time_before(jiffies, timeout)) {
>-		ret = request_firmware(&dp->fw, CDN_DP_FIRMWARE, dp->dev);
>-		if (ret == -ENOENT) {
>-			msleep(sleep);
>-			sleep *= 2;
>-			continue;
>-		} else if (ret) {
>-			DRM_DEV_ERROR(dp->dev,
>-				      "failed to request firmware: %d\n", ret);
>-			goto out;
>-		}
>-
>-		dp->fw_loaded = true;
>-		ret = 0;
>-		goto out;
>-	}
>-
>-	DRM_DEV_ERROR(dp->dev, "Timed out trying to load firmware\n");
>-	ret = -ETIMEDOUT;
>-out:
>-	mutex_lock(&dp->lock);
>-	return ret;
>-}
>-
> static void cdn_dp_pd_event_work(struct work_struct *work)
> {
> 	struct cdn_dp_device *dp = container_of(work, struct cdn_dp_device,
> 						event_work);
> 	struct drm_connector *connector = &dp->connector;
> 	enum drm_connector_status old_status;
>-
> 	int ret;
> 
> 	mutex_lock(&dp->lock);
> 
> 	if (dp->suspended)
> 		goto out;
> 
>-	ret = cdn_dp_request_firmware(dp);
>-	if (ret)
>-		goto out;
>+	if (!dp->fw_loaded) {
>+		ret = request_firmware(&dp->fw, CDN_DP_FIRMWARE, dp->dev);
>+		if (ret) {
>+			DRM_DEV_ERROR(dp->dev, "Loading firmware failed: %d\n", ret);
>+			goto out;
>+		}
>+
>+		dp->fw_loaded = true;
>+	}
> 
> 	dp->connected = true;
>