From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752399AbdBMIIe (ORCPT ); Mon, 13 Feb 2017 03:08:34 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:45857 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752301AbdBMIIc (ORCPT ); Mon, 13 Feb 2017 03:08:32 -0500 Date: Mon, 13 Feb 2017 09:08:27 +0100 From: Maxime Ripard To: Priit Laes Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, Jonathan Liu , Thierry Reding , Russell King , Chen-Yu Tsai , Mark Rutland , Rob Herring , David Airlie , Quentin Schulz , linux-sunxi@googlegroups.com Subject: Re: [PATCH 7/8] drm/sun4i: Add various bits and pieces to enable LVDS support on sun4i Message-ID: <20170213080827.6c6zhr4e6amcwli5@lukather> References: <20170211174405.28395-1-plaes@plaes.org> <20170211174405.28395-8-plaes@plaes.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="w7bnm7jmol4udjdw" Content-Disposition: inline In-Reply-To: <20170211174405.28395-8-plaes@plaes.org> User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --w7bnm7jmol4udjdw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Feb 11, 2017 at 07:44:04PM +0200, Priit Laes wrote: > TODO: We still rely on u-boot for lvds reset bit setup :( That needs to be figured out before merging :/ You also have a number of checkpatch warnings / errors that needs to be fixed. >=20 > Signed-off-by: Priit Laes > --- > drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 ++++++++++++++++++++ > drivers/gpu/drm/sun4i/sun4i_tcon.c | 54 ++++++++++++++++++++++++++++++++= ------ > drivers/gpu/drm/sun4i/sun4i_tcon.h | 15 +++++++++++ > 3 files changed, 90 insertions(+), 8 deletions(-) >=20 > diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/s= un4i_lvds.c > index 2ba4705..de738e5 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c > +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c > @@ -114,6 +114,35 @@ static void sun4i_lvds_encoder_enable(struct drm_enc= oder *encoder) > /* encoder->bridge can be NULL; drm_bridge_enable checks for it */ > drm_bridge_enable(encoder->bridge); > =20 > + /* Enable the LVDS */ > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, > + SUN4I_TCON0_LVDS_IF_ENABLE, > + SUN4I_TCON0_LVDS_IF_ENABLE); > + > + /* > + * TODO: SUN4I_TCON0_LVDS_ANA0_REG_C and SUN4I_TCON0_LVDS_ANA0_PD > + * registers span 3 bits, but we only set upper 2 for both > + * of them based on values taken from Allwinner driver. > + */ > + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN4I_TCON0_LVDS_ANA0_CK_EN | > + SUN4I_TCON0_LVDS_ANA0_REG_V | > + SUN4I_TCON0_LVDS_ANA0_REG_C | > + SUN4I_TCON0_LVDS_ANA0_EN_MB | > + SUN4I_TCON0_LVDS_ANA0_PD | > + SUN4I_TCON0_LVDS_ANA0_DCHS); > + > + udelay(2000); > + > + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > + SUN4I_TCON0_LVDS_ANA1_INIT); > + > + udelay(1000); > + > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > + SUN4I_TCON0_LVDS_ANA1_UPDATE, > + SUN4I_TCON0_LVDS_ANA1_UPDATE); > + > sun4i_tcon_channel_enable(tcon, 0); This should be merged in your patch 6. > } > =20 > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/s= un4i_tcon.c > index 71d0087..468a3ce 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -18,6 +18,7 @@ > #include > =20 > #include > +#include > #include > #include > #include > @@ -29,6 +30,7 @@ > #include "sun4i_crtc.h" > #include "sun4i_dotclock.h" > #include "sun4i_drv.h" > +#include "sun4i_lvds.h" > #include "sun4i_rgb.h" > #include "sun4i_tcon.h" > =20 > @@ -169,12 +171,29 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, > SUN4I_TCON0_BASIC2_V_BACKPORCH(bp)); > =20 > /* Set Hsync and Vsync length */ > - hsync =3D mode->crtc_hsync_end - mode->crtc_hsync_start; > - vsync =3D mode->crtc_vsync_end - mode->crtc_vsync_start; > - DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync); > - regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG, > - SUN4I_TCON0_BASIC3_V_SYNC(vsync) | > - SUN4I_TCON0_BASIC3_H_SYNC(hsync)); > + if (type !=3D DRM_MODE_ENCODER_LVDS) { > + // Not needed for LVDS? > + hsync =3D mode->crtc_hsync_end - mode->crtc_hsync_start; > + vsync =3D mode->crtc_vsync_end - mode->crtc_vsync_start; > + DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync); > + regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG, > + SUN4I_TCON0_BASIC3_V_SYNC(vsync) | > + SUN4I_TCON0_BASIC3_H_SYNC(hsync)); > + } This is your patch 5 (and it would be better to put the condition on what we know rather than what we assume, we know that it's working for RGB, but not for anything else). > + > + if (type =3D=3D DRM_MODE_ENCODER_LVDS) { > + /* Setup bit depth */ > + /* TODO: Figure out where to get display bit depth > + * val =3D (1: 18-bit, 0: 24-bit) > + * TODO: Should we set more registers: > + * BIT(28) - LVDS_DIRECTION > + * BIT(27) - LVDS_MODE > + * BIT(23) - LVDS_CORRECT_MODE > + */ > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, > + SUN4I_TCON0_LVDS_IF_BITWIDTH, > + SUN4I_TCON0_LVDS_IF_BITWIDTH); > + } And this in your patch 6 > =20 > /* Setup the polarity of the various signals */ > if (!(mode->flags & DRM_MODE_FLAG_PHSYNC)) > @@ -183,8 +202,15 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, > if (!(mode->flags & DRM_MODE_FLAG_PVSYNC)) > val |=3D SUN4I_TCON0_IO_POL_VSYNC_POSITIVE; > =20 > + > + /* Set proper DCLK phase value */ > + if (type =3D=3D DRM_MODE_ENCODER_LVDS) > + val |=3D SUN4I_TCON0_IO_POL_DCLK_PHASE(1); > + > regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG, > - SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | SUN4I_TCON0_IO_POL_VSYNC_POSIT= IVE, > + SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | > + SUN4I_TCON0_IO_POL_VSYNC_POSITIVE | > + SUN4I_TCON0_IO_POL_DCLK_PHASE_MASK, > val); This is covered by your clk_set_phase already. > =20 > /* Map output pins to channel 0 */ > @@ -480,6 +506,7 @@ static int sun4i_tcon_bind(struct device *dev, struct= device *master, > struct drm_device *drm =3D data; > struct sun4i_drv *drv =3D drm->dev_private; > struct sun4i_tcon *tcon; > + const char *mode; > int ret; > =20 > tcon =3D devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); > @@ -525,7 +552,18 @@ static int sun4i_tcon_bind(struct device *dev, struc= t device *master, > goto err_free_clocks; > } > =20 > - ret =3D sun4i_rgb_init(drm); > + /* Check which output mode is set, defaulting to RGB */ > + ret =3D of_property_read_string(dev->of_node, "mode", &mode); > + > + if (ret || !strcmp(mode, "rgb")) > + ret =3D sun4i_rgb_init(drm); > + else if (!strcmp(mode, "lvds")) > + ret =3D sun4i_lvds_init(drm); > + else { > + dev_err(dev, "Unknown TCON mode: %s\n", mode); > + ret =3D -1; > + } > + > if (ret < 0) > goto err_free_clocks; > =20 > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/s= un4i_tcon.h > index b040e10..dc4e350 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > @@ -69,8 +69,11 @@ > #define SUN4I_TCON0_TTL3_REG 0x7c > #define SUN4I_TCON0_TTL4_REG 0x80 > #define SUN4I_TCON0_LVDS_IF_REG 0x84 > +#define SUN4I_TCON0_LVDS_IF_ENABLE BIT(31) > +#define SUN4I_TCON0_LVDS_IF_BITWIDTH BIT(26) > #define SUN4I_TCON0_IO_POL_REG 0x88 > #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase) ((phase & 3) << 28) > +#define SUN4I_TCON0_IO_POL_DCLK_PHASE_MASK (3 << 28) > #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE BIT(25) > #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE BIT(24) > =20 > @@ -128,6 +131,18 @@ > #define SUN4I_TCON_CEU_RANGE_G_REG 0x144 > #define SUN4I_TCON_CEU_RANGE_B_REG 0x148 > #define SUN4I_TCON_MUX_CTRL_REG 0x200 > +#define SUN4I_TCON0_LVDS_ANA0_REG 0x220 > +#define SUN4I_TCON0_LVDS_ANA0_CK_EN BIT(29) | BIT(28) > +#define SUN4I_TCON0_LVDS_ANA0_REG_V BIT(27) | BIT(26) > +/* TODO: BIT(23) also belongs to ANA0_REG_C register set */ > +#define SUN4I_TCON0_LVDS_ANA0_REG_C BIT(25) | BIT(24) > +#define SUN4I_TCON0_LVDS_ANA0_EN_MB BIT(22) > +/* TODO: BIT(19) also belongs to ANA0_PD register set */ Why don't you set them then? Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --w7bnm7jmol4udjdw Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYoWl3AAoJEBx+YmzsjxAgy/QP/RuX5n//9c4UUe4p3YEDrIHH 6wkq+KO6VHZO9xQize6+VdTR/R0dgNIC3Y8/QY3bflGjbTMBJ+F62Q0SI03Owo92 y4rhQCcQiBzLHpC9h6i6UWsOu2/iCAqQOhYe2i7JE3u6XS9v1Ohfmt3n3/hXmrUz onLVtSeilHaWZKGsDv1ZtzZOqr9CSmqDzqJ432mpv7+Ek3RKX5Eh28w0Lkw0kziO IDKqD70rGPNHelUzYaXcX0yM61FXfTl+U53Kwj9FKVJYNKulyZ0aAe2J53g82G/b R9VmcMTfj3iSqItsk/LRRhSRYFlIIP7OlZgowl7tVOJLI0n0o2puDQK9pi6q5yyg OegZkBcPnH1WgVYNevEYdR2INwyS+lZFYedbS/VV7YXaH6FyXRWFzpsxwCxJiWXd z4/Os24qSR8oPjMeO6wYl2LFqi2JZDA61YYRjEV+/d81i1lKAbMjjIkuZYkWMg2C lQwhgWKBNewuiuh74VIsUDbAhkcJQfNFQpGKYdbHoRaFsRDm5QOxp7pM8wlI3A2I ZJfmvoJ+WlrHvwsjvYSsafIZ5ZD7MSTJYqHhyKcSOXQqiOn9MislGh0+oYUMPsrF Z8qgqQ5r40aOLbq3Ck72UpNU2bXXh+ivIWJ2LVmeqcFMzCHqcgslQq5PmOhFparT 6ukUYbBGPn9VawGlnXmq =cv72 -----END PGP SIGNATURE----- --w7bnm7jmol4udjdw--