From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932231AbbFHOOT (ORCPT ); Mon, 8 Jun 2015 10:14:19 -0400 Received: from mail-qc0-f177.google.com ([209.85.216.177]:35201 "EHLO mail-qc0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbbFHOOL (ORCPT ); Mon, 8 Jun 2015 10:14:11 -0400 Date: Mon, 8 Jun 2015 16:13:41 +0200 From: Thierry Reding To: Heiko Schocher Cc: linux-kernel@vger.kernel.org, David Airlie , dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/panel: add lg4573 driver Message-ID: <20150608141340.GB10354@ulmo.nvidia.com> References: <1430898573-14783-1-git-send-email-hs@denx.de> <20150605121934.GA26656@ulmo.nvidia.com> <55754EA1.40009@denx.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="oC1+HKm2/end4ao3" Content-Disposition: inline In-Reply-To: <55754EA1.40009@denx.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --oC1+HKm2/end4ao3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 08, 2015 at 10:13:21AM +0200, Heiko Schocher wrote: > Hello Thierry, >=20 > Am 05.06.2015 14:19, schrieb Thierry Reding: > >On Wed, May 06, 2015 at 09:49:33AM +0200, Heiko Schocher wrote: [...] > >>+ - display-timings: timings for the connected panel according to [1] > > > >The timings are already implied by the compatible value, so there's no > >need to list them in DT. >=20 > I look into it ... is there an example for a panel driver with fixed > timings? Should I do it like it is done in the > drivers/gpu/drm/panel/panel-simple.c driver? The simple-panel driver actually implements two methods. The old method is to provide a fixed mode. That works fairly well, but primarily because we don't support very many boards in the upstream kernel. For a couple of panels it was shown that the fixed mode doesn't work very well. One of the reasons was that the mode used values that conflicted with the restrictions of the display controller on another SoC. A more future-proof way would be for the driver to expose the display timings. That allows the driver to compute a "default" mode, but also provide the display driver with a full range of valid timings so that an appropriate mode can be computed, taking into account the restrictions of the display hardware. The two Hannstar panels supported by the driver use this mechanism. Perhaps you can use those for reference. > >>+static void lg4573_display_mode_settings(struct lg4573 *ctx) > >>+{ > >>+ static u16 display_mode_settings[] =3D { > >>+ 0x703A, > >[...] > >>+ 0x7200, > >>+ }; > > > >Please make use of the 78/80 columns. Also, I don't suppose it'd be > >possible to obtain symbolic names for these magic numbers? More of the > >same below. >=20 > Fixed ... I try to find out more about this magic numbers, but I > can;t promise it ... It's not usual to get full documentation on these numbers, but we should at least try. > >>+static int lg4573_get_modes(struct drm_panel *panel) > >>+{ > >>+ struct drm_connector *connector =3D panel->connector; > >>+ struct lg4573 *ctx =3D panel_to_lg4573(panel); > >>+ struct drm_display_mode *mode; > >>+ > >>+ mode =3D drm_mode_create(connector->dev); > >>+ if (!mode) { > >>+ DRM_ERROR("failed to create a new display mode\n"); > >>+ return 0; > >>+ } > >>+ > >>+ drm_display_mode_from_videomode(&ctx->vm, mode); > >>+ > >>+ mode->type =3D DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > >>+ drm_mode_probed_add(connector, mode); > >>+ > >>+ return 1; > >>+} > > > >You can either use a hard-coded mode or use display timings along with > >the helpers to convert the timings to a default mode. No need to parse > >the information from DT. >=20 > Ok... see question above, could I do it like it is done in the > panel-simple driver? Or is there another way? Look at the panel_simple_get_fixed_modes() implementation. That computes a default mode from the typical values of the display timings. Alternatively you can also implement display timings support in your display driver and directly use the ->get_timings() callback for the panel you have. Thierry --oC1+HKm2/end4ao3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVdaMRAAoJEN0jrNd/PrOhIOYP/1Bs4m6B4JoBwkKngD8tgUUD yubJYd8JpRwASKgmXzGy6rCg1PVJKrW3WfemFXF5X+gL9+PefhoMTz01HltleiMM kCR4WDoZY8IAKZydsqEsbjhqEPNK5wGiTsNRgOxXcUq+8N7Yv86qw/vPHxJbW8uW Telv54Q5cAupuQswuSG39m2/RE1oRGbPdqO7ZZ+5pZH+xf05qlMpTSbhPEEJNFyo XcrWkN/YonWr2+r6uBc7Wm8va/OH+ClIwFxPGsgo8tD7pm6jn5GfHEkXi97NSk2l uFMuSgTcl8HRWg04CRhjqvAeh0V+i6A2JNDZisRcEnu3RNgA/PdeiRrDR45KbKP0 V/t9+FBcjL0rPeOCWzrPwwaY7lZTXHFRdxl5gCjak1w1I5VpuDRs23sDceUdFH48 DC0V0c1DILd8G8aETPQftvWDuB8/zu2bKu49KDEQ1eZb1/S13idlFxe4CCTnG8k6 gqHNfljLBTkgKxbqttMS2TmwSX1Quxt0MGFgg+4wZnyIAjteA2Jm+Hi957+KwWY5 ffaV5P4d/JNiyveHnqZeeK3ipCAHt1gGjGy8SUu/zuefnlsJkiX950RLiPQ2m8zr AI8ETN6dLlJhjXjhUYKIutOI5eOYl36JpvK37yiPeN8lGXIV9MxbZt4Cpf0CWXKs QynTGM/wlbunPp2onCCK =qqTJ -----END PGP SIGNATURE----- --oC1+HKm2/end4ao3--