From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 4/5] drm/i915: take power well refs when needed Date: Thu, 17 Oct 2013 16:01:16 +0300 Message-ID: <1382014876.26165.19.camel@intelbox> References: <1381792069-27800-1-git-send-email-jbarnes@virtuousgeek.org> <1381792069-27800-5-git-send-email-jbarnes@virtuousgeek.org> <20131015134044.04cc49de@jbarnes-desktop> <1381921813.22425.13.camel@intelbox> <20131016080843.08cf1f61@jbarnes-desktop> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0422475557==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id A8BDBE6994 for ; Thu, 17 Oct 2013 06:01:19 -0700 (PDT) In-Reply-To: <20131016080843.08cf1f61@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org --===============0422475557== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-YDCyzrqFjrjJAoAdAEKe" --=-YDCyzrqFjrjJAoAdAEKe Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2013-10-16 at 08:08 -0700, Jesse Barnes wrote: > On Wed, 16 Oct 2013 14:10:13 +0300 > Imre Deak wrote: >=20 > > On Tue, 2013-10-15 at 13:40 -0700, Jesse Barnes wrote: > > > On Tue, 15 Oct 2013 16:54:00 -0300 > > > Paulo Zanoni wrote: > > > [...] > > > No that's taken into account here. In __intel_set_mode we take a > > > private ref on the appropriate power well so that we'll preserve stat= e > > > until we do the first crtc_enable. From then on, the ref is tracked > > > there and we drop the private one in __intel_set_mode > > >=20 > > > > > + if (crtc->active) > > > > > + intel_display_power_get(dev, > > > > > + POWER_DOMAIN_PIPE= (crtc->pipe)); > > > > > + > > > >=20 > > > > What about the panel fitter power domains? Sometimes the panel fitt= er > > > > is the thing that makes you require a power well, even though you'r= e > > > > on a pipe that doesn't need it. > > > >=20 > > > > And on Haswell you also have to take into account > > > > TRANSCODER_EDP+PIPE_A versus TRANSCODER_A+PIPE_A, where the first > > > > doesn't need the power well but the second needs it. > > >=20 > > > Yeah I'm still not sure how to handle this in generic code. Maybe th= e > > > power well mapping function Imre added will be enough, but it > > > definitely gets tricky when we look at all the different platforms we > > > have to (and will have to) handle. > >=20 > > Isn't the power domain abstraction a neat idea exactly for the above > > case? Generic code just asks for the domain it needs and doesn't care > > how it maps to power wells on the given platform. So for transcoder_edp > > +pipe_a it'd end up asking for POWER_DOMAIN_PIPE_A and > > POWER_DOMAIN_TRANSCODER_EDP, both of which is a nop on HSW, and for the > > other case POWER_DOMAIN_PIPE_A and POWER_DOMAIN_TRANSCODER_A which woul= d > > enable the power well. You also have the POWER_DOMAIN_PIPE, > > POWER_DOMAIN_TRANSCODER, POWER_DOMAIN_PIPE_PANEL_FITTER helpers already= . >=20 > Yeah I think it can work. I missed your function that takes a crtc > though as well, so we don't end up polluting the generic functions with > TRANSCODER references that don't exist on the Atom platforms for > example. That's the main thing I'm worried about, since as we get more > and more wells I think it'll get easier to get it wrong in the generic > code, if we have to use all the required domains for all platforms > there. Afaics, on VLV for example we'd ask for pipe A/B and transcoder A/B power domains, which is still correct. It's true that there the pipe-transcoder connection is fixed, and so we'll always ask for the same pipe/transcoder power domain pair, but I think it's still ok conceptually. So atm the power domains as defined are platform independent, which is great. If we can't avoid adding a platform specific ones in the future, we could still handle those in platform specific code. --Imre --=-YDCyzrqFjrjJAoAdAEKe Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAABAgAGBQJSX9+cAAoJEORIIAnNuWDFMrAH/1vLVZesIo8DjXMCvL4Nq/52 m/s1XdFVwM48vzfK0JwIWiraQqMfNJag+oqpF47iMEEqxMRk3qJYMbfP2km7zJv+ XmnLAZtpH9b0BgVxiScCyhFsIpwMpfLmx1aQWoKMnfoP5V48ZnvuECWU9jd9r1/r wf83XxZUXQQJdZpnXmHriI98Q6bu3BhhKeMX4ZmOBvFXbhwuqSGtf38gedShmxBN gUf82nygP39SzmD3hVJb62ttlBCuzPAeX/ouPScmeV0wB8UfkymjpNuVd2pEIYwV hjSyrv3tPAAyjupeHhES46X8VN5rYVUirtJUnOinu7IfcACG67xm45iy0VjpXxw= =ME9c -----END PGP SIGNATURE----- --=-YDCyzrqFjrjJAoAdAEKe-- --===============0422475557== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0422475557==--