From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 1/3] drm/panel: Add display_timing support Date: Tue, 24 Mar 2015 12:34:31 +0100 Message-ID: <20150324113430.GN18115@ulmo.nvidia.com> References: <1418319166-23357-1-git-send-email-p.zabel@pengutronix.de> <20150203133015.GH15068@ulmo.nvidia.com> <1424700272.3623.7.camel@pengutronix.de> <1425383383.3146.53.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1852206066==" Return-path: Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com [209.85.220.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 39C5C6E4CA for ; Tue, 24 Mar 2015 04:34:36 -0700 (PDT) Received: by pabxg6 with SMTP id xg6so210366000pab.0 for ; Tue, 24 Mar 2015 04:34:36 -0700 (PDT) In-Reply-To: <1425383383.3146.53.camel@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Philipp Zabel Cc: Steffen Trumtrar , dri-devel@lists.freedesktop.org, kernel@pengutronix.de List-Id: dri-devel@lists.freedesktop.org --===============1852206066== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xdWF/UuCWMRSqXrg" Content-Disposition: inline --xdWF/UuCWMRSqXrg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 03, 2015 at 12:49:43PM +0100, Philipp Zabel wrote: > Hi Thierry, >=20 > Am Montag, den 23.02.2015, 15:04 +0100 schrieb Philipp Zabel: > > Hi Thierry, > >=20 > > do you have any further thoughts on this? >=20 > [...] > > > Sorry for taking so long to get back on this. I generally like the id= ea, > > > though a couple of things are unclear to me. > > >=20 > > > In struct display_timing we have: > > >=20 > > > struct timing_entry hactive; > > > ... > > > struct timing_entry vactive; > > >=20 > > > I wonder if that really makes sense. Are there really datasheets that > > > provide a valid range for the number of horizontal and vertical active > > > pixels? > >=20 > > I could send a patch to change hactive/vactive to a single value > > and change Documentation/devicetree/bindings/video/display-timing.txt > > to clarify ranges are not allowed for these properties until somebody > > digs out a panel that actually needs this. > > Adding Steffen to Cc: in case there was a reason other than symmetry. >=20 > That seems not to be the case so far. >=20 > [...] > > > > /** > > > > * struct drm_panel_funcs - perform operations on a given panel > > > > @@ -38,6 +39,8 @@ struct drm_panel; > > > > * @enable: enable panel (turn on back light, etc.) > > > > * @get_modes: add modes to the connector that the panel is attach= ed to and > > > > * return the number of modes added > > > > + * @get_timings: copy display timings into the provided array and = return > > > > + * the number of display timings available > > > > * > > > > * The .prepare() function is typically called before the display = controller > > > > * starts to transmit video data. Panel drivers can use this to tu= rn the panel > > > > @@ -68,6 +71,8 @@ struct drm_panel_funcs { > > > > int (*prepare)(struct drm_panel *panel); > > > > int (*enable)(struct drm_panel *panel); > > > > int (*get_modes)(struct drm_panel *panel); > > > > + int (*get_timings)(struct drm_panel *panel, unsigned int num_timi= ngs, > > > > + struct display_timing *timings); > > >=20 > > > Also are there even panels that support more than one set of timings? > > > I've never heard of panels that are actually able to do more than one > > > mode, so I'm wondering if num_timings isn't overdoing it a little her= e. > > > I guess it doesn't hurt all that much, though. > >=20 > > Would you prefer > > struct display_timing *(*get_timing)(struct drm_panel *panel); > > ? >=20 > I'd like to resend this. Please let me know if you want me to change > this function prototype. I have no objections to keeping the current prototype. It's something we can always fixup if we want to. Also keeping the symmetry with min/max values for hactive and vactive is okay in my opinion. Were there any other remaining points? If not I'll just apply this as is. Thierry --xdWF/UuCWMRSqXrg Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVEUvDAAoJEN0jrNd/PrOhAcUP/0vHxmKI7g1KiqyTm2CT7NAv SAr6xuF2qYPruxXSTrlGVETGvF30GZeFVksppSmjdITOKf3prIaPA1xv5OrN2qoT qG6RcW0pNNFq5pDkmm37sbtoVLuWWZS/P5PgsXX/SMRexhZGKigxgoliP8FTcS0F G6fzJJE9h/4kLMxrMbEXdegUfVRcpKHi5biG0Rqp2DizmsxnO4utzOuX2uEB9GEC F2/jYfXkWOFonKwkwi6ZnMS33sEkt/Mrai+bA+hgefvfEX7oxl+0dZbKRgLMmh24 8gK0J4vjsDz7DyzyTaR9Z7jSYrk2GGnJHh0aVnTzYyUJUAg9isbMxcwdxCYEeHpv CFNtH97rRgzr3XN5rHucb220VvwCKdMutV+PP9X3kasn0pAdD91xzz+PVo9ewXXe fd0mWlZPlOkPENefvdfH87ljm/vfIeivy7TrAovno17j/uEjLUoyMX6r0sqWIpvG /LZD/DOaVKsSH1Jx9nn+Bl+uF1F4a3MtvwF5xhEA4U3PcAevT/YE5Nj42cfUwY5y sf0AYWrrFZpyhOrFzDIlZamSkLPAIevyjrujfDVDHr/igaIj2ETRcE1tKEI9ZhIx yoic/Lx8N3FGGPFHzAnDXMa9L0MqRS5Ae8e6ZHdzCWcxd0elwWsXlL1aFAEHc5a4 n/2RbYgN0xA63TDPCC2h =aaZm -----END PGP SIGNATURE----- --xdWF/UuCWMRSqXrg-- --===============1852206066== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1852206066==--