From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 04/19] drm/i915: move power domain macros to intel_pm.c Date: Mon, 24 Feb 2014 15:54:46 +0200 Message-ID: <1393250086.13131.105.camel@intelbox> References: <1392674540-10915-1-git-send-email-imre.deak@intel.com> <1392674540-10915-5-git-send-email-imre.deak@intel.com> <20140220112136.3fb5f119@jbarnes-desktop> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0823438082==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id CCF03FAA81 for ; Mon, 24 Feb 2014 05:54:49 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Paulo Zanoni Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org --===============0823438082== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-eRrfHX525XZ/GwaKHlD1" --=-eRrfHX525XZ/GwaKHlD1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2014-02-24 at 10:38 -0300, Paulo Zanoni wrote: > 2014-02-20 16:21 GMT-03:00 Jesse Barnes : > > On Tue, 18 Feb 2014 00:02:05 +0200 > > Imre Deak wrote: > > > >> These macros are used only locally, so move them to the .c file. > >> > >> Also since logically the init power domain should be part of all power > >> wells add it to the always-on power wells too for consistency. Since > >> always-on power wells have noop handlers, this doesn't change the > >> functionality. >=20 > This patch should be split in two: one moves to .c, the other changes > the init power domain. Ok. > But I'm a little confused. I always understood that the "power_on" > domains were things that were always enabled on the hardware, so you > couldn't really turn them "off", no matter how many domains you put. > But the init power domain is different: when you get the init domain, > a lot of stuff can get enabled, and when you put it, a lot of stuff > can be disabled. By this logic, your patch would not make sense. So I > guess we probably have two different definitions for what is a > always_on power domain. I think we probably need to add some comments > in the source code to clarify the definitions of things, to avoid > further confusion :) In general getting a power domain guarantees that you can access the relevant registers, so if you get POWER_DOMAIN_INIT it should enable all the HW resources necessary for this. For always-on power wells that won't involve turning on a power well, but it may involve enabling other resources. A good example is the HSW CRT port power domain, which is on the always-on power well, so we don't need to enable any power well for it, but we still need to disable runtime PM. --Imre >=20 > >> > >> Signed-off-by: Imre Deak > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 10 ---------- > >> drivers/gpu/drm/i915/intel_pm.c | 21 +++++++++++++++++++-- > >> 2 files changed, 19 insertions(+), 12 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i9= 15_drv.h > >> index 796f971..de0c0e0 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -121,8 +121,6 @@ enum intel_display_power_domain { > >> POWER_DOMAIN_NUM, > >> }; > >> > >> -#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) > >> - > >> #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A) > >> #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \ > >> ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER) > >> @@ -130,14 +128,6 @@ enum intel_display_power_domain { > >> ((tran) =3D=3D TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ > >> (tran) + POWER_DOMAIN_TRANSCODER_A) > >> > >> -#define HSW_ALWAYS_ON_POWER_DOMAINS ( \ > >> - BIT(POWER_DOMAIN_PIPE_A) | \ > >> - BIT(POWER_DOMAIN_TRANSCODER_EDP)) > >> -#define BDW_ALWAYS_ON_POWER_DOMAINS ( \ > >> - BIT(POWER_DOMAIN_PIPE_A) | \ > >> - BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ > >> - BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) > >> - > >> enum hpd_pin { > >> HPD_NONE =3D 0, > >> HPD_PORT_A =3D HPD_NONE, /* PORT_A is internal */ > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/in= tel_pm.c > >> index db48d55..9a608f1 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -5384,6 +5384,23 @@ void i915_release_power_well(void) > >> } > >> EXPORT_SYMBOL_GPL(i915_release_power_well); > >> > >> +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) > >> + > >> +#define HSW_ALWAYS_ON_POWER_DOMAINS ( \ > >> + BIT(POWER_DOMAIN_PIPE_A) | \ > >> + BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ > >> + BIT(POWER_DOMAIN_INIT)) > >> +#define HSW_DISPLAY_POWER_DOMAINS ( \ > >> + (POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) | \ > >> + BIT(POWER_DOMAIN_INIT)) > >> + > >> +#define BDW_ALWAYS_ON_POWER_DOMAINS ( \ > >> + HSW_ALWAYS_ON_POWER_DOMAINS | \ > >> + BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) > >> +#define BDW_DISPLAY_POWER_DOMAINS ( \ > >> + (POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) | \ > >> + BIT(POWER_DOMAIN_INIT)) > >> + > >> static struct i915_power_well i9xx_always_on_power_well[] =3D { > >> { > >> .name =3D "always-on", > >> @@ -5400,7 +5417,7 @@ static struct i915_power_well hsw_power_wells[] = =3D { > >> }, > >> { > >> .name =3D "display", > >> - .domains =3D POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DO= MAINS, > >> + .domains =3D HSW_DISPLAY_POWER_DOMAINS, > >> .is_enabled =3D hsw_power_well_enabled, > >> .set =3D hsw_set_power_well, > >> }, > >> @@ -5414,7 +5431,7 @@ static struct i915_power_well bdw_power_wells[] = =3D { > >> }, > >> { > >> .name =3D "display", > >> - .domains =3D POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DO= MAINS, > >> + .domains =3D BDW_DISPLAY_POWER_DOMAINS, > >> .is_enabled =3D hsw_power_well_enabled, > >> .set =3D hsw_set_power_well, > >> }, > > > > Reviewed-by: Jesse Barnes > > > > -- > > Jesse Barnes, Intel Open Source Technology Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 >=20 >=20 --=-eRrfHX525XZ/GwaKHlD1 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.14 (GNU/Linux) iQEcBAABAgAGBQJTC08mAAoJEORIIAnNuWDFPLgH/21RnjtuO2uF6QAOyhfDTNdx +csYdVQ12+lH391ttZBFV8QPZ72L4LPcrKUXb8710s5aOPgxJrtggpcoYyoj08C2 iIqdS8LOsVMDH+8WxSJ9Heb3XTUxKYoZvNMcQ0ZNVRqz46SomJI9IjPkfh5sZfux PRjRVAxYmDfP36k9+GyRkZyxjkBdswA+q7BWsoq70plP9P/W5qQd9cerZoalmDDL ERJ9ZBfhXzwk9zZ4xNFoDrMr78HVmrkHrDei9CJcqADXw9kSespDuv0OZIPJhSe1 +zsEX+YrTc0IjsRP8MQhrB8lAzwZWRWULJJSSQi5Sp/Pl7S1aGicSJ+9rBULkmg= =jYMv -----END PGP SIGNATURE----- --=-eRrfHX525XZ/GwaKHlD1-- --===============0823438082== 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 --===============0823438082==--