From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm/panel: auo novatek 1080p video mode panel Date: Mon, 17 Aug 2015 14:07:10 +0200 Message-ID: <20150817120708.GJ8453@ulmo.nvidia.com> References: <1437507362-20162-1-git-send-email-robdclark@gmail.com> <20150807131931.GA27639@ulmo> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cDtQGJ/EJIRf/Cpq" Return-path: Received: from mail-pa0-f48.google.com ([209.85.220.48]:35762 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750937AbbHQMIR (ORCPT ); Mon, 17 Aug 2015 08:08:17 -0400 Received: by pacgr6 with SMTP id gr6so107542568pac.2 for ; Mon, 17 Aug 2015 05:08:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Rob Clark Cc: "dri-devel@lists.freedesktop.org" , linux-arm-msm , Bjorn Andersson --cDtQGJ/EJIRf/Cpq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 07, 2015 at 12:11:31PM -0400, Rob Clark wrote: > On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding = wrote: > > On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote: [...] > >> +static int auo_panel_on(struct auo_panel *auo) > >> +{ > >> + struct mipi_dsi_device *dsi =3D auo->dsi; > >> + int ret; > >> + > >> + dsi->mode_flags |=3D MIPI_DSI_MODE_LPM; > > > > This is weird. > > > >> + ret =3D mipi_dsi_dcs_set_display_on(dsi); > >> + if (ret < 0) > >> + return ret; > >> + > >> + msleep(40); > >> + > >> + return 0; > >> +} > >> + > >> +static int auo_panel_off(struct auo_panel *auo) > >> +{ > >> + struct mipi_dsi_device *dsi =3D auo->dsi; > >> + int ret; > >> + > >> + dsi->mode_flags &=3D ~MIPI_DSI_MODE_LPM; > > > > And this even more. Doesn't the panel work when you simply send > > everything in low-power mode? >=20 > I wouldn't expect low power mode to have enough bandwidth for the > video signal.. but otoh it seems like I need to use lpm for > power-on/off sequence. Maybe we should wrap that up in a helper to > enable/disable lpm? But that seemed a bit overkill. I think there's a misunderstanding, which arguable might stem from a lack of documentation. The intention for MIPI_DSI_MODE_LPM was to be used in conjunction with "host-driven" command mode. Perhaps I should elaborate on the vocabulary here: Tegra supports two types of command mode: "host-driven" and "DC-driven". Host driven command mode is used to perform panel setup (using DCS and vendor- specific commands). "DC-driven" command mode is used to update the framebuffer using write_memory_start and write_memory_continue DCS commands directly generated by the DSI controller. In the latter case you'd obviously want to run in high-speed mode to achieve the throughput necessary to drive you panel at the requested resolution and framerate. In the former your device should be able to receive command in both high speed and low power modes. However some hardware is known not to work with high speed command transmission. MIPI_DSI_MODE_LPM is targetted at these cases, so that display drivers know not to attempt high-speed transmission of initial command packets. Note how MIPI_DSI_MODE_LPM translates to MIPI_DSI_MSG_USE_LPM when transferring messages (see mipi_dsi_device_transfer()). Looking at the comment for the MIPI_DSI_MODE_LPM definition I realize that it isn't very precise, but I have trouble coming up with anything better. Perhaps: /* transmit command messages (non-video data) in low power mode */ #define MIPI_DSI_MODE_LPM BIT(11) Any ideas? On a semi-related note, some of the other flags are rather badly documented. I do see that both Exynos and MSM implement most of these (specifically the MIPI_DSI_MODE_VIDEO_H* ones), perhaps the comments for all of those should be revisited. Ideally they'd be annotated with a reference to the spec, like we do for MIPI_DSI_CLOCK_NON_CONTINUOUS. Thierry --cDtQGJ/EJIRf/Cpq Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJV0c5pAAoJEN0jrNd/PrOhqO4QAIIO5IWg31+LYUbIEuAkcrhx mQiN1CqcTyVSOrtg7TLD8/zdCBtE2XguwtvjgXO21OK6ZdoWTo26jrbU7wyxt3db 5JZ25d13aB54uVKG1VXpdhMrdOg0i+PE+xOoRRbqyS82vBXjLR6KPFULufOIfgiJ tUXohlsfXm2O1H+p6HwtdH+Zsvrj6YsaHrYylehfFpC7+LY+hn6rPIchVRZmqv5s kwSorW8hp/gqh6I0eeuMBz/gGkqO45SogC0GvYmy6YZVWnhVmeIn7GvxOrOCM+Q0 p4hF3WDvzUtP8NZQ9E985kC5mq0FRo2JuARGZMr7VukJ6z8gI4YSmFlKn9Na9jY7 T+VK6+EPDVYDcERpOZ/vpHLdRYPJxLMWKdi/klbbuRqzjZhhLPGlN33JHEVwsVah eh97sC8XTu+IddZk9mC7+aBpIJpepQBOP1/+bEMmW5JJPHedP+O3KnTn8JZt1cXp cUmp6Ip5QfkUrTLhZ6lumyApprrW2dQZA5ox3QW/rQeW6a2Pwe3zEiY8WwCLeDXZ ws6lW0CY1+67ivFM07I2tGPNyu765MqNI/6+domKuPspPB9Qj4bw5u6EnDVTMtRC 8z4XneTcOdilIDgvI7x99CCbP5C89F08oay9walMpy8jVXtwl+Dh/eHZjBk+hpDX ZetqV+kN/wsaA8s6KdER =9b+2 -----END PGP SIGNATURE----- --cDtQGJ/EJIRf/Cpq--