From patchwork Thu Jul 6 14:19:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9829259 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D4A5A60317 for ; Fri, 7 Jul 2017 00:43:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C53801FF82 for ; Fri, 7 Jul 2017 00:43:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B8D3F28544; Fri, 7 Jul 2017 00:43:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 71FD31FF82 for ; Fri, 7 Jul 2017 00:43:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 13E666E6AE; Fri, 7 Jul 2017 00:43:10 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by gabe.freedesktop.org (Postfix) with ESMTPS id BDAD66E031 for ; Thu, 6 Jul 2017 14:19:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=DwfRCE/rO2VIeRDdqxeiAsmLLzDlN7HYT25zVTn0O8k=; b=U521BrGiF72/9zfRiPvgWHPrH4GFs+MN+UL0xfxu9Thn/6BPUZ/szg/wci7DOsY92MKO1kQRVZTtpBcMNCj4XQNC3MQXxLNvT91urHMPbFWGL9EG0zDcrwAKHSah/GAdXWvXfp+LHBQrxmO3iWEoULlbdKnOoXtD2pwfHivPv50=; Received: from n2100.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:37826) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1dT7dD-00078T-P8; Thu, 06 Jul 2017 15:19:49 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1dT7dA-0007pP-Gm; Thu, 06 Jul 2017 15:19:44 +0100 Date: Thu, 6 Jul 2017 15:19:44 +0100 From: Russell King - ARM Linux To: Neil Armstrong Subject: Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support Message-ID: <20170706141943.GA30053@n2100.armlinux.org.uk> References: <1499337186-24429-1-git-send-email-narmstrong@baylibre.com> <20170706110553.GJ4902@n2100.armlinux.org.uk> <185aa90b-a1b2-d899-40bb-7d333f584bb5@baylibre.com> <20170706114555.GK4902@n2100.armlinux.org.uk> <20170706115643.GA24728@n2100.armlinux.org.uk> <20170706133841.GA28170@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170706133841.GA28170@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-Mailman-Approved-At: Fri, 07 Jul 2017 00:43:09 +0000 Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-amlogic@lists.infradead.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jul 06, 2017 at 02:38:41PM +0100, Russell King - ARM Linux wrote: > On Thu, Jul 06, 2017 at 12:56:43PM +0100, Russell King - ARM Linux wrote: > > On Thu, Jul 06, 2017 at 12:45:55PM +0100, Russell King - ARM Linux wrote: > > > Well, from what I can see in 4.12, the cec-notifier stuff is rather > > > broken (tda998x has stopped working as its stuck with a physical > > > address of f.f.f.f) so I think the whole thing is rather moot right > > > now. I don't yet know what's going on with that, other than the > > > notifier stuff seems to not be working, despite being enabled in > > > the .config. > > > > The problem there appears to be the changes that were made with the > > way the config works - which IMHO are totally broken. > > > > Let's take this scenario: > > > > - You have a HDMI bridge, and you build that into the kernel, because you > > want the display to come up early. > > - You have a CEC driver, which you build as a module. > > > > If the HDMI bridge driver selects CEC_NOTIFIER and the CEC driver selects > > both CEC_NOTIFIER and CEC_CORE, you end up with CEC_NOTIFIER=y and > > CEC_CORE=m. > > > > We now come to this: > > > > #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER) > > > > The definition of IS_REACHABLE() is that it is false if the config symbol > > is selected as a module. So, in this case, we end up compiling out all > > the CEC notifier functions from the HDMI bridge, and building them into > > the CEC driver. > > > > The CEC notifier also gets built as a module, meaning that there's no way > > for the built-in HDMI bridge could ever call the notifier. > > > > The overall result of this is that such a configuration completely breaks > > such a setup - a setup that worked fine before the CEC Kconfig changes. > > > > This isn't limited to tda998x - I'd expect the same to be true of dw-hdmi. > > Fixing this so cec-notifier is built-in isn't sufficient, because we > also need the cec-edid parsing code as well, which is currently part > of cec-core, and that function gets stubbed out if cec-core is not > built-in... > > The patch below works for me. I missed the include/media changes... drivers/media/Makefile | 2 +- drivers/media/cec/Makefile | 6 ++--- drivers/media/cec/cec-core.c | 2 +- include/media/cec-notifier.h | 2 +- include/media/cec.h | 54 ++++++++++++++++++++++++-------------------- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/drivers/media/Makefile b/drivers/media/Makefile index 044503aa8801..0c02fbe4b9c7 100644 --- a/drivers/media/Makefile +++ b/drivers/media/Makefile @@ -24,7 +24,7 @@ obj-$(CONFIG_DVB_CORE) += dvb-core/ # There are both core and drivers at RC subtree - merge before drivers obj-y += rc/ -obj-$(CONFIG_CEC_CORE) += cec/ +obj-y += cec/ # # Finally, merge the drivers that require the core diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile index eaf408e64669..58394b77a328 100644 --- a/drivers/media/cec/Makefile +++ b/drivers/media/cec/Makefile @@ -1,7 +1,7 @@ cec-objs := cec-core.o cec-adap.o cec-api.o cec-edid.o -ifeq ($(CONFIG_CEC_NOTIFIER),y) - cec-objs += cec-notifier.o -endif +obj-$(CONFIG_CEC_NOTIFIER) += cec-notifier.o cec-edid.o + +cec-objs := $(filter-out $(obj-y) $(obj-m), $(cec-objs)) obj-$(CONFIG_CEC_CORE) += cec.o diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c index 2f87748ba4fc..bce26b94c348 100644 --- a/drivers/media/cec/cec-core.c +++ b/drivers/media/cec/cec-core.c @@ -187,7 +187,7 @@ static void cec_devnode_unregister(struct cec_devnode *devnode) put_device(&devnode->dev); } -#ifdef CONFIG_CEC_NOTIFIER +#if IS_ENABLED(CONFIG_CEC_NOTIFIER) static void cec_cec_notify(struct cec_adapter *adap, u16 pa) { cec_s_phys_addr(adap, pa, false); diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h index 298f996969df..83bdc221d0a0 100644 --- a/include/media/cec-notifier.h +++ b/include/media/cec-notifier.h @@ -29,7 +29,7 @@ struct edid; struct cec_adapter; struct cec_notifier; -#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER) +#if IS_ENABLED(CONFIG_CEC_NOTIFIER) /** * cec_notifier_get - find or create a new cec_notifier for the given device. diff --git a/include/media/cec.h b/include/media/cec.h index 201f060978da..039aad98462d 100644 --- a/include/media/cec.h +++ b/include/media/cec.h @@ -225,6 +225,36 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt); void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg); +#if IS_ENABLED(CONFIG_CEC_NOTIFIER) +void cec_register_cec_notifier(struct cec_adapter *adap, + struct cec_notifier *notifier); +#endif + +#else + +static inline int cec_register_adapter(struct cec_adapter *adap, + struct device *parent) +{ + return 0; +} + +static inline void cec_unregister_adapter(struct cec_adapter *adap) +{ +} + +static inline void cec_delete_adapter(struct cec_adapter *adap) +{ +} + +static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, + bool block) +{ +} + +#endif + +#if IS_REACHABLE(CONFIG_CEC_CORE) || IS_REACHABLE(CONFIG_CEC_NOTIFIER) + /** * cec_get_edid_phys_addr() - find and return the physical address * @@ -300,32 +330,8 @@ u16 cec_phys_addr_for_input(u16 phys_addr, u8 input); */ int cec_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port); -#ifdef CONFIG_CEC_NOTIFIER -void cec_register_cec_notifier(struct cec_adapter *adap, - struct cec_notifier *notifier); -#endif - #else -static inline int cec_register_adapter(struct cec_adapter *adap, - struct device *parent) -{ - return 0; -} - -static inline void cec_unregister_adapter(struct cec_adapter *adap) -{ -} - -static inline void cec_delete_adapter(struct cec_adapter *adap) -{ -} - -static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, - bool block) -{ -} - static inline u16 cec_get_edid_phys_addr(const u8 *edid, unsigned int size, unsigned int *offset) {