* [PATCH v2 01/20] drm/atomic: Handle max bpc properties before connector state allocation
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-10 2:48 ` Chaoyi Chen
2026-10-09 3:15 ` [PATCH v2 02/20] drm/connector: Drop early state allocation for max bpc registration Xilin Wu
` (18 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Allow drivers to attach the max bpc property before allocating connector
state, as needed by the upcoming non-HDMI bridge connector support.
Only update the current state when one exists.
Initialize max_requested_bpc and max_bpc from the attached property default
when creating connector state. Use drm_object_property_get_default_value()
rather than the range maximum so that state creation and subsequent resets
restore the value chosen when attaching the property.
Cover deferred allocation, existing state and restoration of a default
that differs from the range maximum in the connector KUnit tests.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 8 +++++
drivers/gpu/drm/drm_connector.c | 6 ++--
drivers/gpu/drm/tests/drm_connector_test.c | 53 ++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index a2ef272e9f27..8352b5a9097a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -494,7 +494,15 @@ void
__drm_atomic_helper_connector_state_init(struct drm_connector_state *conn_state,
struct drm_connector *connector)
{
+ u64 val;
+
conn_state->connector = connector;
+ if (connector->max_bpc_property &&
+ !drm_object_property_get_default_value(&connector->base,
+ connector->max_bpc_property, &val)) {
+ conn_state->max_requested_bpc = val;
+ conn_state->max_bpc = val;
+ }
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_state_init);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 8b4baed060f3..34c30469f405 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2886,8 +2886,10 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
}
drm_object_attach_property(&connector->base, prop, max);
- connector->state->max_requested_bpc = max;
- connector->state->max_bpc = max;
+ if (connector->state) {
+ connector->state->max_requested_bpc = max;
+ connector->state->max_bpc = max;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index beb1d50a6646..1174607441b9 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -12,6 +12,7 @@
#include <drm/drm_file.h>
#include <drm/drm_kunit_helpers.h>
#include <drm/drm_modes.h>
+#include <drm/drm_property.h>
#include <drm/display/drm_hdmi_helper.h>
@@ -187,7 +188,59 @@ KUNIT_ARRAY_PARAM(drm_connector_init_type_valid,
drm_connector_init_type_valid_tests,
drm_connector_init_type_desc);
+/* The attached default need not equal the upper end of the property range. */
+static void drm_test_connector_max_bpc_default(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ int ret;
+
+ ret = drmm_connector_init(&priv->drm, connector, &dummy_funcs,
+ DRM_MODE_CONNECTOR_DisplayPort, NULL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ prop = drm_property_create_range(&priv->drm, 0, "max bpc", 6, 12);
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ connector->max_bpc_property = prop;
+ ret = drm_connector_attach_max_bpc_property(connector, 6, 10);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ KUNIT_EXPECT_NULL(test, connector->state);
+
+ drm_mode_config_reset(&priv->drm);
+ KUNIT_ASSERT_NOT_NULL(test, connector->state);
+ KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
+ KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
+
+ connector->state->max_requested_bpc = 8;
+ connector->state->max_bpc = 8;
+ drm_mode_config_reset(&priv->drm);
+ KUNIT_ASSERT_NOT_NULL(test, connector->state);
+ KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
+ KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
+}
+
+static void drm_test_connector_max_bpc_existing_state(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ int ret;
+
+ ret = drmm_connector_init(&priv->drm, connector, &dummy_funcs,
+ DRM_MODE_CONNECTOR_DisplayPort, NULL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ drm_mode_config_reset(&priv->drm);
+ KUNIT_ASSERT_NOT_NULL(test, connector->state);
+
+ ret = drm_connector_attach_max_bpc_property(connector, 6, 10);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
+ KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
+}
+
static struct kunit_case drmm_connector_init_tests[] = {
+ KUNIT_CASE(drm_test_connector_max_bpc_default),
+ KUNIT_CASE(drm_test_connector_max_bpc_existing_state),
KUNIT_CASE(drm_test_drmm_connector_init),
KUNIT_CASE(drm_test_drmm_connector_init_null_ddc),
KUNIT_CASE_PARAM(drm_test_drmm_connector_init_type_valid,
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 01/20] drm/atomic: Handle max bpc properties before connector state allocation
2026-10-09 3:15 ` [PATCH v2 01/20] drm/atomic: Handle max bpc properties before connector state allocation Xilin Wu
@ 2026-10-10 2:48 ` Chaoyi Chen
0 siblings, 0 replies; 32+ messages in thread
From: Chaoyi Chen @ 2026-10-10 2:48 UTC (permalink / raw)
To: Xilin Wu
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl, Igor Paunovic, dri-devel,
linux-kernel, linux-arm-msm, freedreno, dragon, linux-amlogic,
linux-arm-kernel
Hi Xilin,
On 10/9/2026 11:15 AM, Xilin Wu wrote:
> Allow drivers to attach the max bpc property before allocating connector
> state, as needed by the upcoming non-HDMI bridge connector support.
> Only update the current state when one exists.
>
> Initialize max_requested_bpc and max_bpc from the attached property default
> when creating connector state. Use drm_object_property_get_default_value()
> rather than the range maximum so that state creation and subsequent resets
> restore the value chosen when attaching the property.
>
> Cover deferred allocation, existing state and restoration of a default
> that differs from the range maximum in the connector KUnit tests.
>
> Assisted-by: LLM
> Signed-off-by: Xilin Wu <sophon@radxa.com>
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 8 +++++
> drivers/gpu/drm/drm_connector.c | 6 ++--
> drivers/gpu/drm/tests/drm_connector_test.c | 53 ++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index a2ef272e9f27..8352b5a9097a 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -494,7 +494,15 @@ void
> __drm_atomic_helper_connector_state_init(struct drm_connector_state *conn_state,
> struct drm_connector *connector)
> {
> + u64 val;
> +
> conn_state->connector = connector;
> + if (connector->max_bpc_property &&
> + !drm_object_property_get_default_value(&connector->base,
> + connector->max_bpc_property, &val)) {
> + conn_state->max_requested_bpc = val;
> + conn_state->max_bpc = val;
> + }
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_state_init);
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8b4baed060f3..34c30469f405 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2886,8 +2886,10 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> }
>
> drm_object_attach_property(&connector->base, prop, max);
> - connector->state->max_requested_bpc = max;
> - connector->state->max_bpc = max;
> + if (connector->state) {
> + connector->state->max_requested_bpc = max;
> + connector->state->max_bpc = max;
> + }
>
And for patch1/2. I don't think it's right way to go.
As comment said:
drm_connector_attach_max_bpc_property() requires the connector to have a state.
There are two reasons here. First, most drivers follow the convention
described in this comment, but you only modified some of them. Second,
it appears you are removing the connector state, so the
"if (connector->state)" check here would always evaluate to false,
which doesn't seem to make much sense.
> return 0;
> }
> diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
> index beb1d50a6646..1174607441b9 100644
> --- a/drivers/gpu/drm/tests/drm_connector_test.c
> +++ b/drivers/gpu/drm/tests/drm_connector_test.c
> @@ -12,6 +12,7 @@
> #include <drm/drm_file.h>
> #include <drm/drm_kunit_helpers.h>
> #include <drm/drm_modes.h>
> +#include <drm/drm_property.h>
>
> #include <drm/display/drm_hdmi_helper.h>
>
> @@ -187,7 +188,59 @@ KUNIT_ARRAY_PARAM(drm_connector_init_type_valid,
> drm_connector_init_type_valid_tests,
> drm_connector_init_type_desc);
>
> +/* The attached default need not equal the upper end of the property range. */
> +static void drm_test_connector_max_bpc_default(struct kunit *test)
> +{
> + struct drm_connector_init_priv *priv = test->priv;
> + struct drm_connector *connector = &priv->connector;
> + struct drm_property *prop;
> + int ret;
> +
> + ret = drmm_connector_init(&priv->drm, connector, &dummy_funcs,
> + DRM_MODE_CONNECTOR_DisplayPort, NULL);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> + prop = drm_property_create_range(&priv->drm, 0, "max bpc", 6, 12);
> + KUNIT_ASSERT_NOT_NULL(test, prop);
> + connector->max_bpc_property = prop;
> + ret = drm_connector_attach_max_bpc_property(connector, 6, 10);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + KUNIT_EXPECT_NULL(test, connector->state);
> +
> + drm_mode_config_reset(&priv->drm);
> + KUNIT_ASSERT_NOT_NULL(test, connector->state);
> + KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
> + KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
> +
> + connector->state->max_requested_bpc = 8;
> + connector->state->max_bpc = 8;
> + drm_mode_config_reset(&priv->drm);
> + KUNIT_ASSERT_NOT_NULL(test, connector->state);
> + KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
> + KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
> +}
> +
> +static void drm_test_connector_max_bpc_existing_state(struct kunit *test)
> +{
> + struct drm_connector_init_priv *priv = test->priv;
> + struct drm_connector *connector = &priv->connector;
> + int ret;
> +
> + ret = drmm_connector_init(&priv->drm, connector, &dummy_funcs,
> + DRM_MODE_CONNECTOR_DisplayPort, NULL);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + drm_mode_config_reset(&priv->drm);
> + KUNIT_ASSERT_NOT_NULL(test, connector->state);
> +
> + ret = drm_connector_attach_max_bpc_property(connector, 6, 10);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + KUNIT_EXPECT_EQ(test, connector->state->max_requested_bpc, 10);
> + KUNIT_EXPECT_EQ(test, connector->state->max_bpc, 10);
> +}
> +
> static struct kunit_case drmm_connector_init_tests[] = {
> + KUNIT_CASE(drm_test_connector_max_bpc_default),
> + KUNIT_CASE(drm_test_connector_max_bpc_existing_state),
> KUNIT_CASE(drm_test_drmm_connector_init),
> KUNIT_CASE(drm_test_drmm_connector_init_null_ddc),
> KUNIT_CASE_PARAM(drm_test_drmm_connector_init_type_valid,
>
--
Best,
Chaoyi
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 02/20] drm/connector: Drop early state allocation for max bpc registration
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
2026-10-09 3:15 ` [PATCH v2 01/20] drm/atomic: Handle max bpc properties before connector state allocation Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-09 3:15 ` [PATCH v2 03/20] drm/bridge-connector: Attach max bpc for non-HDMI bridges Xilin Wu
` (17 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
The max bpc property can now be attached before connector state exists,
and the atomic state helper restores its default when creating state.
Remove the early state allocation that was only needed to satisfy the
old property attachment requirement in the HDMI connector helper, Meson
and DW HDMI.
Update the HDMI connector tests to check bit-depth defaults after the
normal mode configuration reset, and check that property registration
itself no longer allocates state.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ------
drivers/gpu/drm/drm_connector.c | 16 ----------------
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 6 ------
drivers/gpu/drm/tests/drm_connector_test.c | 9 +++++++++
4 files changed, 9 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 34c5f583e910..a061a3a23ee5 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2601,12 +2601,6 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
DRM_MODE_CONNECTOR_HDMIA,
hdmi->ddc);
- /*
- * drm_connector_attach_max_bpc_property() requires the
- * connector to have a state.
- */
- drm_atomic_helper_connector_reset(connector);
-
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 34c30469f405..2e0dbc58c13b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -614,22 +614,6 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
strtomem_pad(connector->hdmi.vendor, vendor, 0);
strtomem_pad(connector->hdmi.product, product, 0);
- /*
- * drm_connector_attach_max_bpc_property() requires the
- * connector to have a state.
- */
- if (connector->funcs->atomic_create_state) {
- struct drm_connector_state *state;
-
- state = connector->funcs->atomic_create_state(connector);
- if (IS_ERR(state))
- return PTR_ERR(state);
-
- connector->state = state;
- } else if (connector->funcs->reset) {
- connector->funcs->reset(connector);
- }
-
drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
connector->max_bpc = max_bpc;
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 0c7a72cb514a..3bfb4f8bec2c 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -439,12 +439,6 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
* encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[display connector bridge]->[display connector]
*/
- /*
- * drm_connector_attach_max_bpc_property() requires the
- * connector to have a state.
- */
- drm_atomic_helper_connector_reset(meson_encoder_hdmi->connector);
-
if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) ||
meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 1174607441b9..af5dda3701cf 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -1036,7 +1036,10 @@ static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, 0);
KUNIT_EXPECT_EQ(test, val, 8);
+ KUNIT_EXPECT_NULL(test, connector->state);
+ drm_mode_config_reset(&priv->drm);
state = connector->state;
+ KUNIT_ASSERT_NOT_NULL(test, state);
KUNIT_EXPECT_EQ(test, state->max_bpc, 8);
KUNIT_EXPECT_EQ(test, state->max_requested_bpc, 8);
@@ -1077,7 +1080,10 @@ static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, 0);
KUNIT_EXPECT_EQ(test, val, 10);
+ KUNIT_EXPECT_NULL(test, connector->state);
+ drm_mode_config_reset(&priv->drm);
state = connector->state;
+ KUNIT_ASSERT_NOT_NULL(test, state);
KUNIT_EXPECT_EQ(test, state->max_bpc, 10);
KUNIT_EXPECT_EQ(test, state->max_requested_bpc, 10);
@@ -1118,7 +1124,10 @@ static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, 0);
KUNIT_EXPECT_EQ(test, val, 12);
+ KUNIT_EXPECT_NULL(test, connector->state);
+ drm_mode_config_reset(&priv->drm);
state = connector->state;
+ KUNIT_ASSERT_NOT_NULL(test, state);
KUNIT_EXPECT_EQ(test, state->max_bpc, 12);
KUNIT_EXPECT_EQ(test, state->max_requested_bpc, 12);
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 03/20] drm/bridge-connector: Attach max bpc for non-HDMI bridges
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
2026-10-09 3:15 ` [PATCH v2 01/20] drm/atomic: Handle max bpc properties before connector state allocation Xilin Wu
2026-10-09 3:15 ` [PATCH v2 02/20] drm/connector: Drop early state allocation for max bpc registration Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-10 2:39 ` Chaoyi Chen
2026-10-09 3:15 ` [PATCH v2 04/20] drm/msm/dp: Accept a const SDP header when packing Xilin Wu
` (16 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Allow non-HDMI bridges to advertise their output bit-depth limit through
max_bpc. Attach the max bpc property when the lowest reported limit in the
chain exceeds 8, ignoring bridges that do not report a limit. This lets
DP drivers expose deep-color support without attaching the property in
their connector initialization code.
Use a lower bound of 6 for DP and eDP, which support 6 bpc RGB output, and
8 for other connector types. Keep the existing HDMI registration path.
The property can be attached before connector state allocation; the
atomic state helper initializes its default when state is created.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 18 ++++++++++++++++++
include/drm/drm_bridge.h | 11 ++++++++---
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 8b54069fa53a..be2e08a3694a 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -825,6 +825,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
unsigned int supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444);
unsigned int max_bpc = 8;
+ unsigned int chain_max_bpc = 0;
bool support_hdcp = false;
int connector_type;
int ret;
@@ -985,6 +986,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
panel_bridge = drm_bridge_get(bridge);
}
+ if (bridge->max_bpc)
+ chain_max_bpc = chain_max_bpc ?
+ min(chain_max_bpc, bridge->max_bpc) : bridge->max_bpc;
+
if (bridge->support_hdcp)
support_hdcp = true;
}
@@ -1026,6 +1031,19 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
connector_type, ddc);
if (ret)
return ERR_PTR(ret);
+
+ if (chain_max_bpc > 8) {
+ unsigned int min_bpc = 8;
+
+ if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector_type == DRM_MODE_CONNECTOR_eDP)
+ min_bpc = 6;
+
+ ret = drm_connector_attach_max_bpc_property(connector, min_bpc,
+ chain_max_bpc);
+ if (ret)
+ return ERR_PTR(ret);
+ }
}
if (bridge_connector->bridge_hdmi_audio ||
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 58fff047f43b..01ad1878149e 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1089,9 +1089,14 @@ struct drm_bridge {
unsigned int supported_formats;
/**
- * @max_bpc: Maximum bits per char the HDMI bridge supports. Allowed
- * values are 8, 10 and 12. This is only relevant if
- * @DRM_BRIDGE_OP_HDMI is set.
+ * @max_bpc: Maximum bits per color component the bridge supports.
+ * Zero means that the bridge does not report a limit. For HDMI bridges
+ * with @DRM_BRIDGE_OP_HDMI set, allowed values are 8, 10 and 12.
+ *
+ * For non-HDMI connectors, drm_bridge_connector_init() attaches the
+ * max bpc property when the lowest nonzero limit in the bridge chain
+ * exceeds 8. Bridges remain responsible for validating configurations
+ * against any limits not reported here.
*/
unsigned int max_bpc;
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 03/20] drm/bridge-connector: Attach max bpc for non-HDMI bridges
2026-10-09 3:15 ` [PATCH v2 03/20] drm/bridge-connector: Attach max bpc for non-HDMI bridges Xilin Wu
@ 2026-10-10 2:39 ` Chaoyi Chen
0 siblings, 0 replies; 32+ messages in thread
From: Chaoyi Chen @ 2026-10-10 2:39 UTC (permalink / raw)
To: Xilin Wu
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl, Igor Paunovic, dri-devel,
linux-kernel, linux-arm-msm, freedreno, dragon, linux-amlogic,
linux-arm-kernel
Hi Xilin,
On 10/9/2026 11:15 AM, Xilin Wu wrote:
> Allow non-HDMI bridges to advertise their output bit-depth limit through
> max_bpc. Attach the max bpc property when the lowest reported limit in the
> chain exceeds 8, ignoring bridges that do not report a limit. This lets
> DP drivers expose deep-color support without attaching the property in
> their connector initialization code.
>
> Use a lower bound of 6 for DP and eDP, which support 6 bpc RGB output, and
> 8 for other connector types. Keep the existing HDMI registration path.
> The property can be attached before connector state allocation; the
> atomic state helper initializes its default when state is created.
>
> Assisted-by: LLM
> Signed-off-by: Xilin Wu <sophon@radxa.com>
> ---
> drivers/gpu/drm/display/drm_bridge_connector.c | 18 ++++++++++++++++++
> include/drm/drm_bridge.h | 11 ++++++++---
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 8b54069fa53a..be2e08a3694a 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -825,6 +825,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
> unsigned int supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444);
> unsigned int max_bpc = 8;
> + unsigned int chain_max_bpc = 0;
> bool support_hdcp = false;
> int connector_type;
> int ret;
> @@ -985,6 +986,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> panel_bridge = drm_bridge_get(bridge);
> }
>
> + if (bridge->max_bpc)
> + chain_max_bpc = chain_max_bpc ?
> + min(chain_max_bpc, bridge->max_bpc) : bridge->max_bpc;
> +
The check on chain_max_bpc here doesn't seem to make any sense.
> if (bridge->support_hdcp)
> support_hdcp = true;
> }
> @@ -1026,6 +1031,19 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> connector_type, ddc);
> if (ret)
> return ERR_PTR(ret);
> +
> + if (chain_max_bpc > 8) {
> + unsigned int min_bpc = 8;
> +
> + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> + connector_type == DRM_MODE_CONNECTOR_eDP)
> + min_bpc = 6;
> +
> + ret = drm_connector_attach_max_bpc_property(connector, min_bpc,
> + chain_max_bpc);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> }
>
> if (bridge_connector->bridge_hdmi_audio ||
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 58fff047f43b..01ad1878149e 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -1089,9 +1089,14 @@ struct drm_bridge {
> unsigned int supported_formats;
>
> /**
> - * @max_bpc: Maximum bits per char the HDMI bridge supports. Allowed
> - * values are 8, 10 and 12. This is only relevant if
> - * @DRM_BRIDGE_OP_HDMI is set.
> + * @max_bpc: Maximum bits per color component the bridge supports.
> + * Zero means that the bridge does not report a limit. For HDMI bridges
> + * with @DRM_BRIDGE_OP_HDMI set, allowed values are 8, 10 and 12.
> + *
> + * For non-HDMI connectors, drm_bridge_connector_init() attaches the
> + * max bpc property when the lowest nonzero limit in the bridge chain
> + * exceeds 8. Bridges remain responsible for validating configurations
> + * against any limits not reported here.
> */
> unsigned int max_bpc;
>
>
--
Best,
Chaoyi
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 04/20] drm/msm/dp: Accept a const SDP header when packing
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (2 preceding siblings ...)
2026-10-09 3:15 ` [PATCH v2 03/20] drm/bridge-connector: Attach max bpc for non-HDMI bridges Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-09 3:15 ` [PATCH v2 05/20] drm/msm/dp: Support multiple generic SDP slots Xilin Wu
` (15 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel, Dmitry Baryshkov
The SDP header packing helper only reads the header. Mark its input const
so callers can pack headers from read-only SDP packets without discarding
the qualifier. This prepares for the generic SDP writer accepting const
packets without changing the packed header bytes.
Assisted-by: LLM
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_utils.c | 3 ++-
drivers/gpu/drm/msm/dp/dp_utils.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index 4a5ebb0c33b8..226b370ce8fe 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -74,7 +74,8 @@ u8 msm_dp_utils_calculate_parity(u32 data)
return parity_byte;
}
-void msm_dp_utils_pack_sdp_header(struct dp_sdp_header *sdp_header, u32 header_buff[2])
+void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
+ u32 header_buff[2])
{
header_buff[0] = FIELD_PREP(HEADER_0_MASK, sdp_header->HB0) |
FIELD_PREP(PARITY_0_MASK, msm_dp_utils_calculate_parity(sdp_header->HB0)) |
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index 2e4f98a863c4..2fdb89efc57c 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -31,6 +31,7 @@
u8 msm_dp_utils_get_g0_value(u8 data);
u8 msm_dp_utils_get_g1_value(u8 data);
u8 msm_dp_utils_calculate_parity(u32 data);
-void msm_dp_utils_pack_sdp_header(struct dp_sdp_header *sdp_header, u32 header_buff[2]);
+void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
+ u32 header_buff[2]);
#endif /* _DP_UTILS_H_ */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 05/20] drm/msm/dp: Support multiple generic SDP slots
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (3 preceding siblings ...)
2026-10-09 3:15 ` [PATCH v2 04/20] drm/msm/dp: Accept a const SDP header when packing Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-09 3:15 ` [PATCH v2 06/20] drm/msm/dp: Keep runtime PM calls outside the connection lock Xilin Wu
` (14 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel, Dmitry Baryshkov
The SDP writer currently targets Generic0 for VSC packets. Static HDR
metadata needs a separate slot so that both packets can be sent on the
same stream.
Allow the SDP writer to address a generic packet slot, and define the
Generic2 registers needed for static HDR metadata. Use unaligned
little-endian loads for the payload words.
The existing VSC packet continues to use Generic0 without changing its
layout or enable sequence.
Assisted-by: LLM
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_panel.c | 22 ++++++++++------------
drivers/gpu/drm/msm/dp/dp_reg.h | 4 ++++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index e76dad0f6663..fdf1a3f437ba 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -14,6 +14,7 @@
#include <linux/io.h>
#include <linux/types.h>
+#include <linux/unaligned.h>
#include <asm/byteorder.h>
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
@@ -442,22 +443,19 @@ void msm_dp_panel_clear_dsc_dto(struct msm_dp_panel *msm_dp_panel)
msm_dp_write_p0(panel, MMSS_DP_DSC_DTO, 0x0);
}
-static void msm_dp_panel_send_vsc_sdp(struct msm_dp_panel_private *panel, struct dp_sdp *vsc_sdp)
+static void msm_dp_panel_send_sdp(struct msm_dp_panel_private *panel,
+ const struct dp_sdp *sdp, u32 base)
{
u32 header[2];
- u32 val;
int i;
- msm_dp_utils_pack_sdp_header(&vsc_sdp->sdp_header, header);
+ msm_dp_utils_pack_sdp_header(&sdp->sdp_header, header);
+ msm_dp_write_link(panel, base, header[0]);
+ msm_dp_write_link(panel, base + 4, header[1]);
- msm_dp_write_link(panel, MMSS_DP_GENERIC0_0, header[0]);
- msm_dp_write_link(panel, MMSS_DP_GENERIC0_1, header[1]);
-
- for (i = 0; i < sizeof(vsc_sdp->db); i += 4) {
- val = ((vsc_sdp->db[i]) | (vsc_sdp->db[i + 1] << 8) | (vsc_sdp->db[i + 2] << 16) |
- (vsc_sdp->db[i + 3] << 24));
- msm_dp_write_link(panel, MMSS_DP_GENERIC0_2 + i, val);
- }
+ for (i = 0; i < sizeof(sdp->db); i += 4)
+ msm_dp_write_link(panel, base + 8 + i,
+ get_unaligned_le32(&sdp->db[i]));
}
static void msm_dp_panel_update_sdp(struct msm_dp_panel_private *panel)
@@ -487,7 +485,7 @@ void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, struct dp_sd
cfg2 |= GENERIC0_SDPSIZE_VALID;
msm_dp_write_link(panel, MMSS_DP_SDP_CFG2, cfg2);
- msm_dp_panel_send_vsc_sdp(panel, vsc_sdp);
+ msm_dp_panel_send_sdp(panel, vsc_sdp, MMSS_DP_GENERIC0_0);
/* indicates presence of VSC (BIT(6) of MISC1) */
misc |= DP_MISC1_VSC_SDP;
diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
index dda4b642c81b..6767a215b662 100644
--- a/drivers/gpu/drm/msm/dp/dp_reg.h
+++ b/drivers/gpu/drm/msm/dp/dp_reg.h
@@ -232,10 +232,12 @@
#define MMSS_DP_AUDIO_CTRL_RESET (0x00000214)
#define MMSS_DP_SDP_CFG (0x00000228)
+#define GEN2_SDP_EN BIT(19)
#define GEN0_SDP_EN (0x00020000)
#define MMSS_DP_SDP_CFG2 (0x0000022C)
#define MMSS_DP_AUDIO_TIMESTAMP_0 (0x00000230)
#define MMSS_DP_AUDIO_TIMESTAMP_1 (0x00000234)
+#define GENERIC2_SDPSIZE_VALID BIT(20)
#define GENERIC0_SDPSIZE_VALID (0x00010000)
#define MMSS_DP_AUDIO_STREAM_0 (0x00000240)
@@ -291,6 +293,8 @@
#define MMSS_DP_GENERIC1_8 (0x00000348)
#define MMSS_DP_GENERIC1_9 (0x0000034C)
+#define MMSS_DP_GENERIC2_0 (0x000003D8)
+
#define MMSS_DP_VSCEXT_0 (0x000002D0)
#define MMSS_DP_VSCEXT_1 (0x000002D4)
#define MMSS_DP_VSCEXT_2 (0x000002D8)
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 06/20] drm/msm/dp: Keep runtime PM calls outside the connection lock
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (4 preceding siblings ...)
2026-10-09 3:15 ` [PATCH v2 05/20] drm/msm/dp: Support multiple generic SDP slots Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-09 3:37 ` sashiko-bot
2026-10-09 3:15 ` [PATCH v2 07/20] drm/msm/dp: Serialize stream operations with HPD processing Xilin Wu
` (13 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Runtime suspend disables the display IRQ synchronously. Calling it while
holding plugged_lock can wait for an HPD IRQ thread that needs the same
lock. Acquiring a runtime PM reference under that lock can also wait for
an in-progress suspend with the same dependency.
Acquire runtime PM references before taking plugged_lock in plug and
detect handling, and release them after unlocking in unplug and detect
handling. Keep connection state and PHY operations protected while the
runtime PM reference holds the controller active.
This prepares for serializing stream operations with HPD processing
without extending the connection lock over synchronous runtime PM calls.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 3ca039ff57b8..32a2ac89d8ed 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -409,14 +409,14 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
dp->msm_dp_display.connector_type,
dp->link->sink_count);
- guard(mutex)(&dp->plugged_lock);
-
ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret) {
DRM_ERROR("failed to pm_runtime_resume\n");
return ret;
}
+ mutex_lock(&dp->plugged_lock);
+
msm_dp_aux_enable_xfers(dp->aux, true);
msm_dp_display_host_phy_init(dp);
@@ -428,6 +428,7 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
dp->link->sink_count);
dp->plugged = true;
+ mutex_unlock(&dp->plugged_lock);
return ret;
};
@@ -450,7 +451,7 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
{
struct platform_device *pdev = dp->msm_dp_display.pdev;
- guard(mutex)(&dp->plugged_lock);
+ mutex_lock(&dp->plugged_lock);
dp->panel->video_test = false;
@@ -460,8 +461,10 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
dp->msm_dp_display.connector_type,
dp->link->sink_count);
- if (!dp->plugged)
+ if (!dp->plugged) {
+ mutex_unlock(&dp->plugged_lock);
return 0;
+ }
/* Don't forget modes for eDP */
if (!dp->msm_dp_display.is_edp)
@@ -488,10 +491,10 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
dp->msm_dp_display.connector_type,
dp->link->sink_count);
- if (dp->plugged) {
- pm_runtime_put_sync(&pdev->dev);
- dp->plugged = false;
- }
+ dp->plugged = false;
+ mutex_unlock(&dp->plugged_lock);
+
+ pm_runtime_put_sync(&pdev->dev);
return 0;
}
@@ -899,19 +902,22 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
u8 dpcd[DP_RECEIVER_CAP_SIZE];
struct drm_dp_desc desc;
bool phy_deinit;
+ bool put;
int ret;
dp = to_dp_bridge(bridge)->msm_dp_display;
priv = container_of(dp, struct msm_dp_display_private, msm_dp_display);
- guard(mutex)(&priv->plugged_lock);
ret = pm_runtime_resume_and_get(&dp->pdev->dev);
if (ret) {
DRM_ERROR("failed to pm_runtime_resume\n");
return status;
}
+ mutex_lock(&priv->plugged_lock);
+ put = false;
+
phy_deinit = msm_dp_display_host_phy_init(priv);
msm_dp_aux_enable_xfers(priv->aux, true);
@@ -961,8 +967,12 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
msm_dp_display_host_phy_exit(priv);
}
- pm_runtime_put_sync(&dp->pdev->dev);
+ put = true;
}
+ mutex_unlock(&priv->plugged_lock);
+
+ if (put)
+ pm_runtime_put_sync(&dp->pdev->dev);
return status;
}
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 06/20] drm/msm/dp: Keep runtime PM calls outside the connection lock
2026-10-09 3:15 ` [PATCH v2 06/20] drm/msm/dp: Keep runtime PM calls outside the connection lock Xilin Wu
@ 2026-10-09 3:37 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:37 UTC (permalink / raw)
To: Xilin Wu; +Cc: dri-devel, linux-amlogic, neil.armstrong
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm: Unresolved ABBA deadlock between runtime_suspend and IRQ thread
--
commit 32f0e463b46b824d1b3126d8c017d6f4509a8c7b
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Keep runtime PM calls outside the connection lock
This commit shifts runtime PM calls outside the connection lock in the plug,
unplug, and detect handlers. The goal is to avoid holding the lock during
synchronous PM operations that might deadlock with HPD IRQ threads.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -488,10 +491,10 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
> dp->msm_dp_display.connector_type,
> dp->link->sink_count);
>
> - if (dp->plugged) {
> - pm_runtime_put_sync(&pdev->dev);
> - dp->plugged = false;
> - }
> + dp->plugged = false;
> + mutex_unlock(&dp->plugged_lock);
> +
> + pm_runtime_put_sync(&pdev->dev);
[Severity: High]
Does this still leave an ABBA deadlock, just shifted from the plugged_lock
to the PM runtime wait queue?
If a process thread (e.g., during unbind) calls this pm_runtime_put_sync()
and drops the PM usage count to 0, it triggers msm_dp_pm_runtime_suspend().
The suspend callback calls disable_irq(dp->irq), which synchronously blocks
waiting for the msm_dp_display_irq_thread to complete.
However, if the IRQ thread is concurrently handling an event, it executes
msm_dp_bridge_hpd_notify(), which unconditionally calls:
msm_dp_bridge_hpd_notify()
pm_runtime_resume_and_get(&msm_dp_display->pdev->dev)
Because the device state is RPM_SUSPENDING, the IRQ thread blocks on the
PM runtime wait queue waiting for the suspend to finish. The suspend thread
is blocked on disable_irq() waiting for the IRQ thread, resulting in a
deadlock.
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=6
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 07/20] drm/msm/dp: Serialize stream operations with HPD processing
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (5 preceding siblings ...)
2026-10-09 3:15 ` [PATCH v2 06/20] drm/msm/dp: Keep runtime PM calls outside the connection lock Xilin Wu
@ 2026-10-09 3:15 ` Xilin Wu
2026-10-09 3:30 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 08/20] drm/msm/dp: Track PHY power ownership Xilin Wu
` (12 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:15 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
HPD PHY test requests stop and restart the mainlink under plugged_lock,
but atomic stream enable and disable do not take that lock. Both paths
can check a resource flag before either updates it and acquire duplicate
clock or PHY references. They can also reset a link while the other path
is training it or enabling video.
Hold plugged_lock across link preparation, stream enable and post-enable,
and protect stream disable and PSR transitions with the same lock. Add
lock assertions to the internal stream helpers. Acquire the runtime PM
reference and handle the eDP plug event before entering the critical
section, and leave audio shutdown waits and runtime PM release outside.
This provides the serialization required by PHY ownership tracking and
the subsequent stream enable rollback paths.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 42 ++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 32a2ac89d8ed..787efbcb6c12 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -53,6 +53,7 @@ struct msm_dp_display_private {
bool phy_initialized;
bool audio_supported;
+ /* Serializes HPD, stream and PSR operations while runtime active. */
struct mutex plugged_lock;
bool plugged;
@@ -623,13 +624,10 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
return 0;
}
-static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
+static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
- int rc = 0;
- bool force_link_train = false;
-
- drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
+ int rc;
if (msm_dp_display->is_edp)
msm_dp_hpd_plug_handle(dp);
@@ -640,6 +638,19 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
return rc;
}
+ return 0;
+}
+
+static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
+{
+ struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+ int rc = 0;
+ bool force_link_train = false;
+
+ drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
+
+ lockdep_assert_held(&dp->plugged_lock);
+
if (dp->link->sink_count == 0)
return -ENOTCONN;
@@ -664,6 +675,8 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp,
int rc = 0;
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+ lockdep_assert_held(&dp->plugged_lock);
+
drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
if (msm_dp_display->power_on) {
drm_dbg_dp(dp->drm_dev, "Link already setup, return\n");
@@ -684,6 +697,8 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
+ lockdep_assert_held(&dp->plugged_lock);
+
rate = dp->link->link_params.rate;
if (dp->audio_supported) {
@@ -721,6 +736,8 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+ lockdep_assert_held(&dp->plugged_lock);
+
if (!msm_dp_display->power_on)
return 0;
@@ -880,6 +897,8 @@ void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
}
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
+ guard(mutex)(&dp->plugged_lock);
+
msm_dp_ctrl_set_psr(dp->ctrl, dp->panel, enter);
}
@@ -1449,7 +1468,13 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- rc = msm_dp_display_prepare_link(dp);
+ rc = msm_dp_display_prepare(dp);
+ if (rc)
+ return;
+
+ guard(mutex)(&dp->plugged_lock);
+
+ rc = msm_dp_display_prepare_link_locked(dp);
if (rc) {
DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
return;
@@ -1474,6 +1499,8 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp)
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
+ guard(mutex)(&msm_dp_display->plugged_lock);
+
/*
* If .atomic_enable() bailed out - link training failure is the common
* case - the mainlink was never brought up and ->power_on stayed false.
@@ -1511,7 +1538,8 @@ void msm_dp_display_atomic_post_disable(struct msm_dp *dp)
msm_dp_display_audio_notify_disable(msm_dp_display);
- msm_dp_display_disable(msm_dp_display, msm_dp_display->panel);
+ scoped_guard(mutex, &msm_dp_display->plugged_lock)
+ msm_dp_display_disable(msm_dp_display, msm_dp_display->panel);
msm_dp_display_unprepare(msm_dp_display);
}
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 07/20] drm/msm/dp: Serialize stream operations with HPD processing
2026-10-09 3:15 ` [PATCH v2 07/20] drm/msm/dp: Serialize stream operations with HPD processing Xilin Wu
@ 2026-10-09 3:30 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:30 UTC (permalink / raw)
To: Xilin Wu; +Cc: neil.armstrong, linux-amlogic, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm: sleepable mutex lock in atomic context (plugged_lock)
--
commit cb0347df94f2c11e58ecb3d98faa755fd4b526ef
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Serialize stream operations with HPD processing
HPD PHY test requests stop and restart the mainlink under plugged_lock,
but atomic stream enable and disable do not take that lock. Both paths
can check a resource flag before either updates it and acquire duplicate
clock or PHY references. They can also reset a link while the other path
is training it or enabling video.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 32a2ac89d8ed0..787efbcb6c124 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -1449,7 +1468,13 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
>
> dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>
> - rc = msm_dp_display_prepare_link(dp);
> + rc = msm_dp_display_prepare(dp);
> + if (rc)
> + return;
> +
> + guard(mutex)(&dp->plugged_lock);
[Severity: High]
Does this introduce a sleeping lock into an atomic context?
The DRM subsystem guidelines state that encoder atomic enable/disable
callbacks run in atomic context during non-blocking commits. Calling sleeping
functions, like acquiring a mutex, from an atomic context can cause kernel
warnings, system instability, and potential deadlocks.
> +
> + rc = msm_dp_display_prepare_link_locked(dp);
> if (rc) {
> DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
> return;
[ ... ]
> @@ -1474,6 +1499,8 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp)
>
> msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>
> + guard(mutex)(&msm_dp_display->plugged_lock);
[Severity: High]
Does this also introduce a sleeping lock into an atomic disable callback?
> +
> /*
> * If .atomic_enable() bailed out - link training failure is the common
> * case - the mainlink was never brought up and ->power_on stayed false.
[ ... ]
> @@ -1511,7 +1538,8 @@ void msm_dp_display_atomic_post_disable(struct msm_dp *dp)
>
> msm_dp_display_audio_notify_disable(msm_dp_display);
>
> - msm_dp_display_disable(msm_dp_display, msm_dp_display->panel);
> + scoped_guard(mutex, &msm_dp_display->plugged_lock)
> + msm_dp_display_disable(msm_dp_display, msm_dp_display->panel);
[Severity: High]
Could this scoped_guard() cause a sleep in atomic context during the
post-disable phase of an atomic commit?
>
> msm_dp_display_unprepare(msm_dp_display);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=7
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 08/20] drm/msm/dp: Track PHY power ownership
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (6 preceding siblings ...)
2026-10-09 3:15 ` [PATCH v2 07/20] drm/msm/dp: Serialize stream operations with HPD processing Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 09/20] drm/msm/dp: Unwind resources when enabling a stream fails Xilin Wu
` (11 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Mainlink restart and PHY test paths can power down the PHY before a later
cleanup attempts to release it again. The PHY core counts each power-off
call, so cleanup without an owned reference can underflow that count and
prevent a subsequent enable from calling the hardware power-on operation.
Record successful PHY power acquisition in the controller and route all
power-off paths through an ownership check. Keep the reference recorded
when power-off fails and avoid acquiring a second reference while it is
still held. Also skip releasing link clocks that are already disabled.
The preceding stream serialization change protects these ownership
checks against HPD PHY tests and link maintenance. Error propagation is
left to the subsequent stream enable rollback change.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 59070f399e2d..f005421630c6 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -136,6 +136,7 @@ struct msm_dp_ctrl_private {
u32 hw_revision;
+ bool phy_powered;
bool core_clks_on;
bool link_clks_on;
bool stream_clks_on;
@@ -1813,6 +1814,9 @@ static void msm_dp_ctrl_link_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
+ if (!ctrl->link_clks_on)
+ return;
+
clk_bulk_disable_unprepare(ctrl->num_link_clks, ctrl->link_clks);
ctrl->link_clks_on = false;
@@ -1824,6 +1828,23 @@ static void msm_dp_ctrl_link_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl)
str_on_off(ctrl->core_clks_on));
}
+static int msm_dp_ctrl_phy_power_off(struct msm_dp_ctrl_private *ctrl)
+{
+ int ret;
+
+ if (!ctrl->phy_powered)
+ return 0;
+
+ ret = phy_power_off(ctrl->phy);
+ if (ret) {
+ DRM_ERROR("Failed to power off PHY: %d\n", ret);
+ return ret;
+ }
+
+ ctrl->phy_powered = false;
+ return 0;
+}
+
static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private *ctrl,
struct msm_dp_panel *panel)
{
@@ -1836,7 +1857,11 @@ static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private *ctrl,
ctrl->phy_opts.dp.ssc = drm_dp_max_downspread(dpcd);
phy_configure(phy, &ctrl->phy_opts);
- phy_power_on(phy);
+ if (!ctrl->phy_powered) {
+ ret = phy_power_on(phy);
+ if (!ret)
+ ctrl->phy_powered = true;
+ }
dev_pm_opp_set_rate(ctrl->dev, ctrl->link->link_params.rate * 1000);
ret = msm_dp_ctrl_link_clk_enable(&ctrl->msm_dp_ctrl);
@@ -1996,7 +2021,7 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
*/
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
- phy_power_off(phy);
+ msm_dp_ctrl_phy_power_off(ctrl);
/* hw recommended delay before re-enabling clocks */
msleep(20);
@@ -2022,7 +2047,7 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
- phy_power_off(phy);
+ msm_dp_ctrl_phy_power_off(ctrl);
/* aux channel down, reinit phy */
phy_exit(phy);
@@ -2643,10 +2668,8 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl;
- struct phy *phy;
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- phy = ctrl->phy;
msm_dp_panel_disable_vsc_sdp(panel);
@@ -2656,7 +2679,7 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
- phy_power_off(phy);
+ msm_dp_ctrl_phy_power_off(ctrl);
}
irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 09/20] drm/msm/dp: Unwind resources when enabling a stream fails
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (7 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 08/20] drm/msm/dp: Track PHY power ownership Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:35 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 10/20] drm/msm/dp: Report stream enable failures through link status Xilin Wu
` (10 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Stream enable can fail after acquiring a runtime PM reference or
starting the link. Returning directly leaks those resources, while a
later disable can release a reference that was never acquired.
Track the stream PM reference and whether mainlink startup was
attempted. Use one rollback path for prepare, enable and post-enable
failures, and share the bookkeeping with normal disable to avoid
repeated release.
Keep AUX available for a connected external DP peer, including a
dongle without a downstream sink. If mainlink startup was attempted,
restore the AUX PHY after link shutdown, since partial startup can
power down AUX as well. Use plugged_lock to serialize this decision
with HPD processing, keeping rollback in the same critical section as
the failed enable. Release the lock before eDP unplug handling and
runtime PM put; leave the connection PM reference for the HPD path to
release.
Only access mainlink and SDP registers while the link clocks are
enabled. Startup or link reinitialization can fail with those clocks
already off; keep the controller reset and remaining PHY cleanup
independent of them.
With ownership tracking and rollback in place, propagate errors from
PHY initialization, configuration, power-on, eDP discovery and stream
retraining. Do not mark failed PHY initialization as successful, and
balance resources in the plug, detect and runtime resume paths as
well.
Disable the sink training pattern before returning a retraining error,
while AUX is still available. Preserve the original training error for
the caller to unwind the stream resources.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 42 +++++++----
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 135 +++++++++++++++++++++++++++++-------
3 files changed, 138 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index f005421630c6..f633f6cf5764 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1856,17 +1856,22 @@ static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private *ctrl,
ctrl->phy_opts.dp.link_rate = ctrl->link->link_params.rate / 100;
ctrl->phy_opts.dp.ssc = drm_dp_max_downspread(dpcd);
- phy_configure(phy, &ctrl->phy_opts);
+ ret = phy_configure(phy, &ctrl->phy_opts);
+ if (ret)
+ return ret;
if (!ctrl->phy_powered) {
ret = phy_power_on(phy);
- if (!ret)
- ctrl->phy_powered = true;
+ if (ret)
+ return ret;
+ ctrl->phy_powered = true;
}
dev_pm_opp_set_rate(ctrl->dev, ctrl->link->link_params.rate * 1000);
ret = msm_dp_ctrl_link_clk_enable(&ctrl->msm_dp_ctrl);
- if (ret)
+ if (ret) {
DRM_ERROR("Unable to start link clocks. ret=%d\n", ret);
+ msm_dp_ctrl_phy_power_off(ctrl);
+ }
drm_dbg_dp(ctrl->drm_dev, "link rate=%d\n", ctrl->link->link_params.rate);
@@ -1980,7 +1985,7 @@ static void msm_dp_ctrl_phy_reset(struct msm_dp_ctrl_private *ctrl)
msm_dp_write_ahb(ctrl, REG_DP_PHY_CTRL, 0x0);
}
-void msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl)
+int msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl)
{
struct msm_dp_ctrl_private *ctrl;
struct phy *phy;
@@ -1989,7 +1994,7 @@ void msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl)
phy = ctrl->phy;
msm_dp_ctrl_phy_reset(ctrl);
- phy_init(phy);
+ return phy_init(phy);
}
void msm_dp_ctrl_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl)
@@ -2021,7 +2026,9 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
*/
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
- msm_dp_ctrl_phy_power_off(ctrl);
+ ret = msm_dp_ctrl_phy_power_off(ctrl);
+ if (ret)
+ return ret;
/* hw recommended delay before re-enabling clocks */
msleep(20);
@@ -2038,16 +2045,21 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
struct msm_dp_panel *panel)
{
struct phy *phy;
+ int ret;
phy = ctrl->phy;
- msm_dp_ctrl_mainlink_disable(ctrl);
+ /* Reinitializing the link may already have disabled its clocks. */
+ if (ctrl->link_clks_on)
+ msm_dp_ctrl_mainlink_disable(ctrl);
msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl, panel);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
- msm_dp_ctrl_phy_power_off(ctrl);
+ ret = msm_dp_ctrl_phy_power_off(ctrl);
+ if (ret)
+ return ret;
/* aux channel down, reinit phy */
phy_exit(phy);
@@ -2584,9 +2596,9 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
}
if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
- msm_dp_ctrl_link_retrain(ctrl, panel);
+ ret = msm_dp_ctrl_link_retrain(ctrl, panel);
- /* stop txing train pattern to end link training */
+ /* Stop sink training even if retraining failed. */
msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
return ret;
@@ -2671,9 +2683,11 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- msm_dp_panel_disable_vsc_sdp(panel);
-
- msm_dp_ctrl_mainlink_disable(ctrl);
+ /* Link startup may have failed before enabling the link clocks. */
+ if (ctrl->link_clks_on) {
+ msm_dp_panel_disable_vsc_sdp(panel);
+ msm_dp_ctrl_mainlink_disable(ctrl);
+ }
msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl, panel);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 5902cf7e746a..42c5f847cb02 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -39,7 +39,7 @@ struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev,
void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel);
-void msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl);
+int msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_irq_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 787efbcb6c12..b076d44b5118 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -52,6 +52,8 @@ struct msm_dp_display_private {
bool core_initialized;
bool phy_initialized;
bool audio_supported;
+ bool stream_pm_active;
+ bool stream_link_attempted;
/* Serializes HPD, stream and PSR operations while runtime active. */
struct mutex plugged_lock;
@@ -323,22 +325,26 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
*
* Prepare DP PHY for the AUX transactions to succeed.
*
- * Returns: true if this call has initliazed the PHY and false if the PHY has
- * already been setup beforehand.
+ * Returns: 1 if this call initialized the PHY, 0 if it was already
+ * initialized, or a negative error code on failure.
*/
-static bool msm_dp_display_host_phy_init(struct msm_dp_display_private *dp)
+static int msm_dp_display_host_phy_init(struct msm_dp_display_private *dp)
{
+ int ret;
+
drm_dbg_dp(dp->drm_dev, "type=%d core_init=%d phy_init=%d\n",
dp->msm_dp_display.connector_type, dp->core_initialized,
dp->phy_initialized);
if (!dp->phy_initialized) {
- msm_dp_ctrl_phy_init(dp->ctrl);
+ ret = msm_dp_ctrl_phy_init(dp->ctrl);
+ if (ret)
+ return ret;
dp->phy_initialized = true;
- return true;
+ return 1;
}
- return false;
+ return 0;
}
static void msm_dp_display_host_phy_exit(struct msm_dp_display_private *dp)
@@ -420,7 +426,13 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
msm_dp_aux_enable_xfers(dp->aux, true);
- msm_dp_display_host_phy_init(dp);
+ ret = msm_dp_display_host_phy_init(dp);
+ if (ret < 0) {
+ msm_dp_aux_enable_xfers(dp->aux, false);
+ mutex_unlock(&dp->plugged_lock);
+ pm_runtime_put_sync(&pdev->dev);
+ return ret;
+ }
ret = msm_dp_display_process_hpd_high(dp);
@@ -629,8 +641,11 @@ static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
int rc;
- if (msm_dp_display->is_edp)
- msm_dp_hpd_plug_handle(dp);
+ if (msm_dp_display->is_edp) {
+ rc = msm_dp_hpd_plug_handle(dp);
+ if (rc)
+ return rc;
+ }
rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev);
if (rc) {
@@ -638,6 +653,8 @@ static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
return rc;
}
+ dp->stream_pm_active = true;
+
return 0;
}
@@ -655,10 +672,13 @@ static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
return -ENOTCONN;
if (!msm_dp_display->power_on) {
- msm_dp_display_host_phy_init(dp);
+ rc = msm_dp_display_host_phy_init(dp);
+ if (rc < 0)
+ return rc;
force_link_train = true;
}
+ dp->stream_link_attempted = true;
rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);
if (rc) {
DRM_ERROR("Failed link training (rc=%d)\n", rc);
@@ -750,6 +770,7 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, true);
msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel);
+ dp->stream_link_attempted = false;
if (dp->link->sink_count == 0)
/* re-init the PHY so that we can listen to Dongle disconnect */
@@ -937,7 +958,12 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
mutex_lock(&priv->plugged_lock);
put = false;
- phy_deinit = msm_dp_display_host_phy_init(priv);
+ ret = msm_dp_display_host_phy_init(priv);
+ if (ret < 0) {
+ put = true;
+ goto unlock;
+ }
+ phy_deinit = ret;
msm_dp_aux_enable_xfers(priv->aux, true);
@@ -988,6 +1014,7 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
put = true;
}
+unlock:
mutex_unlock(&priv->plugged_lock);
if (put)
@@ -1323,6 +1350,7 @@ static int msm_dp_pm_runtime_suspend(struct device *dev)
static int msm_dp_pm_runtime_resume(struct device *dev)
{
struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
+ int ret;
/*
* for eDP, host cotroller, HPD block and PHY are enabled here
@@ -1335,7 +1363,12 @@ static int msm_dp_pm_runtime_resume(struct device *dev)
msm_dp_display_host_init(dp);
if (dp->msm_dp_display.is_edp) {
msm_dp_aux_hpd_enable(dp->aux);
- msm_dp_display_host_phy_init(dp);
+ ret = msm_dp_display_host_phy_init(dp);
+ if (ret < 0) {
+ msm_dp_aux_hpd_disable(dp->aux);
+ msm_dp_display_host_deinit(dp);
+ return ret;
+ }
}
enable_irq(dp->irq);
@@ -1460,6 +1493,52 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel);
}
+static void msm_dp_display_abort_enable_locked(struct msm_dp_display_private *dp)
+{
+ bool keep_aux;
+ int ret;
+
+ lockdep_assert_held(&dp->plugged_lock);
+
+ keep_aux = !dp->msm_dp_display.is_edp && dp->plugged &&
+ msm_dp_aux_is_link_connected(dp->aux);
+
+ if (dp->stream_link_attempted) {
+ /* The AUX peer is the remaining dongle, not its absent sink. */
+ if (keep_aux && !dp->link->sink_count && dp->phy_initialized)
+ msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
+
+ msm_dp_ctrl_off_pixel_clk(dp->ctrl);
+ msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+ dp->stream_link_attempted = false;
+
+ /* Mainlink power-off can also power down the AUX circuitry. */
+ msm_dp_display_host_phy_exit(dp);
+ if (keep_aux) {
+ ret = msm_dp_display_host_phy_init(dp);
+ if (ret < 0) {
+ DRM_ERROR("Failed to restore AUX PHY: %d\n", ret);
+ msm_dp_aux_enable_xfers(dp->aux, false);
+ }
+ }
+ } else if (!keep_aux) {
+ msm_dp_display_host_phy_exit(dp);
+ }
+ dp->msm_dp_display.power_on = false;
+}
+
+static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)
+{
+ struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+
+ if (dp->stream_pm_active) {
+ pm_runtime_put_sync(&msm_dp_display->pdev->dev);
+ dp->stream_pm_active = false;
+ }
+
+ drm_dbg_dp(dp->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
+}
+
void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
struct drm_atomic_commit *state)
{
@@ -1470,27 +1549,40 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
rc = msm_dp_display_prepare(dp);
if (rc)
- return;
+ goto err_prepare;
- guard(mutex)(&dp->plugged_lock);
+ mutex_lock(&dp->plugged_lock);
rc = msm_dp_display_prepare_link_locked(dp);
if (rc) {
DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
- return;
+ goto err;
}
rc = msm_dp_display_enable(dp, dp->panel);
- if (rc)
+ if (rc) {
DRM_ERROR("DP display enable failed, rc=%d\n", rc);
+ goto err;
+ }
rc = msm_dp_display_post_enable(msm_dp_display);
if (rc) {
DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
- msm_dp_display_disable(dp, dp->panel);
+ goto err;
}
+ mutex_unlock(&dp->plugged_lock);
+
drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
+ return;
+
+err:
+ msm_dp_display_abort_enable_locked(dp);
+ mutex_unlock(&dp->plugged_lock);
+err_prepare:
+ if (msm_dp_display->is_edp)
+ msm_dp_hpd_unplug_handle(dp);
+ msm_dp_display_unprepare(dp);
}
void msm_dp_display_atomic_disable(struct msm_dp *dp)
@@ -1518,15 +1610,6 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp)
msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
}
-static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)
-{
- struct msm_dp *msm_dp_display = &dp->msm_dp_display;
-
- pm_runtime_put_sync(&msm_dp_display->pdev->dev);
-
- drm_dbg_dp(dp->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
-}
-
void msm_dp_display_atomic_post_disable(struct msm_dp *dp)
{
struct msm_dp_display_private *msm_dp_display;
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 09/20] drm/msm/dp: Unwind resources when enabling a stream fails
2026-10-09 3:16 ` [PATCH v2 09/20] drm/msm/dp: Unwind resources when enabling a stream fails Xilin Wu
@ 2026-10-09 3:35 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:35 UTC (permalink / raw)
To: Xilin Wu; +Cc: linux-amlogic, neil.armstrong, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pm: pm_runtime reference leak in msm_dp_bridge_detect()
- [Medium] drm/msm/dp: dead code in stream enable abort path due to sink_count invariant
--
commit aeec7eb4a5527629640011cba89d7bbaaf90341c
Author: Xilin Wu <sophon@radxa.com>
Subject: drm/msm/dp: Unwind resources when enabling a stream fails
This patch introduces resource unwinding for stream enable failures by tracking
the stream PM reference and mainlink startup status. It unifies the rollback
paths and propagates errors from PHY initialization.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 787efbcb6c124..b076d44b5118c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -988,6 +1014,7 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>
> put = true;
> }
> +unlock:
> mutex_unlock(&priv->plugged_lock);
>
> if (put)
> pm_runtime_put_sync(&dp->pdev->dev);
[Severity: High]
Does msm_dp_bridge_detect() leak a PM runtime reference here?
msm_dp_bridge_detect() unconditionally calls pm_runtime_resume_and_get()
earlier in the function. If the display is already connected (priv->plugged
is true), the exit path skips setting put = true.
As a result, pm_runtime_put_sync() is never called, which leaks the PM
reference. Since userspace can poll the detect callback repeatedly
(e.g., via DRM_IOCTL_MODE_GETCONNECTOR), could this leak the PM reference
indefinitely and permanently prevent the device from suspending?
[ ... ]
> @@ -1460,6 +1493,52 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
> msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel);
> }
>
> +static void msm_dp_display_abort_enable_locked(struct msm_dp_display_private *dp)
> +{
> + bool keep_aux;
> + int ret;
> +
> + lockdep_assert_held(&dp->plugged_lock);
> +
> + keep_aux = !dp->msm_dp_display.is_edp && dp->plugged &&
> + msm_dp_aux_is_link_connected(dp->aux);
> +
> + if (dp->stream_link_attempted) {
> + /* The AUX peer is the remaining dongle, not its absent sink. */
> + if (keep_aux && !dp->link->sink_count && dp->phy_initialized)
> + msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
[Severity: Medium]
Is this Power Save Mode (PSM) configuration block unreachable?
The condition !dp->link->sink_count is evaluated inside the
if (dp->stream_link_attempted) block. However, dp->stream_link_attempted
is only set to true in msm_dp_display_prepare_link_locked() if
dp->link->sink_count is non-zero.
Because both msm_dp_display_prepare_link_locked() and
msm_dp_display_abort_enable_locked() execute sequentially within the same
plugged_lock critical section in msm_dp_display_atomic_enable(), concurrent
HPD events cannot modify sink_count.
Doesn't this mean the inner check !dp->link->sink_count will always evaluate
to false, rendering this fallback dead code?
> +
> + msm_dp_ctrl_off_pixel_clk(dp->ctrl);
> + msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> + dp->stream_link_attempted = false;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=9
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 10/20] drm/msm/dp: Report stream enable failures through link status
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (8 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 09/20] drm/msm/dp: Unwind resources when enabling a stream fails Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:28 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 11/20] drm/msm/dp: Cache eDP link capabilities after successful discovery Xilin Wu
` (9 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Atomic bridge enable callbacks cannot return an error to userspace.
After a failed enable, leaving link-status unchanged gives userspace
no indication that it needs to retry the configuration.
Mark the connector link status bad from a work item after unwinding
the failed enable.
Send one connector hotplug notification per failure episode. The
notification also invokes the fbdev hotplug handler, which can retry
the modeset synchronously. Since atomic enable callbacks cannot
return errors, another stream enable failure can requeue this work
without reporting a failed commit to fbdev. Notifying on every
failure could therefore cause an unbounded retry loop.
Subsequent failures still restore BAD after a retry sets link-status
to GOOD, but suppress further notifications until a successful enable
or an external sink connection change. Do not reset the notification
latch during eDP's internal plug and unplug handling, which runs on
every retry.
Skip failures superseded by recovery or an external unplug before the
worker checks the failure state. Protect the failure state with
plugged_lock, then release it before calling
drm_connector_set_link_status_property(). The helper takes the
connection mutex, which precedes plugged_lock in atomic check's lock
order. Send the notification after the helper releases the mutex.
Recovery concurrent with the property update can cause an extra retry.
Initialize the work at probe and cancel it before unbinding the
display.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 61 +++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index b076d44b5118..816f2dc11aea 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -12,9 +12,11 @@
#include <linux/phy/phy.h>
#include <linux/delay.h>
#include <linux/string_choices.h>
+#include <linux/workqueue.h>
#include <drm/display/drm_dp_aux_bus.h>
#include <drm/display/drm_hdmi_audio_helper.h>
#include <drm/drm_edid.h>
+#include <drm/drm_probe_helper.h>
#include "msm_drv.h"
#include "msm_kms.h"
@@ -54,10 +56,14 @@ struct msm_dp_display_private {
bool audio_supported;
bool stream_pm_active;
bool stream_link_attempted;
+ struct work_struct link_status_work;
/* Serializes HPD, stream and PSR operations while runtime active. */
struct mutex plugged_lock;
bool plugged;
+ /* Protected by plugged_lock, including accesses from link_status_work. */
+ bool link_failed;
+ bool link_status_notified;
struct drm_device *drm_dev;
@@ -205,6 +211,39 @@ void msm_dp_display_signal_audio_complete(struct msm_dp *msm_dp_display)
complete_all(&dp->audio_comp);
}
+static void msm_dp_display_reset_link_status(struct msm_dp_display_private *dp)
+{
+ lockdep_assert_held(&dp->plugged_lock);
+
+ dp->link_failed = false;
+ dp->link_status_notified = false;
+}
+
+static void msm_dp_display_link_status_work(struct work_struct *work)
+{
+ struct msm_dp_display_private *dp = container_of(work,
+ struct msm_dp_display_private, link_status_work);
+ struct drm_connector *connector = dp->msm_dp_display.connector;
+ bool notify;
+
+ scoped_guard(mutex, &dp->plugged_lock) {
+ /* Skip failures superseded before this work starts. */
+ if (!dp->link_failed)
+ return;
+
+ notify = !dp->link_status_notified;
+ dp->link_status_notified = true;
+ }
+
+ /* The helper takes connection_mutex, so drop plugged_lock first. */
+ drm_connector_set_link_status_property(connector,
+ DRM_MODE_LINK_STATUS_BAD);
+
+ /* fbdev can retry the modeset synchronously from this notification. */
+ if (notify)
+ drm_kms_helper_connector_hotplug_event(connector);
+}
+
static int msm_dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -235,6 +274,7 @@ static void msm_dp_display_unbind(struct device *dev, struct device *master,
struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
struct msm_drm_private *priv = dev_get_drvdata(master);
+ cancel_work_sync(&dp->link_status_work);
of_dp_aux_depopulate_bus(dp->aux);
msm_dp_aux_unregister(dp->aux);
@@ -424,6 +464,10 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
mutex_lock(&dp->plugged_lock);
+ /* eDP calls this for every enable, including automatic retries. */
+ if (!dp->msm_dp_display.is_edp && !dp->plugged)
+ msm_dp_display_reset_link_status(dp);
+
msm_dp_aux_enable_xfers(dp->aux, true);
ret = msm_dp_display_host_phy_init(dp);
@@ -466,6 +510,9 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
mutex_lock(&dp->plugged_lock);
+ if (!dp->msm_dp_display.is_edp)
+ msm_dp_display_reset_link_status(dp);
+
dp->panel->video_test = false;
msm_dp_aux_enable_xfers(dp->aux, false);
@@ -529,10 +576,12 @@ static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp)
if (!rc) {
sink_request = dp->link->sink_request;
drm_dbg_dp(dp->drm_dev, "sink_request=%d\n", sink_request);
- if (sink_request & DS_PORT_STATUS_CHANGED)
+ if (sink_request & DS_PORT_STATUS_CHANGED) {
+ msm_dp_display_reset_link_status(dp);
rc = msm_dp_display_process_hpd_high(dp);
- else
+ } else {
rc = msm_dp_display_handle_irq_hpd(dp);
+ }
}
drm_dbg_dp(dp->drm_dev, "After, type=%d, sink_count=%d\n",
@@ -682,7 +731,6 @@ static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);
if (rc) {
DRM_ERROR("Failed link training (rc=%d)\n", rc);
- // TODO: schedule drm_connector_set_link_status_property()
return rc;
}
@@ -1278,6 +1326,7 @@ static int msm_dp_display_probe(struct platform_device *pdev)
dp->hpd_isr_status = 0;
mutex_init(&dp->plugged_lock);
+ INIT_WORK(&dp->link_status_work, msm_dp_display_link_status_work);
rc = msm_dp_display_get_io(dp);
if (rc)
@@ -1571,6 +1620,7 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
goto err;
}
+ msm_dp_display_reset_link_status(dp);
mutex_unlock(&dp->plugged_lock);
drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
@@ -1583,6 +1633,11 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
if (msm_dp_display->is_edp)
msm_dp_hpd_unplug_handle(dp);
msm_dp_display_unprepare(dp);
+ scoped_guard(mutex, &dp->plugged_lock) {
+ dp->link_failed = true;
+ /* Keep reporting BAD, but notify only once until recovery. */
+ schedule_work(&dp->link_status_work);
+ }
}
void msm_dp_display_atomic_disable(struct msm_dp *dp)
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 10/20] drm/msm/dp: Report stream enable failures through link status
2026-10-09 3:16 ` [PATCH v2 10/20] drm/msm/dp: Report stream enable failures through link status Xilin Wu
@ 2026-10-09 3:28 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:28 UTC (permalink / raw)
To: Xilin Wu; +Cc: dri-devel, neil.armstrong, linux-amlogic
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/msm/dp: mixing goto and scoped_guard in msm_dp_display_atomic_enable()
--
commit 5389d76b8ea91b24082be0a6254cd0c780518e0d
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Report stream enable failures through link status
This commit updates the driver to mark the connector link status as bad from
a work item after unwinding a failed enable. It sends a single hotplug
notification per failure episode, allowing fbdev to retry the modeset
synchronously while preventing unbounded retry loops.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index b076d44b5118c..816f2dc11aea1 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -1583,6 +1633,11 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
> if (msm_dp_display->is_edp)
> msm_dp_hpd_unplug_handle(dp);
> msm_dp_display_unprepare(dp);
> + scoped_guard(mutex, &dp->plugged_lock) {
[Severity: Low]
This isn't a functional bug, but does it mix goto-based error handling and
scope-based cleanup in the same function?
The function msm_dp_display_atomic_enable() uses goto-based error handling
with the err and err_prepare labels to manage its cleanup sequence.
The kernel guidelines in include/linux/cleanup.h advise against mixing
goto-based cleanup and guard declarations within the same routine, as it
can create confusing ownership semantics.
Could this be refactored to use standard mutex_lock() and mutex_unlock() to
maintain structural consistency?
> + dp->link_failed = true;
> + /* Keep reporting BAD, but notify only once until recovery. */
> + schedule_work(&dp->link_status_work);
> + }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=10
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 11/20] drm/msm/dp: Cache eDP link capabilities after successful discovery
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (9 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 10/20] drm/msm/dp: Report stream enable failures through link status Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:34 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 12/20] drm/msm/dp: Rebuild the eDP stream on modesets leaving self refresh Xilin Wu
` (8 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
An eDP panel may be powered off during mode validation. Keep its link
capabilities after the first successful discovery so subsequent checks
can use them across disable and suspend. eDP's internal unplug handling
is a power transition, not a change of sink. Continue invalidating
external DP capabilities on unplug and refresh them on discovery.
Discover eDP capabilities from connector detect before the first modeset.
Resume the AUX controller and prepare the fixed panel for the read, then
balance the panel preparation and runtime PM reference. Keep the panel
connected on an AUX error and allow a later probe to retry. Detection
with valid bandwidth limits does not touch panel power.
Track valid bandwidth limits separately from a complete capability cache.
Optional VSC or PSR read failures retain the existing fallback but do not
invalidate known bandwidth limits or permanently cache missing features.
Clear parsed link information before fresh discovery to avoid retaining
rates or flags from a previous sink or partially completed attempt.
Reuse the eDP receiver capabilities without skipping LTTPR setup, EDID
reads, sink requests, power management or link training. Refresh
LINK_BW_SET when selecting LINK_RATE_SET because it is configuration
state and can change across power cycles. A failed configuration read
still fails the enable without discarding known static capabilities.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 73 +++++++++++++++++++++++++++++++--
drivers/gpu/drm/msm/dp/dp_drm.c | 8 ++--
drivers/gpu/drm/msm/dp/dp_drm.h | 2 +
drivers/gpu/drm/msm/dp/dp_panel.c | 81 +++++++++++++++++++++++++++++--------
drivers/gpu/drm/msm/dp/dp_panel.h | 4 ++
5 files changed, 145 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 816f2dc11aea..9b7a1d18dfe0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -16,6 +16,7 @@
#include <drm/display/drm_dp_aux_bus.h>
#include <drm/display/drm_hdmi_audio_helper.h>
#include <drm/drm_edid.h>
+#include <drm/drm_panel.h>
#include <drm/drm_probe_helper.h>
#include "msm_drv.h"
@@ -313,10 +314,16 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
u8 dpcd[DP_RECEIVER_CAP_SIZE];
const struct drm_edid *drm_edid = NULL;
- rc = drm_dp_read_dpcd_caps(dp->aux, dpcd);
- if (rc)
- goto end;
+ if (dp->msm_dp_display.is_edp && dp->panel->caps_cached) {
+ memcpy(dpcd, dp->panel->dpcd, sizeof(dpcd));
+ } else {
+ dp->panel->link_caps_valid = false;
+ rc = drm_dp_read_dpcd_caps(dp->aux, dpcd);
+ if (rc)
+ goto end;
+ }
+ /* Reapply the LTTPR mode even when receiver capabilities are cached. */
dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
rc = msm_dp_panel_read_link_caps(dp->panel, connector);
@@ -521,6 +528,11 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
dp->msm_dp_display.connector_type,
dp->link->sink_count);
+ /* eDP uses this path for power-down, not a change of sink. */
+ if (!dp->msm_dp_display.is_edp) {
+ dp->panel->link_caps_valid = false;
+ dp->panel->caps_cached = false;
+ }
if (!dp->plugged) {
mutex_unlock(&dp->plugged_lock);
return 0;
@@ -971,6 +983,61 @@ void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
msm_dp_ctrl_set_psr(dp->ctrl, dp->panel, enter);
}
+enum drm_connector_status msm_edp_bridge_detect(struct drm_bridge *bridge,
+ struct drm_connector *connector)
+{
+ struct msm_dp *display = to_dp_bridge(bridge)->msm_dp_display;
+ struct msm_dp_display_private *dp = container_of(display,
+ struct msm_dp_display_private, msm_dp_display);
+ struct drm_panel *panel;
+ u8 dpcd[DP_RECEIVER_CAP_SIZE];
+ bool prepared;
+ int ret;
+
+ /* The fixed panel's bandwidth capabilities survive power cycles. */
+ scoped_guard(mutex, &dp->plugged_lock) {
+ if (dp->panel->link_caps_valid)
+ return connector_status_connected;
+ }
+
+ /* Discovery precedes the first modeset; do not power up in atomic check. */
+ panel = of_drm_find_panel(display->next_bridge->of_node);
+ if (IS_ERR(panel))
+ return connector_status_unknown;
+
+ ret = pm_runtime_resume_and_get(&display->pdev->dev);
+ if (ret < 0)
+ goto put_panel;
+
+ prepared = panel->prepared;
+ if (!prepared)
+ drm_panel_prepare(panel);
+ if (!panel->prepared) {
+ ret = -EIO;
+ goto put_pm;
+ }
+
+ scoped_guard(mutex, &dp->plugged_lock) {
+ ret = drm_dp_read_dpcd_caps(dp->aux, dpcd);
+ if (!ret) {
+ dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
+ ret = msm_dp_panel_read_link_caps(dp->panel, connector);
+ }
+ }
+
+ if (!prepared)
+ drm_panel_unprepare(panel);
+put_pm:
+ pm_runtime_put_sync(&display->pdev->dev);
+put_panel:
+ drm_panel_put(panel);
+ if (ret < 0)
+ drm_dbg_dp(dp->drm_dev, "eDP capability discovery failed: %d\n", ret);
+
+ /* A failed AUX access does not mean the fixed panel was disconnected. */
+ return connector_status_connected;
+}
+
/**
* msm_dp_bridge_detect - callback to determine if connector is connected
*
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index c1e4147bfef1..8c4446b12925 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -276,6 +276,7 @@ static void msm_edp_bridge_debugfs_init(struct drm_bridge *bridge, struct dentry
}
static const struct drm_bridge_funcs msm_edp_bridge_ops = {
+ .detect = msm_edp_bridge_detect,
.atomic_pre_enable = msm_dp_bridge_atomic_pre_enable,
.atomic_enable = msm_edp_bridge_atomic_enable,
.atomic_disable = msm_edp_bridge_atomic_disable,
@@ -311,9 +312,8 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
* Many ops only make sense for DP. Why?
* - Detect/HPD are used by DRM to know if a display is _physically_
* there, not whether the display is powered on / finished initting.
- * On eDP we assume the display is always there because you can't
- * know until power is applied. If we don't implement the ops DRM will
- * assume our display is always there.
+ * On eDP the fixed panel is always connected. Detect powers it briefly
+ * to cache link capabilities before the first modeset.
* - Currently eDP mode reading is driven by the panel driver. This
* allows the panel driver to properly power itself on to read the
* modes.
@@ -327,6 +327,8 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
bridge->hdmi_audio_dev = &msm_dp_display->pdev->dev;
bridge->hdmi_audio_max_i2s_playback_channels = 8;
bridge->hdmi_audio_dai_port = -1;
+ } else {
+ bridge->ops = DRM_BRIDGE_OP_DETECT;
}
rc = devm_drm_bridge_add(dev->dev, bridge);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index da412c788503..d37e2ff95f4b 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -25,6 +25,8 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
struct drm_encoder *encoder,
bool yuv_supported);
+enum drm_connector_status msm_edp_bridge_detect(struct drm_bridge *bridge,
+ struct drm_connector *connector);
enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
struct drm_connector *connector);
void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index fdf1a3f437ba..d144af2fe008 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -65,7 +65,7 @@ static inline u32 msm_dp_read_p0(struct msm_dp_panel_private *panel,
return readl_relaxed(panel->p0_base + offset);
}
-static void msm_dp_panel_read_psr_cap(struct msm_dp_panel_private *panel)
+static bool msm_dp_panel_read_psr_cap(struct msm_dp_panel_private *panel)
{
ssize_t rlen;
struct msm_dp_panel *msm_dp_panel;
@@ -81,12 +81,18 @@ static void msm_dp_panel_read_psr_cap(struct msm_dp_panel_private *panel)
"psr version: 0x%x, psr_cap: 0x%x\n",
msm_dp_panel->psr_cap.version,
msm_dp_panel->psr_cap.capabilities);
- } else
+ } else {
+ memset(&msm_dp_panel->psr_cap, 0, sizeof(msm_dp_panel->psr_cap));
DRM_ERROR("failed to read psr info, rlen=%zd\n", rlen);
+ return false;
+ }
}
+
+ return true;
}
-static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
+static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel,
+ bool *caps_valid)
{
int rc, max_lttpr_lanes, max_lttpr_rate;
struct msm_dp_panel_private *panel;
@@ -100,7 +106,20 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
if (rc)
return rc;
- msm_dp_panel->vsc_sdp_supported = drm_dp_vsc_sdp_supported(panel->aux, dpcd);
+ *caps_valid = true;
+ if (dpcd[DP_DPCD_REV] >= DP_DPCD_REV_13) {
+ u8 rx_feature;
+
+ rc = drm_dp_dpcd_read_byte(panel->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
+ &rx_feature);
+ if (rc) {
+ /* Retry discovery rather than caching a failed optional read. */
+ *caps_valid = false;
+ } else {
+ msm_dp_panel->vsc_sdp_supported =
+ rx_feature & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
+ }
+ }
link_info = &msm_dp_panel->link_info;
link_info->revision = dpcd[DP_DPCD_REV];
major = (link_info->revision >> 4) & 0x0f;
@@ -126,7 +145,6 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
/* For eDP v1.4+, parse the SUPPORTED_LINK_RATES table */
if (edp_rev >= DP_EDP_14) {
__le16 rates[DP_MAX_SUPPORTED_RATES];
- u8 bw_set;
int i;
rc = drm_dp_dpcd_read_data(panel->aux, DP_SUPPORTED_LINK_RATES,
@@ -134,10 +152,6 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
if (rc)
return rc;
- rc = drm_dp_dpcd_read_byte(panel->aux, DP_LINK_BW_SET, &bw_set);
- if (rc)
- return rc;
-
/* Find index of max supported link rate that does not exceed dtsi limits */
for (i = 0; i < ARRAY_SIZE(rates); i++) {
/*
@@ -168,10 +182,6 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
link_info->supported_rates[i] = rate;
link_info->rate_set = i;
}
-
- /* Only use LINK_RATE_SET if LINK_BW_SET hasn't already been written to */
- if (!bw_set && link_info->rate)
- link_info->use_rate_set = true;
}
}
@@ -201,15 +211,34 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
drm_dbg_dp(panel->drm_dev, "link_rate_set=%d\n", link_info->rate_set);
- drm_dbg_dp(panel->drm_dev, "use_rate_set=%d\n", link_info->use_rate_set);
drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", link_info->num_lanes);
if (drm_dp_enhanced_frame_cap(dpcd))
link_info->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
- msm_dp_panel_read_psr_cap(panel);
+ if (!msm_dp_panel_read_psr_cap(panel))
+ *caps_valid = false;
- return rc;
+ return 0;
+}
+
+static int msm_dp_panel_read_link_rate_set(struct msm_dp_panel_private *panel)
+{
+ struct msm_dp_link_info *link_info = &panel->msm_dp_panel.link_info;
+ u8 bw_set;
+ int ret;
+
+ if (!link_info->supported_rates[0])
+ return 0;
+
+ /* LINK_BW_SET is configuration state, not a cached capability. */
+ ret = drm_dp_dpcd_read_byte(panel->aux, DP_LINK_BW_SET, &bw_set);
+ if (ret)
+ return ret;
+
+ link_info->use_rate_set = !bw_set;
+ drm_dbg_dp(panel->drm_dev, "use_rate_set=%d\n", link_info->use_rate_set);
+ return 0;
}
static u32 msm_dp_panel_get_supported_bpp(struct msm_dp_panel *msm_dp_panel,
@@ -238,6 +267,7 @@ int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
{
int rc, bw_code;
int count;
+ bool caps_valid;
struct msm_dp_panel_private *panel;
if (!msm_dp_panel || !connector) {
@@ -247,7 +277,16 @@ int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
- rc = msm_dp_panel_read_dpcd(msm_dp_panel);
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
+ msm_dp_panel->caps_cached)
+ return msm_dp_panel_read_link_rate_set(panel);
+
+ msm_dp_panel->link_caps_valid = false;
+ msm_dp_panel->caps_cached = false;
+ memset(&msm_dp_panel->link_info, 0, sizeof(msm_dp_panel->link_info));
+ memset(&msm_dp_panel->psr_cap, 0, sizeof(msm_dp_panel->psr_cap));
+ msm_dp_panel->vsc_sdp_supported = false;
+ rc = msm_dp_panel_read_dpcd(msm_dp_panel, &caps_valid);
if (rc) {
DRM_ERROR("read dpcd failed %d\n", rc);
return rc;
@@ -272,6 +311,14 @@ int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
rc = drm_dp_read_downstream_info(panel->aux, msm_dp_panel->dpcd,
msm_dp_panel->downstream_ports);
+ if (rc)
+ return rc;
+
+ rc = msm_dp_panel_read_link_rate_set(panel);
+ if (!rc) {
+ msm_dp_panel->link_caps_valid = true;
+ msm_dp_panel->caps_cached = caps_valid;
+ }
return rc;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 4519ac374220..a5cb8a4a9498 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -38,6 +38,10 @@ struct msm_dp_panel {
struct msm_dp_panel_psr psr_cap;
bool video_test;
bool vsc_sdp_supported;
+ /* Valid bandwidth limits, independent of optional feature reads. */
+ bool link_caps_valid;
+ /* All capability reads completed; eDP can reuse the full cache. */
+ bool caps_cached;
u32 hw_revision;
u32 max_bw_code;
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 11/20] drm/msm/dp: Cache eDP link capabilities after successful discovery
2026-10-09 3:16 ` [PATCH v2 11/20] drm/msm/dp: Cache eDP link capabilities after successful discovery Xilin Wu
@ 2026-10-09 3:34 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:34 UTC (permalink / raw)
To: Xilin Wu; +Cc: neil.armstrong, linux-amlogic, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/msm/dp: TOCTOU race in eDP detect unexpectedly powers off panel
- [High] drm/msm/dp: eDP power sequencing violation in msm_edp_bridge_detect()
--
commit bb9e355f18a717e81f26745b2bb594514c2f43b0
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Cache eDP link capabilities after successful discovery
An eDP panel may be powered off during mode validation. This commit caches
eDP link capabilities after successful discovery to persist them across
disable and suspend, and discovers capabilities from connector detect before
the first modeset.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -971,6 +983,61 @@ void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
[ ... ]
> +enum drm_connector_status msm_edp_bridge_detect(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
[ ... ]
> + ret = pm_runtime_resume_and_get(&display->pdev->dev);
> + if (ret < 0)
> + goto put_panel;
> +
> + prepared = panel->prepared;
> + if (!prepared)
> + drm_panel_prepare(panel);
[Severity: High]
Does this sequence power up the DP host and PHY before the panel is prepared?
Calling pm_runtime_resume_and_get() triggers msm_dp_pm_runtime_resume(),
which for eDP initializes the DP PHY. Preparing the panel afterwards via
drm_panel_prepare() means the PHY is powered before panel VDD is applied.
Could this drive voltage onto the AUX or main link pins of an unpowered panel
and violate eDP power sequencing requirements?
[Severity: High]
Can a race condition occur here if an asynchronous atomic commit starts
concurrently?
If userspace triggers a non-blocking atomic modeset, it drops locks before
committing. If userspace then immediately issues a GETCONNECTOR ioctl, this
detect function will read prepared == false and call drm_panel_prepare().
The concurrent atomic commit would then execute panel_bridge_pre_enable(),
see that the panel is already prepared, and skip its own preparation.
> + if (!panel->prepared) {
> + ret = -EIO;
> + goto put_pm;
> + }
> +
> + scoped_guard(mutex, &dp->plugged_lock) {
> + ret = drm_dp_read_dpcd_caps(dp->aux, dpcd);
> + if (!ret) {
> + dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
> + ret = msm_dp_panel_read_link_caps(dp->panel, connector);
> + }
> + }
> +
> + if (!prepared)
> + drm_panel_unprepare(panel);
[Severity: High]
Is it possible for this unprepare to shut down the panel while a concurrent
modeset expects it to remain powered?
Because the atomic commit might have skipped preparing the panel (since it
was already prepared by this function), this call to drm_panel_unprepare()
could power off the panel out from under the active modeset based on the
stale local 'prepared' variable.
> +put_pm:
> + pm_runtime_put_sync(&display->pdev->dev);
[Severity: High]
Does powering down the panel before the PHY violate the eDP power-off sequence?
Here drm_panel_unprepare() removes VDD before pm_runtime_put_sync() powers
down the DP host and PHY, which reverses the expected power-down order.
> +put_panel:
> + drm_panel_put(panel);
> + if (ret < 0)
> + drm_dbg_dp(dp->drm_dev, "eDP capability discovery failed: %d\n", ret);
> +
> + /* A failed AUX access does not mean the fixed panel was disconnected. */
> + return connector_status_connected;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=11
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 12/20] drm/msm/dp: Rebuild the eDP stream on modesets leaving self refresh
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (10 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 11/20] drm/msm/dp: Cache eDP link capabilities after successful discovery Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 13/20] drm/msm/dp: Track output bit depth in bridge atomic state Xilin Wu
` (7 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
The atomic helpers skip bridge disable when an active CRTC leaves self
refresh. The eDP enable callback also takes a fast path that only exits
PSR, so a concurrent modeset can leave the retained stream using its old
configuration, including the output depth.
Exit PSR and tear down the retained stream in pre-enable when leaving self
refresh with mode_changed or connectors_changed set. Install the new state
and run the normal stream enable sequence afterwards. Keep the fast path
for ordinary self-refresh exits: active_changed alone does not require
rebuilding the stream.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_drm.c | 24 +++++++++++++++++++++++-
drivers/gpu/drm/msm/dp/dp_utils.c | 7 +++++++
drivers/gpu/drm/msm/dp/dp_utils.h | 4 ++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 8c4446b12925..df224ae46d9d 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -14,6 +14,7 @@
#include "msm_kms.h"
#include "dp_audio.h"
#include "dp_drm.h"
+#include "dp_utils.h"
/**
* msm_dp_bridge_get_modes - callback to add drm modes via drm_mode_probed_add()
@@ -54,6 +55,26 @@ static void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
{
struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
struct msm_dp *dp = dp_bridge->msm_dp_display;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+
+ crtc = drm_atomic_get_new_crtc_for_encoder(state, drm_bridge->encoder);
+ if (!crtc)
+ return;
+
+ old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ /*
+ * The helpers skip bridge disable when waking an active CRTC from
+ * self refresh, even when its color configuration requires a modeset.
+ * Tear down the retained stream before installing the new state.
+ */
+ if (dp->is_edp && dp->power_on && old_crtc_state->self_refresh_active &&
+ new_crtc_state->active && msm_dp_utils_psr_needs_modeset(new_crtc_state)) {
+ msm_dp_display_set_psr(dp, false);
+ msm_dp_display_atomic_disable(dp);
+ msm_dp_display_atomic_post_disable(dp);
+ }
msm_dp_display_atomic_pre_enable(dp, state);
}
@@ -157,7 +178,8 @@ static void msm_edp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
- if (old_crtc_state && old_crtc_state->self_refresh_active) {
+ if (dp->power_on && old_crtc_state && old_crtc_state->self_refresh_active &&
+ !msm_dp_utils_psr_needs_modeset(drm_atomic_get_new_crtc_state(state, crtc))) {
msm_dp_display_set_psr(dp, false);
return;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index 226b370ce8fe..72bae48a1fe0 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -4,6 +4,7 @@
*/
#include <linux/types.h>
+#include <drm/drm_crtc.h>
#include "dp_utils.h"
@@ -87,3 +88,9 @@ void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
FIELD_PREP(HEADER_3_MASK, sdp_header->HB3) |
FIELD_PREP(PARITY_3_MASK, msm_dp_utils_calculate_parity(sdp_header->HB3));
}
+
+bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state)
+{
+ /* active_changed alone also describes a normal self-refresh exit. */
+ return state->mode_changed || state->connectors_changed;
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index 2fdb89efc57c..5d86c978dffd 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -10,6 +10,8 @@
#include <linux/bits.h>
#include <drm/display/drm_dp_helper.h>
+struct drm_crtc_state;
+
#define HEADER_BYTE_0_BIT 0
#define PARITY_BYTE_0_BIT 8
#define HEADER_BYTE_1_BIT 16
@@ -34,4 +36,6 @@ u8 msm_dp_utils_calculate_parity(u32 data);
void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
u32 header_buff[2]);
+bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state);
+
#endif /* _DP_UTILS_H_ */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 13/20] drm/msm/dp: Track output bit depth in bridge atomic state
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (11 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 12/20] drm/msm/dp: Rebuild the eDP stream on modesets leaving self refresh Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 14/20] drm/msm/dp: Clear stale MSA colorimetry bits Xilin Wu
` (6 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Advertise the 10 bpc DP and eDP capability through bridge->max_bpc so
the bridge connector helper attaches the max bpc property. Select a
supported component depth from the new connector state. Keep the result
in a private bridge state so TEST_ONLY atomic commits do not modify the
running stream. Force a modeset when max bpc changes to apply the new
depth to the stream.
Require valid cached bandwidth limits during atomic check. The preceding
eDP discovery change supplies them before the first modeset. Reject
unknown limits instead of accepting a configuration for validation during
enable. Retain support for 6 bpc SDR panels and use an 8 bpc ceiling when
the bridge chain does not expose max bpc.
Keep the selected depth throughout enable. Filter link training fallback
candidates against the checked stream bandwidth, including YUV420, and
retain a final bandwidth check before starting video. A lower-rate link
may be used only if it carries the unchanged stream. Otherwise unwind
and report the link failure rather than silently lowering the depth.
PHY and video compliance tests retain their specified parameters.
Continue searching when a candidate lacks bandwidth: reducing the lane
count restores the maximum rate and can yield a viable combination.
Only successful link training completes the search; successful PHY
reinitialization on the final retry must not masquerade as training
success. Keep sink training cleanup on the failure path.
Retain the generic bridge helper's missing-state guard in the custom
duplicate callback. Initial state allocation can fail at bridge attach;
return NULL in that case so atomic state acquisition reports -ENOMEM
instead of copying from a NULL pointer.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 117 ++++++++++++++++++++++--------------
drivers/gpu/drm/msm/dp/dp_display.c | 39 +++++++++++-
drivers/gpu/drm/msm/dp/dp_display.h | 6 ++
drivers/gpu/drm/msm/dp/dp_drm.c | 82 ++++++++++++++++++++++---
drivers/gpu/drm/msm/dp/dp_drm.h | 7 +++
drivers/gpu/drm/msm/dp/dp_panel.c | 5 --
drivers/gpu/drm/msm/dp/dp_utils.c | 33 ++++++++++
drivers/gpu/drm/msm/dp/dp_utils.h | 6 ++
8 files changed, 232 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index f633f6cf5764..791bb3af2cad 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -23,6 +23,7 @@
#include "dp_reg.h"
#include "dp_ctrl.h"
+#include "dp_utils.h"
#include "dp_link.h"
#define POLLING_SLEEP_US 1000
@@ -2365,10 +2366,56 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
return drm_dp_channel_eq_ok(link_status, num_lanes);
}
+static bool msm_dp_ctrl_link_fits(struct msm_dp_ctrl_private *ctrl,
+ const struct msm_dp_panel *panel)
+{
+ return msm_dp_utils_link_fits(panel->msm_dp_mode.drm_mode.clock,
+ panel->msm_dp_mode.bpp,
+ ctrl->link->link_params.rate,
+ ctrl->link->link_params.num_lanes,
+ panel->msm_dp_mode.out_fmt_is_yuv_420);
+}
+
+static int msm_dp_ctrl_next_link_params(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
+ unsigned int training_step,
+ const u8 *link_status, bool check_bw)
+{
+ unsigned int lanes;
+ int ret;
+
+ do {
+ lanes = ctrl->link->link_params.num_lanes;
+ if (training_step == DP_TRAINING_1) {
+ ret = msm_dp_ctrl_link_rate_down_shift(ctrl);
+ if (ret) {
+ if (!msm_dp_ctrl_clock_recovery_any_ok(link_status, lanes))
+ return ret;
+ ret = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
+ }
+ } else if (training_step == DP_TRAINING_2) {
+ if (!drm_dp_clock_recovery_ok(link_status, lanes))
+ ret = msm_dp_ctrl_link_rate_down_shift(ctrl);
+ else
+ ret = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
+ } else {
+ return -EIO;
+ }
+ if (ret)
+ return ret;
+
+ /* Reducing lanes restores the maximum rate; keep looking. */
+ } while (check_bw && !msm_dp_ctrl_link_fits(ctrl, panel));
+
+ return 0;
+}
+
int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel)
{
int rc = 0;
+ bool trained = false;
+ bool check_bw;
struct msm_dp_ctrl_private *ctrl;
u32 rate;
int link_train_max_retries = 5;
@@ -2382,6 +2429,8 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
+ check_bw = !(ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) &&
+ !panel->video_test;
rate = panel->link_info.rate;
pixel_rate = panel->msm_dp_mode.drm_mode.clock;
@@ -2404,6 +2453,9 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
ctrl->link->link_params.rate, ctrl->link->link_params.num_lanes,
pixel_rate);
+ if (check_bw && !msm_dp_ctrl_link_fits(ctrl, panel))
+ return -ENOSPC;
+
rc = msm_dp_ctrl_enable_mainlink_clocks(ctrl, panel);
if (rc)
return rc;
@@ -2411,55 +2463,22 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
while (--link_train_max_retries) {
training_step = DP_TRAINING_NONE;
rc = msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
- if (rc == 0) {
- /* training completed successfully */
+ if (!rc) {
+ trained = true;
break;
- } else if (training_step == DP_TRAINING_1) {
- /* link train_1 failed */
- if (!msm_dp_aux_is_link_connected(ctrl->aux))
- break;
-
- drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
-
- rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
- if (rc < 0) { /* already in RBR = 1.6G */
- if (msm_dp_ctrl_clock_recovery_any_ok(link_status,
- ctrl->link->link_params.num_lanes)) {
- /*
- * some lanes are ready,
- * reduce lane number
- */
- rc = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
- if (rc < 0) { /* lane == 1 already */
- /* end with failure */
- break;
- }
- } else {
- /* end with failure */
- break; /* lane == 1 already */
- }
- }
- } else if (training_step == DP_TRAINING_2) {
- /* link train_2 failed */
- if (!msm_dp_aux_is_link_connected(ctrl->aux))
- break;
-
- drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ }
- if (!drm_dp_clock_recovery_ok(link_status,
- ctrl->link->link_params.num_lanes))
- rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
- else
- rc = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
+ if (!msm_dp_aux_is_link_connected(ctrl->aux))
+ break;
- if (rc < 0) {
- /* end with failure */
- break; /* lane == 1 already */
- }
+ if (drm_dp_dpcd_read_link_status(ctrl->aux, link_status) < 0)
+ break;
- /* stop link training before start re training */
- msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
- }
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
+ rc = msm_dp_ctrl_next_link_params(ctrl, panel, training_step,
+ link_status, check_bw);
+ if (rc)
+ break;
rc = msm_dp_ctrl_reinitialize_mainlink(ctrl, panel);
if (rc) {
@@ -2468,6 +2487,10 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
}
}
+ /* Reinitializing the PHY is not a successful training attempt. */
+ if (!trained && !rc)
+ rc = -EIO;
+
if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN)
return rc;
@@ -2617,6 +2640,10 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
+ /* Never change the stream depth selected by atomic check. */
+ if (!panel->video_test && !msm_dp_ctrl_link_fits(ctrl, panel))
+ return -ENOSPC;
+
pixel_rate_orig = panel->msm_dp_mode.drm_mode.clock;
pixel_rate = pixel_rate_orig;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 9b7a1d18dfe0..1f9595ff5fd9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -28,6 +28,7 @@
#include "dp_panel.h"
#include "dp_display.h"
#include "dp_drm.h"
+#include "dp_utils.h"
#include "dp_audio.h"
#include "dp_debug.h"
@@ -677,7 +678,8 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
const struct drm_display_mode *adjusted_mode,
- struct msm_dp_panel *msm_dp_panel)
+ struct msm_dp_panel *msm_dp_panel,
+ const struct msm_dp_bridge_state *state)
{
struct msm_dp_display_private *dp;
u32 bpp;
@@ -687,7 +689,7 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
if (msm_dp_display_check_video_test(msm_dp_display))
bpp = msm_dp_display_get_test_bpp(msm_dp_display);
else
- bpp = msm_dp_panel->connector->display_info.bpc * 3;
+ bpp = state->bpp;
msm_dp_panel_init_panel_info(msm_dp_panel, adjusted_mode, bpp ? bpp : 24);
@@ -697,6 +699,34 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
return 0;
}
+int msm_dp_display_atomic_check(struct msm_dp *msm_dp_display,
+ struct msm_dp_bridge_state *state,
+ const struct drm_display_mode *mode,
+ const struct drm_connector_state *conn_state)
+{
+ const struct drm_display_info *info = &conn_state->connector->display_info;
+ struct msm_dp_display_private *dp;
+ unsigned int clock = mode->clock;
+ int bpp;
+
+ dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
+
+ guard(mutex)(&dp->plugged_lock);
+
+ if (!dp->panel->link_caps_valid)
+ return -EINVAL;
+
+ bpp = msm_dp_utils_select_bpp(info->bpc, conn_state->max_requested_bpc ?: 8,
+ clock, dp->panel->link_info.rate,
+ dp->panel->link_info.num_lanes,
+ drm_mode_is_420_only(info, mode));
+ if (bpp < 0)
+ return bpp;
+
+ state->bpp = bpp;
+ return 0;
+}
+
static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
@@ -1593,6 +1623,7 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
struct msm_dp_display_private *dp;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
+ struct drm_bridge_state *bridge_state;
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
@@ -1600,13 +1631,15 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
if (!crtc)
return;
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ bridge_state = drm_atomic_get_new_bridge_state(state, msm_dp_display->bridge);
/*
* The DPU encoder's .atomic_enable() reads the mode's YUV420 / wide bus
* state and runs before the bridge's .atomic_enable(), so the mode must
* be programmed here, in .atomic_pre_enable().
*/
- msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel);
+ msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel,
+ to_dp_bridge_state(bridge_state));
}
static void msm_dp_display_abort_enable_locked(struct msm_dp_display_private *dp)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 83117e7619f3..6a77e5952530 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -11,6 +11,8 @@
#define DP_MAX_PIXEL_CLK_KHZ 675000
+struct msm_dp_bridge_state;
+
struct msm_dp {
struct drm_device *drm_dev;
struct platform_device *pdev;
@@ -43,4 +45,8 @@ enum drm_mode_status msm_dp_display_mode_valid(struct msm_dp *dp,
const struct drm_display_info *info,
const struct drm_display_mode *mode);
+int msm_dp_display_atomic_check(struct msm_dp *dp, struct msm_dp_bridge_state *state,
+ const struct drm_display_mode *mode,
+ const struct drm_connector_state *conn_state);
+
#endif /* _DP_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index df224ae46d9d..9f1aa87a3687 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -4,6 +4,7 @@
*/
#include <linux/string_choices.h>
+#include <linux/slab.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_atomic.h>
#include <drm/drm_bridge.h>
@@ -116,10 +117,71 @@ static enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *drm_brid
return msm_dp_display_mode_valid(dp, info, mode);
}
+static struct drm_bridge_state *msm_dp_bridge_create_state(struct drm_bridge *bridge)
+{
+ struct msm_dp_bridge_state *state;
+
+ state = kzalloc_obj(*state);
+ if (!state)
+ return ERR_PTR(-ENOMEM);
+
+ __drm_atomic_helper_bridge_state_init(&state->base, bridge);
+ return &state->base;
+}
+
+static struct drm_bridge_state *msm_dp_bridge_duplicate_state(struct drm_bridge *bridge)
+{
+ struct msm_dp_bridge_state *state;
+ struct drm_bridge_state *old_state;
+
+ if (WARN_ON(!bridge->base.state))
+ return NULL;
+
+ old_state = drm_priv_to_bridge_state(bridge->base.state);
+ state = kmemdup(to_dp_bridge_state(old_state), sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+
+ __drm_atomic_helper_bridge_duplicate_state(bridge, &state->base);
+ return &state->base;
+}
+
+static void msm_dp_bridge_destroy_state(struct drm_bridge *bridge,
+ struct drm_bridge_state *state)
+{
+ kfree(to_dp_bridge_state(state));
+}
+
+static int msm_dp_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct drm_connector_state *old_conn_state;
+ int ret;
+
+ if (!conn_state->crtc)
+ return 0;
+
+ ret = msm_dp_display_atomic_check(to_dp_bridge(bridge)->msm_dp_display,
+ to_dp_bridge_state(bridge_state),
+ &crtc_state->adjusted_mode, conn_state);
+ if (ret)
+ return ret;
+
+ old_conn_state = drm_atomic_get_old_connector_state(conn_state->state,
+ conn_state->connector);
+ if (old_conn_state->max_requested_bpc != conn_state->max_requested_bpc)
+ crtc_state->mode_changed = true;
+
+ return 0;
+}
+
static const struct drm_bridge_funcs msm_dp_bridge_ops = {
- .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
- .atomic_create_state = drm_atomic_helper_bridge_create_state,
+ .atomic_check = msm_dp_bridge_atomic_check,
+ .atomic_duplicate_state = msm_dp_bridge_duplicate_state,
+ .atomic_destroy_state = msm_dp_bridge_destroy_state,
+ .atomic_create_state = msm_dp_bridge_create_state,
.atomic_pre_enable = msm_dp_bridge_atomic_pre_enable,
.atomic_enable = msm_dp_bridge_atomic_enable,
.atomic_disable = msm_dp_bridge_atomic_disable,
@@ -154,7 +216,7 @@ static int msm_edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
if (crtc_state->self_refresh_active && !dp->psr_supported)
return -EINVAL;
- return 0;
+ return msm_dp_bridge_atomic_check(drm_bridge, bridge_state, crtc_state, conn_state);
}
static void msm_edp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
@@ -283,9 +345,8 @@ static enum drm_mode_status msm_edp_bridge_mode_valid(struct drm_bridge *bridge,
return MODE_CLOCK_HIGH;
/*
- * The eDP controller currently does not have a reliable way of
- * enabling panel power to read sink capabilities. So, we rely
- * on the panel driver to populate only supported modes for now.
+ * Atomic check validates bandwidth for the selected component depth
+ * against the capabilities cached during connector detection.
*/
return MODE_OK;
}
@@ -304,9 +365,9 @@ static const struct drm_bridge_funcs msm_edp_bridge_ops = {
.atomic_disable = msm_edp_bridge_atomic_disable,
.atomic_post_disable = msm_edp_bridge_atomic_post_disable,
.mode_valid = msm_edp_bridge_mode_valid,
- .atomic_create_state = drm_atomic_helper_bridge_create_state,
- .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_create_state = msm_dp_bridge_create_state,
+ .atomic_duplicate_state = msm_dp_bridge_duplicate_state,
+ .atomic_destroy_state = msm_dp_bridge_destroy_state,
.atomic_check = msm_edp_bridge_atomic_check,
.debugfs_init = msm_edp_bridge_debugfs_init,
};
@@ -328,6 +389,7 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
bridge = &msm_dp_bridge->bridge;
bridge->type = msm_dp_display->connector_type;
+ bridge->max_bpc = 10;
bridge->ycbcr_420_allowed = yuv_supported;
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index d37e2ff95f4b..4c651cc40f4d 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -17,6 +17,13 @@ struct msm_dp_bridge {
struct msm_dp *msm_dp_display;
};
+struct msm_dp_bridge_state {
+ struct drm_bridge_state base;
+ u32 bpp;
+};
+
+#define to_dp_bridge_state(x) container_of((x), struct msm_dp_bridge_state, base)
+
#define to_dp_bridge(x) container_of((x), struct msm_dp_bridge, bridge)
struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index d144af2fe008..af05441bc32a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -734,11 +734,6 @@ int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel,
drm_mode->clock);
drm_dbg_dp(panel->drm_dev, "bpp = %d\n", msm_dp_panel->msm_dp_mode.bpp);
- msm_dp_panel->msm_dp_mode.bpp = msm_dp_panel_get_mode_bpp(msm_dp_panel, msm_dp_panel->msm_dp_mode.bpp,
- msm_dp_panel->msm_dp_mode.drm_mode.clock);
-
- drm_dbg_dp(panel->drm_dev, "updated bpp = %d\n",
- msm_dp_panel->msm_dp_mode.bpp);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index 72bae48a1fe0..8b6ef2f6d7f4 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -89,6 +89,39 @@ void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
FIELD_PREP(PARITY_3_MASK, msm_dp_utils_calculate_parity(sdp_header->HB3));
}
+bool msm_dp_utils_link_fits(unsigned int clock, unsigned int bpp,
+ unsigned int rate, unsigned int lanes, bool yuv420)
+{
+ u64 available = (u64)rate * lanes * 8;
+
+ if (!clock || !bpp || !rate || !lanes)
+ return false;
+
+ if (yuv420)
+ available *= 2;
+
+ return (u64)clock * bpp <= available;
+}
+
+int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
+ unsigned int clock, unsigned int rate,
+ unsigned int lanes, bool yuv420)
+{
+ unsigned int bpp;
+
+ bpc = min3(bpc ? bpc : 8, max_bpc, 10U);
+ bpc = round_down(bpc, 2);
+ if (bpc < 6)
+ return -EINVAL;
+
+ for (bpp = bpc * 3; bpp >= 18; bpp -= 6) {
+ if (msm_dp_utils_link_fits(clock, bpp, rate, lanes, yuv420))
+ return bpp;
+ }
+
+ return -EINVAL;
+}
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state)
{
/* active_changed alone also describes a normal self-refresh exit. */
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index 5d86c978dffd..c6b62e3900bb 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -36,6 +36,12 @@ u8 msm_dp_utils_calculate_parity(u32 data);
void msm_dp_utils_pack_sdp_header(const struct dp_sdp_header *sdp_header,
u32 header_buff[2]);
+bool msm_dp_utils_link_fits(unsigned int clock, unsigned int bpp,
+ unsigned int rate, unsigned int lanes, bool yuv420);
+int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
+ unsigned int clock, unsigned int rate,
+ unsigned int lanes, bool yuv420);
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state);
#endif /* _DP_UTILS_H_ */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 14/20] drm/msm/dp: Clear stale MSA colorimetry bits
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (12 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 13/20] drm/msm/dp: Track output bit depth in bridge atomic state Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 15/20] drm/msm/dp: Flush VSC updates when leaving YUV420 Xilin Wu
` (5 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
The MISC0 update clears the bit-depth field but only ORs in the new
colorimetry value. Bits from the previous stream can therefore survive
when the new value clears them.
Clear the colorimetry field together with the bit-depth field before
programming the new stream.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 791bb3af2cad..783cafb22374 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -476,7 +476,8 @@ static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
misc_val = msm_dp_read_link(ctrl, REG_DP_MISC1_MISC0);
/* clear bpp bits */
- misc_val &= ~(0x07 << DP_MISC0_TEST_BITS_DEPTH_SHIFT);
+ misc_val &= ~((0x07 << DP_MISC0_TEST_BITS_DEPTH_SHIFT) |
+ GENMASK(4, 1));
misc_val |= colorimetry_cfg << DP_MISC0_COLORIMETRY_CFG_SHIFT;
misc_val |= test_bits_depth << DP_MISC0_TEST_BITS_DEPTH_SHIFT;
/* Configure clock to synchronous mode */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 15/20] drm/msm/dp: Flush VSC updates when leaving YUV420
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (13 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 14/20] drm/msm/dp: Clear stale MSA colorimetry bits Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams Xilin Wu
` (4 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Peripheral flush is currently requested only for the new YUV420 mode.
Returning to RGB also changes SDP state and must remove the old VSC
packet rather than leaving its enable bits set.
Track whether either stream needs VSC and request peripheral flush for
both enabling and removing it. Explicitly disable VSC for RGB, and write
the packet payload before enabling transmission. Retain the existing
hardware-specific SDP update mechanism.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 7 ++++++-
drivers/gpu/drm/msm/dp/dp_panel.c | 6 ++++--
drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 1f9595ff5fd9..486348113b8a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -691,6 +691,8 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
else
bpp = state->bpp;
+ msm_dp_panel->sdp_needs_flush = msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ||
+ drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode);
msm_dp_panel_init_panel_info(msm_dp_panel, adjusted_mode, bpp ? bpp : 24);
/* populate wide_bus_support to different layers */
@@ -1557,7 +1559,10 @@ void __exit msm_dp_unregister(void)
bool msm_dp_needs_periph_flush(const struct msm_dp *msm_dp_display,
const struct drm_display_mode *mode)
{
- return drm_mode_is_420_only(&msm_dp_display->connector->display_info, mode);
+ struct msm_dp_display_private *dp = container_of(msm_dp_display,
+ struct msm_dp_display_private, msm_dp_display);
+
+ return dp->panel->sdp_needs_flush;
}
bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index af05441bc32a..026074cafc88 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -526,14 +526,14 @@ void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, struct dp_sd
cfg2 = msm_dp_read_link(panel, MMSS_DP_SDP_CFG2);
misc = msm_dp_read_link(panel, REG_DP_MISC1_MISC0);
+ msm_dp_panel_send_sdp(panel, vsc_sdp, MMSS_DP_GENERIC0_0);
+
cfg |= GEN0_SDP_EN;
msm_dp_write_link(panel, MMSS_DP_SDP_CFG, cfg);
cfg2 |= GENERIC0_SDPSIZE_VALID;
msm_dp_write_link(panel, MMSS_DP_SDP_CFG2, cfg2);
- msm_dp_panel_send_sdp(panel, vsc_sdp, MMSS_DP_GENERIC0_0);
-
/* indicates presence of VSC (BIT(6) of MISC1) */
misc |= DP_MISC1_VSC_SDP;
@@ -686,6 +686,8 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
msm_dp_panel_setup_vsc_sdp_yuv_420(msm_dp_panel);
+ else
+ msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
panel->panel_on = true;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index a5cb8a4a9498..31752895f124 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -42,6 +42,7 @@ struct msm_dp_panel {
bool link_caps_valid;
/* All capability reads completed; eDP can reuse the full cache. */
bool caps_cached;
+ bool sdp_needs_flush;
u32 hw_revision;
u32 max_bw_code;
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (14 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 15/20] drm/msm/dp: Flush VSC updates when leaving YUV420 Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:34 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state Xilin Wu
` (3 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
YUV420 uses colorimetry VSC packets, which cannot share the automatic
PSR packet generator. Do not advertise self-refresh eligibility for
these streams while disabling PSR in the controller and sink.
Compute eligibility after the common atomic check and reject an
incompatible self-refresh entry. Clear the automatic VSC generation bit
for YUV420 and restore PSR configuration when returning to RGB.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 19 ++++++++++++-------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 3 ++-
drivers/gpu/drm/msm/dp/dp_drm.c | 16 ++++++++++++----
4 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 783cafb22374..82ef8e405543 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -411,7 +411,7 @@ static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
- if (msm_dp_panel->psr_cap.version)
+ if (msm_dp_panel->psr_cap.version && !msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
config |= DP_CONFIGURATION_CTRL_SEND_VSC;
drm_dbg_dp(ctrl->drm_dev, "stream DP_CONFIGURATION_CTRL=0x%x\n", config);
@@ -1914,7 +1914,7 @@ static void msm_dp_ctrl_psr_exit(struct msm_dp_ctrl_private *ctrl)
}
void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
- struct msm_dp_panel *panel)
+ struct msm_dp_panel *panel, bool enable)
{
struct msm_dp_ctrl_private *ctrl = container_of(msm_dp_ctrl,
struct msm_dp_ctrl_private, msm_dp_ctrl);
@@ -1923,15 +1923,20 @@ void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
if (!panel->psr_cap.version)
return;
- /* enable PSR1 function */
+ /* Colorimetry VSC packets cannot share the PSR packet generator. */
cfg = msm_dp_read_link(ctrl, REG_PSR_CONFIG);
- cfg |= PSR1_SUPPORTED;
+ if (enable)
+ cfg |= PSR1_SUPPORTED;
+ else
+ cfg &= ~PSR1_SUPPORTED;
msm_dp_write_link(ctrl, REG_PSR_CONFIG, cfg);
- msm_dp_ctrl_config_psr_interrupt(ctrl);
- msm_dp_ctrl_enable_sdp(ctrl);
+ if (enable) {
+ msm_dp_ctrl_config_psr_interrupt(ctrl);
+ msm_dp_ctrl_enable_sdp(ctrl);
+ }
- cfg = DP_PSR_ENABLE;
+ cfg = enable ? DP_PSR_ENABLE : 0;
drm_dp_dpcd_write(ctrl->aux, DP_PSR_EN_CFG, &cfg, 1);
}
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 42c5f847cb02..4dd604ef9f1b 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -46,7 +46,7 @@ void msm_dp_ctrl_irq_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel, bool enable);
void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
- struct msm_dp_panel *panel);
+ struct msm_dp_panel *panel, bool enable);
int msm_dp_ctrl_core_clk_enable(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_core_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 486348113b8a..234cb2373451 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -822,7 +822,8 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
msm_dp_display_handle_plugged_change(msm_dp_display, true);
if (msm_dp_display->psr_supported)
- msm_dp_ctrl_config_psr(dp->ctrl, dp->panel);
+ msm_dp_ctrl_config_psr(dp->ctrl, dp->panel,
+ !dp->panel->msm_dp_mode.out_fmt_is_yuv_420);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 9f1aa87a3687..6b01eeeb1aef 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -204,19 +204,27 @@ static int msm_edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
struct drm_connector_state *conn_state)
{
struct msm_dp *dp = to_dp_bridge(drm_bridge)->msm_dp_display;
+ int ret;
if (WARN_ON(!conn_state))
return -ENODEV;
- conn_state->self_refresh_aware = dp->psr_supported;
-
+ conn_state->self_refresh_aware = false;
if (!conn_state->crtc || !crtc_state)
return 0;
- if (crtc_state->self_refresh_active && !dp->psr_supported)
+ ret = msm_dp_bridge_atomic_check(drm_bridge, bridge_state, crtc_state, conn_state);
+ if (ret)
+ return ret;
+
+ /* Match the PSR configuration applied when enabling this stream. */
+ conn_state->self_refresh_aware = dp->psr_supported &&
+ !drm_mode_is_420_only(&conn_state->connector->display_info,
+ &crtc_state->adjusted_mode);
+ if (crtc_state->self_refresh_active && !conn_state->self_refresh_aware)
return -EINVAL;
- return msm_dp_bridge_atomic_check(drm_bridge, bridge_state, crtc_state, conn_state);
+ return 0;
}
static void msm_edp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams
2026-10-09 3:16 ` [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams Xilin Wu
@ 2026-10-09 3:34 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:34 UTC (permalink / raw)
To: Xilin Wu; +Cc: dri-devel, neil.armstrong, linux-amlogic
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm: endianness bug on DP_PSR_EN_CFG write using u32 pointer
--
commit d1403cc5c58c4528368941f08f1ea1ec332d1a49
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Disable PSR for YUV420 streams
This commit disables self-refresh for YUV420 streams because their
colorimetry VSC packets cannot share the automatic PSR packet generator.
It computes eligibility after common atomic checks and restores PSR
configuration accordingly.
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 783cafb223748..82ef8e4055431 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
[ ... ]
> @@ -1923,15 +1923,20 @@ void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
[ ... ]
> - cfg = DP_PSR_ENABLE;
> + cfg = enable ? DP_PSR_ENABLE : 0;
> drm_dp_dpcd_write(ctrl->aux, DP_PSR_EN_CFG, &cfg, 1);
[Severity: Medium]
Could this silently write 0 and fail to enable PSR on big-endian platforms?
The variable cfg is defined as a u32 at the beginning of
msm_dp_ctrl_config_psr(). When its address is passed to drm_dp_dpcd_write()
with a size of 1 byte, little-endian systems will correctly read the least
significant byte containing the enable bit.
However, on big-endian systems, taking the address of a u32 and reading 1
byte will read the most significant byte instead (which is 0x00).
Would using a u8 variable or drm_dp_dpcd_writeb() avoid this structural
mismatch and ensure the correct byte is written regardless of architecture
endianness?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=16
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (15 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:34 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 18/20] drm/msm/dp: Signal BT.2020 RGB on DP and eDP Xilin Wu
` (2 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Move YUV420 VSC construction from panel programming to atomic check.
Validate receiver support and retain the packed packet in private bridge
state. Pass the checked state through stream programming instead of
copying its configuration into the panel.
Program that packet unchanged for normal streams. Only regenerate it for
video compliance tests, which can override the checked bit depth. Keep
hardware PSR configuration and self-refresh eligibility tied to the same
VSC enable state. Derive peripheral flush from the old and new states,
and keep compliance packets local so tests cannot modify atomic state.
Provide a shared packet builder for subsequent colorimetry support. This
change does not expose additional connector colorspaces.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 42 ++++++++++++++++++++++++-----
drivers/gpu/drm/msm/dp/dp_ctrl.h | 5 +++-
drivers/gpu/drm/msm/dp/dp_display.c | 54 ++++++++++++++++++++++++++-----------
drivers/gpu/drm/msm/dp/dp_drm.c | 5 ++--
drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 53 ++----------------------------------
drivers/gpu/drm/msm/dp/dp_panel.h | 3 +--
drivers/gpu/drm/msm/dp/dp_utils.c | 26 ++++++++++++++++++
drivers/gpu/drm/msm/dp/dp_utils.h | 4 +++
9 files changed, 114 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 82ef8e405543..2fd8dd456852 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -23,6 +23,7 @@
#include "dp_reg.h"
#include "dp_ctrl.h"
+#include "dp_drm.h"
#include "dp_utils.h"
#include "dp_link.h"
@@ -390,7 +391,8 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
}
static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
- struct msm_dp_panel *msm_dp_panel)
+ struct msm_dp_panel *msm_dp_panel,
+ bool psr)
{
u32 config = 0, tbd;
@@ -411,7 +413,7 @@ static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
- if (msm_dp_panel->psr_cap.version && !msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
+ if (msm_dp_panel->psr_cap.version && psr)
config |= DP_CONFIGURATION_CTRL_SEND_VSC;
drm_dbg_dp(ctrl->drm_dev, "stream DP_CONFIGURATION_CTRL=0x%x\n", config);
@@ -488,13 +490,20 @@ static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
}
static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl,
- struct msm_dp_panel *panel)
+ struct msm_dp_panel *panel,
+ const struct msm_dp_bridge_state *state,
+ const struct dp_sdp *vsc_sdp)
{
- msm_dp_ctrl_config_ctrl_streams(ctrl, panel);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, panel, !state->vsc_enabled);
msm_dp_ctrl_config_misc1_misc0(ctrl, panel);
msm_dp_panel_timing_cfg(panel, ctrl->msm_dp_ctrl.wide_bus_en);
+
+ if (state->vsc_enabled)
+ msm_dp_panel_enable_vsc_sdp(panel, vsc_sdp);
+ else
+ msm_dp_panel_disable_vsc_sdp(panel);
}
/*
@@ -1664,7 +1673,7 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
struct msm_dp_link_info link_info = {0};
msm_dp_ctrl_config_ctrl_link(ctrl, panel);
- msm_dp_ctrl_config_ctrl_streams(ctrl, panel);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, panel, false);
link_info.num_lanes = ctrl->link->link_params.num_lanes;
link_info.rate = ctrl->link->link_params.rate;
@@ -2633,9 +2642,12 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
return ret;
}
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel)
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel,
+ const struct msm_dp_bridge_state *state)
{
int ret = 0;
+ struct dp_sdp test_vsc_sdp;
+ const struct dp_sdp *vsc_sdp = &state->vsc_sdp;
bool mainlink_ready = false;
struct msm_dp_ctrl_private *ctrl;
unsigned long pixel_rate;
@@ -2650,6 +2662,22 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
if (!panel->video_test && !msm_dp_ctrl_link_fits(ctrl, panel))
return -ENOSPC;
+ if (state->vsc_enabled) {
+ if (!panel->vsc_sdp_supported)
+ return -EINVAL;
+
+ /* Compliance tests may override the checked stream depth. */
+ if (panel->video_test) {
+ ret = msm_dp_utils_pack_vsc_sdp(&test_vsc_sdp,
+ DRM_MODE_COLORIMETRY_DEFAULT,
+ panel->msm_dp_mode.bpp / 3,
+ panel->msm_dp_mode.out_fmt_is_yuv_420);
+ if (ret)
+ return ret;
+ vsc_sdp = &test_vsc_sdp;
+ }
+ }
+
pixel_rate_orig = panel->msm_dp_mode.drm_mode.clock;
pixel_rate = pixel_rate_orig;
@@ -2672,7 +2700,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
msm_dp_setup_peripheral_flush(ctrl);
msm_dp_ctrl_config_ctrl_link(ctrl, panel);
- msm_dp_ctrl_configure_source_params(ctrl, panel);
+ msm_dp_ctrl_configure_source_params(ctrl, panel, state, vsc_sdp);
msm_dp_ctrl_config_msa(ctrl,
ctrl->link->link_params.rate,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 4dd604ef9f1b..e4494a3f4b23 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -10,6 +10,8 @@
#include "dp_panel.h"
#include "dp_link.h"
+struct msm_dp_bridge_state;
+
struct msm_dp_ctrl {
bool wide_bus_en;
};
@@ -18,7 +20,8 @@ struct phy;
int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel);
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel);
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel,
+ const struct msm_dp_bridge_state *state);
int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel,
bool force_link_train);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 234cb2373451..5a9162712540 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -56,6 +56,8 @@ struct msm_dp_display_private {
bool core_initialized;
bool phy_initialized;
bool audio_supported;
+ /* Pending DPU peripheral flush, set before encoder enable. */
+ bool sdp_needs_flush;
bool stream_pm_active;
bool stream_link_attempted;
struct work_struct link_status_work;
@@ -679,7 +681,8 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
const struct drm_display_mode *adjusted_mode,
struct msm_dp_panel *msm_dp_panel,
- const struct msm_dp_bridge_state *state)
+ const struct msm_dp_bridge_state *state,
+ const struct msm_dp_bridge_state *old_state)
{
struct msm_dp_display_private *dp;
u32 bpp;
@@ -691,8 +694,7 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
else
bpp = state->bpp;
- msm_dp_panel->sdp_needs_flush = msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ||
- drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode);
+ dp->sdp_needs_flush = old_state->vsc_enabled || state->vsc_enabled;
msm_dp_panel_init_panel_info(msm_dp_panel, adjusted_mode, bpp ? bpp : 24);
/* populate wide_bus_support to different layers */
@@ -726,7 +728,15 @@ int msm_dp_display_atomic_check(struct msm_dp *msm_dp_display,
return bpp;
state->bpp = bpp;
- return 0;
+ state->vsc_enabled = drm_mode_is_420_only(info, mode);
+ if (!state->vsc_enabled)
+ return 0;
+
+ if (dp->panel->link_caps_valid && !dp->panel->vsc_sdp_supported)
+ return -EINVAL;
+
+ return msm_dp_utils_pack_vsc_sdp(&state->vsc_sdp, DRM_MODE_COLORIMETRY_DEFAULT,
+ bpp / 3, drm_mode_is_420_only(info, mode));
}
static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
@@ -751,7 +761,8 @@ static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
return 0;
}
-static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
+static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp,
+ const struct msm_dp_bridge_state *state)
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
int rc = 0;
@@ -761,6 +772,9 @@ static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
lockdep_assert_held(&dp->plugged_lock);
+ if (state->vsc_enabled && !dp->panel->vsc_sdp_supported)
+ return -EINVAL;
+
if (dp->link->sink_count == 0)
return -ENOTCONN;
@@ -782,7 +796,8 @@ static int msm_dp_display_prepare_link_locked(struct msm_dp_display_private *dp)
}
static int msm_dp_display_enable(struct msm_dp_display_private *dp,
- struct msm_dp_panel *msm_dp_panel)
+ struct msm_dp_panel *msm_dp_panel,
+ const struct msm_dp_bridge_state *state)
{
int rc = 0;
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
@@ -795,14 +810,15 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp,
return 0;
}
- rc = msm_dp_ctrl_on_stream(dp->ctrl, msm_dp_panel);
+ rc = msm_dp_ctrl_on_stream(dp->ctrl, msm_dp_panel, state);
if (!rc)
msm_dp_display->power_on = true;
return rc;
}
-static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
+static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display,
+ const struct msm_dp_bridge_state *state)
{
struct msm_dp_display_private *dp;
u32 rate;
@@ -822,8 +838,7 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
msm_dp_display_handle_plugged_change(msm_dp_display, true);
if (msm_dp_display->psr_supported)
- msm_dp_ctrl_config_psr(dp->ctrl, dp->panel,
- !dp->panel->msm_dp_mode.out_fmt_is_yuv_420);
+ msm_dp_ctrl_config_psr(dp->ctrl, dp->panel, !state->vsc_enabled);
return 0;
}
@@ -1563,7 +1578,7 @@ bool msm_dp_needs_periph_flush(const struct msm_dp *msm_dp_display,
struct msm_dp_display_private *dp = container_of(msm_dp_display,
struct msm_dp_display_private, msm_dp_display);
- return dp->panel->sdp_needs_flush;
+ return dp->sdp_needs_flush;
}
bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display)
@@ -1629,7 +1644,7 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
struct msm_dp_display_private *dp;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
- struct drm_bridge_state *bridge_state;
+ struct drm_bridge_state *bridge_state, *old_bridge_state;
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
@@ -1638,6 +1653,7 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
return;
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
bridge_state = drm_atomic_get_new_bridge_state(state, msm_dp_display->bridge);
+ old_bridge_state = drm_atomic_get_old_bridge_state(state, msm_dp_display->bridge);
/*
* The DPU encoder's .atomic_enable() reads the mode's YUV420 / wide bus
@@ -1645,7 +1661,8 @@ void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
* be programmed here, in .atomic_pre_enable().
*/
msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel,
- to_dp_bridge_state(bridge_state));
+ to_dp_bridge_state(bridge_state),
+ to_dp_bridge_state(old_bridge_state));
}
static void msm_dp_display_abort_enable_locked(struct msm_dp_display_private *dp)
@@ -1697,30 +1714,35 @@ static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)
void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
struct drm_atomic_commit *state)
{
+ struct drm_bridge_state *bridge_state;
+ const struct msm_dp_bridge_state *dp_state;
int rc = 0;
struct msm_dp_display_private *dp;
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
+ bridge_state = drm_atomic_get_new_bridge_state(state, msm_dp_display->bridge);
+ dp_state = to_dp_bridge_state(bridge_state);
+
rc = msm_dp_display_prepare(dp);
if (rc)
goto err_prepare;
mutex_lock(&dp->plugged_lock);
- rc = msm_dp_display_prepare_link_locked(dp);
+ rc = msm_dp_display_prepare_link_locked(dp, dp_state);
if (rc) {
DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
goto err;
}
- rc = msm_dp_display_enable(dp, dp->panel);
+ rc = msm_dp_display_enable(dp, dp->panel, dp_state);
if (rc) {
DRM_ERROR("DP display enable failed, rc=%d\n", rc);
goto err;
}
- rc = msm_dp_display_post_enable(msm_dp_display);
+ rc = msm_dp_display_post_enable(msm_dp_display, dp_state);
if (rc) {
DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
goto err;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 6b01eeeb1aef..808b6fe3d650 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -204,6 +204,7 @@ static int msm_edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
struct drm_connector_state *conn_state)
{
struct msm_dp *dp = to_dp_bridge(drm_bridge)->msm_dp_display;
+ struct msm_dp_bridge_state *state = to_dp_bridge_state(bridge_state);
int ret;
if (WARN_ON(!conn_state))
@@ -218,9 +219,7 @@ static int msm_edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
return ret;
/* Match the PSR configuration applied when enabling this stream. */
- conn_state->self_refresh_aware = dp->psr_supported &&
- !drm_mode_is_420_only(&conn_state->connector->display_info,
- &crtc_state->adjusted_mode);
+ conn_state->self_refresh_aware = dp->psr_supported && !state->vsc_enabled;
if (crtc_state->self_refresh_active && !conn_state->self_refresh_aware)
return -EINVAL;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 4c651cc40f4d..122150d95bbb 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -20,6 +20,8 @@ struct msm_dp_bridge {
struct msm_dp_bridge_state {
struct drm_bridge_state base;
u32 bpp;
+ bool vsc_enabled;
+ struct dp_sdp vsc_sdp;
};
#define to_dp_bridge_state(x) container_of((x), struct msm_dp_bridge_state, base)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 026074cafc88..bbbe7dc62550 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -516,7 +516,7 @@ static void msm_dp_panel_update_sdp(struct msm_dp_panel_private *panel)
}
}
-void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, struct dp_sdp *vsc_sdp)
+void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, const struct dp_sdp *vsc_sdp)
{
struct msm_dp_panel_private *panel =
container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
@@ -572,49 +572,6 @@ void msm_dp_panel_disable_vsc_sdp(struct msm_dp_panel *msm_dp_panel)
msm_dp_panel_update_sdp(panel);
}
-static int msm_dp_panel_setup_vsc_sdp_yuv_420(struct msm_dp_panel *msm_dp_panel)
-{
- struct msm_dp_display_mode *msm_dp_mode;
- struct drm_dp_vsc_sdp vsc_sdp_data;
- struct dp_sdp vsc_sdp;
- ssize_t len;
-
- if (!msm_dp_panel) {
- DRM_ERROR("invalid input\n");
- return -EINVAL;
- }
-
- msm_dp_mode = &msm_dp_panel->msm_dp_mode;
-
- memset(&vsc_sdp_data, 0, sizeof(vsc_sdp_data));
-
- /* VSC SDP header as per table 2-118 of DP 1.4 specification */
- vsc_sdp_data.sdp_type = DP_SDP_VSC;
- vsc_sdp_data.revision = 0x05;
- vsc_sdp_data.length = 0x13;
-
- /* VSC SDP Payload for DB16 */
- vsc_sdp_data.pixelformat = DP_PIXELFORMAT_YUV420;
- vsc_sdp_data.colorimetry = DP_COLORIMETRY_DEFAULT;
-
- /* VSC SDP Payload for DB17 */
- vsc_sdp_data.bpc = msm_dp_mode->bpp / 3;
- vsc_sdp_data.dynamic_range = DP_DYNAMIC_RANGE_CTA;
-
- /* VSC SDP Payload for DB18 */
- vsc_sdp_data.content_type = DP_CONTENT_TYPE_GRAPHICS;
-
- len = drm_dp_vsc_sdp_pack(&vsc_sdp_data, &vsc_sdp);
- if (len < 0) {
- DRM_ERROR("unable to pack vsc sdp\n");
- return len;
- }
-
- msm_dp_panel_enable_vsc_sdp(msm_dp_panel, &vsc_sdp);
-
- return 0;
-}
-
int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
{
u32 data, total_ver, total_hor;
@@ -684,11 +641,6 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
msm_dp_write_p0(panel, MMSS_DP_INTF_CONFIG, reg);
- if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
- msm_dp_panel_setup_vsc_sdp_yuv_420(msm_dp_panel);
- else
- msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
-
panel->panel_on = true;
return 0;
@@ -710,8 +662,7 @@ int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel,
msm_dp_panel->msm_dp_mode.h_active_low =
!!(adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC);
msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 =
- drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode) &&
- msm_dp_panel->vsc_sdp_supported;
+ drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode);
drm_mode = &msm_dp_panel->msm_dp_mode.drm_mode;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 31752895f124..9b81042e1036 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -42,7 +42,6 @@ struct msm_dp_panel {
bool link_caps_valid;
/* All capability reads completed; eDP can reuse the full cache. */
bool caps_cached;
- bool sdp_needs_flush;
u32 hw_revision;
u32 max_bw_code;
@@ -63,7 +62,7 @@ void msm_dp_panel_tpg_config(struct msm_dp_panel *msm_dp_panel, bool enable);
void msm_dp_panel_clear_dsc_dto(struct msm_dp_panel *msm_dp_panel);
-void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, struct dp_sdp *vsc_sdp);
+void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, const struct dp_sdp *vsc_sdp);
void msm_dp_panel_disable_vsc_sdp(struct msm_dp_panel *msm_dp_panel);
/**
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index 8b6ef2f6d7f4..bde627a6701b 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -122,6 +122,32 @@ int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
return -EINVAL;
}
+int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace,
+ unsigned int bpc, bool yuv420)
+{
+ struct drm_dp_vsc_sdp vsc = {
+ .sdp_type = DP_SDP_VSC,
+ .revision = 0x05,
+ .length = 0x13,
+ .pixelformat = yuv420 ? DP_PIXELFORMAT_YUV420 : DP_PIXELFORMAT_RGB,
+ .bpc = bpc,
+ .dynamic_range = yuv420 ? DP_DYNAMIC_RANGE_CTA : DP_DYNAMIC_RANGE_VESA,
+ .content_type = DP_CONTENT_TYPE_GRAPHICS,
+ };
+ ssize_t ret;
+
+ switch (colorspace) {
+ case DRM_MODE_COLORIMETRY_DEFAULT:
+ vsc.colorimetry = DP_COLORIMETRY_DEFAULT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = drm_dp_vsc_sdp_pack(&vsc, sdp);
+ return ret < 0 ? ret : 0;
+}
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state)
{
/* active_changed alone also describes a normal self-refresh exit. */
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index c6b62e3900bb..e916de2a5929 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -9,6 +9,7 @@
#include <linux/bitfield.h>
#include <linux/bits.h>
#include <drm/display/drm_dp_helper.h>
+#include <drm/drm_connector.h>
struct drm_crtc_state;
@@ -42,6 +43,9 @@ int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
unsigned int clock, unsigned int rate,
unsigned int lanes, bool yuv420);
+int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace,
+ unsigned int bpc, bool yuv420);
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state);
#endif /* _DP_UTILS_H_ */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state
2026-10-09 3:16 ` [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state Xilin Wu
@ 2026-10-09 3:34 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:34 UTC (permalink / raw)
To: Xilin Wu; +Cc: neil.armstrong, linux-amlogic, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm/dp: persistent peripheral flush after disabling VSC SDP (state stickiness)
--
commit b5e9083e5d055244149a4e0660254522762ffc0e
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Track VSC packets in bridge atomic state
This commit moves YUV420 VSC construction to the atomic check phase and
retains the packed packet in the private bridge state. It also programs
the packet unchanged for normal streams while deriving the peripheral
flush from both the old and new states.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -679,7 +681,8 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
> static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
> const struct drm_display_mode *adjusted_mode,
> struct msm_dp_panel *msm_dp_panel,
> - const struct msm_dp_bridge_state *state)
> + const struct msm_dp_bridge_state *state,
> + const struct msm_dp_bridge_state *old_state)
> {
> struct msm_dp_display_private *dp;
> u32 bpp;
> @@ -691,8 +694,7 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
> else
> bpp = state->bpp;
>
> - msm_dp_panel->sdp_needs_flush = msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ||
> - drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode);
> + dp->sdp_needs_flush = old_state->vsc_enabled || state->vsc_enabled;
[Severity: Medium]
When transitioning from a mode with VSC SDP enabled (e.g., YUV420) to a mode
without it (RGB), the display controller requires a peripheral flush to apply
the disabled SDP configuration. Here, the expression correctly evaluates to
true for that transition frame.
However, since msm_dp_display_set_mode() is only executed during
atomic_pre_enable (on modesets), does this code leave the flush flag
persistently enabled?
Because there is no mechanism here to clear dp->sdp_needs_flush after the
transition frame completes, it appears the flag will stay true for all
subsequent non-modeset page flips.
[ ... ]
> @@ -1563,7 +1578,7 @@ bool msm_dp_needs_periph_flush(const struct msm_dp *msm_dp_display,
> struct msm_dp_display_private *dp = container_of(msm_dp_display,
> struct msm_dp_display_private, msm_dp_display);
>
> - return dp->panel->sdp_needs_flush;
> + return dp->sdp_needs_flush;
[Severity: Medium]
The DPU encoder queries msm_dp_needs_periph_flush() on every frame kickoff.
If the value remains stuck at true after a transition away from a VSC SDP
mode, does this cause the hardware to perform unnecessary peripheral flushes
on every VSYNC during the entire duration of the new mode?
Could we provide a way to clear the flush flag once the transition frame has
been successfully kicked off?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=17
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 18/20] drm/msm/dp: Signal BT.2020 RGB on DP and eDP
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (16 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:16 ` [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes Xilin Wu
2026-10-09 3:16 ` [PATCH v2 20/20] drm/msm/dp: Add static HDR metadata support for DP and eDP Xilin Wu
19 siblings, 0 replies; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Expose the standard DP Colorspace property with BT2020_RGB support and
carry the selected colorspace in bridge atomic state. Force a modeset
when it changes so the stream receives the corresponding VSC packet.
Signal full-range RGB to match the existing pixel path and retain CTA
range for YUV420. Reject BT2020_RGB with a YUV420-only mode. Reuse the
preceding VSC state, update and PSR handling for the new colorimetry.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++--
drivers/gpu/drm/msm/dp/dp_drm.c | 9 ++++++++-
drivers/gpu/drm/msm/dp/dp_drm.h | 1 +
drivers/gpu/drm/msm/dp/dp_utils.c | 12 ++++++++++++
drivers/gpu/drm/msm/dp/dp_utils.h | 3 +++
6 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 2fd8dd456852..a1a649c636c7 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2669,7 +2669,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
/* Compliance tests may override the checked stream depth. */
if (panel->video_test) {
ret = msm_dp_utils_pack_vsc_sdp(&test_vsc_sdp,
- DRM_MODE_COLORIMETRY_DEFAULT,
+ state->colorspace,
panel->msm_dp_mode.bpp / 3,
panel->msm_dp_mode.out_fmt_is_yuv_420);
if (ret)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 5a9162712540..34e1c48cdd79 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -728,14 +728,16 @@ int msm_dp_display_atomic_check(struct msm_dp *msm_dp_display,
return bpp;
state->bpp = bpp;
- state->vsc_enabled = drm_mode_is_420_only(info, mode);
+ state->colorspace = conn_state->colorspace;
+ state->vsc_enabled = drm_mode_is_420_only(info, mode) ||
+ state->colorspace != DRM_MODE_COLORIMETRY_DEFAULT;
if (!state->vsc_enabled)
return 0;
if (dp->panel->link_caps_valid && !dp->panel->vsc_sdp_supported)
return -EINVAL;
- return msm_dp_utils_pack_vsc_sdp(&state->vsc_sdp, DRM_MODE_COLORIMETRY_DEFAULT,
+ return msm_dp_utils_pack_vsc_sdp(&state->vsc_sdp, state->colorspace,
bpp / 3, drm_mode_is_420_only(info, mode));
}
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 808b6fe3d650..0d17536f02fd 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -171,7 +171,7 @@ static int msm_dp_bridge_atomic_check(struct drm_bridge *bridge,
old_conn_state = drm_atomic_get_old_connector_state(conn_state->state,
conn_state->connector);
- if (old_conn_state->max_requested_bpc != conn_state->max_requested_bpc)
+ if (msm_dp_utils_needs_modeset(old_conn_state, conn_state))
crtc_state->mode_changed = true;
return 0;
@@ -456,11 +456,18 @@ struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
struct drm_encoder *encoder)
{
struct drm_connector *connector = NULL;
+ int ret;
connector = drm_bridge_connector_init(msm_dp_display->drm_dev, encoder);
if (IS_ERR(connector))
return connector;
+ ret = drm_mode_create_dp_colorspace_property(connector,
+ BIT(DRM_MODE_COLORIMETRY_BT2020_RGB));
+ if (ret)
+ return ERR_PTR(ret);
+ drm_connector_attach_colorspace_property(connector);
+
if (!msm_dp_display->is_edp)
drm_connector_attach_dp_subconnector_property(connector);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 122150d95bbb..0e85a1a55c88 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -20,6 +20,7 @@ struct msm_dp_bridge {
struct msm_dp_bridge_state {
struct drm_bridge_state base;
u32 bpp;
+ enum drm_colorspace colorspace;
bool vsc_enabled;
struct dp_sdp vsc_sdp;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index bde627a6701b..431c18aa2096 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -140,6 +140,11 @@ int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace
case DRM_MODE_COLORIMETRY_DEFAULT:
vsc.colorimetry = DP_COLORIMETRY_DEFAULT;
break;
+ case DRM_MODE_COLORIMETRY_BT2020_RGB:
+ if (yuv420)
+ return -EINVAL;
+ vsc.colorimetry = DP_COLORIMETRY_BT2020_RGB;
+ break;
default:
return -EINVAL;
}
@@ -148,6 +153,13 @@ int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace
return ret < 0 ? ret : 0;
}
+bool msm_dp_utils_needs_modeset(struct drm_connector_state *old_state,
+ struct drm_connector_state *new_state)
+{
+ return old_state->max_requested_bpc != new_state->max_requested_bpc ||
+ old_state->colorspace != new_state->colorspace;
+}
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state)
{
/* active_changed alone also describes a normal self-refresh exit. */
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index e916de2a5929..81773db75c60 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -46,6 +46,9 @@ int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace,
unsigned int bpc, bool yuv420);
+bool msm_dp_utils_needs_modeset(struct drm_connector_state *old_state,
+ struct drm_connector_state *new_state);
+
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state);
#endif /* _DP_UTILS_H_ */
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (17 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 18/20] drm/msm/dp: Signal BT.2020 RGB on DP and eDP Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:44 ` sashiko-bot
2026-10-09 3:16 ` [PATCH v2 20/20] drm/msm/dp: Add static HDR metadata support for DP and eDP Xilin Wu
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
The debugfs test control writes video_test directly, racing HPD handling
and mode programming. Reading the test flag and its requested depth in
separate calls also allows those values to change between the reads.
Route test state access through the display and use plugged_lock for both
HPD and debugfs updates. Hold the same lock while selecting the test depth
and installing the mode. Pass the display instance to debugfs for access
to its connector and the serialized test controls.
Preserve the existing SDR test behavior. The shared setter provides the
entry point for rejecting incompatible tests when HDR is added.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/dp/dp_debug.c | 27 ++++++++++++++-------------
drivers/gpu/drm/msm/dp/dp_debug.h | 8 +++++---
drivers/gpu/drm/msm/dp/dp_display.c | 34 ++++++++++++++++++++++------------
drivers/gpu/drm/msm/dp/dp_display.h | 2 +-
4 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
index 00fb1a7ba40e..c26b356f998a 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -21,7 +21,7 @@
struct msm_dp_debug_private {
struct msm_dp_link *link;
struct msm_dp_panel *panel;
- struct drm_connector *connector;
+ struct msm_dp *dp;
};
static int msm_dp_debug_show(struct seq_file *seq, void *p)
@@ -89,7 +89,7 @@ DEFINE_SHOW_ATTRIBUTE(msm_dp_debug);
static int msm_dp_test_data_show(struct seq_file *m, void *data)
{
const struct msm_dp_debug_private *debug = m->private;
- const struct drm_connector *connector = debug->connector;
+ const struct drm_connector *connector = debug->dp->connector;
u32 bpc;
if (connector->status == connector_status_connected) {
@@ -111,7 +111,7 @@ DEFINE_SHOW_ATTRIBUTE(msm_dp_test_data);
static int msm_dp_test_type_show(struct seq_file *m, void *data)
{
const struct msm_dp_debug_private *debug = m->private;
- const struct drm_connector *connector = debug->connector;
+ const struct drm_connector *connector = debug->dp->connector;
if (connector->status == connector_status_connected)
seq_printf(m, "%02x", DP_TEST_LINK_VIDEO_PATTERN);
@@ -133,7 +133,7 @@ static ssize_t msm_dp_test_active_write(struct file *file,
int val = 0;
debug = ((struct seq_file *)file->private_data)->private;
- connector = debug->connector;
+ connector = debug->dp->connector;
if (len == 0)
return 0;
@@ -154,10 +154,11 @@ static ssize_t msm_dp_test_active_write(struct file *file,
/* To prevent erroneous activation of the compliance
* testing code, only accept an actual value of 1 here
*/
- if (val == 1)
- debug->panel->video_test = true;
- else
- debug->panel->video_test = false;
+ status = msm_dp_display_set_video_test(debug->dp, val == 1);
+ if (status) {
+ kfree(input_buffer);
+ return status;
+ }
}
kfree(input_buffer);
@@ -168,10 +169,10 @@ static ssize_t msm_dp_test_active_write(struct file *file,
static int msm_dp_test_active_show(struct seq_file *m, void *data)
{
struct msm_dp_debug_private *debug = m->private;
- struct drm_connector *connector = debug->connector;
+ struct drm_connector *connector = debug->dp->connector;
if (connector->status == connector_status_connected) {
- if (debug->panel->video_test)
+ if (msm_dp_display_check_video_test(debug->dp))
seq_puts(m, "1");
else
seq_puts(m, "0");
@@ -200,12 +201,12 @@ static const struct file_operations test_active_fops = {
int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
struct msm_dp_link *link,
- struct drm_connector *connector,
+ struct msm_dp *dp,
struct dentry *root, bool is_edp)
{
struct msm_dp_debug_private *debug;
- if (!dev || !panel || !link) {
+ if (!dev || !panel || !link || !dp || !dp->connector) {
DRM_ERROR("invalid input\n");
return -EINVAL;
}
@@ -216,7 +217,7 @@ int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
debug->link = link;
debug->panel = panel;
- debug->connector = connector;
+ debug->dp = dp;
debugfs_create_file("dp_debug", 0444, root,
debug, &msm_dp_debug_fops);
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h b/drivers/gpu/drm/msm/dp/dp_debug.h
index a90083fec856..430891e110fb 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.h
+++ b/drivers/gpu/drm/msm/dp/dp_debug.h
@@ -9,6 +9,8 @@
#include "dp_panel.h"
#include "dp_link.h"
+struct msm_dp;
+
#if defined(CONFIG_DEBUG_FS)
/**
@@ -17,7 +19,7 @@
* @dev: device instance of the caller
* @panel: instance of panel module
* @link: instance of link module
- * @connector: double pointer to display connector
+ * @dp: display instance owning the connector and video test state
* @root: connector's debugfs root
* @is_edp: set for eDP connectors / panels
* return: pointer to allocated debug module data
@@ -27,7 +29,7 @@
*/
int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
struct msm_dp_link *link,
- struct drm_connector *connector,
+ struct msm_dp *dp,
struct dentry *root,
bool is_edp);
@@ -36,7 +38,7 @@ int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
static inline
int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
struct msm_dp_link *link,
- struct drm_connector *connector,
+ struct msm_dp *dp,
struct dentry *root,
bool is_edp)
{
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 34e1c48cdd79..c51d033598a5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -435,10 +435,20 @@ static void msm_dp_display_host_deinit(struct msm_dp_display_private *dp)
dp->core_initialized = false;
}
+static int msm_dp_display_set_video_test_locked(struct msm_dp_display_private *dp,
+ bool enable)
+{
+ lockdep_assert_held(&dp->plugged_lock);
+
+ dp->panel->video_test = enable;
+ return 0;
+}
+
static void msm_dp_display_handle_video_request(struct msm_dp_display_private *dp)
{
if (dp->link->sink_request & DP_TEST_LINK_VIDEO_PATTERN) {
- dp->panel->video_test = true;
+ if (msm_dp_display_set_video_test_locked(dp, true))
+ dp->link->test_response = DP_TEST_NAK;
msm_dp_link_send_test_response(dp->link);
}
}
@@ -689,8 +699,10 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- if (msm_dp_display_check_video_test(msm_dp_display))
- bpp = msm_dp_display_get_test_bpp(msm_dp_display);
+ guard(mutex)(&dp->plugged_lock);
+
+ if (dp->panel->video_test)
+ bpp = msm_dp_link_bit_depth_to_bpp(dp->link->test_video.test_bit_depth);
else
bpp = state->bpp;
@@ -974,22 +986,20 @@ bool msm_dp_display_check_video_test(struct msm_dp *dp)
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
+ guard(mutex)(&msm_dp_display->plugged_lock);
+
return msm_dp_display->panel->video_test;
}
-int msm_dp_display_get_test_bpp(struct msm_dp *dp)
+int msm_dp_display_set_video_test(struct msm_dp *dp, bool enable)
{
struct msm_dp_display_private *msm_dp_display;
- if (!dp) {
- DRM_ERROR("invalid params\n");
- return 0;
- }
-
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
- return msm_dp_link_bit_depth_to_bpp(
- msm_dp_display->link->test_video.test_bit_depth);
+ guard(mutex)(&msm_dp_display->plugged_lock);
+
+ return msm_dp_display_set_video_test_locked(msm_dp_display, enable);
}
void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp)
@@ -1604,7 +1614,7 @@ void msm_dp_display_debugfs_init(struct msm_dp *msm_dp_display, struct dentry *r
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
dev = &dp->msm_dp_display.pdev->dev;
- rc = msm_dp_debug_init(dev, dp->panel, dp->link, dp->msm_dp_display.connector, root, is_edp);
+ rc = msm_dp_debug_init(dev, dp->panel, dp->link, msm_dp_display, root, is_edp);
if (rc)
DRM_ERROR("failed to initialize debug, rc = %d\n", rc);
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 6a77e5952530..ac64c1d3227a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -30,7 +30,7 @@ struct msm_dp {
int msm_dp_display_get_modes(struct msm_dp *msm_dp_display);
bool msm_dp_display_check_video_test(struct msm_dp *msm_dp_display);
-int msm_dp_display_get_test_bpp(struct msm_dp *msm_dp_display);
+int msm_dp_display_set_video_test(struct msm_dp *dp, bool enable);
void msm_dp_display_signal_audio_start(struct msm_dp *msm_dp_display);
void msm_dp_display_signal_audio_complete(struct msm_dp *msm_dp_display);
void msm_dp_display_set_psr(struct msm_dp *dp, bool enter);
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes
2026-10-09 3:16 ` [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes Xilin Wu
@ 2026-10-09 3:44 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:44 UTC (permalink / raw)
To: Xilin Wu; +Cc: dri-devel, neil.armstrong, linux-amlogic
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm/dp: missing plugged_lock in msm_dp_panel_get_mode_bpp() leaves TOCTOU race
--
commit 016f17497dc5f474b86e152324fba4d9409781b6
Author: Xilin Wu <sophon@radxa.com>
drm/msm/dp: Serialize video test state changes
This commit serializes video test state access by routing it through the
display and using plugged_lock for HPD and debugfs updates. This prevents
race conditions during mode programming.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 34e1c48cdd794..c51d033598a5b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -689,8 +699,10 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
>
> dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>
> - if (msm_dp_display_check_video_test(msm_dp_display))
> - bpp = msm_dp_display_get_test_bpp(msm_dp_display);
> + guard(mutex)(&dp->plugged_lock);
> +
> + if (dp->panel->video_test)
> + bpp = msm_dp_link_bit_depth_to_bpp(dp->link->test_video.test_bit_depth);
[Severity: Medium]
The commit message notes that reading the test flag and requested depth
separately allows values to change mid-read, and this function was safely
updated to hold plugged_lock.
However, does this patch miss the similar reads in msm_dp_panel_get_mode_bpp()
in drivers/gpu/drm/msm/dp/dp_panel.c?
That function is called during DRM mode validation and still reads these
fields sequentially without holding plugged_lock:
drivers/gpu/drm/msm/dp/dp_panel.c:msm_dp_panel_get_mode_bpp() {
...
if (msm_dp_panel->video_test)
bpp = msm_dp_link_bit_depth_to_bpp(
panel->link->test_video.test_bit_depth);
...
}
If HPD or debugfs updates these values concurrently, could this function
observe a torn state where the test flag is true but the bit depth has not
yet been updated, causing it to incorrectly accept or reject a mode based
on inconsistent parameters?
> else
> bpp = state->bpp;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=19
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 20/20] drm/msm/dp: Add static HDR metadata support for DP and eDP
2026-10-09 3:15 [PATCH v2 00/20] drm/msm/dp: Add static HDR support for DP and eDP Xilin Wu
` (18 preceding siblings ...)
2026-10-09 3:16 ` [PATCH v2 19/20] drm/msm/dp: Serialize video test state changes Xilin Wu
@ 2026-10-09 3:16 ` Xilin Wu
2026-10-09 3:40 ` sashiko-bot
19 siblings, 1 reply; 32+ messages in thread
From: Xilin Wu @ 2026-10-09 3:16 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, dragon,
Xilin Wu, linux-amlogic, linux-arm-kernel
Expose HDR_OUTPUT_METADATA and validate and pack it during atomic check.
Program Generic2 from the checked bridge state on every stream enable.
Reuse the DRM metadata conversion and infoframe packing helpers, omit
the HDMI checksum when constructing the DP SDP, and zero reserved bytes.
Check the cached controller revision and sink HDR capabilities before
accepting the atomic state. Require 10 bpc for HDR transfer functions
and validate link bandwidth during atomic check. Training must preserve
that depth; report a link failure if the trained link cannot carry the
stream.
Force a modeset for metadata changes and include metadata enable and
disable in peripheral flush requests. Keep HDR streams out of PSR and
clear Generic2 enable state when disabling the stream. Preserve the
checked metadata in bridge state so every stream rebuild restores its
packets. Pass state through stream programming without copying the
configuration into the panel.
Give checked HDR modesets precedence over SDR video-pattern tests.
Reject new test requests with NAK while HDR is pending or active and
return -EBUSY from the debugfs test control. Clear the test override
under plugged_lock when applying HDR so that a request arriving after
atomic check cannot replace the checked output depth. Keep pending test
colorimetry from overriding HDR signalling. Release the test exclusion
on disable or enable failure. Atomic checks leave the running test state
untouched.
Assisted-by: LLM
Signed-off-by: Xilin Wu <sophon@radxa.com>
---
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 +++++++--
drivers/gpu/drm/msm/dp/dp_display.c | 35 ++++++++++++++++++---
drivers/gpu/drm/msm/dp/dp_drm.c | 1 +
drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++
drivers/gpu/drm/msm/dp/dp_panel.c | 29 ++++++++++++++++++
drivers/gpu/drm/msm/dp/dp_panel.h | 3 ++
drivers/gpu/drm/msm/dp/dp_utils.c | 61 ++++++++++++++++++++++++++++++++++---
drivers/gpu/drm/msm/dp/dp_utils.h | 5 ++-
9 files changed, 140 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index ed1c4775d1e9..e988e6509ec8 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -119,6 +119,7 @@ config DRM_MSM_DP
depends on DRM_MSM
depends on DRM_MSM_KMS
select DRM_DISPLAY_HDMI_AUDIO_HELPER
+ select DRM_DISPLAY_HDMI_HELPER
select RATIONAL
select DRM_DISPLAY_DP_AUX_BUS
select DRM_DISPLAY_DP_HELPER
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index a1a649c636c7..cc1914f7cf18 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -467,13 +467,16 @@ static void msm_dp_ctrl_lane_mapping(struct msm_dp_ctrl_private *ctrl)
}
static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
- struct msm_dp_panel *msm_dp_panel)
+ struct msm_dp_panel *msm_dp_panel,
+ const struct msm_dp_bridge_state *state)
{
u32 colorimetry_cfg, test_bits_depth, misc_val;
test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link,
msm_dp_panel->msm_dp_mode.bpp);
- colorimetry_cfg = msm_dp_link_get_colorimetry_config(ctrl->link);
+ /* A video test request must not override the HDR quantization range. */
+ colorimetry_cfg = state->hdr_enabled ? DP_MISC0_COLORIMERY_CFG_LEGACY_RGB :
+ msm_dp_link_get_colorimetry_config(ctrl->link);
misc_val = msm_dp_read_link(ctrl, REG_DP_MISC1_MISC0);
@@ -496,7 +499,7 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
{
msm_dp_ctrl_config_ctrl_streams(ctrl, panel, !state->vsc_enabled);
- msm_dp_ctrl_config_misc1_misc0(ctrl, panel);
+ msm_dp_ctrl_config_misc1_misc0(ctrl, panel, state);
msm_dp_panel_timing_cfg(panel, ctrl->msm_dp_ctrl.wide_bus_en);
@@ -504,6 +507,11 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
msm_dp_panel_enable_vsc_sdp(panel, vsc_sdp);
else
msm_dp_panel_disable_vsc_sdp(panel);
+
+ if (state->hdr_enabled)
+ msm_dp_panel_enable_hdr_sdp(panel, &state->hdr_sdp);
+ else
+ msm_dp_panel_disable_hdr_sdp(panel);
}
/*
@@ -2747,6 +2755,7 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
/* Link startup may have failed before enabling the link clocks. */
if (ctrl->link_clks_on) {
msm_dp_panel_disable_vsc_sdp(panel);
+ msm_dp_panel_disable_hdr_sdp(panel);
msm_dp_ctrl_mainlink_disable(ctrl);
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c51d033598a5..7104822b03e0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -58,6 +58,8 @@ struct msm_dp_display_private {
bool audio_supported;
/* Pending DPU peripheral flush, set before encoder enable. */
bool sdp_needs_flush;
+ /* Block asynchronous video tests while an HDR stream is pending or active. */
+ bool video_test_blocked;
bool stream_pm_active;
bool stream_link_attempted;
struct work_struct link_status_work;
@@ -440,6 +442,9 @@ static int msm_dp_display_set_video_test_locked(struct msm_dp_display_private *d
{
lockdep_assert_held(&dp->plugged_lock);
+ if (enable && dp->video_test_blocked)
+ return -EBUSY;
+
dp->panel->video_test = enable;
return 0;
}
@@ -701,12 +706,19 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
guard(mutex)(&dp->plugged_lock);
+ dp->video_test_blocked = state->hdr_enabled;
+
+ /* A checked HDR modeset supersedes any intervening video test request. */
+ if (state->hdr_enabled)
+ dp->panel->video_test = false;
+
if (dp->panel->video_test)
bpp = msm_dp_link_bit_depth_to_bpp(dp->link->test_video.test_bit_depth);
else
bpp = state->bpp;
- dp->sdp_needs_flush = old_state->vsc_enabled || state->vsc_enabled;
+ dp->sdp_needs_flush = old_state->vsc_enabled || state->vsc_enabled ||
+ old_state->hdr_enabled || state->hdr_enabled;
msm_dp_panel_init_panel_info(msm_dp_panel, adjusted_mode, bpp ? bpp : 24);
/* populate wide_bus_support to different layers */
@@ -723,7 +735,7 @@ int msm_dp_display_atomic_check(struct msm_dp *msm_dp_display,
const struct drm_display_info *info = &conn_state->connector->display_info;
struct msm_dp_display_private *dp;
unsigned int clock = mode->clock;
- int bpp;
+ int bpp, ret;
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
@@ -732,17 +744,28 @@ int msm_dp_display_atomic_check(struct msm_dp *msm_dp_display,
if (!dp->panel->link_caps_valid)
return -EINVAL;
+ ret = msm_dp_utils_pack_hdr_sdp(conn_state, &state->hdr_sdp);
+ if (ret)
+ return ret;
+
+ state->hdr_enabled = !!conn_state->hdr_output_metadata;
+ if (state->hdr_enabled && dp->panel->hw_revision < DP_HW_VERSION_1_0)
+ return -EINVAL;
+
bpp = msm_dp_utils_select_bpp(info->bpc, conn_state->max_requested_bpc ?: 8,
clock, dp->panel->link_info.rate,
dp->panel->link_info.num_lanes,
- drm_mode_is_420_only(info, mode));
+ drm_mode_is_420_only(info, mode),
+ state->hdr_enabled && state->hdr_sdp.db[2] !=
+ HDMI_EOTF_TRADITIONAL_GAMMA_SDR);
if (bpp < 0)
return bpp;
state->bpp = bpp;
state->colorspace = conn_state->colorspace;
state->vsc_enabled = drm_mode_is_420_only(info, mode) ||
- state->colorspace != DRM_MODE_COLORIMETRY_DEFAULT;
+ state->colorspace != DRM_MODE_COLORIMETRY_DEFAULT ||
+ state->hdr_enabled;
if (!state->vsc_enabled)
return 0;
@@ -880,10 +903,13 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
lockdep_assert_held(&dp->plugged_lock);
+ dp->video_test_blocked = false;
+
if (!msm_dp_display->power_on)
return 0;
msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
+ msm_dp_panel_disable_hdr_sdp(msm_dp_panel);
msm_dp_ctrl_off_pixel_clk(dp->ctrl);
@@ -1774,6 +1800,7 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
msm_dp_hpd_unplug_handle(dp);
msm_dp_display_unprepare(dp);
scoped_guard(mutex, &dp->plugged_lock) {
+ dp->video_test_blocked = false;
dp->link_failed = true;
/* Keep reporting BAD, but notify only once until recovery. */
schedule_work(&dp->link_status_work);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 0d17536f02fd..16c9bd3afc1b 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -467,6 +467,7 @@ struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
if (ret)
return ERR_PTR(ret);
drm_connector_attach_colorspace_property(connector);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!msm_dp_display->is_edp)
drm_connector_attach_dp_subconnector_property(connector);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 0e85a1a55c88..89c0123af641 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -23,6 +23,8 @@ struct msm_dp_bridge_state {
enum drm_colorspace colorspace;
bool vsc_enabled;
struct dp_sdp vsc_sdp;
+ bool hdr_enabled;
+ struct dp_sdp hdr_sdp;
};
#define to_dp_bridge_state(x) container_of((x), struct msm_dp_bridge_state, base)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index bbbe7dc62550..78ab2d280e11 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -572,6 +572,35 @@ void msm_dp_panel_disable_vsc_sdp(struct msm_dp_panel *msm_dp_panel)
msm_dp_panel_update_sdp(panel);
}
+void msm_dp_panel_enable_hdr_sdp(struct msm_dp_panel *msm_dp_panel,
+ const struct dp_sdp *hdr_sdp)
+{
+ struct msm_dp_panel_private *panel =
+ container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
+ u32 cfg, cfg2;
+
+ msm_dp_panel_send_sdp(panel, hdr_sdp, MMSS_DP_GENERIC2_0);
+
+ cfg2 = msm_dp_read_link(panel, MMSS_DP_SDP_CFG2);
+ msm_dp_write_link(panel, MMSS_DP_SDP_CFG2, cfg2 | GENERIC2_SDPSIZE_VALID);
+ cfg = msm_dp_read_link(panel, MMSS_DP_SDP_CFG);
+ msm_dp_write_link(panel, MMSS_DP_SDP_CFG, cfg | GEN2_SDP_EN);
+ msm_dp_panel_update_sdp(panel);
+}
+
+void msm_dp_panel_disable_hdr_sdp(struct msm_dp_panel *msm_dp_panel)
+{
+ struct msm_dp_panel_private *panel =
+ container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
+ u32 cfg, cfg2;
+
+ cfg = msm_dp_read_link(panel, MMSS_DP_SDP_CFG);
+ msm_dp_write_link(panel, MMSS_DP_SDP_CFG, cfg & ~GEN2_SDP_EN);
+ cfg2 = msm_dp_read_link(panel, MMSS_DP_SDP_CFG2);
+ msm_dp_write_link(panel, MMSS_DP_SDP_CFG2, cfg2 & ~GENERIC2_SDPSIZE_VALID);
+ msm_dp_panel_update_sdp(panel);
+}
+
int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
{
u32 data, total_ver, total_hor;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9b81042e1036..58a5f846f788 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -64,6 +64,9 @@ void msm_dp_panel_clear_dsc_dto(struct msm_dp_panel *msm_dp_panel);
void msm_dp_panel_enable_vsc_sdp(struct msm_dp_panel *msm_dp_panel, const struct dp_sdp *vsc_sdp);
void msm_dp_panel_disable_vsc_sdp(struct msm_dp_panel *msm_dp_panel);
+void msm_dp_panel_enable_hdr_sdp(struct msm_dp_panel *msm_dp_panel,
+ const struct dp_sdp *hdr_sdp);
+void msm_dp_panel_disable_hdr_sdp(struct msm_dp_panel *msm_dp_panel);
/**
* is_link_rate_valid() - validates the link rate
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.c b/drivers/gpu/drm/msm/dp/dp_utils.c
index 431c18aa2096..c7a42f130d42 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.c
+++ b/drivers/gpu/drm/msm/dp/dp_utils.c
@@ -4,6 +4,8 @@
*/
#include <linux/types.h>
+#include <drm/display/drm_hdmi_helper.h>
+#include <drm/drm_property.h>
#include <drm/drm_crtc.h>
#include "dp_utils.h"
@@ -105,16 +107,19 @@ bool msm_dp_utils_link_fits(unsigned int clock, unsigned int bpp,
int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
unsigned int clock, unsigned int rate,
- unsigned int lanes, bool yuv420)
+ unsigned int lanes, bool yuv420, bool hdr)
{
unsigned int bpp;
+ if (hdr && bpc < 10)
+ return -EINVAL;
+
bpc = min3(bpc ? bpc : 8, max_bpc, 10U);
bpc = round_down(bpc, 2);
- if (bpc < 6)
+ if (bpc < (hdr ? 10 : 6))
return -EINVAL;
- for (bpp = bpc * 3; bpp >= 18; bpp -= 6) {
+ for (bpp = bpc * 3; bpp >= (hdr ? 30 : 18); bpp -= 6) {
if (msm_dp_utils_link_fits(clock, bpp, rate, lanes, yuv420))
return bpp;
}
@@ -153,11 +158,59 @@ int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace
return ret < 0 ? ret : 0;
}
+int msm_dp_utils_pack_hdr_sdp(const struct drm_connector_state *conn_state,
+ struct dp_sdp *sdp)
+{
+ const struct hdr_output_metadata *metadata;
+ const struct hdr_static_metadata *sink;
+ struct hdmi_drm_infoframe frame;
+ u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_DRM_INFOFRAME_SIZE];
+ int ret;
+
+ memset(sdp, 0, sizeof(*sdp));
+ if (!conn_state->hdr_output_metadata)
+ return 0;
+
+ if (conn_state->hdr_output_metadata->length != sizeof(*metadata))
+ return -EINVAL;
+
+ metadata = conn_state->hdr_output_metadata->data;
+ sink = &conn_state->connector->display_info.hdr_sink_metadata.hdmi_type1;
+ if (metadata->metadata_type != HDMI_STATIC_METADATA_TYPE1 ||
+ metadata->hdmi_metadata_type1.metadata_type != HDMI_STATIC_METADATA_TYPE1 ||
+ metadata->hdmi_metadata_type1.eotf > HDMI_EOTF_BT_2100_HLG)
+ return -EINVAL;
+
+ if (!(sink->metadata_type & BIT(HDMI_STATIC_METADATA_TYPE1)) ||
+ !(sink->eotf & BIT(metadata->hdmi_metadata_type1.eotf)))
+ return -EINVAL;
+
+ ret = drm_hdmi_infoframe_set_hdr_metadata(&frame, conn_state);
+ if (ret)
+ return ret;
+
+ ret = hdmi_drm_infoframe_pack_only(&frame, buffer, sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ sdp->sdp_header.HB1 = HDMI_INFOFRAME_TYPE_DRM;
+ sdp->sdp_header.HB2 = 0x1d;
+ sdp->sdp_header.HB3 = 0x13 << 2;
+ sdp->db[0] = frame.version;
+ sdp->db[1] = frame.length;
+ /* DP carries the version and length, but no HDMI checksum. */
+ memcpy(&sdp->db[2], &buffer[HDMI_INFOFRAME_HEADER_SIZE],
+ HDMI_DRM_INFOFRAME_SIZE);
+
+ return 0;
+}
+
bool msm_dp_utils_needs_modeset(struct drm_connector_state *old_state,
struct drm_connector_state *new_state)
{
return old_state->max_requested_bpc != new_state->max_requested_bpc ||
- old_state->colorspace != new_state->colorspace;
+ old_state->colorspace != new_state->colorspace ||
+ !drm_connector_atomic_hdr_metadata_equal(old_state, new_state);
}
bool msm_dp_utils_psr_needs_modeset(const struct drm_crtc_state *state)
diff --git a/drivers/gpu/drm/msm/dp/dp_utils.h b/drivers/gpu/drm/msm/dp/dp_utils.h
index 81773db75c60..397a40a58e1c 100644
--- a/drivers/gpu/drm/msm/dp/dp_utils.h
+++ b/drivers/gpu/drm/msm/dp/dp_utils.h
@@ -41,11 +41,14 @@ bool msm_dp_utils_link_fits(unsigned int clock, unsigned int bpp,
unsigned int rate, unsigned int lanes, bool yuv420);
int msm_dp_utils_select_bpp(unsigned int bpc, unsigned int max_bpc,
unsigned int clock, unsigned int rate,
- unsigned int lanes, bool yuv420);
+ unsigned int lanes, bool yuv420, bool hdr);
int msm_dp_utils_pack_vsc_sdp(struct dp_sdp *sdp, enum drm_colorspace colorspace,
unsigned int bpc, bool yuv420);
+int msm_dp_utils_pack_hdr_sdp(const struct drm_connector_state *conn_state,
+ struct dp_sdp *sdp);
+
bool msm_dp_utils_needs_modeset(struct drm_connector_state *old_state,
struct drm_connector_state *new_state);
--
2.56.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 20/20] drm/msm/dp: Add static HDR metadata support for DP and eDP
2026-10-09 3:16 ` [PATCH v2 20/20] drm/msm/dp: Add static HDR metadata support for DP and eDP Xilin Wu
@ 2026-10-09 3:40 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-10-09 3:40 UTC (permalink / raw)
To: Xilin Wu; +Cc: neil.armstrong, linux-amlogic, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/msm/dp: unclocked access crash in msm_dp_display_disable() after failed PHY test
- [Medium] cleanup: mixed goto and scoped_guard in msm_dp_display_atomic_enable
--
commit 3c44a9bdc2d3792773d111df209f6cd19e02a7d6
Author: Xilin Wu <sophon@radxa.com>
Subject: drm/msm/dp: Add static HDR metadata support for DP and eDP
Expose HDR_OUTPUT_METADATA and validate and pack it during atomic check.
Program Generic2 from the checked bridge state on every stream enable.
Reuse the DRM metadata conversion and infoframe packing helpers.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index c51d033598a5b..7104822b03e05 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -880,10 +903,13 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
>
> lockdep_assert_held(&dp->plugged_lock);
>
> + dp->video_test_blocked = false;
> +
> if (!msm_dp_display->power_on)
> return 0;
>
> msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
> + msm_dp_panel_disable_hdr_sdp(msm_dp_panel);
[Severity: High]
Can this unconditionally access hardware registers when the DP link clocks
are disabled?
If a DP sink requests a PHY test pattern,
msm_dp_ctrl_process_phy_test_request() intentionally disables the link
clocks via msm_dp_ctrl_off_link().
If msm_dp_ctrl_on_link() subsequently fails, it returns early leaving the
link clocks disabled, but the display power_on flag remains true.
When msm_dp_display_disable() is then called (e.g. on monitor unplug), it
proceeds because power_on is true, and calls msm_dp_panel_disable_hdr_sdp()
and msm_dp_panel_disable_vsc_sdp(). These functions write to hardware link
registers (like MMSS_DP_SDP_CFG).
Accessing these registers while the link clocks are off can cause a
synchronous external abort (kernel panic) which is reachable via a faulty
or malicious DP sink.
[ ... ]
> @@ -1774,6 +1800,7 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
> msm_dp_hpd_unplug_handle(dp);
> msm_dp_display_unprepare(dp);
> scoped_guard(mutex, &dp->plugged_lock) {
> + dp->video_test_blocked = false;
> dp->link_failed = true;
> /* Keep reporting BAD, but notify only once until recovery. */
> schedule_work(&dp->link_status_work);
[Severity: Medium]
Does this mix goto-based error handling and scope-based cleanup in the same
function?
The msm_dp_display_atomic_enable() function uses traditional goto-based error
handling (goto err; goto err_prepare;) but also includes a scoped_guard()
block at the end.
According to kernel cleanup guidelines (include/linux/cleanup.h), mixing
goto and scope-based cleanup within the same function is explicitly
prohibited because it creates confusing ownership semantics and increases
the risk of double-free or locking bugs during maintenance.
Could this be converted to use scope-based cleanup entirely, or use
traditional locking for this block to match the rest of the function?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=20
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 32+ messages in thread