[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <082379e2-2d00-4f2f-82dd-cff93d2867af@ideasonboard.com>
Date: Fri, 21 Nov 2025 14:02:18 +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 RESEND v9 5/6] cadence: cdns-mhdp8546*: Change
drm_connector from structure to pointer
Hi,
On 20/11/2025 14:14, Harikrishna Shenoy wrote:
> After enabling DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, 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 DRM_BRIDGE_ATTACH_NO_CONNECTOR
> 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
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag set while maintaining all
> connector-dependent functionality in the driver.
Now the code looks fine, but you didn't update the description, which
now looks to be quite wrong for this.
Tomi
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
> Signed-off-by: Harikrishna Shenoy <h-shenoy@...com>
> ---
> .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 14 +++++++-------
> .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.h | 3 +--
> .../gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 16 ++++++++--------
> 3 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 816d5d87b45fe..002b4be3de674 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);
> }
>
> @@ -1964,7 +1964,7 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *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_ptr;
> + struct drm_connector *conn = mhdp->connector;
> u64 old_cp, new_cp;
>
> mutex_lock(&mhdp->link_mutex);
> @@ -2179,7 +2179,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 a76775c768956..b297db53ba283 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 5ac2fad2f0078..1d433ad3fe878 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> @@ -393,9 +393,9 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device *mhdp)
> {
> int ret;
>
> - if (mhdp->connector_ptr) {
> + if (mhdp->connector) {
> dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
> - mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
> + mhdp->connector->name, mhdp->connector->base.id);
> }
>
> ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
> @@ -445,10 +445,10 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
> if (!ret && hdcp_port_status & HDCP_PORT_STS_AUTH)
> goto out;
>
> - if (mhdp->connector_ptr) {
> + if (mhdp->connector) {
> dev_err(mhdp->dev,
> "[%s:%d] HDCP link failed, retrying authentication\n",
> - mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
> + mhdp->connector->name, mhdp->connector->base.id);
> }
>
> ret = _cdns_mhdp_hdcp_disable(mhdp);
> @@ -494,16 +494,16 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct *work)
> struct drm_device *dev = NULL;
> struct drm_connector_state *state;
>
> - if (mhdp->connector_ptr)
> - dev = mhdp->connector_ptr->dev;
> + if (mhdp->connector)
> + dev = mhdp->connector->dev;
>
> if (!dev)
> return;
>
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> mutex_lock(&mhdp->hdcp.mutex);
> - if (mhdp->connector_ptr && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> - state = mhdp->connector_ptr->state;
> + if (mhdp->connector && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> + state = mhdp->connector->state;
> state->content_protection = mhdp->hdcp.value;
> }
> mutex_unlock(&mhdp->hdcp.mutex);
Powered by blists - more mailing lists