Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752336AbdLNDnW (ORCPT ); Wed, 13 Dec 2017 22:43:22 -0500 Received: from smtp.csie.ntu.edu.tw ([140.112.30.61]:36502 "EHLO smtp.csie.ntu.edu.tw" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751917AbdLNDnU (ORCPT ); Wed, 13 Dec 2017 22:43:20 -0500 X-Google-Smtp-Source: ACJfBosDT0uH/kJOcsXKR9X+v6Am+uRyQTUw3Dsrv4632gOv7wh7Dk1LhZhxLMuQ21wcBZB4ar/bCkd5tsH30ilW9nA= MIME-Version: 1.0 In-Reply-To: References: From: Chen-Yu Tsai Date: Thu, 14 Dec 2017 11:42:55 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 08/15] drm/sun4i: Add LVDS support To: Maxime Ripard Cc: Daniel Vetter , David Airlie , Chen-Yu Tsai , dri-devel , linux-kernel , Mark Rutland , Rob Herring , linux-arm-kernel , Priit Laes , Icenowy Zheng , Thomas Petazzoni , Jernej Skrabec , devicetree , Thierry Reding Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4367 Lines: 132 On Thu, Dec 7, 2017 at 11:58 PM, Maxime Ripard wrote: > The TCON supports the LVDS interface to output to a panel or a bridge. > Let's add support for it. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/sun4i/Makefile | 1 +- > drivers/gpu/drm/sun4i/sun4i_lvds.c | 177 ++++++++++++++++++++++- > drivers/gpu/drm/sun4i/sun4i_lvds.h | 18 ++- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 242 +++++++++++++++++++++++++++++- > drivers/gpu/drm/sun4i/sun4i_tcon.h | 29 ++++- > 5 files changed, 465 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/sun4i/sun4i_lvds.c > create mode 100644 drivers/gpu/drm/sun4i/sun4i_lvds.h > [...] > diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.h b/drivers/gpu/drm/sun4i/sun4i_lvds.h > new file mode 100644 > index 000000000000..1b8fad4b82c3 > --- /dev/null > +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.h > @@ -0,0 +1,18 @@ > +/* > + * Copyright (C) 2015 NextThing Co > + * Copyright (C) 2015-2017 Free Electrons > + * > + * Maxime Ripard > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ SPDX? > + > +#ifndef _SUN4I_LVDS_H_ > +#define _SUN4I_LVDS_H_ > + > +int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon); > + > +#endif /* _SUN4I_LVDS_H_ */ > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 46e28ca1f676..777c7348d0cf 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c [...] > @@ -698,6 +873,54 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, > return ret; > } > > + /* > + * This can only be made optional since we've had DT nodes > + * without the LVDS reset properties. > + * > + * If the property is missing, just disable LVDS, and print a > + * warning. > + */ > + tcon->lvds_rst = devm_reset_control_get_optional(dev, "lvds"); > + if (IS_ERR(tcon->lvds_rst)) { > + dev_err(dev, "Couldn't get our reset line\n"); > + return PTR_ERR(tcon->lvds_rst); > + } else if (tcon->lvds_rst) { > + has_lvds_rst = true; > + reset_control_reset(tcon->lvds_rst); > + } else { > + has_lvds_rst = false; > + } > + > + /* > + * This can only be made optional since we've had DT nodes > + * without the LVDS reset properties. > + * > + * If the property is missing, just disable LVDS, and print a > + * warning. > + */ > + if (tcon->quirks->has_lvds_pll) { Care to change these names to match the DT property as well? [...] > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > index bd3ad7684870..23db06cdc461 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h [...] > @@ -149,6 +173,7 @@ struct sun4i_tcon; > > struct sun4i_tcon_quirks { > bool has_channel_1; /* a33 does not have channel 1 */ > + bool has_lvds_pll; /* Can we mux the LVDS clock to a PLL? */ This could then read "Does LVDS clock have parent other than TCON clock?" Otherwise, Reviewed-by: Chen-Yu Tsai Also see my other (late) reply to the previous version. > bool needs_de_be_mux; /* sun6i needs mux to select backend */ > > /* callback to handle tcon muxing options */ > @@ -167,6 +192,9 @@ struct sun4i_tcon { > struct clk *sclk0; > struct clk *sclk1; > > + /* Possible mux for the LVDS clock */ > + struct clk *lvds_pll; > + > /* Pixel clock */ > struct clk *dclk; > u8 dclk_max_div; > @@ -174,6 +202,7 @@ struct sun4i_tcon { > > /* Reset control */ > struct reset_control *lcd_rst; > + struct reset_control *lvds_rst; > > struct drm_panel *panel; > > -- > git-series 0.9.1