drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++--------------- drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +- 2 files changed, 110 insertions(+), 178 deletions(-)
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Convert it to drm bridge driver, it will be convenient for us to
migrate the connector part to the display driver later.
Considering that some code depend on the connector, the following
changes have been made:
- Only process edid in &drm_bridge_funcs.edid_read(), so no need to
store additional edid info.
- Now cdn_dp_get_sink_capability() only focused on reading DPCD_REV.
- Update bpc info in cdn_dp_bridge_atomic_enable() instead of
cdn_dp_encoder_mode_set(). Actually, the bpc data will be used in
cdn_dp_bridge_atomic_enable().
- Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers.
This patch also convert to use devm_drm_bridge_alloc() API.
Tested with RK3399 EVB IND board.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
Changes in v3:
- Link to V2: https://lore.kernel.org/all/20250523011310.120-1-kernel@airkyi.com/
- Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers
- Remove the dependency for connector
- Remove the extra stored edid
- Code cleanup
Changes in v2:
- Link to V1: https://lore.kernel.org/all/20250507035148.415-1-kernel@airkyi.com/
- Use drm_atomic_get_new_connector_for_encoder() to get connector
- Convert to use devm_drm_bridge_alloc() API
- Fix typo: cdn_dp_connector_edid_read -> cdn_dp_bridge_edid_read
drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++---------------
drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +-
2 files changed, 110 insertions(+), 178 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 292c31de18f1..5e116d3e516b 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -16,6 +16,7 @@
#include <sound/hdmi-codec.h>
#include <drm/display/drm_dp_helper.h>
+#include <drm/display/drm_hdmi_audio_helper.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_edid.h>
#include <drm/drm_of.h>
@@ -25,9 +26,9 @@
#include "cdn-dp-core.h"
#include "cdn-dp-reg.h"
-static inline struct cdn_dp_device *connector_to_dp(struct drm_connector *connector)
+static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
{
- return container_of(connector, struct cdn_dp_device, connector);
+ return container_of(bridge, struct cdn_dp_device, bridge);
}
static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder)
@@ -231,9 +232,9 @@ static bool cdn_dp_check_sink_connection(struct cdn_dp_device *dp)
}
static enum drm_connector_status
-cdn_dp_connector_detect(struct drm_connector *connector, bool force)
+cdn_dp_bridge_detect(struct drm_bridge *bridge)
{
- struct cdn_dp_device *dp = connector_to_dp(connector);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
enum drm_connector_status status = connector_status_disconnected;
mutex_lock(&dp->lock);
@@ -244,41 +245,25 @@ cdn_dp_connector_detect(struct drm_connector *connector, bool force)
return status;
}
-static void cdn_dp_connector_destroy(struct drm_connector *connector)
+static const struct drm_edid *
+cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector)
{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_funcs cdn_dp_atomic_connector_funcs = {
- .detect = cdn_dp_connector_detect,
- .destroy = cdn_dp_connector_destroy,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static int cdn_dp_connector_get_modes(struct drm_connector *connector)
-{
- struct cdn_dp_device *dp = connector_to_dp(connector);
- int ret = 0;
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
+ const struct drm_edid *drm_edid;
mutex_lock(&dp->lock);
-
- ret = drm_edid_connector_add_modes(connector);
-
+ drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
mutex_unlock(&dp->lock);
- return ret;
+ return drm_edid;
}
static enum drm_mode_status
-cdn_dp_connector_mode_valid(struct drm_connector *connector,
- const struct drm_display_mode *mode)
+cdn_dp_bridge_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_info *display_info,
+ const struct drm_display_mode *mode)
{
- struct cdn_dp_device *dp = connector_to_dp(connector);
- struct drm_display_info *display_info = &dp->connector.display_info;
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
u32 requested, actual, rate, sink_max, source_max = 0;
u8 lanes, bpc;
@@ -323,11 +308,6 @@ cdn_dp_connector_mode_valid(struct drm_connector *connector,
return MODE_OK;
}
-static struct drm_connector_helper_funcs cdn_dp_connector_helper_funcs = {
- .get_modes = cdn_dp_connector_get_modes,
- .mode_valid = cdn_dp_connector_mode_valid,
-};
-
static int cdn_dp_firmware_init(struct cdn_dp_device *dp)
{
int ret;
@@ -360,7 +340,6 @@ static int cdn_dp_firmware_init(struct cdn_dp_device *dp)
static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
{
- const struct drm_display_info *info = &dp->connector.display_info;
int ret;
if (!cdn_dp_check_sink_connection(dp))
@@ -373,17 +352,6 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
return ret;
}
- drm_edid_free(dp->drm_edid);
- dp->drm_edid = drm_edid_read_custom(&dp->connector,
- cdn_dp_get_edid_block, dp);
- drm_edid_connector_update(&dp->connector, dp->drm_edid);
-
- dp->sink_has_audio = info->has_audio;
-
- if (dp->drm_edid)
- DRM_DEV_DEBUG_KMS(dp->dev, "got edid: width[%d] x height[%d]\n",
- info->width_mm / 10, info->height_mm / 10);
-
return 0;
}
@@ -488,10 +456,6 @@ static int cdn_dp_disable(struct cdn_dp_device *dp)
dp->active = false;
dp->max_lanes = 0;
dp->max_rate = 0;
- if (!dp->connected) {
- drm_edid_free(dp->drm_edid);
- dp->drm_edid = NULL;
- }
return 0;
}
@@ -551,21 +515,8 @@ static void cdn_dp_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *adjusted)
{
struct cdn_dp_device *dp = encoder_to_dp(encoder);
- struct drm_display_info *display_info = &dp->connector.display_info;
struct video_info *video = &dp->video_info;
- switch (display_info->bpc) {
- case 10:
- video->color_depth = 10;
- break;
- case 6:
- video->color_depth = 6;
- break;
- default:
- video->color_depth = 8;
- break;
- }
-
video->color_fmt = PXL_RGB;
video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
@@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
bool plugged)
{
- if (dp->codec_dev)
- dp->plugged_cb(dp->codec_dev, plugged);
+ if (dp->sink_has_audio)
+ drm_connector_hdmi_audio_plugged_notify(dp->connector, plugged);
}
-static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
+static void cdn_dp_display_info_update(struct cdn_dp_device *dp, struct drm_display_info *display_info)
{
- struct cdn_dp_device *dp = encoder_to_dp(encoder);
+ struct video_info *video = &dp->video_info;
+
+ switch (display_info->bpc) {
+ case 10:
+ video->color_depth = 10;
+ break;
+ case 6:
+ video->color_depth = 6;
+ break;
+ default:
+ video->color_depth = 8;
+ break;
+ }
+
+ dp->sink_has_audio = display_info->has_audio;
+}
+
+static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
int ret, val;
- ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+ dp->connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+ if (!dp->connector)
+ return;
+
+ cdn_dp_display_info_update(dp, &dp->connector->display_info);
+
+ ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp->encoder.encoder);
if (ret < 0) {
DRM_DEV_ERROR(dp->dev, "Could not get vop id, %d", ret);
return;
@@ -625,7 +601,7 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
ret = cdn_dp_enable(dp);
if (ret) {
- DRM_DEV_ERROR(dp->dev, "Failed to enable encoder %d\n",
+ DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n",
ret);
goto out;
}
@@ -661,9 +637,9 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
mutex_unlock(&dp->lock);
}
-static void cdn_dp_encoder_disable(struct drm_encoder *encoder)
+static void cdn_dp_bridge_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
{
- struct cdn_dp_device *dp = encoder_to_dp(encoder);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
int ret;
mutex_lock(&dp->lock);
@@ -672,7 +648,7 @@ static void cdn_dp_encoder_disable(struct drm_encoder *encoder)
if (dp->active) {
ret = cdn_dp_disable(dp);
if (ret) {
- DRM_DEV_ERROR(dp->dev, "Failed to disable encoder %d\n",
+ DRM_DEV_ERROR(dp->dev, "Failed to disable bridge %d\n",
ret);
}
}
@@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct drm_encoder *encoder,
static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs = {
.mode_set = cdn_dp_encoder_mode_set,
- .enable = cdn_dp_encoder_enable,
- .disable = cdn_dp_encoder_disable,
.atomic_check = cdn_dp_encoder_atomic_check,
};
@@ -779,11 +753,12 @@ static int cdn_dp_parse_dt(struct cdn_dp_device *dp)
return 0;
}
-static int cdn_dp_audio_hw_params(struct device *dev, void *data,
- struct hdmi_codec_daifmt *daifmt,
- struct hdmi_codec_params *params)
+static int cdn_dp_audio_prepare(struct drm_connector *connector,
+ struct drm_bridge *bridge,
+ struct hdmi_codec_daifmt *daifmt,
+ struct hdmi_codec_params *params)
{
- struct cdn_dp_device *dp = dev_get_drvdata(dev);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
struct audio_info audio = {
.sample_width = params->sample_width,
.sample_rate = params->sample_rate,
@@ -805,7 +780,7 @@ static int cdn_dp_audio_hw_params(struct device *dev, void *data,
audio.format = AFMT_SPDIF;
break;
default:
- DRM_DEV_ERROR(dev, "Invalid format %d\n", daifmt->fmt);
+ DRM_DEV_ERROR(bridge->dev, "Invalid format %d\n", daifmt->fmt);
ret = -EINVAL;
goto out;
}
@@ -819,9 +794,10 @@ static int cdn_dp_audio_hw_params(struct device *dev, void *data,
return ret;
}
-static void cdn_dp_audio_shutdown(struct device *dev, void *data)
+static void cdn_dp_audio_shutdown(struct drm_connector *connector,
+ struct drm_bridge *bridge)
{
- struct cdn_dp_device *dp = dev_get_drvdata(dev);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
int ret;
mutex_lock(&dp->lock);
@@ -835,10 +811,11 @@ static void cdn_dp_audio_shutdown(struct device *dev, void *data)
mutex_unlock(&dp->lock);
}
-static int cdn_dp_audio_mute_stream(struct device *dev, void *data,
+static int cdn_dp_audio_mute_stream(struct drm_connector *connector,
+ struct drm_bridge *bridge,
bool enable, int direction)
{
- struct cdn_dp_device *dp = dev_get_drvdata(dev);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
int ret;
mutex_lock(&dp->lock);
@@ -854,57 +831,21 @@ static int cdn_dp_audio_mute_stream(struct device *dev, void *data,
return ret;
}
-static int cdn_dp_audio_get_eld(struct device *dev, void *data,
- u8 *buf, size_t len)
-{
- struct cdn_dp_device *dp = dev_get_drvdata(dev);
-
- memcpy(buf, dp->connector.eld, min(sizeof(dp->connector.eld), len));
-
- return 0;
-}
-
-static int cdn_dp_audio_hook_plugged_cb(struct device *dev, void *data,
- hdmi_codec_plugged_cb fn,
- struct device *codec_dev)
-{
- struct cdn_dp_device *dp = dev_get_drvdata(dev);
-
- mutex_lock(&dp->lock);
- dp->plugged_cb = fn;
- dp->codec_dev = codec_dev;
- cdn_dp_audio_handle_plugged_change(dp, dp->connected);
- mutex_unlock(&dp->lock);
-
- return 0;
-}
-
-static const struct hdmi_codec_ops audio_codec_ops = {
- .hw_params = cdn_dp_audio_hw_params,
- .audio_shutdown = cdn_dp_audio_shutdown,
- .mute_stream = cdn_dp_audio_mute_stream,
- .get_eld = cdn_dp_audio_get_eld,
- .hook_plugged_cb = cdn_dp_audio_hook_plugged_cb,
+static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .detect = cdn_dp_bridge_detect,
+ .edid_read = cdn_dp_bridge_edid_read,
+ .atomic_enable = cdn_dp_bridge_atomic_enable,
+ .atomic_disable = cdn_dp_bridge_atomic_disable,
+ .mode_valid = cdn_dp_bridge_mode_valid,
+
+ .dp_audio_prepare = cdn_dp_audio_prepare,
+ .dp_audio_mute_stream = cdn_dp_audio_mute_stream,
+ .dp_audio_shutdown = cdn_dp_audio_shutdown,
};
-static int cdn_dp_audio_codec_init(struct cdn_dp_device *dp,
- struct device *dev)
-{
- struct hdmi_codec_pdata codec_data = {
- .i2s = 1,
- .spdif = 1,
- .ops = &audio_codec_ops,
- .max_i2s_channels = 8,
- .no_capture_mute = 1,
- };
-
- dp->audio_pdev = platform_device_register_data(
- dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
- &codec_data, sizeof(codec_data));
-
- return PTR_ERR_OR_ZERO(dp->audio_pdev);
-}
-
static int cdn_dp_request_firmware(struct cdn_dp_device *dp)
{
int ret;
@@ -1006,7 +947,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
out:
mutex_unlock(&dp->lock);
- drm_connector_helper_hpd_irq_event(&dp->connector);
+ if (dp->connector)
+ drm_connector_helper_hpd_irq_event(dp->connector);
}
static int cdn_dp_pd_event(struct notifier_block *nb,
@@ -1062,26 +1004,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs);
- connector = &dp->connector;
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- connector->dpms = DRM_MODE_DPMS_OFF;
-
- ret = drm_connector_init(drm_dev, connector,
- &cdn_dp_atomic_connector_funcs,
- DRM_MODE_CONNECTOR_DisplayPort);
- if (ret) {
- DRM_ERROR("failed to initialize connector with drm\n");
- goto err_free_encoder;
- }
+ dp->bridge.ops =
+ DRM_BRIDGE_OP_DETECT |
+ DRM_BRIDGE_OP_EDID |
+ DRM_BRIDGE_OP_HPD |
+ DRM_BRIDGE_OP_DP_AUDIO;
+ dp->bridge.of_node = dp->dev->of_node;
+ dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
+ dp->bridge.hdmi_audio_dev = dp->dev;
+ dp->bridge.hdmi_audio_max_i2s_playback_channels = 8;
+ dp->bridge.hdmi_audio_spdif_playback = 1;
+ dp->bridge.hdmi_audio_dai_port = -1;
+
+ ret = devm_drm_bridge_add(dev, &dp->bridge);
+ if (ret)
+ return ret;
- drm_connector_helper_add(connector, &cdn_dp_connector_helper_funcs);
+ ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (ret)
+ return ret;
- ret = drm_connector_attach_encoder(connector, encoder);
- if (ret) {
- DRM_ERROR("failed to attach connector and encoder\n");
- goto err_free_connector;
+ connector = drm_bridge_connector_init(drm_dev, encoder);
+ if (IS_ERR(connector)) {
+ ret = PTR_ERR(connector);
+ dev_err(dp->dev, "failed to init bridge connector: %d\n", ret);
+ return ret;
}
+ drm_connector_attach_encoder(connector, encoder);
+
for (i = 0; i < dp->ports; i++) {
port = dp->port[i];
@@ -1101,30 +1052,19 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
schedule_work(&dp->event_work);
return 0;
-
-err_free_connector:
- drm_connector_cleanup(connector);
-err_free_encoder:
- drm_encoder_cleanup(encoder);
- return ret;
}
static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
{
struct cdn_dp_device *dp = dev_get_drvdata(dev);
struct drm_encoder *encoder = &dp->encoder.encoder;
- struct drm_connector *connector = &dp->connector;
cancel_work_sync(&dp->event_work);
- cdn_dp_encoder_disable(encoder);
encoder->funcs->destroy(encoder);
- connector->funcs->destroy(connector);
pm_runtime_disable(dev);
if (dp->fw_loaded)
release_firmware(dp->fw);
- drm_edid_free(dp->drm_edid);
- dp->drm_edid = NULL;
}
static const struct component_ops cdn_dp_component_ops = {
@@ -1171,9 +1111,10 @@ static int cdn_dp_probe(struct platform_device *pdev)
int ret;
int i;
- dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
- if (!dp)
- return -ENOMEM;
+ dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
+ &cdn_dp_bridge_funcs);
+ if (IS_ERR(dp))
+ return PTR_ERR(dp);
dp->dev = dev;
match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node);
@@ -1209,19 +1150,11 @@ static int cdn_dp_probe(struct platform_device *pdev)
mutex_init(&dp->lock);
dev_set_drvdata(dev, dp);
- ret = cdn_dp_audio_codec_init(dp, dev);
- if (ret)
- return ret;
-
ret = component_add(dev, &cdn_dp_component_ops);
if (ret)
- goto err_audio_deinit;
+ return ret;
return 0;
-
-err_audio_deinit:
- platform_device_unregister(dp->audio_pdev);
- return ret;
}
static void cdn_dp_remove(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
index 17498f576ce7..6aa3ce489382 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
@@ -8,6 +8,8 @@
#define _CDN_DP_CORE_H
#include <drm/display/drm_dp_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
#include <drm/drm_panel.h>
#include <drm/drm_probe_helper.h>
#include <sound/hdmi-codec.h>
@@ -65,12 +67,12 @@ struct cdn_dp_port {
struct cdn_dp_device {
struct device *dev;
struct drm_device *drm_dev;
- struct drm_connector connector;
+ struct drm_bridge bridge;
+ struct drm_connector *connector;
struct rockchip_encoder encoder;
struct drm_display_mode mode;
struct platform_device *audio_pdev;
struct work_struct event_work;
- const struct drm_edid *drm_edid;
struct mutex lock;
bool connected;
@@ -102,8 +104,5 @@ struct cdn_dp_device {
u8 dpcd[DP_RECEIVER_CAP_SIZE];
bool sink_has_audio;
-
- hdmi_codec_plugged_cb plugged_cb;
- struct device *codec_dev;
};
#endif /* _CDN_DP_CORE_H */
--
2.49.0On Tue, May 27, 2025 at 04:14:47PM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>
> Convert it to drm bridge driver, it will be convenient for us to
> migrate the connector part to the display driver later.
> Considering that some code depend on the connector, the following
> changes have been made:
> - Only process edid in &drm_bridge_funcs.edid_read(), so no need to
> store additional edid info.
> - Now cdn_dp_get_sink_capability() only focused on reading DPCD_REV.
> - Update bpc info in cdn_dp_bridge_atomic_enable() instead of
> cdn_dp_encoder_mode_set(). Actually, the bpc data will be used in
> cdn_dp_bridge_atomic_enable().
> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers.
>
> This patch also convert to use devm_drm_bridge_alloc() API.
>
> Tested with RK3399 EVB IND board.
>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
>
> Changes in v3:
> - Link to V2: https://lore.kernel.org/all/20250523011310.120-1-kernel@airkyi.com/
> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers
> - Remove the dependency for connector
> - Remove the extra stored edid
> - Code cleanup
>
> Changes in v2:
> - Link to V1: https://lore.kernel.org/all/20250507035148.415-1-kernel@airkyi.com/
> - Use drm_atomic_get_new_connector_for_encoder() to get connector
> - Convert to use devm_drm_bridge_alloc() API
> - Fix typo: cdn_dp_connector_edid_read -> cdn_dp_bridge_edid_read
>
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++---------------
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +-
> 2 files changed, 110 insertions(+), 178 deletions(-)
>
> @@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
> bool plugged)
> {
> - if (dp->codec_dev)
> - dp->plugged_cb(dp->codec_dev, plugged);
> + if (dp->sink_has_audio)
> + drm_connector_hdmi_audio_plugged_notify(dp->connector, plugged);
I'd say, notify always and let userspace figure it out via the ELD. Then
you shouldn't need sink_has_audio. This would match the behaviour of
HDMI drivers.
> }
>
[...]
> @@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct drm_encoder *encoder,
>
> static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs = {
> .mode_set = cdn_dp_encoder_mode_set,
> - .enable = cdn_dp_encoder_enable,
> - .disable = cdn_dp_encoder_disable,
> .atomic_check = cdn_dp_encoder_atomic_check,
Nit: for the future cleanup, it should probably be possible to get rid
of these encoder ops too by moving them to the bridge ops.
> };
>
[...]
> @@ -1006,7 +947,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
>
> out:
> mutex_unlock(&dp->lock);
> - drm_connector_helper_hpd_irq_event(&dp->connector);
> + if (dp->connector)
> + drm_connector_helper_hpd_irq_event(dp->connector);
drm_bridge_hpd_notify(). I think then you don't need dp->connector.
> }
>
> static int cdn_dp_pd_event(struct notifier_block *nb,
> @@ -1062,26 +1004,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>
> drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs);
>
> - connector = &dp->connector;
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> - connector->dpms = DRM_MODE_DPMS_OFF;
> -
> - ret = drm_connector_init(drm_dev, connector,
> - &cdn_dp_atomic_connector_funcs,
> - DRM_MODE_CONNECTOR_DisplayPort);
> - if (ret) {
> - DRM_ERROR("failed to initialize connector with drm\n");
> - goto err_free_encoder;
> - }
> + dp->bridge.ops =
> + DRM_BRIDGE_OP_DETECT |
> + DRM_BRIDGE_OP_EDID |
> + DRM_BRIDGE_OP_HPD |
> + DRM_BRIDGE_OP_DP_AUDIO;
> + dp->bridge.of_node = dp->dev->of_node;
> + dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> + dp->bridge.hdmi_audio_dev = dp->dev;
> + dp->bridge.hdmi_audio_max_i2s_playback_channels = 8;
> + dp->bridge.hdmi_audio_spdif_playback = 1;
> + dp->bridge.hdmi_audio_dai_port = -1;
> +
> + ret = devm_drm_bridge_add(dev, &dp->bridge);
> + if (ret)
> + return ret;
>
> - drm_connector_helper_add(connector, &cdn_dp_connector_helper_funcs);
> + ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret)
> + return ret;
>
> - ret = drm_connector_attach_encoder(connector, encoder);
> - if (ret) {
> - DRM_ERROR("failed to attach connector and encoder\n");
> - goto err_free_connector;
> + connector = drm_bridge_connector_init(drm_dev, encoder);
> + if (IS_ERR(connector)) {
> + ret = PTR_ERR(connector);
> + dev_err(dp->dev, "failed to init bridge connector: %d\n", ret);
> + return ret;
> }
>
> + drm_connector_attach_encoder(connector, encoder);
> +
> for (i = 0; i < dp->ports; i++) {
> port = dp->port[i];
>
> @@ -1101,30 +1052,19 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> schedule_work(&dp->event_work);
>
> return 0;
> -
> -err_free_connector:
> - drm_connector_cleanup(connector);
> -err_free_encoder:
> - drm_encoder_cleanup(encoder);
> - return ret;
> }
>
> static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> struct drm_encoder *encoder = &dp->encoder.encoder;
> - struct drm_connector *connector = &dp->connector;
>
> cancel_work_sync(&dp->event_work);
> - cdn_dp_encoder_disable(encoder);
> encoder->funcs->destroy(encoder);
> - connector->funcs->destroy(connector);
>
> pm_runtime_disable(dev);
> if (dp->fw_loaded)
> release_firmware(dp->fw);
> - drm_edid_free(dp->drm_edid);
> - dp->drm_edid = NULL;
> }
>
> static const struct component_ops cdn_dp_component_ops = {
> @@ -1171,9 +1111,10 @@ static int cdn_dp_probe(struct platform_device *pdev)
> int ret;
> int i;
>
> - dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
> - if (!dp)
> - return -ENOMEM;
> + dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
> + &cdn_dp_bridge_funcs);
> + if (IS_ERR(dp))
> + return PTR_ERR(dp);
> dp->dev = dev;
>
> match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node);
> @@ -1209,19 +1150,11 @@ static int cdn_dp_probe(struct platform_device *pdev)
> mutex_init(&dp->lock);
> dev_set_drvdata(dev, dp);
>
> - ret = cdn_dp_audio_codec_init(dp, dev);
> - if (ret)
> - return ret;
> -
> ret = component_add(dev, &cdn_dp_component_ops);
> if (ret)
> - goto err_audio_deinit;
> + return ret;
>
> return 0;
> -
> -err_audio_deinit:
> - platform_device_unregister(dp->audio_pdev);
> - return ret;
> }
>
> static void cdn_dp_remove(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 17498f576ce7..6aa3ce489382 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -8,6 +8,8 @@
> #define _CDN_DP_CORE_H
>
> #include <drm/display/drm_dp_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
This include can go to the C file instead of the header.
> #include <drm/drm_panel.h>
> #include <drm/drm_probe_helper.h>
> #include <sound/hdmi-codec.h>
> @@ -65,12 +67,12 @@ struct cdn_dp_port {
> struct cdn_dp_device {
> struct device *dev;
> struct drm_device *drm_dev;
> - struct drm_connector connector;
> + struct drm_bridge bridge;
> + struct drm_connector *connector;
> struct rockchip_encoder encoder;
> struct drm_display_mode mode;
> struct platform_device *audio_pdev;
> struct work_struct event_work;
> - const struct drm_edid *drm_edid;
>
> struct mutex lock;
> bool connected;
> @@ -102,8 +104,5 @@ struct cdn_dp_device {
>
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> bool sink_has_audio;
> -
> - hdmi_codec_plugged_cb plugged_cb;
> - struct device *codec_dev;
> };
> #endif /* _CDN_DP_CORE_H */
> --
> 2.49.0
>
--
With best wishes
Dmitry
Hi Dmitry,
On 2025/5/28 4:25, Dmitry Baryshkov wrote:
> On Tue, May 27, 2025 at 04:14:47PM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> Convert it to drm bridge driver, it will be convenient for us to
>> migrate the connector part to the display driver later.
>> Considering that some code depend on the connector, the following
>> changes have been made:
>> - Only process edid in &drm_bridge_funcs.edid_read(), so no need to
>> store additional edid info.
>> - Now cdn_dp_get_sink_capability() only focused on reading DPCD_REV.
>> - Update bpc info in cdn_dp_bridge_atomic_enable() instead of
>> cdn_dp_encoder_mode_set(). Actually, the bpc data will be used in
>> cdn_dp_bridge_atomic_enable().
>> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers.
>>
>> This patch also convert to use devm_drm_bridge_alloc() API.
>>
>> Tested with RK3399 EVB IND board.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>> ---
>>
>> Changes in v3:
>> - Link to V2: https://lore.kernel.org/all/20250523011310.120-1-kernel@airkyi.com/
>> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers
>> - Remove the dependency for connector
>> - Remove the extra stored edid
>> - Code cleanup
>>
>> Changes in v2:
>> - Link to V1: https://lore.kernel.org/all/20250507035148.415-1-kernel@airkyi.com/
>> - Use drm_atomic_get_new_connector_for_encoder() to get connector
>> - Convert to use devm_drm_bridge_alloc() API
>> - Fix typo: cdn_dp_connector_edid_read -> cdn_dp_bridge_edid_read
>>
>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++---------------
>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +-
>> 2 files changed, 110 insertions(+), 178 deletions(-)
>>
>
>> @@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>> static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
>> bool plugged)
>> {
>> - if (dp->codec_dev)
>> - dp->plugged_cb(dp->codec_dev, plugged);
>> + if (dp->sink_has_audio)
>> + drm_connector_hdmi_audio_plugged_notify(dp->connector, plugged);
> I'd say, notify always and let userspace figure it out via the ELD. Then
> you shouldn't need sink_has_audio. This would match the behaviour of
> HDMI drivers.
Oh, I find that there are similar usages in qcom msm driver. Is there
any more progress?
>
>> }
>>
> [...]
>
>> @@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct drm_encoder *encoder,
>>
>> static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs = {
>> .mode_set = cdn_dp_encoder_mode_set,
>> - .enable = cdn_dp_encoder_enable,
>> - .disable = cdn_dp_encoder_disable,
>> .atomic_check = cdn_dp_encoder_atomic_check,
> Nit: for the future cleanup, it should probably be possible to get rid
> of these encoder ops too by moving them to the bridge ops.
Interesting, have these patches been submitted upstream yet?
>
>> };
>>
> [...]
>
>> @@ -1006,7 +947,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
>>
>> out:
>> mutex_unlock(&dp->lock);
>> - drm_connector_helper_hpd_irq_event(&dp->connector);
>> + if (dp->connector)
>> + drm_connector_helper_hpd_irq_event(dp->connector);
> drm_bridge_hpd_notify(). I think then you don't need dp->connector.
That make sense! Will fix in v4.
>
>> }
>>
>> static int cdn_dp_pd_event(struct notifier_block *nb,
>> @@ -1062,26 +1004,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>
>> drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs);
>>
>> - connector = &dp->connector;
>> - connector->polled = DRM_CONNECTOR_POLL_HPD;
>> - connector->dpms = DRM_MODE_DPMS_OFF;
>> -
>> - ret = drm_connector_init(drm_dev, connector,
>> - &cdn_dp_atomic_connector_funcs,
>> - DRM_MODE_CONNECTOR_DisplayPort);
>> - if (ret) {
>> - DRM_ERROR("failed to initialize connector with drm\n");
>> - goto err_free_encoder;
>> - }
>> + dp->bridge.ops =
>> + DRM_BRIDGE_OP_DETECT |
>> + DRM_BRIDGE_OP_EDID |
>> + DRM_BRIDGE_OP_HPD |
>> + DRM_BRIDGE_OP_DP_AUDIO;
>> + dp->bridge.of_node = dp->dev->of_node;
>> + dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
>> + dp->bridge.hdmi_audio_dev = dp->dev;
>> + dp->bridge.hdmi_audio_max_i2s_playback_channels = 8;
>> + dp->bridge.hdmi_audio_spdif_playback = 1;
>> + dp->bridge.hdmi_audio_dai_port = -1;
>> +
>> + ret = devm_drm_bridge_add(dev, &dp->bridge);
>> + if (ret)
>> + return ret;
>>
>> - drm_connector_helper_add(connector, &cdn_dp_connector_helper_funcs);
>> + ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>> + if (ret)
>> + return ret;
>>
>> - ret = drm_connector_attach_encoder(connector, encoder);
>> - if (ret) {
>> - DRM_ERROR("failed to attach connector and encoder\n");
>> - goto err_free_connector;
>> + connector = drm_bridge_connector_init(drm_dev, encoder);
>> + if (IS_ERR(connector)) {
>> + ret = PTR_ERR(connector);
>> + dev_err(dp->dev, "failed to init bridge connector: %d\n", ret);
>> + return ret;
>> }
>>
>> + drm_connector_attach_encoder(connector, encoder);
>> +
>> for (i = 0; i < dp->ports; i++) {
>> port = dp->port[i];
>>
>> @@ -1101,30 +1052,19 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> schedule_work(&dp->event_work);
>>
>> return 0;
>> -
>> -err_free_connector:
>> - drm_connector_cleanup(connector);
>> -err_free_encoder:
>> - drm_encoder_cleanup(encoder);
>> - return ret;
>> }
>>
>> static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
>> {
>> struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> struct drm_encoder *encoder = &dp->encoder.encoder;
>> - struct drm_connector *connector = &dp->connector;
>>
>> cancel_work_sync(&dp->event_work);
>> - cdn_dp_encoder_disable(encoder);
>> encoder->funcs->destroy(encoder);
>> - connector->funcs->destroy(connector);
>>
>> pm_runtime_disable(dev);
>> if (dp->fw_loaded)
>> release_firmware(dp->fw);
>> - drm_edid_free(dp->drm_edid);
>> - dp->drm_edid = NULL;
>> }
>>
>> static const struct component_ops cdn_dp_component_ops = {
>> @@ -1171,9 +1111,10 @@ static int cdn_dp_probe(struct platform_device *pdev)
>> int ret;
>> int i;
>>
>> - dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
>> - if (!dp)
>> - return -ENOMEM;
>> + dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
>> + &cdn_dp_bridge_funcs);
>> + if (IS_ERR(dp))
>> + return PTR_ERR(dp);
>> dp->dev = dev;
>>
>> match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node);
>> @@ -1209,19 +1150,11 @@ static int cdn_dp_probe(struct platform_device *pdev)
>> mutex_init(&dp->lock);
>> dev_set_drvdata(dev, dp);
>>
>> - ret = cdn_dp_audio_codec_init(dp, dev);
>> - if (ret)
>> - return ret;
>> -
>> ret = component_add(dev, &cdn_dp_component_ops);
>> if (ret)
>> - goto err_audio_deinit;
>> + return ret;
>>
>> return 0;
>> -
>> -err_audio_deinit:
>> - platform_device_unregister(dp->audio_pdev);
>> - return ret;
>> }
>>
>> static void cdn_dp_remove(struct platform_device *pdev)
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> index 17498f576ce7..6aa3ce489382 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> @@ -8,6 +8,8 @@
>> #define _CDN_DP_CORE_H
>>
>> #include <drm/display/drm_dp_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
> This include can go to the C file instead of the header.
Okay, will fix in v4.
>
>> #include <drm/drm_panel.h>
>> #include <drm/drm_probe_helper.h>
>> #include <sound/hdmi-codec.h>
>> @@ -65,12 +67,12 @@ struct cdn_dp_port {
>> struct cdn_dp_device {
>> struct device *dev;
>> struct drm_device *drm_dev;
>> - struct drm_connector connector;
>> + struct drm_bridge bridge;
>> + struct drm_connector *connector;
>> struct rockchip_encoder encoder;
>> struct drm_display_mode mode;
>> struct platform_device *audio_pdev;
>> struct work_struct event_work;
>> - const struct drm_edid *drm_edid;
>>
>> struct mutex lock;
>> bool connected;
>> @@ -102,8 +104,5 @@ struct cdn_dp_device {
>>
>> u8 dpcd[DP_RECEIVER_CAP_SIZE];
>> bool sink_has_audio;
>> -
>> - hdmi_codec_plugged_cb plugged_cb;
>> - struct device *codec_dev;
>> };
>> #endif /* _CDN_DP_CORE_H */
>> --
>> 2.49.0
>>
On Wed, 28 May 2025 at 04:57, Chaoyi Chen <chaoyi.chen@rock-chips.com> wrote:
>
> Hi Dmitry,
>
> On 2025/5/28 4:25, Dmitry Baryshkov wrote:
> > On Tue, May 27, 2025 at 04:14:47PM +0800, Chaoyi Chen wrote:
> >> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >>
> >> Convert it to drm bridge driver, it will be convenient for us to
> >> migrate the connector part to the display driver later.
> >> Considering that some code depend on the connector, the following
> >> changes have been made:
> >> - Only process edid in &drm_bridge_funcs.edid_read(), so no need to
> >> store additional edid info.
> >> - Now cdn_dp_get_sink_capability() only focused on reading DPCD_REV.
> >> - Update bpc info in cdn_dp_bridge_atomic_enable() instead of
> >> cdn_dp_encoder_mode_set(). Actually, the bpc data will be used in
> >> cdn_dp_bridge_atomic_enable().
> >> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers.
> >>
> >> This patch also convert to use devm_drm_bridge_alloc() API.
> >>
> >> Tested with RK3399 EVB IND board.
> >>
> >> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >> ---
> >>
> >> Changes in v3:
> >> - Link to V2: https://lore.kernel.org/all/20250523011310.120-1-kernel@airkyi.com/
> >> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers
> >> - Remove the dependency for connector
> >> - Remove the extra stored edid
> >> - Code cleanup
> >>
> >> Changes in v2:
> >> - Link to V1: https://lore.kernel.org/all/20250507035148.415-1-kernel@airkyi.com/
> >> - Use drm_atomic_get_new_connector_for_encoder() to get connector
> >> - Convert to use devm_drm_bridge_alloc() API
> >> - Fix typo: cdn_dp_connector_edid_read -> cdn_dp_bridge_edid_read
> >>
> >> drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++---------------
> >> drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +-
> >> 2 files changed, 110 insertions(+), 178 deletions(-)
> >>
> >
> >> @@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> >> static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
> >> bool plugged)
> >> {
> >> - if (dp->codec_dev)
> >> - dp->plugged_cb(dp->codec_dev, plugged);
> >> + if (dp->sink_has_audio)
> >> + drm_connector_hdmi_audio_plugged_notify(dp->connector, plugged);
> > I'd say, notify always and let userspace figure it out via the ELD. Then
> > you shouldn't need sink_has_audio. This would match the behaviour of
> > HDMI drivers.
>
> Oh, I find that there are similar usages in qcom msm driver. Is there
> any more progress?
For msm driver it is required as DSP requires HDMI to be plugged for
the audio path to work.
>
>
> >
> >> }
> >>
> > [...]
> >
> >> @@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct drm_encoder *encoder,
> >>
> >> static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs = {
> >> .mode_set = cdn_dp_encoder_mode_set,
> >> - .enable = cdn_dp_encoder_enable,
> >> - .disable = cdn_dp_encoder_disable,
> >> .atomic_check = cdn_dp_encoder_atomic_check,
> > Nit: for the future cleanup, it should probably be possible to get rid
> > of these encoder ops too by moving them to the bridge ops.
>
> Interesting, have these patches been submitted upstream yet?
Everything is already there, see drm_bridge_funcs::mode_set() and
drm_bridge_funcs::atomic_check().
--
With best wishes
Dmitry
Hi Dmitry,
On 2025/5/29 0:09, Dmitry Baryshkov wrote:
>>>> @@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>>>> static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
>>>> bool plugged)
>>>> {
>>>> - if (dp->codec_dev)
>>>> - dp->plugged_cb(dp->codec_dev, plugged);
>>>> + if (dp->sink_has_audio)
>>>> + drm_connector_hdmi_audio_plugged_notify(dp->connector, plugged);
>>> I'd say, notify always and let userspace figure it out via the ELD. Then
>>> you shouldn't need sink_has_audio. This would match the behaviour of
>>> HDMI drivers.
>> Oh, I find that there are similar usages in qcom msm driver. Is there
>> any more progress?
> For msm driver it is required as DSP requires HDMI to be plugged for
> the audio path to work.
I see, will fix in v4.
>>>> @@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct drm_encoder *encoder,
>>>>
>>>> static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs = {
>>>> .mode_set = cdn_dp_encoder_mode_set,
>>>> - .enable = cdn_dp_encoder_enable,
>>>> - .disable = cdn_dp_encoder_disable,
>>>> .atomic_check = cdn_dp_encoder_atomic_check,
>>> Nit: for the future cleanup, it should probably be possible to get rid
>>> of these encoder ops too by moving them to the bridge ops.
>> Interesting, have these patches been submitted upstream yet?
> Everything is already there, see drm_bridge_funcs::mode_set() and
> drm_bridge_funcs::atomic_check().
Thanks for the clarification. I will move mode_set() to bridge ops.
And for the drm_encoder_helper_funcs::atomic_check(), most Rockchip
drivers will set some Rockchip-specific properties here so that the VOP
driver can process them. In the future, we may integrate a new encoder
driver to process these private properties. So, I prefer to keep this as
it is.
Hi Chaoyi,
kernel test robot noticed the following build errors:
[auto build test ERROR on rockchip/for-next]
[also build test ERROR on linus/master v6.15 next-20250527]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Chaoyi-Chen/drm-rockchip-cdn-dp-Convert-to-drm-bridge/20250527-161941
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link: https://lore.kernel.org/r/20250527081447.304-1-kernel%40airkyi.com
patch subject: [PATCH v3] drm/rockchip: cdn-dp: Convert to drm bridge
config: i386-buildonly-randconfig-002-20250527 (https://download.01.org/0day-ci/archive/20250528/202505280026.8VDl0TWN-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250528/202505280026.8VDl0TWN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505280026.8VDl0TWN-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:783:17: error: incompatible pointer types passing 'struct drm_device *' to parameter of type 'const struct device *' [-Werror,-Wincompatible-pointer-types]
783 | DRM_DEV_ERROR(bridge->dev, "Invalid format %d\n", daifmt->fmt);
| ^~~~~~~~~~~
include/drm/drm_print.h:504:17: note: expanded from macro 'DRM_DEV_ERROR'
504 | drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~
include/drm/drm_print.h:488:42: note: passing argument to parameter 'dev' here
488 | void drm_dev_printk(const struct device *dev, const char *level,
| ^
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:844:3: error: field designator 'dp_audio_prepare' does not refer to any field in type 'const struct drm_bridge_funcs'; did you mean 'hdmi_audio_prepare'?
844 | .dp_audio_prepare = cdn_dp_audio_prepare,
| ^~~~~~~~~~~~~~~~
| hdmi_audio_prepare
include/drm/drm_bridge.h:702:8: note: 'hdmi_audio_prepare' declared here
702 | int (*hdmi_audio_prepare)(struct drm_connector *connector,
| ^
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:845:3: error: field designator 'dp_audio_mute_stream' does not refer to any field in type 'const struct drm_bridge_funcs'; did you mean 'hdmi_audio_mute_stream'?
845 | .dp_audio_mute_stream = cdn_dp_audio_mute_stream,
| ^~~~~~~~~~~~~~~~~~~~
| hdmi_audio_mute_stream
include/drm/drm_bridge.h:728:8: note: 'hdmi_audio_mute_stream' declared here
728 | int (*hdmi_audio_mute_stream)(struct drm_connector *connector,
| ^
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:846:3: error: field designator 'dp_audio_shutdown' does not refer to any field in type 'const struct drm_bridge_funcs'; did you mean 'hdmi_audio_shutdown'?
846 | .dp_audio_shutdown = cdn_dp_audio_shutdown,
| ^~~~~~~~~~~~~~~~~
| hdmi_audio_shutdown
include/drm/drm_bridge.h:716:9: note: 'hdmi_audio_shutdown' declared here
716 | void (*hdmi_audio_shutdown)(struct drm_connector *connector,
| ^
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:1011:4: error: use of undeclared identifier 'DRM_BRIDGE_OP_DP_AUDIO'
1011 | DRM_BRIDGE_OP_DP_AUDIO;
| ^
>> drivers/gpu/drm/rockchip/cdn-dp-core.c:1046:9: error: use of undeclared label 'err_free_connector'
1046 | goto err_free_connector;
| ^
drivers/gpu/drm/rockchip/cdn-dp-core.c:1114:7: error: call to undeclared function 'devm_drm_bridge_alloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1114 | dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
| ^
drivers/gpu/drm/rockchip/cdn-dp-core.c:1114:7: note: did you mean 'devm_drm_bridge_add'?
include/drm/drm_bridge.h:945:5: note: 'devm_drm_bridge_add' declared here
945 | int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
| ^
drivers/gpu/drm/rockchip/cdn-dp-core.c:1114:34: error: expected expression
1114 | dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
| ^
drivers/gpu/drm/rockchip/cdn-dp-core.c:1114:56: error: use of undeclared identifier 'bridge'
1114 | dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
| ^
9 errors generated.
vim +783 drivers/gpu/drm/rockchip/cdn-dp-core.c
755
756 static int cdn_dp_audio_prepare(struct drm_connector *connector,
757 struct drm_bridge *bridge,
758 struct hdmi_codec_daifmt *daifmt,
759 struct hdmi_codec_params *params)
760 {
761 struct cdn_dp_device *dp = bridge_to_dp(bridge);
762 struct audio_info audio = {
763 .sample_width = params->sample_width,
764 .sample_rate = params->sample_rate,
765 .channels = params->channels,
766 };
767 int ret;
768
769 mutex_lock(&dp->lock);
770 if (!dp->active) {
771 ret = -ENODEV;
772 goto out;
773 }
774
775 switch (daifmt->fmt) {
776 case HDMI_I2S:
777 audio.format = AFMT_I2S;
778 break;
779 case HDMI_SPDIF:
780 audio.format = AFMT_SPDIF;
781 break;
782 default:
> 783 DRM_DEV_ERROR(bridge->dev, "Invalid format %d\n", daifmt->fmt);
784 ret = -EINVAL;
785 goto out;
786 }
787
788 ret = cdn_dp_audio_config(dp, &audio);
789 if (!ret)
790 dp->audio_info = audio;
791
792 out:
793 mutex_unlock(&dp->lock);
794 return ret;
795 }
796
797 static void cdn_dp_audio_shutdown(struct drm_connector *connector,
798 struct drm_bridge *bridge)
799 {
800 struct cdn_dp_device *dp = bridge_to_dp(bridge);
801 int ret;
802
803 mutex_lock(&dp->lock);
804 if (!dp->active)
805 goto out;
806
807 ret = cdn_dp_audio_stop(dp, &dp->audio_info);
808 if (!ret)
809 dp->audio_info.format = AFMT_UNUSED;
810 out:
811 mutex_unlock(&dp->lock);
812 }
813
814 static int cdn_dp_audio_mute_stream(struct drm_connector *connector,
815 struct drm_bridge *bridge,
816 bool enable, int direction)
817 {
818 struct cdn_dp_device *dp = bridge_to_dp(bridge);
819 int ret;
820
821 mutex_lock(&dp->lock);
822 if (!dp->active) {
823 ret = -ENODEV;
824 goto out;
825 }
826
827 ret = cdn_dp_audio_mute(dp, enable);
828
829 out:
830 mutex_unlock(&dp->lock);
831 return ret;
832 }
833
834 static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
835 .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
836 .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
837 .atomic_reset = drm_atomic_helper_bridge_reset,
838 .detect = cdn_dp_bridge_detect,
839 .edid_read = cdn_dp_bridge_edid_read,
840 .atomic_enable = cdn_dp_bridge_atomic_enable,
841 .atomic_disable = cdn_dp_bridge_atomic_disable,
842 .mode_valid = cdn_dp_bridge_mode_valid,
843
> 844 .dp_audio_prepare = cdn_dp_audio_prepare,
> 845 .dp_audio_mute_stream = cdn_dp_audio_mute_stream,
> 846 .dp_audio_shutdown = cdn_dp_audio_shutdown,
847 };
848
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.