From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04D21C433DB for ; Mon, 1 Mar 2021 10:57:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BAEB064EBA for ; Mon, 1 Mar 2021 10:57:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232455AbhCAK52 (ORCPT ); Mon, 1 Mar 2021 05:57:28 -0500 Received: from lb2-smtp-cloud9.xs4all.net ([194.109.24.26]:44497 "EHLO lb2-smtp-cloud9.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232203AbhCAK5U (ORCPT ); Mon, 1 Mar 2021 05:57:20 -0500 Received: from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d]) by smtp-cloud9.xs4all.net with ESMTPA id GgE0lCjqUC40pGgE3lhfts; Mon, 01 Mar 2021 11:56:32 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s2; t=1614596192; bh=X/+Eyi00WPvKMSECicb3RzcDT/xWAIhYgln3dL3QJtg=; h=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From: Subject; b=OTLNkhirsaEo9Fptb/KOGIrsTEpfdgqj4+DA1ExVmF29XtDRwh/r49zC17PrD5vX4 vLYgoV/mAQ3BEte1XgZkloo+hnZvDUJ/gubbv2d/B4BM8URdnZX60whHVaVb9/qMOH yXKE5DNQ3sRpTWrOGy/fnofQbxMA3BE8r7NP8onubzsjujrlwvPreZHwvf2GWx+5ep hfJ59sVFnbZxVkzr9u/BQjegt+5REya2tGADjztE7mjBymShwm5Y2Ot7zN5j2ljgUl 6tFd+Ilmm5RSZdkE0oEvXoM5xCyfys4IZwUKCOxYev6bWueOvomq1W4nz4b1bl1BVg 4nZw5dnufy9/Q== Subject: Re: [PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops To: Laurent Pinchart Cc: linux-media@vger.kernel.org, Tomi Valkeinen , dri-devel@lists.freedesktop.org, linux-omap@vger.kernel.org, Sekhar Nori , Tony Lindgren References: <20210211103703.444625-1-hverkuil-cisco@xs4all.nl> <20210211103703.444625-2-hverkuil-cisco@xs4all.nl> From: Hans Verkuil Message-ID: <663ec8d4-eb1a-a6ed-056f-cc51a544e6a7@xs4all.nl> Date: Mon, 1 Mar 2021 11:56:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4xfCB5yZR8/GRlwKkaZLMHSpy3X+eQIjc84djTeI3exsO02J3VgjLPE+XiAReoFVKqwEvev5J0pE/T1J2EOMJTmoCZugpBZmWAxRHGU2k1ic8X6eCWXvmM y5wVEuoRuAVpSfck5TTE+wa1wWJvuvSz7c0FYZAh0EoAIlVVFq+ER+oCpgdPqPK2PJXTw1ba+0qcsRUHV6qXBaDIlF8KysNVAYlBLVGGTNw6T4F6P7noyzoG 9imS4GsXUUyUao7hNe9gVUKpP6ennyw9nujB/TkxNVDpn/SMI4d3V+6v2Uwi6TdRnze+i5273Zb1t0tmN6OptTUjo0qDwCjglDup7J0IiEDNrhgsFR4LkEP7 BwsZGhBy4gG05AJJsyN7QSgLB56ujoyN+TEaBTuoXmkkMo5ytNZmNyj4RhlTz+VAlos+QMdD Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On 19/02/2021 13:02, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Thu, Feb 11, 2021 at 11:36:59AM +0100, Hans Verkuil wrote: >> Add bridge cec_init/exit ops. These ops will be responsible for >> creating and destroying the CEC adapter for the bridge that supports >> CEC. >> >> Signed-off-by: Hans Verkuil >> --- >> drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++ >> include/drm/drm_bridge.h | 31 ++++++++++++++++++++++++++ >> 2 files changed, 54 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c >> index 791379816837..2ff90f5e468c 100644 >> --- a/drivers/gpu/drm/drm_bridge_connector.c >> +++ b/drivers/gpu/drm/drm_bridge_connector.c >> @@ -84,6 +84,13 @@ struct drm_bridge_connector { >> * connector modes detection, if any (see &DRM_BRIDGE_OP_MODES). >> */ >> struct drm_bridge *bridge_modes; >> + /** >> + * @bridge_cec: >> + * >> + * The last bridge in the chain (closest to the connector) that provides >> + * cec adapter support, if any (see &DRM_BRIDGE_OP_CEC). >> + */ >> + struct drm_bridge *bridge_cec; >> }; >> >> #define to_drm_bridge_connector(x) \ >> @@ -204,6 +211,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector) >> struct drm_bridge_connector *bridge_connector = >> to_drm_bridge_connector(connector); >> >> + if (bridge_connector->bridge_cec) { >> + struct drm_bridge *cec = bridge_connector->bridge_cec; >> + >> + cec->funcs->cec_exit(cec); >> + } >> if (bridge_connector->bridge_hpd) { >> struct drm_bridge *hpd = bridge_connector->bridge_hpd; >> >> @@ -352,6 +364,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, >> bridge_connector->bridge_detect = bridge; >> if (bridge->ops & DRM_BRIDGE_OP_MODES) >> bridge_connector->bridge_modes = bridge; >> + if (bridge->ops & DRM_BRIDGE_OP_CEC) >> + bridge_connector->bridge_cec = bridge; >> >> if (!drm_bridge_get_next_bridge(bridge)) >> connector_type = bridge->type; >> @@ -374,6 +388,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, >> else if (bridge_connector->bridge_detect) >> connector->polled = DRM_CONNECTOR_POLL_CONNECT >> | DRM_CONNECTOR_POLL_DISCONNECT; >> + if (bridge_connector->bridge_cec) { >> + struct drm_bridge *bridge = bridge_connector->bridge_cec; >> + int ret = bridge->funcs->cec_init(bridge, connector); >> + >> + if (ret) { >> + drm_bridge_connector_destroy(connector); >> + return ERR_PTR(ret); >> + } >> + } >> >> return connector; >> } >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index 2195daa289d2..4c83c2657e87 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -629,6 +629,30 @@ struct drm_bridge_funcs { >> * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. >> */ >> void (*hpd_disable)(struct drm_bridge *bridge); >> + >> + /** >> + * @cec_init: >> + * >> + * Initialize the CEC adapter. >> + * >> + * This callback is optional and shall only be implemented by bridges >> + * that support a CEC adapter. Bridges that implement it shall also >> + * implement the @cec_exit callback and set the DRM_BRIDGE_OP_CEC flag >> + * in their &drm_bridge->ops. >> + */ >> + int (*cec_init)(struct drm_bridge *bridge, struct drm_connector *conn); >> + >> + /** >> + * @cec_exit: >> + * >> + * Terminate the CEC adapter. >> + * >> + * This callback is optional and shall only be implemented by bridges >> + * that support a CEC adapter. Bridges that implement it shall also >> + * implement the @cec_init callback and set the DRM_BRIDGE_OP_CEC flag >> + * in their &drm_bridge->ops. >> + */ >> + void (*cec_exit)(struct drm_bridge *bridge); > > These are very ad-hoc operations. Would it make sense to have something > that could also be reused for other type of intiialization and cleanup > that require access to the drm_connector ? I do not have a very strong opinion, to be honest. How about this: /** * @DRM_BRIDGE_OP_CONN: The bridge can do additional work when * a drm_connector is created or destroyed, such as creating or * removing a CEC adapter. * &drm_bridge_funcs->conn_init and &drm_bridge_funcs->conn_exit * callbacks. */ DRM_BRIDGE_OP_CONN = BIT(4), Would that work better for you? Regards, Hans > >> }; >> >> /** >> @@ -698,6 +722,13 @@ enum drm_bridge_ops { >> * this flag shall implement the &drm_bridge_funcs->get_modes callback. >> */ >> DRM_BRIDGE_OP_MODES = BIT(3), >> + /** >> + * @DRM_BRIDGE_OP_CEC: The bridge supports a CEC adapter. >> + * Bridges that set this flag shall implement the >> + * &drm_bridge_funcs->cec_init and &drm_bridge_funcs->cec_exit >> + * callbacks. >> + */ >> + DRM_BRIDGE_OP_CEC = BIT(4), >> }; >> >> /** >