| 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: <0ad1e053-d655-4b57-a6d1-d74b560aa1fc@ti.com>
Date: Tue, 2 Sep 2025 13:58:08 +0530
From: Harikrishna Shenoy <h-shenoy@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC: <jonas@...boo.se>, <jernej.skrabec@...il.com>,
<maarten.lankhorst@...ux.intel.com>, <tzimmermann@...e.de>,
<airlied@...il.com>, <simona@...ll.ch>, <lyude@...hat.com>,
<luca.ceresoli@...tlin.com>, <viro@...iv.linux.org.uk>,
<andy.yan@...k-chips.com>, <linux@...blig.org>, <javierm@...hat.com>,
<linux-kernel@...r.kernel.org>, <devarsht@...com>,
<j-choudhary@...com>, <u-kumar1@...com>, <s-jain1@...com>,
<andrzej.hajda@...el.com>, <neil.armstrong@...aro.org>,
<rfoss@...nel.org>, <Laurent.pinchart@...asonboard.com>,
<mripard@...nel.org>, <lumag@...nel.org>, <dianders@...omium.org>,
<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v5 1/6] drm/bridge: cadence: cdns-mhdp8546-core: Remove
legacy support for connector initialisation in bridge
On 9/1/25 15:22, Tomi Valkeinen wrote:
> Hi,
>
> On 11/08/2025 10:58, Harikrishna Shenoy wrote:
>> From: Jayesh Choudhary <j-choudhary@...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.
>>
>> Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
> You have a fixes tag here. What bug does this fix?
https://lore.kernel.org/all/05948e1c-fa08-4aca-b705-b2e3a228f758@ti.com/
will include explanation in cover-letter.
>> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
>> ---
>> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 187 +-----------------
>> 1 file changed, 10 insertions(+), 177 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> index a614d1384f71..08702ade2903 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.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,114 +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;
>> - }
>> -
>> - 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)
>> @@ -1671,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);
>> @@ -2368,17 +2208,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.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);
> We don't check the return value anymore... This function is void, so we
> can't propagate the error further. We could change
> cdns_mhdp_update_link_status to return void, but maybe it's better to
> catch the error here, and print an error.
>
> Tomi
>
will print a warn about return value in hpd_work function.
>> +
>> + drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>> }
>>
>> static int cdns_mhdp_probe(struct platform_device *pdev)
Powered by blists - more mailing lists