* [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void
@ 2025-04-01 5:11 Dmitry Baryshkov
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
` (8 more replies)
0 siblings, 9 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
While it makes sense for panel callbacks to be able to return an error,
the state-management functions from drm_panel API are usually called
from atomic callbacks, which can not fails and must return void. Make
drm_panel_prepare(), drm_panel_enable(), drm_panel_disable() and
drm_panel_unprepare() follow the same calling convention and return
void.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (7):
drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
drm/bridge: analogic_dp: drop panel_is_modeset
drm/bridge: analogic_dp: drop panel_lock
drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
drm/panel: make prepare/enable and disable/unprepare calls return void
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 107 ++-------------------
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 -
drivers/gpu/drm/drm_panel.c | 54 ++++-------
drivers/gpu/drm/panel/panel-newvision-nv3051d.c | 9 +-
include/drm/drm_panel.h | 8 +-
5 files changed, 33 insertions(+), 148 deletions(-)
---
base-commit: bfb20a655848a9088e3e9ae24b1dcce1bbf016c2
change-id: 20250401-panel-return-void-d461c71ebd35
Best regards,
--
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:20 ` Damon Ding
2025-04-17 15:46 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument " Dmitry Baryshkov
` (7 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
The analogix_dp_prepare_panel() returns immediately if there is no
attached panel. Drop several calls to this function which are performed
when dp->plat_data->panel is NULL.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 042154e2d8ccd74ac2dc27fe527e810a50e08a09..ab1cfc340aa95bbf13fe021bd33227b565a5458d 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1005,17 +1005,11 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
{
struct analogix_dp_device *dp = to_dp(connector);
const struct drm_edid *drm_edid;
- int ret, num_modes = 0;
+ int num_modes = 0;
if (dp->plat_data->panel) {
num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
} else {
- ret = analogix_dp_prepare_panel(dp, true, false);
- if (ret) {
- DRM_ERROR("Failed to prepare panel (%d)\n", ret);
- return 0;
- }
-
drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
drm_edid_connector_update(&dp->connector, drm_edid);
@@ -1024,10 +1018,6 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
num_modes += drm_edid_connector_add_modes(&dp->connector);
drm_edid_free(drm_edid);
}
-
- ret = analogix_dp_prepare_panel(dp, false, false);
- if (ret)
- DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
}
if (dp->plat_data->get_modes)
@@ -1087,19 +1077,9 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
if (dp->plat_data->panel)
return connector_status_connected;
- ret = analogix_dp_prepare_panel(dp, true, false);
- if (ret) {
- DRM_ERROR("Failed to prepare panel (%d)\n", ret);
- return connector_status_disconnected;
- }
-
if (!analogix_dp_detect_hpd(dp))
status = connector_status_connected;
- ret = analogix_dp_prepare_panel(dp, false, false);
- if (ret)
- DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
-
return status;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:38 ` Damon Ding
2025-04-17 15:46 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
` (6 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
After previous cleanup all calling sites pass true as is_modeset_prepare
argument to analogix_dp_prepare_panel(). Drop dead code depending on
that argument being false.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index ab1cfc340aa95bbf13fe021bd33227b565a5458d..82dc4b01806f9728dc882b0128171838e81f21b0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -963,13 +963,13 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
* If @prepare is true, this function will prepare the panel. Conversely, if it
* is false, the panel will be unprepared.
*
- * If @is_modeset_prepare is true, the function will disregard the current state
+ * The function will disregard the current state
* of the panel and either prepare/unprepare the panel based on @prepare. Once
* it finishes, it will update dp->panel_is_modeset to reflect the current state
* of the panel.
*/
static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
- bool prepare, bool is_modeset_prepare)
+ bool prepare)
{
int ret = 0;
@@ -978,13 +978,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
mutex_lock(&dp->panel_lock);
- /*
- * Exit early if this is a temporary prepare/unprepare and we're already
- * modeset (since we neither want to prepare twice or unprepare early).
- */
- if (dp->panel_is_modeset && !is_modeset_prepare)
- goto out;
-
if (prepare)
ret = drm_panel_prepare(dp->plat_data->panel);
else
@@ -993,8 +986,7 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
if (ret)
goto out;
- if (is_modeset_prepare)
- dp->panel_is_modeset = prepare;
+ dp->panel_is_modeset = prepare;
out:
mutex_unlock(&dp->panel_lock);
@@ -1072,7 +1064,6 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
{
struct analogix_dp_device *dp = to_dp(connector);
enum drm_connector_status status = connector_status_disconnected;
- int ret;
if (dp->plat_data->panel)
return connector_status_connected;
@@ -1194,7 +1185,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
if (old_crtc_state && old_crtc_state->self_refresh_active)
return;
- ret = analogix_dp_prepare_panel(dp, true, true);
+ ret = analogix_dp_prepare_panel(dp, true);
if (ret)
DRM_ERROR("failed to setup the panel ret = %d\n", ret);
}
@@ -1294,7 +1285,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
pm_runtime_put_sync(dp->dev);
- ret = analogix_dp_prepare_panel(dp, false, true);
+ ret = analogix_dp_prepare_panel(dp, false);
if (ret)
DRM_ERROR("failed to setup the panel ret = %d\n", ret);
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
2025-04-01 5:11 ` [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument " Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:46 ` Damon Ding
` (2 more replies)
2025-04-01 5:11 ` [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock Dmitry Baryshkov
` (5 subsequent siblings)
8 siblings, 3 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
The dp->panel_is_modeset is now a write-only field. Drop it completely.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +----------
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 1 -
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 82dc4b01806f9728dc882b0128171838e81f21b0..704c6169116eb2601d2ad02dc7294455ceff5460 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -964,9 +964,7 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
* is false, the panel will be unprepared.
*
* The function will disregard the current state
- * of the panel and either prepare/unprepare the panel based on @prepare. Once
- * it finishes, it will update dp->panel_is_modeset to reflect the current state
- * of the panel.
+ * of the panel and either prepare/unprepare the panel based on @prepare.
*/
static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
bool prepare)
@@ -983,12 +981,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
else
ret = drm_panel_unprepare(dp->plat_data->panel);
- if (ret)
- goto out;
-
- dp->panel_is_modeset = prepare;
-
-out:
mutex_unlock(&dp->panel_lock);
return ret;
}
@@ -1532,7 +1524,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
dp->dpms_mode = DRM_MODE_DPMS_OFF;
mutex_init(&dp->panel_lock);
- dp->panel_is_modeset = false;
/*
* platform dp driver need containor_of the plat_data to get
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 774d11574b095b093ddf2818ad5b84be6605c9bf..b679d5b71d276f458d905c936160f107225bc6c5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -170,7 +170,6 @@ struct analogix_dp_device {
bool psr_supported;
struct mutex panel_lock;
- bool panel_is_modeset;
struct analogix_dp_plat_data *plat_data;
};
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (2 preceding siblings ...)
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:50 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel() Dmitry Baryshkov
` (4 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
The analogix_dp_prepare_panel() function is called from bridge's
atomic_pre_enable() and atomic_post_disable() callbacks, which can not
happen simultaneously. Drop the useless mutex.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -----
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 --
2 files changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 704c6169116eb2601d2ad02dc7294455ceff5460..1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -974,14 +974,11 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
if (!dp->plat_data->panel)
return 0;
- mutex_lock(&dp->panel_lock);
-
if (prepare)
ret = drm_panel_prepare(dp->plat_data->panel);
else
ret = drm_panel_unprepare(dp->plat_data->panel);
- mutex_unlock(&dp->panel_lock);
return ret;
}
@@ -1523,8 +1520,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
dp->dev = &pdev->dev;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
- mutex_init(&dp->panel_lock);
-
/*
* platform dp driver need containor_of the plat_data to get
* the driver private data, so we need to store the point of
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index b679d5b71d276f458d905c936160f107225bc6c5..2b54120ba4a3ef55af518a0629a5c8469f275a0f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -169,8 +169,6 @@ struct analogix_dp_device {
bool fast_train_enable;
bool psr_supported;
- struct mutex panel_lock;
-
struct analogix_dp_plat_data *plat_data;
};
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (3 preceding siblings ...)
2025-04-01 5:11 ` [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:54 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls Dmitry Baryshkov
` (3 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
The analogix_dp_prepare_panel() is now only calling a corresponding
drm_panel function. Inline it to simplify the code.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 43 +++++-----------------
1 file changed, 10 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978..f60068011008482f7b7b2edfcab5fb1b3e9e130f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -955,33 +955,6 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
}
-/*
- * This function is a bit of a catch-all for panel preparation, hopefully
- * simplifying the logic of functions that need to prepare/unprepare the panel
- * below.
- *
- * If @prepare is true, this function will prepare the panel. Conversely, if it
- * is false, the panel will be unprepared.
- *
- * The function will disregard the current state
- * of the panel and either prepare/unprepare the panel based on @prepare.
- */
-static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
- bool prepare)
-{
- int ret = 0;
-
- if (!dp->plat_data->panel)
- return 0;
-
- if (prepare)
- ret = drm_panel_prepare(dp->plat_data->panel);
- else
- ret = drm_panel_unprepare(dp->plat_data->panel);
-
- return ret;
-}
-
static int analogix_dp_get_modes(struct drm_connector *connector)
{
struct analogix_dp_device *dp = to_dp(connector);
@@ -1174,9 +1147,11 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
if (old_crtc_state && old_crtc_state->self_refresh_active)
return;
- ret = analogix_dp_prepare_panel(dp, true);
- if (ret)
- DRM_ERROR("failed to setup the panel ret = %d\n", ret);
+ if (dp->plat_data->panel) {
+ ret = drm_panel_prepare(dp->plat_data->panel);
+ if (ret)
+ DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
+ }
}
static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
@@ -1274,9 +1249,11 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
pm_runtime_put_sync(dp->dev);
- ret = analogix_dp_prepare_panel(dp, false);
- if (ret)
- DRM_ERROR("failed to setup the panel ret = %d\n", ret);
+ if (dp->plat_data->panel) {
+ ret = drm_panel_unprepare(dp->plat_data->panel);
+ if (ret)
+ DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
+ }
dp->fast_train_enable = false;
dp->psr_supported = false;
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (4 preceding siblings ...)
2025-04-01 5:11 ` [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel() Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-14 1:58 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void Dmitry Baryshkov
` (2 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
Follow the example of other drivers and ignore return values of the
drm_panel_prepare() / unprepare() / enable() / disable() calls. There is
no possible error recovery, so the driver just logs a message.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 39 ++++------------------
1 file changed, 6 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index f60068011008482f7b7b2edfcab5fb1b3e9e130f..c7dffdae31877ae194fc6b0a5bf21be203f7dcc4 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -838,10 +838,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
int ret;
/* Keep the panel disabled while we configure video */
- if (dp->plat_data->panel) {
- if (drm_panel_disable(dp->plat_data->panel))
- DRM_ERROR("failed to disable the panel\n");
- }
+ drm_panel_disable(dp->plat_data->panel);
ret = analogix_dp_train_link(dp);
if (ret) {
@@ -863,13 +860,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
}
/* Safe to enable the panel now */
- if (dp->plat_data->panel) {
- ret = drm_panel_enable(dp->plat_data->panel);
- if (ret) {
- DRM_ERROR("failed to enable the panel\n");
- return ret;
- }
- }
+ drm_panel_enable(dp->plat_data->panel);
/* Check whether panel supports fast training */
ret = analogix_dp_fast_link_train_detection(dp);
@@ -1136,7 +1127,6 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
- int ret;
crtc = analogix_dp_get_new_crtc(dp, old_state);
if (!crtc)
@@ -1147,11 +1137,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
if (old_crtc_state && old_crtc_state->self_refresh_active)
return;
- if (dp->plat_data->panel) {
- ret = drm_panel_prepare(dp->plat_data->panel);
- if (ret)
- DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
- }
+ drm_panel_prepare(dp->plat_data->panel);
}
static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
@@ -1231,17 +1217,11 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
{
struct analogix_dp_device *dp = bridge->driver_private;
- int ret;
if (dp->dpms_mode != DRM_MODE_DPMS_ON)
return;
- if (dp->plat_data->panel) {
- if (drm_panel_disable(dp->plat_data->panel)) {
- DRM_ERROR("failed to disable the panel\n");
- return;
- }
- }
+ drm_panel_disable(dp->plat_data->panel);
disable_irq(dp->irq);
@@ -1249,11 +1229,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
pm_runtime_put_sync(dp->dev);
- if (dp->plat_data->panel) {
- ret = drm_panel_unprepare(dp->plat_data->panel);
- if (ret)
- DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
- }
+ drm_panel_unprepare(dp->plat_data->panel);
dp->fast_train_enable = false;
dp->psr_supported = false;
@@ -1678,10 +1654,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
analogix_dp_bridge_disable(dp->bridge);
dp->connector.funcs->destroy(&dp->connector);
- if (dp->plat_data->panel) {
- if (drm_panel_unprepare(dp->plat_data->panel))
- DRM_ERROR("failed to turnoff the panel\n");
- }
+ drm_panel_unprepare(dp->plat_data->panel);
drm_dp_aux_unregister(&dp->aux);
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (5 preceding siblings ...)
2025-04-01 5:11 ` [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls Dmitry Baryshkov
@ 2025-04-01 5:11 ` Dmitry Baryshkov
2025-04-11 8:55 ` Neil Armstrong
2025-04-18 13:22 ` [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare " Neil Armstrong
2025-04-22 13:56 ` Dmitry Baryshkov
8 siblings, 1 reply; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-01 5:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
Now there are no users of the return value of the drm_panel_prepare(),
drm_panel_unprepare(), drm_panel_enable() and drm_panel_disable() calls.
Usually these calls are performed from the atomic callbacks, where it is
impossible to return an error. Stop returning error codes and return
void instead.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/drm_panel.c | 54 +++++++++----------------
drivers/gpu/drm/panel/panel-newvision-nv3051d.c | 9 +----
include/drm/drm_panel.h | 8 ++--
3 files changed, 26 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index c627e42a7ce70459f50eb5095fffc806ca45dabf..faa7a76b63b53b3a45b3400d8bf3b58a027a340e 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -105,21 +105,21 @@ EXPORT_SYMBOL(drm_panel_remove);
*
* Calling this function will enable power and deassert any reset signals to
* the panel. After this has completed it is possible to communicate with any
- * integrated circuitry via a command bus.
- *
- * Return: 0 on success or a negative error code on failure.
+ * integrated circuitry via a command bus. This function cannot fail (as it is
+ * called from the pre_enable call chain). There will always be a call to
+ * drm_panel_disable() afterwards.
*/
-int drm_panel_prepare(struct drm_panel *panel)
+void drm_panel_prepare(struct drm_panel *panel)
{
struct drm_panel_follower *follower;
int ret;
if (!panel)
- return -EINVAL;
+ return;
if (panel->prepared) {
dev_warn(panel->dev, "Skipping prepare of already prepared panel\n");
- return 0;
+ return;
}
mutex_lock(&panel->follower_lock);
@@ -138,11 +138,8 @@ int drm_panel_prepare(struct drm_panel *panel)
follower->funcs->panel_prepared, ret);
}
- ret = 0;
exit:
mutex_unlock(&panel->follower_lock);
-
- return ret;
}
EXPORT_SYMBOL(drm_panel_prepare);
@@ -154,16 +151,14 @@ EXPORT_SYMBOL(drm_panel_prepare);
* reset, turn off power supplies, ...). After this function has completed, it
* is usually no longer possible to communicate with the panel until another
* call to drm_panel_prepare().
- *
- * Return: 0 on success or a negative error code on failure.
*/
-int drm_panel_unprepare(struct drm_panel *panel)
+void drm_panel_unprepare(struct drm_panel *panel)
{
struct drm_panel_follower *follower;
int ret;
if (!panel)
- return -EINVAL;
+ return;
/*
* If you are seeing the warning below it likely means one of two things:
@@ -176,7 +171,7 @@ int drm_panel_unprepare(struct drm_panel *panel)
*/
if (!panel->prepared) {
dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n");
- return 0;
+ return;
}
mutex_lock(&panel->follower_lock);
@@ -195,11 +190,8 @@ int drm_panel_unprepare(struct drm_panel *panel)
}
panel->prepared = false;
- ret = 0;
exit:
mutex_unlock(&panel->follower_lock);
-
- return ret;
}
EXPORT_SYMBOL(drm_panel_unprepare);
@@ -209,26 +201,26 @@ EXPORT_SYMBOL(drm_panel_unprepare);
*
* Calling this function will cause the panel display drivers to be turned on
* and the backlight to be enabled. Content will be visible on screen after
- * this call completes.
- *
- * Return: 0 on success or a negative error code on failure.
+ * this call completes. This function cannot fail (as it is called from the
+ * enable call chain). There will always be a call to drm_panel_disable()
+ * afterwards.
*/
-int drm_panel_enable(struct drm_panel *panel)
+void drm_panel_enable(struct drm_panel *panel)
{
int ret;
if (!panel)
- return -EINVAL;
+ return;
if (panel->enabled) {
dev_warn(panel->dev, "Skipping enable of already enabled panel\n");
- return 0;
+ return;
}
if (panel->funcs && panel->funcs->enable) {
ret = panel->funcs->enable(panel);
if (ret < 0)
- return ret;
+ return;
}
panel->enabled = true;
@@ -236,8 +228,6 @@ int drm_panel_enable(struct drm_panel *panel)
if (ret < 0)
DRM_DEV_INFO(panel->dev, "failed to enable backlight: %d\n",
ret);
-
- return 0;
}
EXPORT_SYMBOL(drm_panel_enable);
@@ -248,15 +238,13 @@ EXPORT_SYMBOL(drm_panel_enable);
* This will typically turn off the panel's backlight or disable the display
* drivers. For smart panels it should still be possible to communicate with
* the integrated circuitry via any command bus after this call.
- *
- * Return: 0 on success or a negative error code on failure.
*/
-int drm_panel_disable(struct drm_panel *panel)
+void drm_panel_disable(struct drm_panel *panel)
{
int ret;
if (!panel)
- return -EINVAL;
+ return;
/*
* If you are seeing the warning below it likely means one of two things:
@@ -269,7 +257,7 @@ int drm_panel_disable(struct drm_panel *panel)
*/
if (!panel->enabled) {
dev_warn(panel->dev, "Skipping disable of already disabled panel\n");
- return 0;
+ return;
}
ret = backlight_disable(panel->backlight);
@@ -280,11 +268,9 @@ int drm_panel_disable(struct drm_panel *panel)
if (panel->funcs && panel->funcs->disable) {
ret = panel->funcs->disable(panel);
if (ret < 0)
- return ret;
+ return;
}
panel->enabled = false;
-
- return 0;
}
EXPORT_SYMBOL(drm_panel_disable);
diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
index 5d115ecd5dd44c8e5e7d1fb8afe573324e987f59..b6429795e8f518646443dd8179f3ec28cef4dc0f 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
@@ -413,15 +413,10 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
static void panel_nv3051d_shutdown(struct mipi_dsi_device *dsi)
{
struct panel_nv3051d *ctx = mipi_dsi_get_drvdata(dsi);
- int ret;
- ret = drm_panel_unprepare(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
+ drm_panel_unprepare(&ctx->panel);
- ret = drm_panel_disable(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
+ drm_panel_disable(&ctx->panel);
}
static void panel_nv3051d_remove(struct mipi_dsi_device *dsi)
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index a9c042c8dea1a82ef979c7a68204e0b55483fc28..18bf338c8b96254dc3f2880106b944e71ea4c9a7 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -275,11 +275,11 @@ void drm_panel_init(struct drm_panel *panel, struct device *dev,
void drm_panel_add(struct drm_panel *panel);
void drm_panel_remove(struct drm_panel *panel);
-int drm_panel_prepare(struct drm_panel *panel);
-int drm_panel_unprepare(struct drm_panel *panel);
+void drm_panel_prepare(struct drm_panel *panel);
+void drm_panel_unprepare(struct drm_panel *panel);
-int drm_panel_enable(struct drm_panel *panel);
-int drm_panel_disable(struct drm_panel *panel);
+void drm_panel_enable(struct drm_panel *panel);
+void drm_panel_disable(struct drm_panel *panel);
int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector *connector);
--
2.39.5
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void
2025-04-01 5:11 ` [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void Dmitry Baryshkov
@ 2025-04-11 8:55 ` Neil Armstrong
0 siblings, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-11 8:55 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> Now there are no users of the return value of the drm_panel_prepare(),
> drm_panel_unprepare(), drm_panel_enable() and drm_panel_disable() calls.
> Usually these calls are performed from the atomic callbacks, where it is
> impossible to return an error. Stop returning error codes and return
> void instead.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/drm_panel.c | 54 +++++++++----------------
> drivers/gpu/drm/panel/panel-newvision-nv3051d.c | 9 +----
> include/drm/drm_panel.h | 8 ++--
> 3 files changed, 26 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index c627e42a7ce70459f50eb5095fffc806ca45dabf..faa7a76b63b53b3a45b3400d8bf3b58a027a340e 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -105,21 +105,21 @@ EXPORT_SYMBOL(drm_panel_remove);
> *
> * Calling this function will enable power and deassert any reset signals to
> * the panel. After this has completed it is possible to communicate with any
> - * integrated circuitry via a command bus.
> - *
> - * Return: 0 on success or a negative error code on failure.
> + * integrated circuitry via a command bus. This function cannot fail (as it is
> + * called from the pre_enable call chain). There will always be a call to
> + * drm_panel_disable() afterwards.
> */
> -int drm_panel_prepare(struct drm_panel *panel)
> +void drm_panel_prepare(struct drm_panel *panel)
> {
> struct drm_panel_follower *follower;
> int ret;
>
> if (!panel)
> - return -EINVAL;
> + return;
>
> if (panel->prepared) {
> dev_warn(panel->dev, "Skipping prepare of already prepared panel\n");
> - return 0;
> + return;
> }
>
> mutex_lock(&panel->follower_lock);
> @@ -138,11 +138,8 @@ int drm_panel_prepare(struct drm_panel *panel)
> follower->funcs->panel_prepared, ret);
> }
>
> - ret = 0;
> exit:
> mutex_unlock(&panel->follower_lock);
> -
> - return ret;
> }
> EXPORT_SYMBOL(drm_panel_prepare);
>
> @@ -154,16 +151,14 @@ EXPORT_SYMBOL(drm_panel_prepare);
> * reset, turn off power supplies, ...). After this function has completed, it
> * is usually no longer possible to communicate with the panel until another
> * call to drm_panel_prepare().
> - *
> - * Return: 0 on success or a negative error code on failure.
> */
> -int drm_panel_unprepare(struct drm_panel *panel)
> +void drm_panel_unprepare(struct drm_panel *panel)
> {
> struct drm_panel_follower *follower;
> int ret;
>
> if (!panel)
> - return -EINVAL;
> + return;
>
> /*
> * If you are seeing the warning below it likely means one of two things:
> @@ -176,7 +171,7 @@ int drm_panel_unprepare(struct drm_panel *panel)
> */
> if (!panel->prepared) {
> dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n");
> - return 0;
> + return;
> }
>
> mutex_lock(&panel->follower_lock);
> @@ -195,11 +190,8 @@ int drm_panel_unprepare(struct drm_panel *panel)
> }
> panel->prepared = false;
>
> - ret = 0;
> exit:
> mutex_unlock(&panel->follower_lock);
> -
> - return ret;
> }
> EXPORT_SYMBOL(drm_panel_unprepare);
>
> @@ -209,26 +201,26 @@ EXPORT_SYMBOL(drm_panel_unprepare);
> *
> * Calling this function will cause the panel display drivers to be turned on
> * and the backlight to be enabled. Content will be visible on screen after
> - * this call completes.
> - *
> - * Return: 0 on success or a negative error code on failure.
> + * this call completes. This function cannot fail (as it is called from the
> + * enable call chain). There will always be a call to drm_panel_disable()
> + * afterwards.
> */
> -int drm_panel_enable(struct drm_panel *panel)
> +void drm_panel_enable(struct drm_panel *panel)
> {
> int ret;
>
> if (!panel)
> - return -EINVAL;
> + return;
>
> if (panel->enabled) {
> dev_warn(panel->dev, "Skipping enable of already enabled panel\n");
> - return 0;
> + return;
> }
>
> if (panel->funcs && panel->funcs->enable) {
> ret = panel->funcs->enable(panel);
> if (ret < 0)
> - return ret;
> + return;
> }
> panel->enabled = true;
>
> @@ -236,8 +228,6 @@ int drm_panel_enable(struct drm_panel *panel)
> if (ret < 0)
> DRM_DEV_INFO(panel->dev, "failed to enable backlight: %d\n",
> ret);
> -
> - return 0;
> }
> EXPORT_SYMBOL(drm_panel_enable);
>
> @@ -248,15 +238,13 @@ EXPORT_SYMBOL(drm_panel_enable);
> * This will typically turn off the panel's backlight or disable the display
> * drivers. For smart panels it should still be possible to communicate with
> * the integrated circuitry via any command bus after this call.
> - *
> - * Return: 0 on success or a negative error code on failure.
> */
> -int drm_panel_disable(struct drm_panel *panel)
> +void drm_panel_disable(struct drm_panel *panel)
> {
> int ret;
>
> if (!panel)
> - return -EINVAL;
> + return;
>
> /*
> * If you are seeing the warning below it likely means one of two things:
> @@ -269,7 +257,7 @@ int drm_panel_disable(struct drm_panel *panel)
> */
> if (!panel->enabled) {
> dev_warn(panel->dev, "Skipping disable of already disabled panel\n");
> - return 0;
> + return;
> }
>
> ret = backlight_disable(panel->backlight);
> @@ -280,11 +268,9 @@ int drm_panel_disable(struct drm_panel *panel)
> if (panel->funcs && panel->funcs->disable) {
> ret = panel->funcs->disable(panel);
> if (ret < 0)
> - return ret;
> + return;
> }
> panel->enabled = false;
> -
> - return 0;
> }
> EXPORT_SYMBOL(drm_panel_disable);
>
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> index 5d115ecd5dd44c8e5e7d1fb8afe573324e987f59..b6429795e8f518646443dd8179f3ec28cef4dc0f 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> @@ -413,15 +413,10 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
> static void panel_nv3051d_shutdown(struct mipi_dsi_device *dsi)
> {
> struct panel_nv3051d *ctx = mipi_dsi_get_drvdata(dsi);
> - int ret;
>
> - ret = drm_panel_unprepare(&ctx->panel);
> - if (ret < 0)
> - dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
> + drm_panel_unprepare(&ctx->panel);
>
> - ret = drm_panel_disable(&ctx->panel);
> - if (ret < 0)
> - dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
> + drm_panel_disable(&ctx->panel);
> }
>
> static void panel_nv3051d_remove(struct mipi_dsi_device *dsi)
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index a9c042c8dea1a82ef979c7a68204e0b55483fc28..18bf338c8b96254dc3f2880106b944e71ea4c9a7 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -275,11 +275,11 @@ void drm_panel_init(struct drm_panel *panel, struct device *dev,
> void drm_panel_add(struct drm_panel *panel);
> void drm_panel_remove(struct drm_panel *panel);
>
> -int drm_panel_prepare(struct drm_panel *panel);
> -int drm_panel_unprepare(struct drm_panel *panel);
> +void drm_panel_prepare(struct drm_panel *panel);
> +void drm_panel_unprepare(struct drm_panel *panel);
>
> -int drm_panel_enable(struct drm_panel *panel);
> -int drm_panel_disable(struct drm_panel *panel);
> +void drm_panel_enable(struct drm_panel *panel);
> +void drm_panel_disable(struct drm_panel *panel);
>
> int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector *connector);
>
>
LGTM
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
@ 2025-04-14 1:20 ` Damon Ding
2025-04-14 2:01 ` Damon Ding
2025-04-17 15:46 ` Neil Armstrong
1 sibling, 1 reply; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:20 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() returns immediately if there is no
> attached panel. Drop several calls to this function which are performed
> when dp->plat_data->panel is NULL.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 22 +---------------------
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 042154e2d8ccd74ac2dc27fe527e810a50e08a09..ab1cfc340aa95bbf13fe021bd33227b565a5458d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1005,17 +1005,11 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> const struct drm_edid *drm_edid;
> - int ret, num_modes = 0;
> + int num_modes = 0;
>
> if (dp->plat_data->panel) {
> num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
> } else {
> - ret = analogix_dp_prepare_panel(dp, true, false);
> - if (ret) {
> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> - return 0;
> - }
> -
> drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>
> drm_edid_connector_update(&dp->connector, drm_edid);
> @@ -1024,10 +1018,6 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
> num_modes += drm_edid_connector_add_modes(&dp->connector);
> drm_edid_free(drm_edid);
> }
> -
> - ret = analogix_dp_prepare_panel(dp, false, false);
> - if (ret)
> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> }
>
> if (dp->plat_data->get_modes)
> @@ -1087,19 +1077,9 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
> if (dp->plat_data->panel)
> return connector_status_connected;
>
> - ret = analogix_dp_prepare_panel(dp, true, false);
> - if (ret) {
> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> - return connector_status_disconnected;
> - }
> -
> if (!analogix_dp_detect_hpd(dp))
> status = connector_status_connected;
>
> - ret = analogix_dp_prepare_panel(dp, false, false);
> - if (ret)
> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> -
> return status;
> }
>
>
With the patch series:
https://patchwork.kernel.org/project/linux-rockchip/list/?series=942183
When I verify this patch, there will be a small warning:
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1075:6: warning:
unused variable 'ret' [-Wunused-variable]
int ret;
^
It should be better to remove the unused 'ret'.
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument " Dmitry Baryshkov
@ 2025-04-14 1:38 ` Damon Ding
2025-04-17 15:46 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:38 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> After previous cleanup all calling sites pass true as is_modeset_prepare
> argument to analogix_dp_prepare_panel(). Drop dead code depending on
> that argument being false.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index ab1cfc340aa95bbf13fe021bd33227b565a5458d..82dc4b01806f9728dc882b0128171838e81f21b0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -963,13 +963,13 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> * If @prepare is true, this function will prepare the panel. Conversely, if it
> * is false, the panel will be unprepared.
> *
> - * If @is_modeset_prepare is true, the function will disregard the current state
> + * The function will disregard the current state
> * of the panel and either prepare/unprepare the panel based on @prepare. Once
> * it finishes, it will update dp->panel_is_modeset to reflect the current state
> * of the panel.
> */
> static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> - bool prepare, bool is_modeset_prepare)
> + bool prepare)
> {
> int ret = 0;
>
> @@ -978,13 +978,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
>
> mutex_lock(&dp->panel_lock);
>
> - /*
> - * Exit early if this is a temporary prepare/unprepare and we're already
> - * modeset (since we neither want to prepare twice or unprepare early).
> - */
> - if (dp->panel_is_modeset && !is_modeset_prepare)
> - goto out;
> -
> if (prepare)
> ret = drm_panel_prepare(dp->plat_data->panel);
> else
> @@ -993,8 +986,7 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> if (ret)
> goto out;
>
> - if (is_modeset_prepare)
> - dp->panel_is_modeset = prepare;
> + dp->panel_is_modeset = prepare;
>
> out:
> mutex_unlock(&dp->panel_lock);
> @@ -1072,7 +1064,6 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> enum drm_connector_status status = connector_status_disconnected;
> - int ret;
>
> if (dp->plat_data->panel)
> return connector_status_connected;
> @@ -1194,7 +1185,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - ret = analogix_dp_prepare_panel(dp, true, true);
> + ret = analogix_dp_prepare_panel(dp, true);
> if (ret)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> }
> @@ -1294,7 +1285,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - ret = analogix_dp_prepare_panel(dp, false, true);
> + ret = analogix_dp_prepare_panel(dp, false);
> if (ret)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
>
>
The patch has been verified with the eDP panel "lg,lp079qx1-sp0v" in
RK3588S EVB1 board.
Tested-by: Damon Ding <damon.ding@rock-chips.com>
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
@ 2025-04-14 1:46 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
2025-04-22 13:42 ` Neil Armstrong
2 siblings, 0 replies; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:46 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> The dp->panel_is_modeset is now a write-only field. Drop it completely.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +----------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 1 -
> 2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 82dc4b01806f9728dc882b0128171838e81f21b0..704c6169116eb2601d2ad02dc7294455ceff5460 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -964,9 +964,7 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> * is false, the panel will be unprepared.
> *
> * The function will disregard the current state
> - * of the panel and either prepare/unprepare the panel based on @prepare. Once
> - * it finishes, it will update dp->panel_is_modeset to reflect the current state
> - * of the panel.
> + * of the panel and either prepare/unprepare the panel based on @prepare.
> */
> static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> bool prepare)
> @@ -983,12 +981,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> else
> ret = drm_panel_unprepare(dp->plat_data->panel);
>
> - if (ret)
> - goto out;
> -
> - dp->panel_is_modeset = prepare;
> -
> -out:
> mutex_unlock(&dp->panel_lock);
> return ret;
> }
> @@ -1532,7 +1524,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
>
> mutex_init(&dp->panel_lock);
> - dp->panel_is_modeset = false;
>
> /*
> * platform dp driver need containor_of the plat_data to get
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 774d11574b095b093ddf2818ad5b84be6605c9bf..b679d5b71d276f458d905c936160f107225bc6c5 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -170,7 +170,6 @@ struct analogix_dp_device {
> bool psr_supported;
>
> struct mutex panel_lock;
> - bool panel_is_modeset;
>
> struct analogix_dp_plat_data *plat_data;
> };
>
The patch has been verified with the eDP panel "lg,lp079qx1-sp0v" in
RK3588S EVB1 board.
Tested-by: Damon Ding <damon.ding@rock-chips.com>
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock
2025-04-01 5:11 ` [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock Dmitry Baryshkov
@ 2025-04-14 1:50 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:50 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() function is called from bridge's
> atomic_pre_enable() and atomic_post_disable() callbacks, which can not
> happen simultaneously. Drop the useless mutex.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 --
> 2 files changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 704c6169116eb2601d2ad02dc7294455ceff5460..1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -974,14 +974,11 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> if (!dp->plat_data->panel)
> return 0;
>
> - mutex_lock(&dp->panel_lock);
> -
> if (prepare)
> ret = drm_panel_prepare(dp->plat_data->panel);
> else
> ret = drm_panel_unprepare(dp->plat_data->panel);
>
> - mutex_unlock(&dp->panel_lock);
> return ret;
> }
>
> @@ -1523,8 +1520,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> dp->dev = &pdev->dev;
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
>
> - mutex_init(&dp->panel_lock);
> -
> /*
> * platform dp driver need containor_of the plat_data to get
> * the driver private data, so we need to store the point of
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b679d5b71d276f458d905c936160f107225bc6c5..2b54120ba4a3ef55af518a0629a5c8469f275a0f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -169,8 +169,6 @@ struct analogix_dp_device {
> bool fast_train_enable;
> bool psr_supported;
>
> - struct mutex panel_lock;
> -
> struct analogix_dp_plat_data *plat_data;
> };
>
>
The patch has been verified with the eDP panel "lg,lp079qx1-sp0v" in
RK3588S EVB1 board.
Tested-by: Damon Ding <damon.ding@rock-chips.com>
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel() Dmitry Baryshkov
@ 2025-04-14 1:54 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:54 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() is now only calling a corresponding
> drm_panel function. Inline it to simplify the code.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 43 +++++-----------------
> 1 file changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978..f60068011008482f7b7b2edfcab5fb1b3e9e130f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -955,33 +955,6 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
> }
>
> -/*
> - * This function is a bit of a catch-all for panel preparation, hopefully
> - * simplifying the logic of functions that need to prepare/unprepare the panel
> - * below.
> - *
> - * If @prepare is true, this function will prepare the panel. Conversely, if it
> - * is false, the panel will be unprepared.
> - *
> - * The function will disregard the current state
> - * of the panel and either prepare/unprepare the panel based on @prepare.
> - */
> -static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> - bool prepare)
> -{
> - int ret = 0;
> -
> - if (!dp->plat_data->panel)
> - return 0;
> -
> - if (prepare)
> - ret = drm_panel_prepare(dp->plat_data->panel);
> - else
> - ret = drm_panel_unprepare(dp->plat_data->panel);
> -
> - return ret;
> -}
> -
> static int analogix_dp_get_modes(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> @@ -1174,9 +1147,11 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - ret = analogix_dp_prepare_panel(dp, true);
> - if (ret)
> - DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> + if (dp->plat_data->panel) {
> + ret = drm_panel_prepare(dp->plat_data->panel);
> + if (ret)
> + DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
> + }
> }
>
> static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> @@ -1274,9 +1249,11 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - ret = analogix_dp_prepare_panel(dp, false);
> - if (ret)
> - DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> + if (dp->plat_data->panel) {
> + ret = drm_panel_unprepare(dp->plat_data->panel);
> + if (ret)
> + DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
> + }
>
> dp->fast_train_enable = false;
> dp->psr_supported = false;
>
The patch has been verified with the eDP panel "lg,lp079qx1-sp0v" in
RK3588S EVB1 board.
Tested-by: Damon Ding <damon.ding@rock-chips.com>
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
2025-04-01 5:11 ` [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls Dmitry Baryshkov
@ 2025-04-14 1:58 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Damon Ding @ 2025-04-14 1:58 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
Hi Dmitry,
On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> Follow the example of other drivers and ignore return values of the
> drm_panel_prepare() / unprepare() / enable() / disable() calls. There is
> no possible error recovery, so the driver just logs a message.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 39 ++++------------------
> 1 file changed, 6 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index f60068011008482f7b7b2edfcab5fb1b3e9e130f..c7dffdae31877ae194fc6b0a5bf21be203f7dcc4 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -838,10 +838,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
> int ret;
>
> /* Keep the panel disabled while we configure video */
> - if (dp->plat_data->panel) {
> - if (drm_panel_disable(dp->plat_data->panel))
> - DRM_ERROR("failed to disable the panel\n");
> - }
> + drm_panel_disable(dp->plat_data->panel);
>
> ret = analogix_dp_train_link(dp);
> if (ret) {
> @@ -863,13 +860,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
> }
>
> /* Safe to enable the panel now */
> - if (dp->plat_data->panel) {
> - ret = drm_panel_enable(dp->plat_data->panel);
> - if (ret) {
> - DRM_ERROR("failed to enable the panel\n");
> - return ret;
> - }
> - }
> + drm_panel_enable(dp->plat_data->panel);
>
> /* Check whether panel supports fast training */
> ret = analogix_dp_fast_link_train_detection(dp);
> @@ -1136,7 +1127,6 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> struct analogix_dp_device *dp = bridge->driver_private;
> struct drm_crtc *crtc;
> struct drm_crtc_state *old_crtc_state;
> - int ret;
>
> crtc = analogix_dp_get_new_crtc(dp, old_state);
> if (!crtc)
> @@ -1147,11 +1137,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - if (dp->plat_data->panel) {
> - ret = drm_panel_prepare(dp->plat_data->panel);
> - if (ret)
> - DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
> - }
> + drm_panel_prepare(dp->plat_data->panel);
> }
>
> static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> @@ -1231,17 +1217,11 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> {
> struct analogix_dp_device *dp = bridge->driver_private;
> - int ret;
>
> if (dp->dpms_mode != DRM_MODE_DPMS_ON)
> return;
>
> - if (dp->plat_data->panel) {
> - if (drm_panel_disable(dp->plat_data->panel)) {
> - DRM_ERROR("failed to disable the panel\n");
> - return;
> - }
> - }
> + drm_panel_disable(dp->plat_data->panel);
>
> disable_irq(dp->irq);
>
> @@ -1249,11 +1229,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - if (dp->plat_data->panel) {
> - ret = drm_panel_unprepare(dp->plat_data->panel);
> - if (ret)
> - DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
> - }
> + drm_panel_unprepare(dp->plat_data->panel);
>
> dp->fast_train_enable = false;
> dp->psr_supported = false;
> @@ -1678,10 +1654,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
> analogix_dp_bridge_disable(dp->bridge);
> dp->connector.funcs->destroy(&dp->connector);
>
> - if (dp->plat_data->panel) {
> - if (drm_panel_unprepare(dp->plat_data->panel))
> - DRM_ERROR("failed to turnoff the panel\n");
> - }
> + drm_panel_unprepare(dp->plat_data->panel);
>
> drm_dp_aux_unregister(&dp->aux);
>
>
The patch has been verified with the eDP panel "lg,lp079qx1-sp0v" in
RK3588S EVB1 board.
Tested-by: Damon Ding <damon.ding@rock-chips.com>
Best regards,
Damon
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
2025-04-14 1:20 ` Damon Ding
@ 2025-04-14 2:01 ` Damon Ding
2025-04-20 22:56 ` Dmitry Baryshkov
0 siblings, 1 reply; 27+ messages in thread
From: Damon Ding @ 2025-04-14 2:01 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
On 2025/4/14 9:20, Damon Ding wrote:
> Hi Dmitry,
>
> On 2025/4/1 13:11, Dmitry Baryshkov wrote:
>> The analogix_dp_prepare_panel() returns immediately if there is no
>> attached panel. Drop several calls to this function which are performed
>> when dp->plat_data->panel is NULL.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 22
>> +---------------------
>> 1 file changed, 1 insertion(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index
>> 042154e2d8ccd74ac2dc27fe527e810a50e08a09..ab1cfc340aa95bbf13fe021bd33227b565a5458d 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1005,17 +1005,11 @@ static int analogix_dp_get_modes(struct
>> drm_connector *connector)
>> {
>> struct analogix_dp_device *dp = to_dp(connector);
>> const struct drm_edid *drm_edid;
>> - int ret, num_modes = 0;
>> + int num_modes = 0;
>> if (dp->plat_data->panel) {
>> num_modes += drm_panel_get_modes(dp->plat_data->panel,
>> connector);
>> } else {
>> - ret = analogix_dp_prepare_panel(dp, true, false);
>> - if (ret) {
>> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
>> - return 0;
>> - }
>> -
>> drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>> drm_edid_connector_update(&dp->connector, drm_edid);
>> @@ -1024,10 +1018,6 @@ static int analogix_dp_get_modes(struct
>> drm_connector *connector)
>> num_modes += drm_edid_connector_add_modes(&dp->connector);
>> drm_edid_free(drm_edid);
>> }
>> -
>> - ret = analogix_dp_prepare_panel(dp, false, false);
>> - if (ret)
>> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
>> }
>> if (dp->plat_data->get_modes)
>> @@ -1087,19 +1077,9 @@ analogix_dp_detect(struct drm_connector
>> *connector, bool force)
>> if (dp->plat_data->panel)
>> return connector_status_connected;
>> - ret = analogix_dp_prepare_panel(dp, true, false);
>> - if (ret) {
>> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
>> - return connector_status_disconnected;
>> - }
>> -
>> if (!analogix_dp_detect_hpd(dp))
>> status = connector_status_connected;
>> - ret = analogix_dp_prepare_panel(dp, false, false);
>> - if (ret)
>> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
>> -
>> return status;
>> }
>>
>
> With the patch series:
> https://patchwork.kernel.org/project/linux-rockchip/list/?series=942183
>
> When I verify this patch, there will be a small warning:
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1075:6: warning:
> unused variable 'ret' [-Wunused-variable]
> int ret;
> ^
>
> It should be better to remove the unused 'ret'.
>
> Best regards,
> Damon
>
>
>
Tested-by: Damon Ding <damon.ding@rock-chips.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
2025-04-14 1:20 ` Damon Ding
@ 2025-04-17 15:46 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:46 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() returns immediately if there is no
> attached panel. Drop several calls to this function which are performed
> when dp->plat_data->panel is NULL.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 22 +---------------------
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 042154e2d8ccd74ac2dc27fe527e810a50e08a09..ab1cfc340aa95bbf13fe021bd33227b565a5458d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1005,17 +1005,11 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> const struct drm_edid *drm_edid;
> - int ret, num_modes = 0;
> + int num_modes = 0;
>
> if (dp->plat_data->panel) {
> num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
> } else {
> - ret = analogix_dp_prepare_panel(dp, true, false);
> - if (ret) {
> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> - return 0;
> - }
> -
> drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>
> drm_edid_connector_update(&dp->connector, drm_edid);
> @@ -1024,10 +1018,6 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
> num_modes += drm_edid_connector_add_modes(&dp->connector);
> drm_edid_free(drm_edid);
> }
> -
> - ret = analogix_dp_prepare_panel(dp, false, false);
> - if (ret)
> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> }
>
> if (dp->plat_data->get_modes)
> @@ -1087,19 +1077,9 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
> if (dp->plat_data->panel)
> return connector_status_connected;
>
> - ret = analogix_dp_prepare_panel(dp, true, false);
> - if (ret) {
> - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> - return connector_status_disconnected;
> - }
> -
> if (!analogix_dp_detect_hpd(dp))
> status = connector_status_connected;
>
> - ret = analogix_dp_prepare_panel(dp, false, false);
> - if (ret)
> - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> -
> return status;
> }
>
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument " Dmitry Baryshkov
2025-04-14 1:38 ` Damon Ding
@ 2025-04-17 15:46 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:46 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> After previous cleanup all calling sites pass true as is_modeset_prepare
> argument to analogix_dp_prepare_panel(). Drop dead code depending on
> that argument being false.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index ab1cfc340aa95bbf13fe021bd33227b565a5458d..82dc4b01806f9728dc882b0128171838e81f21b0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -963,13 +963,13 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> * If @prepare is true, this function will prepare the panel. Conversely, if it
> * is false, the panel will be unprepared.
> *
> - * If @is_modeset_prepare is true, the function will disregard the current state
> + * The function will disregard the current state
> * of the panel and either prepare/unprepare the panel based on @prepare. Once
> * it finishes, it will update dp->panel_is_modeset to reflect the current state
> * of the panel.
> */
> static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> - bool prepare, bool is_modeset_prepare)
> + bool prepare)
> {
> int ret = 0;
>
> @@ -978,13 +978,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
>
> mutex_lock(&dp->panel_lock);
>
> - /*
> - * Exit early if this is a temporary prepare/unprepare and we're already
> - * modeset (since we neither want to prepare twice or unprepare early).
> - */
> - if (dp->panel_is_modeset && !is_modeset_prepare)
> - goto out;
> -
> if (prepare)
> ret = drm_panel_prepare(dp->plat_data->panel);
> else
> @@ -993,8 +986,7 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> if (ret)
> goto out;
>
> - if (is_modeset_prepare)
> - dp->panel_is_modeset = prepare;
> + dp->panel_is_modeset = prepare;
>
> out:
> mutex_unlock(&dp->panel_lock);
> @@ -1072,7 +1064,6 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> enum drm_connector_status status = connector_status_disconnected;
> - int ret;
>
> if (dp->plat_data->panel)
> return connector_status_connected;
> @@ -1194,7 +1185,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - ret = analogix_dp_prepare_panel(dp, true, true);
> + ret = analogix_dp_prepare_panel(dp, true);
> if (ret)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> }
> @@ -1294,7 +1285,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - ret = analogix_dp_prepare_panel(dp, false, true);
> + ret = analogix_dp_prepare_panel(dp, false);
> if (ret)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
>
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
2025-04-14 1:46 ` Damon Ding
@ 2025-04-17 15:47 ` Neil Armstrong
2025-04-20 22:55 ` Dmitry Baryshkov
2025-04-22 13:42 ` Neil Armstrong
2 siblings, 1 reply; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:47 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> The dp->panel_is_modeset is now a write-only field. Drop it completely.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +----------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 1 -
> 2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 82dc4b01806f9728dc882b0128171838e81f21b0..704c6169116eb2601d2ad02dc7294455ceff5460 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -964,9 +964,7 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> * is false, the panel will be unprepared.
> *
> * The function will disregard the current state
> - * of the panel and either prepare/unprepare the panel based on @prepare. Once
> - * it finishes, it will update dp->panel_is_modeset to reflect the current state
> - * of the panel.
> + * of the panel and either prepare/unprepare the panel based on @prepare.
> */
> static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> bool prepare)
> @@ -983,12 +981,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> else
> ret = drm_panel_unprepare(dp->plat_data->panel);
>
> - if (ret)
> - goto out;
> -
> - dp->panel_is_modeset = prepare;
> -
> -out:
> mutex_unlock(&dp->panel_lock);
> return ret;
> }
> @@ -1532,7 +1524,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
>
> mutex_init(&dp->panel_lock);
> - dp->panel_is_modeset = false;
>
> /*
> * platform dp driver need containor_of the plat_data to get
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 774d11574b095b093ddf2818ad5b84be6605c9bf..b679d5b71d276f458d905c936160f107225bc6c5 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -170,7 +170,6 @@ struct analogix_dp_device {
> bool psr_supported;
>
> struct mutex panel_lock;
> - bool panel_is_modeset;
>
> struct analogix_dp_plat_data *plat_data;
> };
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock
2025-04-01 5:11 ` [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock Dmitry Baryshkov
2025-04-14 1:50 ` Damon Ding
@ 2025-04-17 15:47 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:47 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() function is called from bridge's
> atomic_pre_enable() and atomic_post_disable() callbacks, which can not
> happen simultaneously. Drop the useless mutex.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 --
> 2 files changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 704c6169116eb2601d2ad02dc7294455ceff5460..1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -974,14 +974,11 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> if (!dp->plat_data->panel)
> return 0;
>
> - mutex_lock(&dp->panel_lock);
> -
> if (prepare)
> ret = drm_panel_prepare(dp->plat_data->panel);
> else
> ret = drm_panel_unprepare(dp->plat_data->panel);
>
> - mutex_unlock(&dp->panel_lock);
> return ret;
> }
>
> @@ -1523,8 +1520,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> dp->dev = &pdev->dev;
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
>
> - mutex_init(&dp->panel_lock);
> -
> /*
> * platform dp driver need containor_of the plat_data to get
> * the driver private data, so we need to store the point of
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b679d5b71d276f458d905c936160f107225bc6c5..2b54120ba4a3ef55af518a0629a5c8469f275a0f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -169,8 +169,6 @@ struct analogix_dp_device {
> bool fast_train_enable;
> bool psr_supported;
>
> - struct mutex panel_lock;
> -
> struct analogix_dp_plat_data *plat_data;
> };
>
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
2025-04-01 5:11 ` [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel() Dmitry Baryshkov
2025-04-14 1:54 ` Damon Ding
@ 2025-04-17 15:48 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:48 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> The analogix_dp_prepare_panel() is now only calling a corresponding
> drm_panel function. Inline it to simplify the code.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 43 +++++-----------------
> 1 file changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 1ec4d277fe6bb03ad9eb8451ae2af5ad5ada0978..f60068011008482f7b7b2edfcab5fb1b3e9e130f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -955,33 +955,6 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
> }
>
> -/*
> - * This function is a bit of a catch-all for panel preparation, hopefully
> - * simplifying the logic of functions that need to prepare/unprepare the panel
> - * below.
> - *
> - * If @prepare is true, this function will prepare the panel. Conversely, if it
> - * is false, the panel will be unprepared.
> - *
> - * The function will disregard the current state
> - * of the panel and either prepare/unprepare the panel based on @prepare.
> - */
> -static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> - bool prepare)
> -{
> - int ret = 0;
> -
> - if (!dp->plat_data->panel)
> - return 0;
> -
> - if (prepare)
> - ret = drm_panel_prepare(dp->plat_data->panel);
> - else
> - ret = drm_panel_unprepare(dp->plat_data->panel);
> -
> - return ret;
> -}
> -
> static int analogix_dp_get_modes(struct drm_connector *connector)
> {
> struct analogix_dp_device *dp = to_dp(connector);
> @@ -1174,9 +1147,11 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - ret = analogix_dp_prepare_panel(dp, true);
> - if (ret)
> - DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> + if (dp->plat_data->panel) {
> + ret = drm_panel_prepare(dp->plat_data->panel);
> + if (ret)
> + DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
> + }
> }
>
> static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> @@ -1274,9 +1249,11 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - ret = analogix_dp_prepare_panel(dp, false);
> - if (ret)
> - DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> + if (dp->plat_data->panel) {
> + ret = drm_panel_unprepare(dp->plat_data->panel);
> + if (ret)
> + DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
> + }
>
> dp->fast_train_enable = false;
> dp->psr_supported = false;
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
2025-04-01 5:11 ` [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls Dmitry Baryshkov
2025-04-14 1:58 ` Damon Ding
@ 2025-04-17 15:48 ` Neil Armstrong
1 sibling, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-17 15:48 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> Follow the example of other drivers and ignore return values of the
> drm_panel_prepare() / unprepare() / enable() / disable() calls. There is
> no possible error recovery, so the driver just logs a message.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 39 ++++------------------
> 1 file changed, 6 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index f60068011008482f7b7b2edfcab5fb1b3e9e130f..c7dffdae31877ae194fc6b0a5bf21be203f7dcc4 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -838,10 +838,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
> int ret;
>
> /* Keep the panel disabled while we configure video */
> - if (dp->plat_data->panel) {
> - if (drm_panel_disable(dp->plat_data->panel))
> - DRM_ERROR("failed to disable the panel\n");
> - }
> + drm_panel_disable(dp->plat_data->panel);
>
> ret = analogix_dp_train_link(dp);
> if (ret) {
> @@ -863,13 +860,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
> }
>
> /* Safe to enable the panel now */
> - if (dp->plat_data->panel) {
> - ret = drm_panel_enable(dp->plat_data->panel);
> - if (ret) {
> - DRM_ERROR("failed to enable the panel\n");
> - return ret;
> - }
> - }
> + drm_panel_enable(dp->plat_data->panel);
>
> /* Check whether panel supports fast training */
> ret = analogix_dp_fast_link_train_detection(dp);
> @@ -1136,7 +1127,6 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> struct analogix_dp_device *dp = bridge->driver_private;
> struct drm_crtc *crtc;
> struct drm_crtc_state *old_crtc_state;
> - int ret;
>
> crtc = analogix_dp_get_new_crtc(dp, old_state);
> if (!crtc)
> @@ -1147,11 +1137,7 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> if (old_crtc_state && old_crtc_state->self_refresh_active)
> return;
>
> - if (dp->plat_data->panel) {
> - ret = drm_panel_prepare(dp->plat_data->panel);
> - if (ret)
> - DRM_ERROR("failed to prepare the panel ret = %d\n", ret);
> - }
> + drm_panel_prepare(dp->plat_data->panel);
> }
>
> static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> @@ -1231,17 +1217,11 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> {
> struct analogix_dp_device *dp = bridge->driver_private;
> - int ret;
>
> if (dp->dpms_mode != DRM_MODE_DPMS_ON)
> return;
>
> - if (dp->plat_data->panel) {
> - if (drm_panel_disable(dp->plat_data->panel)) {
> - DRM_ERROR("failed to disable the panel\n");
> - return;
> - }
> - }
> + drm_panel_disable(dp->plat_data->panel);
>
> disable_irq(dp->irq);
>
> @@ -1249,11 +1229,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>
> pm_runtime_put_sync(dp->dev);
>
> - if (dp->plat_data->panel) {
> - ret = drm_panel_unprepare(dp->plat_data->panel);
> - if (ret)
> - DRM_ERROR("failed to unprepare the panel ret = %d\n", ret);
> - }
> + drm_panel_unprepare(dp->plat_data->panel);
>
> dp->fast_train_enable = false;
> dp->psr_supported = false;
> @@ -1678,10 +1654,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
> analogix_dp_bridge_disable(dp->bridge);
> dp->connector.funcs->destroy(&dp->connector);
>
> - if (dp->plat_data->panel) {
> - if (drm_panel_unprepare(dp->plat_data->panel))
> - DRM_ERROR("failed to turnoff the panel\n");
> - }
> + drm_panel_unprepare(dp->plat_data->panel);
>
> drm_dp_aux_unregister(&dp->aux);
>
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (6 preceding siblings ...)
2025-04-01 5:11 ` [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void Dmitry Baryshkov
@ 2025-04-18 13:22 ` Neil Armstrong
2025-04-22 13:56 ` Dmitry Baryshkov
8 siblings, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-18 13:22 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> While it makes sense for panel callbacks to be able to return an error,
> the state-management functions from drm_panel API are usually called
> from atomic callbacks, which can not fails and must return void. Make
> drm_panel_prepare(), drm_panel_enable(), drm_panel_disable() and
> drm_panel_unprepare() follow the same calling convention and return
> void.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> Dmitry Baryshkov (7):
> drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
> drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
> drm/bridge: analogic_dp: drop panel_is_modeset
> drm/bridge: analogic_dp: drop panel_lock
> drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
> drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
> drm/panel: make prepare/enable and disable/unprepare calls return void
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 107 ++-------------------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 -
> drivers/gpu/drm/drm_panel.c | 54 ++++-------
> drivers/gpu/drm/panel/panel-newvision-nv3051d.c | 9 +-
> include/drm/drm_panel.h | 8 +-
> 5 files changed, 33 insertions(+), 148 deletions(-)
> ---
> base-commit: bfb20a655848a9088e3e9ae24b1dcce1bbf016c2
> change-id: 20250401-panel-return-void-d461c71ebd35
>
> Best regards,
I think you can respin with the warning fix on patch 1 and then apply
Neil
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset
2025-04-17 15:47 ` Neil Armstrong
@ 2025-04-20 22:55 ` Dmitry Baryshkov
0 siblings, 0 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-20 22:55 UTC (permalink / raw)
To: Neil Armstrong
Cc: Andrzej Hajda, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang,
dri-devel, linux-kernel
On Thu, Apr 17, 2025 at 05:47:00PM +0200, Neil Armstrong wrote:
> On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> > The dp->panel_is_modeset is now a write-only field. Drop it completely.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Neil, it looks like you wanted to respond with either a comment or an
r-b tag, but none were present in the email...
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +----------
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 1 -
> > 2 files changed, 1 insertion(+), 11 deletions(-)
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
2025-04-14 2:01 ` Damon Ding
@ 2025-04-20 22:56 ` Dmitry Baryshkov
0 siblings, 0 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-20 22:56 UTC (permalink / raw)
To: Damon Ding
Cc: Laurent.pinchart, airlied, andrzej.hajda, dri-devel,
jernej.skrabec, jonas, linux-kernel, maarten.lankhorst, mripard,
neil.armstrong, quic_jesszhan, rfoss, simona, tzimmermann
On Mon, Apr 14, 2025 at 10:01:16AM +0800, Damon Ding wrote:
> On 2025/4/14 9:20, Damon Ding wrote:
> > Hi Dmitry,
> >
> > On 2025/4/1 13:11, Dmitry Baryshkov wrote:
> > > The analogix_dp_prepare_panel() returns immediately if there is no
> > > attached panel. Drop several calls to this function which are performed
> > > when dp->plat_data->panel is NULL.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > ---
> > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 22
> > > +---------------------
> > > 1 file changed, 1 insertion(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/
> > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > index 042154e2d8ccd74ac2dc27fe527e810a50e08a09..ab1cfc340aa95bbf13fe021bd33227b565a5458d
> > > 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > @@ -1005,17 +1005,11 @@ static int analogix_dp_get_modes(struct
> > > drm_connector *connector)
> > > {
> > > struct analogix_dp_device *dp = to_dp(connector);
> > > const struct drm_edid *drm_edid;
> > > - int ret, num_modes = 0;
> > > + int num_modes = 0;
> > > if (dp->plat_data->panel) {
> > > num_modes += drm_panel_get_modes(dp->plat_data->panel,
> > > connector);
> > > } else {
> > > - ret = analogix_dp_prepare_panel(dp, true, false);
> > > - if (ret) {
> > > - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> > > - return 0;
> > > - }
> > > -
> > > drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
> > > drm_edid_connector_update(&dp->connector, drm_edid);
> > > @@ -1024,10 +1018,6 @@ static int analogix_dp_get_modes(struct
> > > drm_connector *connector)
> > > num_modes += drm_edid_connector_add_modes(&dp->connector);
> > > drm_edid_free(drm_edid);
> > > }
> > > -
> > > - ret = analogix_dp_prepare_panel(dp, false, false);
> > > - if (ret)
> > > - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> > > }
> > > if (dp->plat_data->get_modes)
> > > @@ -1087,19 +1077,9 @@ analogix_dp_detect(struct drm_connector
> > > *connector, bool force)
> > > if (dp->plat_data->panel)
> > > return connector_status_connected;
> > > - ret = analogix_dp_prepare_panel(dp, true, false);
> > > - if (ret) {
> > > - DRM_ERROR("Failed to prepare panel (%d)\n", ret);
> > > - return connector_status_disconnected;
> > > - }
> > > -
> > > if (!analogix_dp_detect_hpd(dp))
> > > status = connector_status_connected;
> > > - ret = analogix_dp_prepare_panel(dp, false, false);
> > > - if (ret)
> > > - DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> > > -
> > > return status;
> > > }
> > >
> >
> > With the patch series:
> > https://patchwork.kernel.org/project/linux-rockchip/list/?series=942183
> >
> > When I verify this patch, there will be a small warning:
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1075:6: warning:
> > unused variable 'ret' [-Wunused-variable]
> > int ret;
> > ^
> >
> > It should be better to remove the unused 'ret'.
I can't reproduce the warning, so I'd like to push the series once patch
3/7 gets an R-B.
> >
> > Best regards,
> > Damon
> >
> >
> >
>
> Tested-by: Damon Ding <damon.ding@rock-chips.com>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
2025-04-14 1:46 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
@ 2025-04-22 13:42 ` Neil Armstrong
2 siblings, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2025-04-22 13:42 UTC (permalink / raw)
To: Dmitry Baryshkov, Andrzej Hajda, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang
Cc: dri-devel, linux-kernel
On 01/04/2025 07:11, Dmitry Baryshkov wrote:
> The dp->panel_is_modeset is now a write-only field. Drop it completely.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +----------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 1 -
> 2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 82dc4b01806f9728dc882b0128171838e81f21b0..704c6169116eb2601d2ad02dc7294455ceff5460 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -964,9 +964,7 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> * is false, the panel will be unprepared.
> *
> * The function will disregard the current state
> - * of the panel and either prepare/unprepare the panel based on @prepare. Once
> - * it finishes, it will update dp->panel_is_modeset to reflect the current state
> - * of the panel.
> + * of the panel and either prepare/unprepare the panel based on @prepare.
> */
> static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> bool prepare)
> @@ -983,12 +981,6 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,
> else
> ret = drm_panel_unprepare(dp->plat_data->panel);
>
> - if (ret)
> - goto out;
> -
> - dp->panel_is_modeset = prepare;
> -
> -out:
> mutex_unlock(&dp->panel_lock);
> return ret;
> }
> @@ -1532,7 +1524,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
>
> mutex_init(&dp->panel_lock);
> - dp->panel_is_modeset = false;
>
> /*
> * platform dp driver need containor_of the plat_data to get
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 774d11574b095b093ddf2818ad5b84be6605c9bf..b679d5b71d276f458d905c936160f107225bc6c5 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -170,7 +170,6 @@ struct analogix_dp_device {
> bool psr_supported;
>
> struct mutex panel_lock;
> - bool panel_is_modeset;
>
> struct analogix_dp_plat_data *plat_data;
> };
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
` (7 preceding siblings ...)
2025-04-18 13:22 ` [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare " Neil Armstrong
@ 2025-04-22 13:56 ` Dmitry Baryshkov
8 siblings, 0 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2025-04-22 13:56 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jessica Zhang,
Dmitry Baryshkov
Cc: dri-devel, linux-kernel
On Tue, 01 Apr 2025 08:11:32 +0300, Dmitry Baryshkov wrote:
> While it makes sense for panel callbacks to be able to return an error,
> the state-management functions from drm_panel API are usually called
> from atomic callbacks, which can not fails and must return void. Make
> drm_panel_prepare(), drm_panel_enable(), drm_panel_disable() and
> drm_panel_unprepare() follow the same calling convention and return
> void.
>
> [...]
Applied to drm-misc-next, thanks!
[1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel()
commit: b65cbfe0e1d8bcb74dbeb221d423dd926f326d05
[2/7] drm/bridge: analogix_dp: drop unused argument to analogix_dp_prepare_panel()
commit: 0ce432e18c932a7fd219b9c2bf1db623fb0f7840
[3/7] drm/bridge: analogic_dp: drop panel_is_modeset
commit: 4fc72e1fc46b0dca74c1f8c94091e524ad5b9fdb
[4/7] drm/bridge: analogic_dp: drop panel_lock
commit: 62ca1c3a146c73f8b8a381bf4461a82951defb8b
[5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel()
commit: d5f34ca5df7c0ae075fdfeba20edd6f81c792554
[6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls
commit: 58a71d2bf103550f809987e16d8c1949be719040
[7/7] drm/panel: make prepare/enable and disable/unprepare calls return void
commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
Best regards,
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-04-22 13:56 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-01 5:11 [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare return void Dmitry Baryshkov
2025-04-01 5:11 ` [PATCH 1/7] drm/bridge: analogix_dp: drop extra calls to analogix_dp_prepare_panel() Dmitry Baryshkov
2025-04-14 1:20 ` Damon Ding
2025-04-14 2:01 ` Damon Ding
2025-04-20 22:56 ` Dmitry Baryshkov
2025-04-17 15:46 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 2/7] drm/bridge: analogix_dp: drop unused argument " Dmitry Baryshkov
2025-04-14 1:38 ` Damon Ding
2025-04-17 15:46 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 3/7] drm/bridge: analogic_dp: drop panel_is_modeset Dmitry Baryshkov
2025-04-14 1:46 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
2025-04-20 22:55 ` Dmitry Baryshkov
2025-04-22 13:42 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 4/7] drm/bridge: analogic_dp: drop panel_lock Dmitry Baryshkov
2025-04-14 1:50 ` Damon Ding
2025-04-17 15:47 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 5/7] drm/bridge: analogix_dp: inline analogix_dp_prepare_panel() Dmitry Baryshkov
2025-04-14 1:54 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 6/7] drm/bridge: analogix_dp: ignore return values of drm_panel_* calls Dmitry Baryshkov
2025-04-14 1:58 ` Damon Ding
2025-04-17 15:48 ` Neil Armstrong
2025-04-01 5:11 ` [PATCH 7/7] drm/panel: make prepare/enable and disable/unprepare calls return void Dmitry Baryshkov
2025-04-11 8:55 ` Neil Armstrong
2025-04-18 13:22 ` [PATCH 0/7] drm/panel: make prepare / enable / disable / unprepare " Neil Armstrong
2025-04-22 13:56 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox