* [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase
@ 2025-10-14 9:45 Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector earlier in atomic_enable() Harikrishna Shenoy
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
With the DBANC framework, the connector is no longer initialized in
bridge_attach()when the display controller sets the
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn()
when trying to access &conn->dev->mode_config.mutex.
Observed on a board where EDID read failed.
(log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)
Patch 1 adds a connector_ptr which takes care of both
DBANC and !DBANC case by setting the pointer in appropriate hooks
and checking for pointer validity before accessing the connector.
Patch 2 adds mode validation hook to bridge fucntions.
Patches 3,4 do necessary cleanup and alignment for using
connector pointer.
The rationale behind the sequence of commits is we can cleanly
switch to drm_connector pointer after removal of connector helper
code blocks, which are anyways not touch after DBANC has been
enabled in driver.
The later patches make smaller adjustments: lowering the log level for
noisy DPCD transfer errors and moving HDCP state handling into the bridge
atomic check in line with the DBANC model.
v7 patch link:
<https://lore.kernel.org/all/20250929083936.1575685-1-h-shenoy@ti.com/>
Changelog v7-v8:
-Move patches with firxes tag to top of series with appropriate changes
to them.
-Add R/B tag to patch
https://lore.kernel.org/all/ae3snoap64r252sbqhsshsadxfmlqdfn6b4o5fgfcmxppglkqf@2lsstfsghzwb/
v6 patch link:
<https://lore.kernel.org/all/20250909090824.1655537-1-h-shenoy@ti.com/>
Changelog v6-v7:
-Update cover letter to explain the series.
-Add R/B tag in PATCH 1 and drop fixes tag as suggested.
-Drop fixes tag in PATCH 2.
-Update the commit messages for clear understanding of changes done in patches.
v5 patch link:
<https://lore.kernel.org/all/20250811075904.1613519-1-h-shenoy@ti.com/>
Changelog v5 -> v6:
-Update cover letter to clarify the series in better way.
-Add Reviewed-by tag to relevant patches.
v4 patch link:
<https://lore.kernel.org/all/20250624054448.192801-1-j-choudhary@ti.com>
Changelog v4->v5:
- Handle HDCP state in bridge atomic check instead of connector
atomic check
v3 patch link:
<https://lore.kernel.org/all/20250529142517.188786-1-j-choudhary@ti.com/>
Changelog v3->v4:
- Fix kernel test robot build warning:
<https://lore.kernel.org/all/202505300201.2s6r12yc-lkp@intel.com/>
v2 patch link:
<https://lore.kernel.org/all/20250521073237.366463-1-j-choudhary@ti.com/>
Changelog v2->v3:
- Add mode_valid in drm_bridge_funcs to a separate patch
- Remove "if (mhdp->connector.dev)" conditions that were missed in v2
- Split out the move of drm_atomic_get_new_connector_for_encoder()
to a separate patch
- Drop "R-by" considering the changes in v2[1/3]
- Add Fixes tag to first 4 patches:
commit c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
This added DBANC flag in tidss while attaching bridge to the encoder
- Drop RFC prefix
v1 patch link:
<https://lore.kernel.org/all/20250116111636.157641-1-j-choudhary@ti.com/>
Changelog v1->v2:
- Remove !DRM_BRIDGE_ATTACH_NO_CONNECTOR entirely
- Add mode_valid in drm_bridge_funcs[0]
- Fix NULL POINTER differently since we cannot access atomic_state
- Reduce log level in cdns_mhdp_transfer call
[0]: https://lore.kernel.org/all/20240530091757.433106-1-j-choudhary@ti.com/
Harikrishna Shenoy (1):
drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge
atomic check
Jayesh Choudhary (5):
drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector
earlier in atomic_enable()
drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to
drm_bridge_funcs
drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for
connector initialisation in bridge
drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from
structure to pointer
drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD
read/write
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 258 +++++-------------
.../drm/bridge/cadence/cdns-mhdp8546-core.h | 2 +-
.../drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 8 +-
3 files changed, 72 insertions(+), 196 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector earlier in atomic_enable()
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 2/6] drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to drm_bridge_funcs Harikrishna Shenoy
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
From: Jayesh Choudhary <j-choudhary@ti.com>
In case if we get errors in cdns_mhdp_link_up() or cdns_mhdp_reg_read()
in atomic_enable, we will go to cdns_mhdp_modeset_retry_fn() and will hit
NULL pointer while trying to access the mutex. We need the connector to
be set before that. Unlike in legacy !(DBANC) cases, we do not have
connector initialised in bridge_attach(), so add the mhdp->connector_ptr
in device structure to handle both DBANC and !DBANC case and set it in
atomic_enable() earlier to avoid possible NULL pointer dereference in
recovery paths like modeset_retry_fn() with the DBANC flag set.
Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 29 ++++++++++---------
.../drm/bridge/cadence/cdns-mhdp8546-core.h | 1 +
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 38726ae1bf15..f3076e9cdabb 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -740,7 +740,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
bridge_attached = mhdp->bridge_attached;
spin_unlock(&mhdp->start_lock);
if (bridge_attached) {
- if (mhdp->connector.dev)
+ if (mhdp->connector_ptr && mhdp->connector_ptr->dev)
drm_kms_helper_hotplug_event(mhdp->bridge.dev);
else
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
@@ -1636,6 +1636,7 @@ static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
return ret;
}
+ mhdp->connector_ptr = conn;
drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
ret = drm_display_info_set_bus_formats(&conn->display_info,
@@ -1915,17 +1916,25 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
struct cdns_mhdp_bridge_state *mhdp_state;
struct drm_crtc_state *crtc_state;
- struct drm_connector *connector;
struct drm_connector_state *conn_state;
struct drm_bridge_state *new_state;
const struct drm_display_mode *mode;
u32 resp;
- int ret;
+ int ret = 0;
dev_dbg(mhdp->dev, "bridge enable\n");
mutex_lock(&mhdp->link_mutex);
+ mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
+ bridge->encoder);
+ if (WARN_ON(!mhdp->connector_ptr))
+ goto out;
+
+ conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector_ptr);
+ if (WARN_ON(!conn_state))
+ goto out;
+
if (mhdp->plugged && !mhdp->link_up) {
ret = cdns_mhdp_link_up(mhdp);
if (ret < 0)
@@ -1945,15 +1954,6 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
cdns_mhdp_reg_write(mhdp, CDNS_DPTX_CAR,
resp | CDNS_VIF_CLK_EN | CDNS_VIF_CLK_RSTN);
- connector = drm_atomic_get_new_connector_for_encoder(state,
- bridge->encoder);
- if (WARN_ON(!connector))
- goto out;
-
- conn_state = drm_atomic_get_new_connector_state(state, connector);
- if (WARN_ON(!conn_state))
- goto out;
-
if (mhdp->hdcp_supported &&
mhdp->hw_state == MHDP_HW_READY &&
conn_state->content_protection ==
@@ -2030,6 +2030,7 @@ static void cdns_mhdp_atomic_disable(struct drm_bridge *bridge,
if (mhdp->info && mhdp->info->ops && mhdp->info->ops->disable)
mhdp->info->ops->disable(mhdp);
+ mhdp->connector_ptr = NULL;
mutex_unlock(&mhdp->link_mutex);
}
@@ -2296,7 +2297,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
- conn = &mhdp->connector;
+ conn = mhdp->connector_ptr;
/* Grab the locks before changing connector property */
mutex_lock(&conn->dev->mode_config.mutex);
@@ -2373,7 +2374,7 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
int ret;
ret = cdns_mhdp_update_link_status(mhdp);
- if (mhdp->connector.dev) {
+ if (mhdp->connector_ptr && mhdp->connector_ptr->dev) {
if (ret < 0)
schedule_work(&mhdp->modeset_retry_work);
else
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index bad2fc0c7306..a76775c76895 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -376,6 +376,7 @@ struct cdns_mhdp_device {
struct mutex link_mutex;
struct drm_connector connector;
+ struct drm_connector *connector_ptr;
struct drm_bridge bridge;
struct cdns_mhdp_link link;
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 2/6] drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to drm_bridge_funcs
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector earlier in atomic_enable() Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 3/6] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge Harikrishna Shenoy
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
From: Jayesh Choudhary <j-choudhary@ti.com>
Add cdns_mhdp_bridge_mode_valid() to check if specific mode is valid for
this bridge or not. In the legacy !(DBANC) usecase, we were using the hook
from drm_connector_helper_funcs but with DBANC we need to have mode_valid()
in drm_bridge_funcs.
Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f3076e9cdabb..7178a01e4d4d 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2162,6 +2162,25 @@ static const struct drm_edid *cdns_mhdp_bridge_edid_read(struct drm_bridge *brid
return cdns_mhdp_edid_read(mhdp, connector);
}
+static enum drm_mode_status
+cdns_mhdp_bridge_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode)
+{
+ struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
+
+ mutex_lock(&mhdp->link_mutex);
+
+ if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
+ mhdp->link.rate)) {
+ mutex_unlock(&mhdp->link_mutex);
+ return MODE_CLOCK_HIGH;
+ }
+
+ mutex_unlock(&mhdp->link_mutex);
+ return MODE_OK;
+}
+
static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
.atomic_enable = cdns_mhdp_atomic_enable,
.atomic_disable = cdns_mhdp_atomic_disable,
@@ -2176,6 +2195,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
.edid_read = cdns_mhdp_bridge_edid_read,
.hpd_enable = cdns_mhdp_bridge_hpd_enable,
.hpd_disable = cdns_mhdp_bridge_hpd_disable,
+ .mode_valid = cdns_mhdp_bridge_mode_valid,
};
static bool cdns_mhdp_detect_hpd(struct cdns_mhdp_device *mhdp, bool *hpd_pulse)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 3/6] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector earlier in atomic_enable() Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 2/6] drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to drm_bridge_funcs Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer Harikrishna Shenoy
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
From: Jayesh Choudhary <j-choudhary@ti.com>
Now that we have DBANC framework, remove the connector initialisation code
as that piece of code is not called if DRM_BRIDGE_ATTACH_NO_CONNECTOR flag
is used. Only TI K3 platforms consume this driver and tidss (their display
controller) has this flag set. So this legacy support can be dropped.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 188 +-----------------
1 file changed, 10 insertions(+), 178 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 7178a01e4d4d..31256ad69602 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -739,12 +739,8 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
spin_lock(&mhdp->start_lock);
bridge_attached = mhdp->bridge_attached;
spin_unlock(&mhdp->start_lock);
- if (bridge_attached) {
- if (mhdp->connector_ptr && mhdp->connector_ptr->dev)
- drm_kms_helper_hotplug_event(mhdp->bridge.dev);
- else
- drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
- }
+ if (bridge_attached)
+ drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
}
static int cdns_mhdp_load_firmware(struct cdns_mhdp_device *mhdp)
@@ -1444,56 +1440,6 @@ static const struct drm_edid *cdns_mhdp_edid_read(struct cdns_mhdp_device *mhdp,
return drm_edid_read_custom(connector, cdns_mhdp_get_edid_block, mhdp);
}
-static int cdns_mhdp_get_modes(struct drm_connector *connector)
-{
- struct cdns_mhdp_device *mhdp = connector_to_mhdp(connector);
- const struct drm_edid *drm_edid;
- int num_modes;
-
- if (!mhdp->plugged)
- return 0;
-
- drm_edid = cdns_mhdp_edid_read(mhdp, connector);
-
- drm_edid_connector_update(connector, drm_edid);
-
- if (!drm_edid) {
- dev_err(mhdp->dev, "Failed to read EDID\n");
- return 0;
- }
-
- num_modes = drm_edid_connector_add_modes(connector);
- drm_edid_free(drm_edid);
-
- /*
- * HACK: Warn about unsupported display formats until we deal
- * with them correctly.
- */
- if (connector->display_info.color_formats &&
- !(connector->display_info.color_formats &
- mhdp->display_fmt.color_format))
- dev_warn(mhdp->dev,
- "%s: No supported color_format found (0x%08x)\n",
- __func__, connector->display_info.color_formats);
-
- if (connector->display_info.bpc &&
- connector->display_info.bpc < mhdp->display_fmt.bpc)
- dev_warn(mhdp->dev, "%s: Display bpc only %d < %d\n",
- __func__, connector->display_info.bpc,
- mhdp->display_fmt.bpc);
-
- return num_modes;
-}
-
-static int cdns_mhdp_connector_detect(struct drm_connector *conn,
- struct drm_modeset_acquire_ctx *ctx,
- bool force)
-{
- struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
-
- return cdns_mhdp_detect(mhdp);
-}
-
static u32 cdns_mhdp_get_bpp(struct cdns_mhdp_display_fmt *fmt)
{
u32 bpp;
@@ -1547,115 +1493,6 @@ bool cdns_mhdp_bandwidth_ok(struct cdns_mhdp_device *mhdp,
return true;
}
-static
-enum drm_mode_status cdns_mhdp_mode_valid(struct drm_connector *conn,
- const struct drm_display_mode *mode)
-{
- struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
-
- mutex_lock(&mhdp->link_mutex);
-
- if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
- mhdp->link.rate)) {
- mutex_unlock(&mhdp->link_mutex);
- return MODE_CLOCK_HIGH;
- }
-
- mutex_unlock(&mhdp->link_mutex);
- return MODE_OK;
-}
-
-static int cdns_mhdp_connector_atomic_check(struct drm_connector *conn,
- struct drm_atomic_state *state)
-{
- struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
- struct drm_connector_state *old_state, *new_state;
- struct drm_crtc_state *crtc_state;
- u64 old_cp, new_cp;
-
- if (!mhdp->hdcp_supported)
- return 0;
-
- old_state = drm_atomic_get_old_connector_state(state, conn);
- new_state = drm_atomic_get_new_connector_state(state, conn);
- old_cp = old_state->content_protection;
- new_cp = new_state->content_protection;
-
- if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
- new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
- new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
- goto mode_changed;
- }
-
- if (!new_state->crtc) {
- if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
- new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
- return 0;
- }
-
- if (old_cp == new_cp ||
- (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
- new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
- return 0;
-
-mode_changed:
- crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
- crtc_state->mode_changed = true;
-
- return 0;
-}
-
-static const struct drm_connector_helper_funcs cdns_mhdp_conn_helper_funcs = {
- .detect_ctx = cdns_mhdp_connector_detect,
- .get_modes = cdns_mhdp_get_modes,
- .mode_valid = cdns_mhdp_mode_valid,
- .atomic_check = cdns_mhdp_connector_atomic_check,
-};
-
-static const struct drm_connector_funcs cdns_mhdp_conn_funcs = {
- .fill_modes = drm_helper_probe_single_connector_modes,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
- .reset = drm_atomic_helper_connector_reset,
- .destroy = drm_connector_cleanup,
-};
-
-static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
-{
- u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
- struct drm_connector *conn = &mhdp->connector;
- struct drm_bridge *bridge = &mhdp->bridge;
- int ret;
-
- conn->polled = DRM_CONNECTOR_POLL_HPD;
-
- ret = drm_connector_init(bridge->dev, conn, &cdns_mhdp_conn_funcs,
- DRM_MODE_CONNECTOR_DisplayPort);
- if (ret) {
- dev_err(mhdp->dev, "Failed to initialize connector with drm\n");
- return ret;
- }
-
- mhdp->connector_ptr = conn;
- drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
-
- ret = drm_display_info_set_bus_formats(&conn->display_info,
- &bus_format, 1);
- if (ret)
- return ret;
-
- ret = drm_connector_attach_encoder(conn, bridge->encoder);
- if (ret) {
- dev_err(mhdp->dev, "Failed to attach connector to encoder\n");
- return ret;
- }
-
- if (mhdp->hdcp_supported)
- ret = drm_connector_attach_content_protection_property(conn, true);
-
- return ret;
-}
-
static int cdns_mhdp_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
@@ -1672,9 +1509,11 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
return ret;
if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
- ret = cdns_mhdp_connector_init(mhdp);
- if (ret)
- goto aux_unregister;
+ ret = -EINVAL;
+ dev_err(mhdp->dev,
+ "Connector initialisation not supported in bridge_attach %d\n",
+ ret);
+ goto aux_unregister;
}
spin_lock(&mhdp->start_lock);
@@ -2391,17 +2230,10 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
struct cdns_mhdp_device *mhdp = container_of(work,
struct cdns_mhdp_device,
hpd_work);
- int ret;
- ret = cdns_mhdp_update_link_status(mhdp);
- if (mhdp->connector_ptr && mhdp->connector_ptr->dev) {
- if (ret < 0)
- schedule_work(&mhdp->modeset_retry_work);
- else
- drm_kms_helper_hotplug_event(mhdp->bridge.dev);
- } else {
- drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
- }
+ cdns_mhdp_update_link_status(mhdp);
+
+ drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
}
static int cdns_mhdp_probe(struct platform_device *pdev)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
` (2 preceding siblings ...)
2025-10-14 9:45 ` [PATCH v8 3/6] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-11-12 9:15 ` Tomi Valkeinen
2025-10-14 9:45 ` [PATCH v8 5/6] drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD read/write Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check Harikrishna Shenoy
5 siblings, 1 reply; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
From: Jayesh Choudhary <j-choudhary@ti.com>
After adding DBANC framework, mhdp->connector is not initialised during
bridge_attach(). The connector is however required in few driver calls
like cdns_mhdp_hdcp_enable() and cdns_mhdp_modeset_retry_fn().
Now that we have dropped the legacy code which became redundant
with introduction of DBNAC usecase in driver, we can cleanly switch
to drm_connector pointer instead of structure.
Set it in bridge_enable() and clear it in bridge_disable(),
and make appropriate changes.
This allows us to dynamically set the reference in bridge_enable() when
the connector becomes available and clear it in bridge_disable().
This change is necessary to properly integrate with the DBANC framework
while maintaining all connector-dependent functionality in the driver.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 12 ++++++------
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h | 3 +--
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 8 ++++----
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 31256ad69602..fe2da567ec66 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1765,12 +1765,12 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
mutex_lock(&mhdp->link_mutex);
- mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
- bridge->encoder);
- if (WARN_ON(!mhdp->connector_ptr))
+ mhdp->connector = drm_atomic_get_new_connector_for_encoder(state,
+ bridge->encoder);
+ if (WARN_ON(!mhdp->connector))
goto out;
- conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector_ptr);
+ conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector);
if (WARN_ON(!conn_state))
goto out;
@@ -1869,7 +1869,7 @@ static void cdns_mhdp_atomic_disable(struct drm_bridge *bridge,
if (mhdp->info && mhdp->info->ops && mhdp->info->ops->disable)
mhdp->info->ops->disable(mhdp);
- mhdp->connector_ptr = NULL;
+ mhdp->connector = NULL;
mutex_unlock(&mhdp->link_mutex);
}
@@ -2156,7 +2156,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
- conn = mhdp->connector_ptr;
+ conn = mhdp->connector;
/* Grab the locks before changing connector property */
mutex_lock(&conn->dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index a76775c76895..b297db53ba28 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -375,8 +375,7 @@ struct cdns_mhdp_device {
*/
struct mutex link_mutex;
- struct drm_connector connector;
- struct drm_connector *connector_ptr;
+ struct drm_connector *connector;
struct drm_bridge bridge;
struct cdns_mhdp_link link;
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
index 42248f179b69..59f18c3281ef 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
@@ -394,7 +394,7 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device *mhdp)
int ret;
dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
- mhdp->connector.name, mhdp->connector.base.id);
+ mhdp->connector->name, mhdp->connector->base.id);
ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
@@ -445,7 +445,7 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
dev_err(mhdp->dev,
"[%s:%d] HDCP link failed, retrying authentication\n",
- mhdp->connector.name, mhdp->connector.base.id);
+ mhdp->connector->name, mhdp->connector->base.id);
ret = _cdns_mhdp_hdcp_disable(mhdp);
if (ret) {
@@ -487,13 +487,13 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct *work)
struct cdns_mhdp_device *mhdp = container_of(hdcp,
struct cdns_mhdp_device,
hdcp);
- struct drm_device *dev = mhdp->connector.dev;
+ struct drm_device *dev = mhdp->connector->dev;
struct drm_connector_state *state;
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
mutex_lock(&mhdp->hdcp.mutex);
if (mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
- state = mhdp->connector.state;
+ state = mhdp->connector->state;
state->content_protection = mhdp->hdcp.value;
}
mutex_unlock(&mhdp->hdcp.mutex);
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 5/6] drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD read/write
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
` (3 preceding siblings ...)
2025-10-14 9:45 ` [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check Harikrishna Shenoy
5 siblings, 0 replies; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
From: Jayesh Choudhary <j-choudhary@ti.com>
Reduce the log level for cdns_mhdp_dpcd_read and cdns_mhdp_dpcd_write
errors in cdns_mhdp_transfer function as in case of failure, there is
flooding of these prints along with other indicators like EDID failure
logs which are fairly intuitive in themselves rendering these error logs
useless.
Also, the caller functions for the cdns_mhdp_transfer in drm_dp_helper.c
(which calls it 32 times), has debug log level in case transfer fails.
So having a superseding log level in cdns_mhdp_transfer seems bad.
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index fe2da567ec66..06ac5c2ee78f 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -778,7 +778,7 @@ static ssize_t cdns_mhdp_transfer(struct drm_dp_aux *aux,
if (!ret)
continue;
- dev_err(mhdp->dev,
+ dev_dbg(mhdp->dev,
"Failed to write DPCD addr %u\n",
msg->address + i);
@@ -788,7 +788,7 @@ static ssize_t cdns_mhdp_transfer(struct drm_dp_aux *aux,
ret = cdns_mhdp_dpcd_read(mhdp, msg->address,
msg->buffer, msg->size);
if (ret) {
- dev_err(mhdp->dev,
+ dev_dbg(mhdp->dev,
"Failed to read DPCD addr %u\n",
msg->address);
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
` (4 preceding siblings ...)
2025-10-14 9:45 ` [PATCH v8 5/6] drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD read/write Harikrishna Shenoy
@ 2025-10-14 9:45 ` Harikrishna Shenoy
2025-11-12 9:22 ` Tomi Valkeinen
5 siblings, 1 reply; 10+ messages in thread
From: Harikrishna Shenoy @ 2025-10-14 9:45 UTC (permalink / raw)
To: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tomi.valkeinen, tzimmermann, u-kumar1
Now that we have DBANC framework and legacy connector functions removed,
handle the HDCP disabling in bridge atomic check rather than in connector
atomic check in !(DBANC) usecase.
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 06ac5c2ee78f..120eb7ffe20c 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1962,6 +1962,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
{
struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
+ struct drm_connector_state *old_state, *new_state;
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *conn = mhdp->connector;
+ u64 old_cp, new_cp;
mutex_lock(&mhdp->link_mutex);
@@ -1981,6 +1985,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
if (mhdp->info)
bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
+ if (conn && mhdp->hdcp_supported) {
+ old_state = drm_atomic_get_old_connector_state(state, conn);
+ new_state = drm_atomic_get_new_connector_state(state, conn);
+ old_cp = old_state->content_protection;
+ new_cp = new_state->content_protection;
+
+ if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
+ new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
+ new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+ crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
+ crtc_state->mode_changed = true;
+ }
+
+ if (!new_state->crtc) {
+ if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
+ new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+ }
+ }
+
mutex_unlock(&mhdp->link_mutex);
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer
2025-10-14 9:45 ` [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer Harikrishna Shenoy
@ 2025-11-12 9:15 ` Tomi Valkeinen
0 siblings, 0 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2025-11-12 9:15 UTC (permalink / raw)
To: Harikrishna Shenoy
Cc: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tzimmermann, u-kumar1
Hi,
On 14/10/2025 12:45, Harikrishna Shenoy wrote:
> From: Jayesh Choudhary <j-choudhary@ti.com>
>
> After adding DBANC framework, mhdp->connector is not initialised during
> bridge_attach(). The connector is however required in few driver calls
> like cdns_mhdp_hdcp_enable() and cdns_mhdp_modeset_retry_fn().
> Now that we have dropped the legacy code which became redundant
> with introduction of DBNAC usecase in driver, we can cleanly switch
> to drm_connector pointer instead of structure.
>
> Set it in bridge_enable() and clear it in bridge_disable(),
> and make appropriate changes.
>
> This allows us to dynamically set the reference in bridge_enable() when
> the connector becomes available and clear it in bridge_disable().
> This change is necessary to properly integrate with the DBANC framework
> while maintaining all connector-dependent functionality in the driver.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
> ---
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 12 ++++++------
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h | 3 +--
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 8 ++++----
> 3 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 31256ad69602..fe2da567ec66 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -1765,12 +1765,12 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
>
> mutex_lock(&mhdp->link_mutex);
>
> - mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
> - bridge->encoder);
> - if (WARN_ON(!mhdp->connector_ptr))
> + mhdp->connector = drm_atomic_get_new_connector_for_encoder(state,
> + bridge->encoder);
> + if (WARN_ON(!mhdp->connector))
> goto out;
>
> - conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector_ptr);
> + conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector);
These kind of changes make sense: earlier we had connector and
connector_ptr:
struct drm_connector connector;
struct drm_connector *connector_ptr;
and for !DBANC, ptr pointed to the above connector struct, whereas for
DBANC ptr pointed to the separate connector. Now we drop the above
connector field, and rename connector_ptr. So that's fine. But...
> if (WARN_ON(!conn_state))
> goto out;
>
> @@ -1869,7 +1869,7 @@ static void cdns_mhdp_atomic_disable(struct drm_bridge *bridge,
> if (mhdp->info && mhdp->info->ops && mhdp->info->ops->disable)
> mhdp->info->ops->disable(mhdp);
>
> - mhdp->connector_ptr = NULL;
> + mhdp->connector = NULL;
> mutex_unlock(&mhdp->link_mutex);
> }
>
> @@ -2156,7 +2156,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
>
> mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
>
> - conn = mhdp->connector_ptr;
> + conn = mhdp->connector;
>
> /* Grab the locks before changing connector property */
> mutex_lock(&conn->dev->mode_config.mutex);
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> index a76775c76895..b297db53ba28 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> @@ -375,8 +375,7 @@ struct cdns_mhdp_device {
> */
> struct mutex link_mutex;
>
> - struct drm_connector connector;
> - struct drm_connector *connector_ptr;
> + struct drm_connector *connector;
> struct drm_bridge bridge;
>
> struct cdns_mhdp_link link;
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> index 42248f179b69..59f18c3281ef 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> @@ -394,7 +394,7 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device *mhdp)
> int ret;
>
> dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
> - mhdp->connector.name, mhdp->connector.base.id);
> + mhdp->connector->name, mhdp->connector->base.id);
... what's this? Here the code being removed still uses mhdp->connector.
But that was essentially disabled in the previous patch. I would expect
the only changes in this patch to be renaming connector_ptr ->
connector, but this change and the ones below hint that there's an issue
in the earlier patches. So if, before this patch, we ever ran DBANC and
called this function, we'd be using mhdp->connector, which hasn't been
initialized.
The mdhp->connector should go away in the previous patch. This patch
should only do the rename.
However, I think the use of mhdp->connector with DBANC should be fixed
already earlier. Sounds like patch 1 content, which already fixes
similar issues?
Tomi
>
> ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
>
> @@ -445,7 +445,7 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
>
> dev_err(mhdp->dev,
> "[%s:%d] HDCP link failed, retrying authentication\n",
> - mhdp->connector.name, mhdp->connector.base.id);
> + mhdp->connector->name, mhdp->connector->base.id);
>
> ret = _cdns_mhdp_hdcp_disable(mhdp);
> if (ret) {
> @@ -487,13 +487,13 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct *work)
> struct cdns_mhdp_device *mhdp = container_of(hdcp,
> struct cdns_mhdp_device,
> hdcp);
> - struct drm_device *dev = mhdp->connector.dev;
> + struct drm_device *dev = mhdp->connector->dev;
> struct drm_connector_state *state;
>
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> mutex_lock(&mhdp->hdcp.mutex);
> if (mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> - state = mhdp->connector.state;
> + state = mhdp->connector->state;
> state->content_protection = mhdp->hdcp.value;
> }
> mutex_unlock(&mhdp->hdcp.mutex);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
2025-10-14 9:45 ` [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check Harikrishna Shenoy
@ 2025-11-12 9:22 ` Tomi Valkeinen
2025-11-18 4:54 ` Harikrishna shenoy
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2025-11-12 9:22 UTC (permalink / raw)
To: Harikrishna Shenoy
Cc: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tzimmermann, u-kumar1
Hi,
On 14/10/2025 12:45, Harikrishna Shenoy wrote:
> Now that we have DBANC framework and legacy connector functions removed,
> handle the HDCP disabling in bridge atomic check rather than in connector
> atomic check in !(DBANC) usecase.
This sounds odd. The patch is only adding new code, so "handle the HDCP
disabling in bridge atomic check rather than in connector atomic check
in !(DBANC) usecase." doesn't quite make sense.
Afaiu in patch 3 you removed the HDCP code for !DBANC, and here you add
it for DBANC.
Correct me if I'm wrong, but HDCP support for DBANC is currently broken
in upstream, as the HDCP code is only for the !DBANC case?
Isn't this patch then similar to patch 2, which is a fix for a missing
feature with DBANC? So should this also be a fix, and perhaps be moved
as after patch 2?
Tomi
> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
> ---
> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 23 +++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 06ac5c2ee78f..120eb7ffe20c 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -1962,6 +1962,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
> {
> struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
> const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> + struct drm_connector_state *old_state, *new_state;
> + struct drm_atomic_state *state = crtc_state->state;
> + struct drm_connector *conn = mhdp->connector;
> + u64 old_cp, new_cp;
>
> mutex_lock(&mhdp->link_mutex);
>
> @@ -1981,6 +1985,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
> if (mhdp->info)
> bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
>
> + if (conn && mhdp->hdcp_supported) {
> + old_state = drm_atomic_get_old_connector_state(state, conn);
> + new_state = drm_atomic_get_new_connector_state(state, conn);
> + old_cp = old_state->content_protection;
> + new_cp = new_state->content_protection;
> +
> + if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
> + new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> + new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
> + crtc_state->mode_changed = true;
> + }
> +
> + if (!new_state->crtc) {
> + if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + }
> + }
> +
> mutex_unlock(&mhdp->link_mutex);
> return 0;
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
2025-11-12 9:22 ` Tomi Valkeinen
@ 2025-11-18 4:54 ` Harikrishna shenoy
0 siblings, 0 replies; 10+ messages in thread
From: Harikrishna shenoy @ 2025-11-18 4:54 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Laurent.pinchart, airlied, andrzej.hajda, andy.yan,
aradhya.bhatia, devarsht, dianders, dri-devel, javierm,
jernej.skrabec, jonas, linux-kernel, linux, luca.ceresoli, lumag,
lyude, maarten.lankhorst, mordan, mripard, neil.armstrong, rfoss,
s-jain1, simona, tzimmermann, u-kumar1
On 12/11/25 14:52, Tomi Valkeinen wrote:
> Hi,
>
> On 14/10/2025 12:45, Harikrishna Shenoy wrote:
>> Now that we have DBANC framework and legacy connector functions removed,
>> handle the HDCP disabling in bridge atomic check rather than in connector
>> atomic check in !(DBANC) usecase.
>
> This sounds odd. The patch is only adding new code, so "handle the HDCP
> disabling in bridge atomic check rather than in connector atomic check
> in !(DBANC) usecase." doesn't quite make sense.
>
> Afaiu in patch 3 you removed the HDCP code for !DBANC, and here you add
> it for DBANC.
>
> Correct me if I'm wrong, but HDCP support for DBANC is currently broken
> in upstream, as the HDCP code is only for the !DBANC case?
>
> Isn't this patch then similar to patch 2, which is a fix for a missing
> feature with DBANC? So should this also be a fix, and perhaps be moved
> as after patch 2?
>
> Tomi
>
Hi Tomi,
Thanks for the review, yes will update the sequence of patch and move
this after patch 2 and also will add fixes patch.
Regards,
Hari
>> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
>> ---
>> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 23 +++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> index 06ac5c2ee78f..120eb7ffe20c 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> @@ -1962,6 +1962,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>> {
>> struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
>> const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
>> + struct drm_connector_state *old_state, *new_state;
>> + struct drm_atomic_state *state = crtc_state->state;
>> + struct drm_connector *conn = mhdp->connector;
>> + u64 old_cp, new_cp;
>>
>> mutex_lock(&mhdp->link_mutex);
>>
>> @@ -1981,6 +1985,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>> if (mhdp->info)
>> bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
>>
>> + if (conn && mhdp->hdcp_supported) {
>> + old_state = drm_atomic_get_old_connector_state(state, conn);
>> + new_state = drm_atomic_get_new_connector_state(state, conn);
>> + old_cp = old_state->content_protection;
>> + new_cp = new_state->content_protection;
>> +
>> + if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
>> + new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> + new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> + crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
>> + crtc_state->mode_changed = true;
>> + }
>> +
>> + if (!new_state->crtc) {
>> + if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
>> + new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> + }
>> + }
>> +
>> mutex_unlock(&mhdp->link_mutex);
>> return 0;
>> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-11-18 4:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-14 9:45 [PATCH v8 0/6] MHDP8546 fixes related to DBANC usecase Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector earlier in atomic_enable() Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 2/6] drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to drm_bridge_funcs Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 3/6] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer Harikrishna Shenoy
2025-11-12 9:15 ` Tomi Valkeinen
2025-10-14 9:45 ` [PATCH v8 5/6] drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD read/write Harikrishna Shenoy
2025-10-14 9:45 ` [PATCH v8 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check Harikrishna Shenoy
2025-11-12 9:22 ` Tomi Valkeinen
2025-11-18 4:54 ` Harikrishna shenoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®