From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fllvem-ot04.ext.ti.com (fllvem-ot04.ext.ti.com [198.47.19.246]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CFF0424BCF5 for ; Tue, 27 May 2025 08:42:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.47.19.246 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748335349; cv=none; b=iha3R1ruL9hudWaySKu8Gc4GEJYjWUu/fIl9D7LL9PVzAszv593T4sNrjsVJ26YXCP3MZMozFHkjfu9wfqiwB1R3PqEs+0iXFBHSIw5hOauZ2hduQv0ssaN8Nc0WDMCgFK25H0oJUImZayb0ILX+x5gqXpnZJeptBXuQF0wPI5Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748335349; c=relaxed/simple; bh=rmnoFRpaJtdUkwh1OUQWOVnR+3qLFYMDpGhs2h75mKY=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=C1e6ZlqNPUlwX7cH+UOjOX56xuejmBdRAv7MO5Wb2c4/xSW/FK9bs5NyAbbPKIUB2v8a0d5ejHslh97y8RFDkl/9DF6NTJbg0pKqKoG5hjrzAIEPWG5z8otVKOGgmcEU2w1nTkwpPAZYbZTvHSQHAby3GKArLPMVQEmMXRmCxvQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com; spf=pass smtp.mailfrom=ti.com; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b=bRPUpfUV; arc=none smtp.client-ip=198.47.19.246 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ti.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="bRPUpfUV" Received: from lelvem-sh01.itg.ti.com ([10.180.77.71]) by fllvem-ot04.ext.ti.com (8.15.2/8.15.2) with ESMTP id 54R8fcPE1702550; Tue, 27 May 2025 03:41:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1748335298; bh=uxm6tPLxie47q8JFR/KqL0jy99upZdjfBAJB9Y77QkY=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=bRPUpfUVJylm1BQo6OmDuRfKA9Q1TpK8W0eEdS2mi3YjWcKtVE1LRWytRbkIyylCP k4laGkECXlRKNiRsL+Enn52sEBHobShfn9iG32c3Ys/8A2feFRKCgD8iqOtLn3OnXl +FI+BDBt4JQeQ3LwBFnRGrSfp3mhUyw8G93IboFw= Received: from DFLE112.ent.ti.com (dfle112.ent.ti.com [10.64.6.33]) by lelvem-sh01.itg.ti.com (8.18.1/8.18.1) with ESMTPS id 54R8fc9x2512021 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA256 bits=128 verify=FAIL); Tue, 27 May 2025 03:41:38 -0500 Received: from DFLE113.ent.ti.com (10.64.6.34) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Tue, 27 May 2025 03:41:37 -0500 Received: from lelvem-mr05.itg.ti.com (10.180.75.9) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Tue, 27 May 2025 03:41:37 -0500 Received: from [10.24.72.182] (jayesh-hp-z2-tower-g5-workstation.dhcp.ti.com [10.24.72.182]) by lelvem-mr05.itg.ti.com (8.18.1/8.18.1) with ESMTP id 54R8fUL33286987; Tue, 27 May 2025 03:41:30 -0500 Message-ID: Date: Tue, 27 May 2025 14:11:29 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v2 1/3] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge To: Tomi Valkeinen CC: , , , , , , , , , , , , , , , , , , , , , , , , , References: <20250521073237.366463-1-j-choudhary@ti.com> <20250521073237.366463-2-j-choudhary@ti.com> Content-Language: en-US From: Jayesh Choudhary In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-C2ProcessedOrg: 333ef613-75bf-4e12-a4b1-8e3623f5dcea Hello Tomi, On 27/05/25 13:08, Tomi Valkeinen wrote: > Hi, > > On 21/05/2025 10:32, Jayesh Choudhary wrote: >> 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. >> > > Why is the series RFC? Does it not work? Is there something here you're > not comfortable with? These changes work without any issue. I was a little doubtful about the second patch so kept it as RFC. > >> Signed-off-by: Jayesh Choudhary >> --- >> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 186 +++--------------- >> 1 file changed, 25 insertions(+), 161 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> index b431e7efd1f0..66bd916c2fe9 100644 >> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> @@ -1444,56 +1444,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 +1497,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 +1513,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); >> @@ -2158,6 +2002,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, >> @@ -2172,6 +2035,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) > > Why do you need to add bridge mode_valid() when removing the legacy > non-DRM_BRIDGE_ATTACH_NO_CONNECTOR code? Okay. Then I will add the bridge mode_valid as a separate patch. Warm Regards, Jayesh > > Tomi >