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 16F60C43462 for ; Fri, 16 Apr 2021 07:46:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC1F1611AF for ; Fri, 16 Apr 2021 07:46:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237041AbhDPHqh (ORCPT ); Fri, 16 Apr 2021 03:46:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231898AbhDPHqh (ORCPT ); Fri, 16 Apr 2021 03:46:37 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1EA3BC061574; Fri, 16 Apr 2021 00:46:13 -0700 (PDT) Received: from [192.168.1.111] (91-157-208-71.elisa-laajakaista.fi [91.157.208.71]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BCBE05A5; Fri, 16 Apr 2021 09:46:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1618559170; bh=P7wwiy1ARCZJKs2Gzsnd/CbbG6MTgpoebNlqrb2EiGU=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=vGl2HQqQ6d+qaDSlFxoNL4JTPaQwlXITDJiAMAOMY+J0JwWUXmbYafcnXHvGemzzx erq15XghRnW1hyHPgDs5Zr/tWfnTI8El+9yrE4habSs5L8IQ/qRqpkT1mT6X/HKYvv 2sVY8iONFHBD5NoFAmWxXnt6R5js5/1ipVHnY0AQ= Subject: Re: [PATCHv2 1/6] drm: drm_bridge: add connector_attach/detach bridge ops To: Hans Verkuil , linux-media@vger.kernel.org Cc: Tony Lindgren , Sekhar Nori , dri-devel@lists.freedesktop.org, Laurent Pinchart , linux-omap@vger.kernel.org, devicetree@vger.kernel.org References: <20210302162403.983585-1-hverkuil-cisco@xs4all.nl> <20210302162403.983585-2-hverkuil-cisco@xs4all.nl> From: Tomi Valkeinen Message-ID: <3ba8c7c3-2e86-964d-2e5b-5cdd805def5c@ideasonboard.com> Date: Fri, 16 Apr 2021 10:46:08 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210302162403.983585-2-hverkuil-cisco@xs4all.nl> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Hans, On 02/03/2021 18:23, Hans Verkuil wrote: > Add bridge connector_attach/detach ops. These ops are called when a > bridge is attached or detached to a drm_connector. These ops can be > used to register and unregister an HDMI CEC adapter for a bridge that > supports CEC. > > Signed-off-by: Hans Verkuil > --- > drivers/gpu/drm/drm_bridge_connector.c | 9 +++++++++ > include/drm/drm_bridge.h | 27 ++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..07db71d4f5b3 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -203,6 +203,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector) > { > struct drm_bridge_connector *bridge_connector = > to_drm_bridge_connector(connector); > + struct drm_bridge *bridge; > + > + drm_for_each_bridge_in_chain(bridge_connector->encoder, bridge) > + if (bridge->funcs->connector_detach) > + bridge->funcs->connector_detach(bridge, connector); > > if (bridge_connector->bridge_hpd) { > struct drm_bridge *hpd = bridge_connector->bridge_hpd; > @@ -375,6 +380,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; > > + drm_for_each_bridge_in_chain(encoder, bridge) > + if (bridge->funcs->connector_attach) > + bridge->funcs->connector_attach(bridge, connector); > + > return connector; > } > EXPORT_SYMBOL_GPL(drm_bridge_connector_init); > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 2195daa289d2..3320a6ebd253 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -629,6 +629,33 @@ struct drm_bridge_funcs { > * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. > */ > void (*hpd_disable)(struct drm_bridge *bridge); > + > + /** > + * @connector_attach: > + * > + * This callback is invoked whenever our bridge is being attached to a > + * &drm_connector. This is where an HDMI CEC adapter can be registered. > + * Note that this callback expects that this op always succeeds. Since > + * HDMI CEC support is an optional feature, any failure to register a > + * CEC adapter must be ignored since video output will still work > + * without CEC. > + * Even if CEC support is optional, the callback itself is generic. Wouldn't it be better to make this function return an error, and for CEC, just return 0 if CEC won't get registered correctly? Also, I personally like things to fail if something doesn't go right, instead of continuing, if that thing is never supposed to happen in normal situations. E.g. if CEC registration fails because we're out of memory, I think the op should fail too. Tomi 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=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham 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 E1B41C43460 for ; Fri, 16 Apr 2021 07:46:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4F5D161107 for ; Fri, 16 Apr 2021 07:46:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4F5D161107 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AEDFB6E194; Fri, 16 Apr 2021 07:46:13 +0000 (UTC) Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id BB2006E194 for ; Fri, 16 Apr 2021 07:46:11 +0000 (UTC) Received: from [192.168.1.111] (91-157-208-71.elisa-laajakaista.fi [91.157.208.71]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BCBE05A5; Fri, 16 Apr 2021 09:46:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1618559170; bh=P7wwiy1ARCZJKs2Gzsnd/CbbG6MTgpoebNlqrb2EiGU=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=vGl2HQqQ6d+qaDSlFxoNL4JTPaQwlXITDJiAMAOMY+J0JwWUXmbYafcnXHvGemzzx erq15XghRnW1hyHPgDs5Zr/tWfnTI8El+9yrE4habSs5L8IQ/qRqpkT1mT6X/HKYvv 2sVY8iONFHBD5NoFAmWxXnt6R5js5/1ipVHnY0AQ= Subject: Re: [PATCHv2 1/6] drm: drm_bridge: add connector_attach/detach bridge ops To: Hans Verkuil , linux-media@vger.kernel.org References: <20210302162403.983585-1-hverkuil-cisco@xs4all.nl> <20210302162403.983585-2-hverkuil-cisco@xs4all.nl> From: Tomi Valkeinen Message-ID: <3ba8c7c3-2e86-964d-2e5b-5cdd805def5c@ideasonboard.com> Date: Fri, 16 Apr 2021 10:46:08 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210302162403.983585-2-hverkuil-cisco@xs4all.nl> Content-Language: en-US X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Tony Lindgren , Sekhar Nori , dri-devel@lists.freedesktop.org, Laurent Pinchart , linux-omap@vger.kernel.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Hans, On 02/03/2021 18:23, Hans Verkuil wrote: > Add bridge connector_attach/detach ops. These ops are called when a > bridge is attached or detached to a drm_connector. These ops can be > used to register and unregister an HDMI CEC adapter for a bridge that > supports CEC. > > Signed-off-by: Hans Verkuil > --- > drivers/gpu/drm/drm_bridge_connector.c | 9 +++++++++ > include/drm/drm_bridge.h | 27 ++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..07db71d4f5b3 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -203,6 +203,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector) > { > struct drm_bridge_connector *bridge_connector = > to_drm_bridge_connector(connector); > + struct drm_bridge *bridge; > + > + drm_for_each_bridge_in_chain(bridge_connector->encoder, bridge) > + if (bridge->funcs->connector_detach) > + bridge->funcs->connector_detach(bridge, connector); > > if (bridge_connector->bridge_hpd) { > struct drm_bridge *hpd = bridge_connector->bridge_hpd; > @@ -375,6 +380,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; > > + drm_for_each_bridge_in_chain(encoder, bridge) > + if (bridge->funcs->connector_attach) > + bridge->funcs->connector_attach(bridge, connector); > + > return connector; > } > EXPORT_SYMBOL_GPL(drm_bridge_connector_init); > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 2195daa289d2..3320a6ebd253 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -629,6 +629,33 @@ struct drm_bridge_funcs { > * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. > */ > void (*hpd_disable)(struct drm_bridge *bridge); > + > + /** > + * @connector_attach: > + * > + * This callback is invoked whenever our bridge is being attached to a > + * &drm_connector. This is where an HDMI CEC adapter can be registered. > + * Note that this callback expects that this op always succeeds. Since > + * HDMI CEC support is an optional feature, any failure to register a > + * CEC adapter must be ignored since video output will still work > + * without CEC. > + * Even if CEC support is optional, the callback itself is generic. Wouldn't it be better to make this function return an error, and for CEC, just return 0 if CEC won't get registered correctly? Also, I personally like things to fail if something doesn't go right, instead of continuing, if that thing is never supposed to happen in normal situations. E.g. if CEC registration fails because we're out of memory, I think the op should fail too. Tomi _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel