| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0aabf4ae-2877-409c-a047-5286085bd9a3@ideasonboard.com>
Date: Wed, 12 Nov 2025 11:15:25 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Harikrishna Shenoy <h-shenoy@...com>
Cc: Laurent.pinchart@...asonboard.com, airlied@...il.com,
andrzej.hajda@...el.com, andy.yan@...k-chips.com, aradhya.bhatia@...ux.dev,
devarsht@...com, dianders@...omium.org, dri-devel@...ts.freedesktop.org,
javierm@...hat.com, jernej.skrabec@...il.com, jonas@...boo.se,
linux-kernel@...r.kernel.org, linux@...blig.org, luca.ceresoli@...tlin.com,
lumag@...nel.org, lyude@...hat.com, maarten.lankhorst@...ux.intel.com,
mordan@...ras.ru, mripard@...nel.org, neil.armstrong@...aro.org,
rfoss@...nel.org, s-jain1@...com, simona@...ll.ch, tzimmermann@...e.de,
u-kumar1@...com
Subject: Re: [PATCH v8 4/6] drm/bridge: cadence: cdns-mhdp8546*: Change
drm_connector from structure to pointer
Hi,
On 14/10/2025 12:45, Harikrishna Shenoy wrote:
> From: Jayesh Choudhary <j-choudhary@...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@....qualcomm.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
> Signed-off-by: Harikrishna Shenoy <h-shenoy@...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);
Powered by blists - more mailing lists