* [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format
@ 2026-05-10 18:31 Jonas Karlman
2026-05-10 18:31 ` [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data Jonas Karlman
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-rockchip, linux-arm-kernel,
linux-kernel, Jonas Karlman
This series include misc cleanup of the dwhdmi-rockchip driver and
prepares for future support of YCbCr output and Deep Color modes.
Patch 1-7 cleanup and changes to use drmres helpers for the encoder.
Patch 8 prepare for use of a display-connector bridge for RK3568/RK3566.
Patch 9-10 prepares for future support of YCbCr and Deep Color modes.
This series depends on the patch "drm/rockchip: dw_hdmi: avoid direct
dereference of phy->dev.of_node" [1] from the series "Split Generic PHY
consumer and provider API" [2].
[1] https://lore.kernel.org/linux-phy/20260505100523.1922388-16-vladimir.oltean@nxp.com/
[2] https://lore.kernel.org/linux-phy/20260505100523.1922388-1-vladimir.oltean@nxp.com/
This series is part of a multi series effort to:
- phy: rockchip: inno-hdmi: Change TMDS rate handling to configure() ops [v2]
- drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format [v1]
- drm: bridge: dw_hdmi: Misc enable/disable, CEC and EDID cleanup [v5]
- drm/bridge: dw-hdmi: Improve input/output bus format handling
- drm/bridge: dw-hdmi: Convert to a HDMI bridge and use of bridge connector
- drm/bridge: dw-hdmi: Add and use tmds_char_rate_valid() plat data ops
- drm/meson: hdmi: Misc cleanup and use CEC notifier helpers
- drm/rockchip: dw_hdmi: Enable YCbCr and Deep Color modes
Link to snapshot: https://github.com/Kwiboo/linux-rockchip/commits/next-20260508-rk-hdmi-v3/
Jonas Karlman (10):
drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match
data
drm/rockchip: dw_hdmi: Use local dev variable consistently in bind()
drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources
drm/rockchip: dw_hdmi: Inline resource lookup into bind()
drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge
drm/rockchip: dw_hdmi: Remove empty encoder helper funcs
drm/rockchip: dw_hdmi: Clean up whitespace
drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566
drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()
drm/rockchip: dw_hdmi: Propagate bus format to display driver
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 220 ++++++++++++--------
1 file changed, 129 insertions(+), 91 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 02/10] drm/rockchip: dw_hdmi: Use local dev variable consistently in bind() Jonas Karlman
` (9 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Change to use of_device_get_match_data() to get match data prior to
allocating private data. All current entries in the of_device_id match
table provide match data, so no functional change is intended.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 8c0e433fbda2..34a2248f6b4b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -536,8 +536,8 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
{
struct platform_device *pdev = to_platform_device(dev);
struct device_node *np = dev_of_node(dev);
+ const struct dw_hdmi_plat_data *drv_data;
struct dw_hdmi_plat_data *plat_data;
- const struct of_device_id *match;
struct drm_device *drm = data;
struct drm_encoder *encoder;
struct rockchip_hdmi *hdmi;
@@ -546,13 +546,16 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
if (!np)
return -ENODEV;
+ drv_data = of_device_get_match_data(dev);
+ if (!drv_data)
+ return -ENODEV;
+
hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
- match = of_match_node(dw_hdmi_rockchip_dt_ids, np);
- plat_data = devm_kmemdup(&pdev->dev, match->data,
- sizeof(*plat_data), GFP_KERNEL);
+ plat_data = devm_kmemdup(&pdev->dev, drv_data,
+ sizeof(*drv_data), GFP_KERNEL);
if (!plat_data)
return -ENOMEM;
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 02/10] drm/rockchip: dw_hdmi: Use local dev variable consistently in bind()
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
2026-05-10 18:31 ` [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 03/10] drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources Jonas Karlman
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Replace indirect struct device accesses via hdmi->dev and pdev->dev with
the local dev parameter already available in dw_hdmi_rockchip_bind(),
for consistency and readability.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 34a2248f6b4b..435352f91b88 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -550,16 +550,15 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
if (!drv_data)
return -ENODEV;
- hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+ hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
- plat_data = devm_kmemdup(&pdev->dev, drv_data,
- sizeof(*drv_data), GFP_KERNEL);
+ plat_data = devm_kmemdup(dev, drv_data, sizeof(*drv_data), GFP_KERNEL);
if (!plat_data)
return -ENOMEM;
- hdmi->dev = &pdev->dev;
+ hdmi->dev = dev;
hdmi->plat_data = plat_data;
hdmi->chip_data = plat_data->phy_data;
plat_data->phy_data = hdmi;
@@ -581,13 +580,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
ret = rockchip_hdmi_parse_dt(hdmi);
if (ret) {
- return dev_err_probe(hdmi->dev, ret, "Unable to parse OF data\n");
+ return dev_err_probe(dev, ret, "Unable to parse OF data\n");
}
hdmi->phy = devm_phy_optional_get(dev, "hdmi");
if (IS_ERR(hdmi->phy)) {
ret = PTR_ERR(hdmi->phy);
- return dev_err_probe(hdmi->dev, ret, "failed to get phy\n");
+ return dev_err_probe(dev, ret, "failed to get phy\n");
}
index = of_property_match_string(np, "phy-names", "hdmi");
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 03/10] drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
2026-05-10 18:31 ` [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data Jonas Karlman
2026-05-10 18:31 ` [PATCH 02/10] drm/rockchip: dw_hdmi: Use local dev variable consistently in bind() Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 04/10] drm/rockchip: dw_hdmi: Inline resource lookup into bind() Jonas Karlman
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Change to use drmres helpers drmm_kzalloc() to allocate driver data
and drmm_encoder_init() to initialize the encoder. With use of drmres
the manual encoder cleanup in failure path and unbind is also removed.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 29 ++++++++-------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 435352f91b88..982d0829f278 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -14,6 +14,7 @@
#include <drm/bridge/dw_hdmi.h>
#include <drm/drm_edid.h>
+#include <drm/drm_managed.h>
#include <drm/drm_of.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
@@ -550,13 +551,14 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
if (!drv_data)
return -ENODEV;
- hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
+ hdmi = drmm_kzalloc(drm, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
- plat_data = devm_kmemdup(dev, drv_data, sizeof(*drv_data), GFP_KERNEL);
+ plat_data = drmm_kzalloc(drm, sizeof(*drv_data), GFP_KERNEL);
if (!plat_data)
return -ENOMEM;
+ memcpy(plat_data, drv_data, sizeof(*drv_data));
hdmi->dev = dev;
hdmi->plat_data = plat_data;
@@ -608,28 +610,20 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
FIELD_PREP_WM16(RK3568_HDMI_SCLIN_MSK, 1));
}
+ ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to init encoder\n");
+
drm_encoder_helper_add(encoder, &dw_hdmi_rockchip_encoder_helper_funcs);
- drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
platform_set_drvdata(pdev, hdmi);
hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
-
- /*
- * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
- * which would have called the encoder cleanup. Do it manually.
- */
- if (IS_ERR(hdmi->hdmi)) {
- ret = PTR_ERR(hdmi->hdmi);
- goto err_bind;
- }
+ if (IS_ERR(hdmi->hdmi))
+ return dev_err_probe(dev, PTR_ERR(hdmi->hdmi),
+ "failed to bind encoder\n");
return 0;
-
-err_bind:
- drm_encoder_cleanup(encoder);
-
- return ret;
}
static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
@@ -638,7 +632,6 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
dw_hdmi_unbind(hdmi->hdmi);
- drm_encoder_cleanup(&hdmi->encoder.encoder);
}
static const struct component_ops dw_hdmi_rockchip_ops = {
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 04/10] drm/rockchip: dw_hdmi: Inline resource lookup into bind()
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (2 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 03/10] drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 05/10] drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge Jonas Karlman
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Inline rockchip_hdmi_parse_dt() into dw_hdmi_rockchip_bind() so the
probe path is easier to follow in one place. Also ensure failures in
bind() use dev_err_probe() so probe deferrals and errors are reported
consistently.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 74 +++++++++------------
1 file changed, 30 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 982d0829f278..c5cdf2d1f04b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -196,41 +196,6 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
{ ~0UL, 0x0000, 0x0000, 0x0000}
};
-static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
-{
- struct device_node *np = hdmi->dev->of_node;
- int ret;
-
- hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
- if (IS_ERR(hdmi->regmap)) {
- dev_err(hdmi->dev, "Unable to get rockchip,grf\n");
- return PTR_ERR(hdmi->regmap);
- }
-
- hdmi->ref_clk = devm_clk_get_optional_enabled(hdmi->dev, "ref");
- if (!hdmi->ref_clk)
- hdmi->ref_clk = devm_clk_get_optional_enabled(hdmi->dev, "vpll");
-
- if (IS_ERR(hdmi->ref_clk)) {
- ret = PTR_ERR(hdmi->ref_clk);
- return dev_err_probe(hdmi->dev, ret, "failed to get reference clock\n");
- }
-
- hdmi->grf_clk = devm_clk_get_optional(hdmi->dev, "grf");
- if (IS_ERR(hdmi->grf_clk)) {
- ret = PTR_ERR(hdmi->grf_clk);
- return dev_err_probe(hdmi->dev, ret, "failed to get grf clock\n");
- }
-
- ret = devm_regulator_get_enable(hdmi->dev, "avdd-0v9");
- if (ret)
- return ret;
-
- ret = devm_regulator_get_enable(hdmi->dev, "avdd-1v8");
-
- return ret;
-}
-
static enum drm_mode_status
dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
const struct drm_display_info *info,
@@ -578,18 +543,39 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
* the required CRTC is added later.
*/
if (encoder->possible_crtcs == 0)
- return -EPROBE_DEFER;
+ return dev_err_probe(dev, -EPROBE_DEFER,
+ "failed to find possible crtcs\n");
- ret = rockchip_hdmi_parse_dt(hdmi);
- if (ret) {
- return dev_err_probe(dev, ret, "Unable to parse OF data\n");
- }
+ hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+ if (IS_ERR(hdmi->regmap))
+ return dev_err_probe(dev, PTR_ERR(hdmi->regmap),
+ "failed to get rockchip,grf\n");
+
+ hdmi->ref_clk = devm_clk_get_optional_enabled(dev, "ref");
+ if (!hdmi->ref_clk)
+ hdmi->ref_clk = devm_clk_get_optional_enabled(dev, "vpll");
+
+ if (IS_ERR(hdmi->ref_clk))
+ return dev_err_probe(dev, PTR_ERR(hdmi->ref_clk),
+ "failed to get reference clock\n");
+
+ hdmi->grf_clk = devm_clk_get_optional(dev, "grf");
+ if (IS_ERR(hdmi->grf_clk))
+ return dev_err_probe(dev, PTR_ERR(hdmi->grf_clk),
+ "failed to get grf clock\n");
+
+ ret = devm_regulator_get_enable(dev, "avdd-0v9");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable avdd-0v9\n");
+
+ ret = devm_regulator_get_enable(dev, "avdd-1v8");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable avdd-1v8\n");
hdmi->phy = devm_phy_optional_get(dev, "hdmi");
- if (IS_ERR(hdmi->phy)) {
- ret = PTR_ERR(hdmi->phy);
- return dev_err_probe(dev, ret, "failed to get phy\n");
- }
+ if (IS_ERR(hdmi->phy))
+ return dev_err_probe(dev, PTR_ERR(hdmi->phy),
+ "failed to get phy\n");
index = of_property_match_string(np, "phy-names", "hdmi");
if (index >= 0) {
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 05/10] drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (3 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 04/10] drm/rockchip: dw_hdmi: Inline resource lookup into bind() Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 06/10] drm/rockchip: dw_hdmi: Remove empty encoder helper funcs Jonas Karlman
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Take a reference on the dw-hdmi bridge during bind and drop it again
from unbind to ensure the bridge is kept alive for the lifetime of the
encoder component.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index c5cdf2d1f04b..c7933f3eb64f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -79,6 +79,7 @@ struct rockchip_hdmi {
struct clk *hdmiphy_clk;
struct clk *ref_clk;
struct clk *grf_clk;
+ struct drm_bridge *bridge;
struct dw_hdmi *hdmi;
struct phy *phy;
};
@@ -609,6 +610,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
return dev_err_probe(dev, PTR_ERR(hdmi->hdmi),
"failed to bind encoder\n");
+ hdmi->bridge = of_drm_find_and_get_bridge(np);
+ if (!hdmi->bridge) {
+ dw_hdmi_remove(hdmi->hdmi);
+ return dev_err_probe(dev, -ENODEV,
+ "failed to find bridge\n");
+ }
+
return 0;
}
@@ -617,6 +625,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
{
struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+ drm_bridge_put(hdmi->bridge);
dw_hdmi_unbind(hdmi->hdmi);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 06/10] drm/rockchip: dw_hdmi: Remove empty encoder helper funcs
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (4 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 05/10] drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 07/10] drm/rockchip: dw_hdmi: Clean up whitespace Jonas Karlman
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Remove the empty disable() and static return true mode_fixup() encoder
helper funcs as they do not provide any useful functionality.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index c7933f3eb64f..6aefdba0f1f9 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -226,18 +226,6 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
return MODE_OK;
}
-static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder)
-{
-}
-
-static bool
-dw_hdmi_rockchip_encoder_mode_fixup(struct drm_encoder *encoder,
- const struct drm_display_mode *mode,
- struct drm_display_mode *adj_mode)
-{
- return true;
-}
-
static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adj_mode)
@@ -290,10 +278,8 @@ dw_hdmi_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
}
static const struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_funcs = {
- .mode_fixup = dw_hdmi_rockchip_encoder_mode_fixup,
- .mode_set = dw_hdmi_rockchip_encoder_mode_set,
- .enable = dw_hdmi_rockchip_encoder_enable,
- .disable = dw_hdmi_rockchip_encoder_disable,
+ .mode_set = dw_hdmi_rockchip_encoder_mode_set,
+ .enable = dw_hdmi_rockchip_encoder_enable,
.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
};
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 07/10] drm/rockchip: dw_hdmi: Clean up whitespace
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (5 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 06/10] drm/rockchip: dw_hdmi: Remove empty encoder helper funcs Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 08/10] drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566 Jonas Karlman
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
Move the blank line before the RK3328 definitions for readability and
make the phy_config table spacing consistent with other tables.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 6aefdba0f1f9..a55a89040590 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -31,8 +31,8 @@
#define RK3288_GRF_SOC_CON6 0x025C
#define RK3288_HDMI_LCDC_SEL BIT(4)
-#define RK3328_GRF_SOC_CON2 0x0408
+#define RK3328_GRF_SOC_CON2 0x0408
#define RK3328_HDMI_SDAIN_MSK BIT(11)
#define RK3328_HDMI_SCLIN_MSK BIT(10)
#define RK3328_HDMI_HPD_IOE BIT(2)
@@ -190,11 +190,11 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
/*pixelclk symbol term vlev*/
- { 74250000, 0x8009, 0x0004, 0x0272},
- { 165000000, 0x802b, 0x0004, 0x0209},
- { 297000000, 0x8039, 0x0005, 0x028d},
- { 594000000, 0x8039, 0x0000, 0x019d},
- { ~0UL, 0x0000, 0x0000, 0x0000}
+ { 74250000, 0x8009, 0x0004, 0x0272 },
+ { 165000000, 0x802b, 0x0004, 0x0209 },
+ { 297000000, 0x8039, 0x0005, 0x028d },
+ { 594000000, 0x8039, 0x0000, 0x019d },
+ { ~0UL, 0x0000, 0x0000, 0x0000 },
};
static enum drm_mode_status
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 08/10] drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (6 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 07/10] drm/rockchip: dw_hdmi: Clean up whitespace Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 09/10] drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set() Jonas Karlman
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
All in-tree RK3568/RK3566 device trees using HDMI also include the
required hdmi-connector node at port@1 since their introduction.
Define the output_port for RK3568 so that dw-hdmi bridge driver can pick
up the display-connector bridge once the dw-hdmi connector is replaced
with a bridge connector in a future change.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index a55a89040590..9d3bb6cd5670 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -459,6 +459,7 @@ static const struct dw_hdmi_plat_data rk3568_hdmi_drv_data = {
.phy_config = rockchip_phy_config,
.phy_data = &rk3568_chip_data,
.use_drm_infoframe = true,
+ .output_port = 1,
};
static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 09/10] drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (7 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 08/10] drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566 Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 10/10] drm/rockchip: dw_hdmi: Propagate bus format to display driver Jonas Karlman
2026-05-16 6:14 ` Claude review: drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Claude Code Review Bot
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
The HDMI helpers negotiated TMDS character rate and output bpc are
available from the connector state. Change the encoder helper from
mode_set() to atomic_mode_set() so these values can be used to configure
the HDMI PHY using phy_configure().
This has no impact until the dw-hdmi bridge is fully converted into a
HDMI bridge and HDMI helpers are used to assign hdmi.tmds_char_rate.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 29 +++++++++++++++++----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 9d3bb6cd5670..94a30579a736 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -226,11 +226,22 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
return MODE_OK;
}
-static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adj_mode)
+static void
+dw_hdmi_rockchip_encoder_atomic_mode_set(struct drm_encoder *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
{
struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
+ struct drm_display_mode *adj_mode = &crtc_state->adjusted_mode;
+
+ if (hdmi->phy && conn_state->hdmi.tmds_char_rate) {
+ union phy_configure_opts opts = {};
+
+ opts.hdmi.bpc = conn_state->hdmi.output_bpc;
+ opts.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+
+ phy_configure(hdmi->phy, &opts);
+ }
clk_set_rate(hdmi->ref_clk, adj_mode->clock * 1000);
}
@@ -270,15 +281,23 @@ dw_hdmi_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_connector_state *conn_state)
{
struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+ struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
+ union phy_configure_opts opts = {};
s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
s->output_type = DRM_MODE_CONNECTOR_HDMIA;
- return 0;
+ if (!hdmi->phy || !conn_state->hdmi.tmds_char_rate)
+ return 0;
+
+ opts.hdmi.bpc = conn_state->hdmi.output_bpc;
+ opts.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+
+ return phy_validate(hdmi->phy, PHY_MODE_HDMI, PHY_HDMI_MODE_TMDS, &opts);
}
static const struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_funcs = {
- .mode_set = dw_hdmi_rockchip_encoder_mode_set,
+ .atomic_mode_set = dw_hdmi_rockchip_encoder_atomic_mode_set,
.enable = dw_hdmi_rockchip_encoder_enable,
.atomic_check = dw_hdmi_rockchip_encoder_atomic_check,
};
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 10/10] drm/rockchip: dw_hdmi: Propagate bus format to display driver
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (8 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 09/10] drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set() Jonas Karlman
@ 2026-05-10 18:31 ` Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-16 6:14 ` Claude review: drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Claude Code Review Bot
10 siblings, 1 reply; 22+ messages in thread
From: Jonas Karlman @ 2026-05-10 18:31 UTC (permalink / raw)
To: Heiko Stübner, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel,
Jonas Karlman
The HDMI block is currently hardcoded to expect RGB output from the
display controller. However, the VOP in some SoCs are capable of YCbCr
output to the HDMI block.
Read the negotiated bus format from the bridge state and propagate it to
the CRCT state in form of output mode and bus format. Treat the format
MEDIA_BUS_FMT_FIXED as RGB888 and reject any unsupported formats.
This has no inpact until dw-hdmi bridge is fully converted to a HDMI
bridge and also adds support for the "color format" connector property.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 44 ++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 94a30579a736..b52114d5fe9c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -5,6 +5,7 @@
#include <linux/clk.h>
#include <linux/hw_bitfield.h>
+#include <linux/media-bus-format.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -275,6 +276,26 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
dev_dbg(hdmi->dev, "vop %s output to hdmi\n", ret ? "LIT" : "BIG");
}
+static u32 dw_hdmi_rockchip_get_bus_format(struct drm_encoder *encoder,
+ struct drm_connector_state *conn_state)
+{
+ struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
+ struct drm_bridge_state *bridge_state;
+
+ bridge = drm_bridge_chain_get_first_bridge(encoder);
+ if (!bridge)
+ return 0;
+
+ bridge_state = drm_atomic_get_bridge_state(conn_state->state, bridge);
+ if (!bridge_state)
+ return 0;
+
+ if (bridge_state->input_bus_cfg.format != MEDIA_BUS_FMT_FIXED)
+ return bridge_state->input_bus_cfg.format;
+
+ return bridge_state->output_bus_cfg.format;
+}
+
static int
dw_hdmi_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
@@ -283,9 +304,30 @@ dw_hdmi_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
union phy_configure_opts opts = {};
+ u32 bus_format;
+
+ bus_format = dw_hdmi_rockchip_get_bus_format(encoder, conn_state);
+
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_FIXED:
+ bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ fallthrough;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ case MEDIA_BUS_FMT_YUV8_1X24:
+ case MEDIA_BUS_FMT_YUV10_1X30:
+ s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+ break;
+ case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+ case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+ s->output_mode = ROCKCHIP_OUT_MODE_YUV420;
+ break;
+ default:
+ return -EINVAL;
+ }
- s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+ s->bus_format = bus_format;
if (!hdmi->phy || !conn_state->hdmi.tmds_char_rate)
return 0;
--
2.54.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
` (9 preceding siblings ...)
2026-05-10 18:31 ` [PATCH 10/10] drm/rockchip: dw_hdmi: Propagate bus format to display driver Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
10 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format
Author: Jonas Karlman <jonas@kwiboo.se>
Patches: 11
Reviewed: 2026-05-16T16:14:37.782316
---
This is a well-structured 10-patch cleanup and preparation series for the `dw_hdmi-rockchip` driver by Jonas Karlman. Patches 1-7 are straightforward cleanups: modernizing match data retrieval, using `drmm_*` managed resources, inlining a helper, adding bridge refcounting, removing dead code, and cosmetic whitespace fixes. Patches 8-10 prepare for future YCbCr/Deep Color support by setting `output_port`, migrating to `atomic_mode_set()` with PHY configuration, and propagating bus format from the bridge state.
The series is logically ordered and each patch is well-scoped. There is one **real bug** in patch 10 (incorrect error handling for `drm_atomic_get_bridge_state`), and one potential concern in patch 5 regarding error handling ordering. The rest is clean.
**Verdict: Mostly good, one bug needs fixing in patch 10.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data
2026-05-10 18:31 ` [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
This replaces `of_match_node()` + `match->data` with the simpler `of_device_get_match_data()`. The early NULL check before allocation is a good improvement -- if the match table entry somehow lacks data, we bail before allocating.
The `sizeof(*drv_data)` in the `devm_kmemdup` call is correct since `drv_data` is `const struct dw_hdmi_plat_data *`, same type as the old `sizeof(*plat_data)`.
The removal of the `struct device_node *np = dev_of_node(dev)` variable is not done here but `np` is still used later, and the `of_device_get_match_data()` takes `dev` directly rather than `np`. The `np` NULL check remains, which is fine since it's used elsewhere.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Use local dev variable consistently in bind()
2026-05-10 18:31 ` [PATCH 02/10] drm/rockchip: dw_hdmi: Use local dev variable consistently in bind() Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Straightforward replacement of `&pdev->dev` and `hdmi->dev` with the already-available `dev` parameter. `dev` and `&pdev->dev` are the same pointer since `pdev = to_platform_device(dev)`, so this is purely cosmetic consistency.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources
2026-05-10 18:31 ` [PATCH 03/10] drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Switches from `devm_kzalloc`/`devm_kmemdup` to `drmm_kzalloc` and from `drm_simple_encoder_init` to `drmm_encoder_init`. This ties the lifetime of the `hdmi` struct and encoder to the DRM device rather than the platform device, which is the correct lifetime for DRM objects.
The change from `devm_kmemdup` to `drmm_kzalloc` + `memcpy` is functionally equivalent and fine.
The removal of the `err_bind` label and `drm_encoder_cleanup()` calls (both in error path and in `unbind`) is correct since `drmm_encoder_init` handles cleanup automatically.
One ordering subtlety: `drmm_encoder_init()` is called **before** `dw_hdmi_bind()`, and the old code called `drm_simple_encoder_init()` at the same point. The `drm_encoder_helper_add()` is moved after `drmm_encoder_init()` which is correct -- helpers must be added after init.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Inline resource lookup into bind()
2026-05-10 18:31 ` [PATCH 04/10] drm/rockchip: dw_hdmi: Inline resource lookup into bind() Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Inlines `rockchip_hdmi_parse_dt()` into `dw_hdmi_rockchip_bind()`. The inlining is straightforward and faithful. The improvements are:
1. `hdmi->dev` is replaced with `dev` (consistent with patch 2).
2. Error messages for regulators now use `dev_err_probe()` with descriptive messages instead of silently returning error codes.
3. The braces around `if (ret) { return ...; }` are removed for single-statement bodies.
The regulator error handling is slightly improved: the original code didn't log failures for `avdd-0v9` and `avdd-1v8` regulators, now they both use `dev_err_probe()`.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge
2026-05-10 18:31 ` [PATCH 05/10] drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Minor concern**
This adds `of_drm_find_and_get_bridge(np)` after `dw_hdmi_bind()` succeeds and calls `drm_bridge_put()` in unbind. The purpose is to hold a reference on the bridge for the lifetime of the encoder component.
**Concern**: The error path calls `dw_hdmi_remove(hdmi->hdmi)` on bridge lookup failure, but the bind function called `dw_hdmi_bind()` above. I'd expect `dw_hdmi_unbind()` to be used here for symmetry, matching what the `unbind` function does. Let me check:
```c
hdmi->bridge = of_drm_find_and_get_bridge(np);
if (!hdmi->bridge) {
dw_hdmi_remove(hdmi->hdmi);
return dev_err_probe(dev, -ENODEV,
"failed to find bridge\n");
}
```
The use of `dw_hdmi_remove()` vs `dw_hdmi_unbind()` may be intentional if the APIs differ, but it's worth the author confirming this is the right teardown function. The `unbind` path uses `dw_hdmi_unbind()`.
Also, `of_drm_find_and_get_bridge(np)` searches for a bridge associated with the device node. After `dw_hdmi_bind()` registers the bridge, this should find it. But the check `if (!hdmi->bridge)` only checks for NULL -- `of_drm_find_and_get_bridge` could also return an `ERR_PTR`. Checking the API: looking at the header, it returns `struct drm_bridge *` which can be NULL or ERR_PTR, so `IS_ERR_OR_NULL()` would be safer.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Remove empty encoder helper funcs
2026-05-10 18:31 ` [PATCH 06/10] drm/rockchip: dw_hdmi: Remove empty encoder helper funcs Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Removes the empty `dw_hdmi_rockchip_encoder_disable()` and the always-true `dw_hdmi_rockchip_encoder_mode_fixup()`. The DRM core already handles these cases: a NULL `disable` callback is a no-op, and a NULL `mode_fixup` defaults to true.
The whitespace alignment cleanup in the helper funcs struct (removing extra spaces for alignment) is a reasonable companion change.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Clean up whitespace
2026-05-10 18:31 ` [PATCH 07/10] drm/rockchip: dw_hdmi: Clean up whitespace Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Two cosmetic changes:
1. Moves a blank line to properly separate the RK3288 and RK3328 define blocks.
2. Adds spaces before closing braces in the `phy_config` table entries and adds a trailing comma to the sentinel entry.
The trailing comma on the sentinel entry is fine stylistically.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566
2026-05-10 18:31 ` [PATCH 08/10] drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566 Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Adds `.output_port = 1` to `rk3568_hdmi_drv_data`. The commit message explains this is needed so the dw-hdmi bridge driver can find the display-connector bridge at port@1 once the dw-hdmi connector is replaced with a bridge connector.
The claim that all in-tree RK3568/RK3566 DTs with HDMI include the hdmi-connector node at port@1 is stated but not verified here -- this is a reasonable assertion given the author's domain expertise.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()
2026-05-10 18:31 ` [PATCH 09/10] drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set() Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Clean**
Replaces the legacy `mode_set()` encoder helper with `atomic_mode_set()`, which provides access to `crtc_state` and `conn_state`. This enables reading `conn_state->hdmi.tmds_char_rate` and `conn_state->hdmi.output_bpc` to configure the PHY.
The `phy_configure()` call is guarded by `hdmi->phy && conn_state->hdmi.tmds_char_rate`, which is correct -- only SoCs with an external PHY and a valid TMDS rate will configure it. The `phy_validate()` in `atomic_check` mirrors the same guard.
The `clk_set_rate()` call now uses `&crtc_state->adjusted_mode` instead of the `adj_mode` parameter, which is the same data accessed differently.
The commit message correctly notes this has no functional impact until the dw-hdmi bridge is fully converted.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Claude review: drm/rockchip: dw_hdmi: Propagate bus format to display driver
2026-05-10 18:31 ` [PATCH 10/10] drm/rockchip: dw_hdmi: Propagate bus format to display driver Jonas Karlman
@ 2026-05-16 6:14 ` Claude Code Review Bot
0 siblings, 0 replies; 22+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:14 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Bug found**
This patch reads the negotiated bus format from the bridge state and maps it to the rockchip output mode (AAAA or YUV420).
**Bug**: In `dw_hdmi_rockchip_get_bus_format()`:
```c
bridge_state = drm_atomic_get_bridge_state(conn_state->state, bridge);
if (!bridge_state)
return 0;
```
`drm_atomic_get_bridge_state()` returns `ERR_CAST()` on failure (i.e., `ERR_PTR`), **not** NULL. This NULL check will never catch an error. The code will proceed with an `ERR_PTR` value in `bridge_state` and dereference it, causing a crash.
This should be:
```c
if (IS_ERR(bridge_state))
return 0;
```
Or better, since this is called from `atomic_check`, the error should be propagated:
```c
if (IS_ERR(bridge_state))
return PTR_ERR(bridge_state);
```
But that would require changing the return type of `dw_hdmi_rockchip_get_bus_format()` to handle errors. The simplest safe fix is to use `IS_ERR(bridge_state)`.
**Minor**: The commit message has a typo: "inpact" should be "impact".
**Additional note**: The `__free(drm_bridge_put)` cleanup annotation on `bridge` is a nice use of scope-based resource management for the bridge reference obtained from `drm_bridge_chain_get_first_bridge()`.
The `MEDIA_BUS_FMT_FIXED` fallthrough to `MEDIA_BUS_FMT_RGB888_1X24` is the right default behavior. The switch covers RGB, YCbCr 4:4:4, and YCbCr 4:2:0 at both 8 and 10 bit depths.
The `s->bus_format = bus_format` assignment correctly propagates the format to `rockchip_crtc_state`, which has the `bus_format` field (verified in `rockchip_drm_drv.h`).
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2026-05-16 6:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 18:31 [PATCH 00/10] drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Jonas Karlman
2026-05-10 18:31 ` [PATCH 01/10] drm/rockchip: dw_hdmi: Use of_device_get_match_data() to get match data Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 02/10] drm/rockchip: dw_hdmi: Use local dev variable consistently in bind() Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 03/10] drm/rockchip: dw_hdmi: Use drmres helpers for encoder resources Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 04/10] drm/rockchip: dw_hdmi: Inline resource lookup into bind() Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 05/10] drm/rockchip: dw_hdmi: Hold a reference to the dw-hdmi bridge Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 06/10] drm/rockchip: dw_hdmi: Remove empty encoder helper funcs Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 07/10] drm/rockchip: dw_hdmi: Clean up whitespace Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 08/10] drm/rockchip: dw_hdmi: Set output_port for RK3568/RK3566 Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 09/10] drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set() Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-10 18:31 ` [PATCH 10/10] drm/rockchip: dw_hdmi: Propagate bus format to display driver Jonas Karlman
2026-05-16 6:14 ` Claude review: " Claude Code Review Bot
2026-05-16 6:14 ` Claude review: drm/rockchip: dw_hdmi: Misc cleanup and propagate bus format Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox