From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:57256 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726008AbeIESb7 (ORCPT ); Wed, 5 Sep 2018 14:31:59 -0400 From: Laurent Pinchart To: Geert Uytterhoeven Cc: Laurent Pinchart , DRI Development , Linux-Renesas Subject: Re: [PATCH 05/16] drm: rcar-du: lvds: D3/E3 support Date: Wed, 05 Sep 2018 17:01:43 +0300 Message-ID: <4417675.15G9f8jK7e@avalon> In-Reply-To: References: <20180904121027.24031-1-laurent.pinchart+renesas@ideasonboard.com> <20180904121027.24031-6-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Geert, On Tuesday, 4 September 2018 17:29:29 EEST Geert Uytterhoeven wrote: > On Tue, Sep 4, 2018 at 2:10 PM Laurent Pinchart wrote: > > The LVDS encoders in the D3 and E3 SoCs differ significantly from those > > in the other R-Car Gen3 family members: > > > > - The LVDS PLL architecture is more complex and requires computing PLL > > parameters manually. > > > > - The PLL uses external clocks as inputs, which need to be retrieved > > from DT. > > > > - In addition to the different PLL setup, the startup sequence has > > changed *again* (seems someone had trouble making his/her mind). > > > > Supporting all this requires DT bindings extensions for external clocks, > > brand new PLL setup code, and a few quirks to handle the differences in > > the startup sequence. > > > > The implementation doesn't support all hardware features yet, namely > > > > - Using the LV[01] clocks generated by the CPG as PLL input. > > - Providing the LVDS PLL clock to the DU for use with the RGB output. > > > > Those features can be added later when the need will arise. > > > > Signed-off-by: Laurent Pinchart > > > > Thanks for your patch! > > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > > > +static void rcar_lvds_d3_e3_pll_calc(struct rcar_lvds *lvds, struct clk > > *clk, + unsigned long target, struct > > pll_info *pll) +{ > > > > +#if defined(CONFIG_DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) > > + { > > As this code has no dependencies, you can improve compile coverage: > > if (IS_ENABLED(...) || ...) { > > BTW, was CONFIG_DEBUG intended? Or just DEBUG? It should have been just DEBUG, yes. I was trying to guard against dev_dbg() being defined as a no-op and gcc throwing unused variables warnings, but it looks like that has never been an issue in the first place, so I'll drop the guard. -- Regards, Laurent Pinchart