diff mbox series

[1/5] drm/i915: Use separate "DC off" power well for ADL-P and DG2

Message ID 20230316202549.1764024-2-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series More MTL WA and powerwell patches | expand

Commit Message

Sripada, Radhakrishna March 16, 2023, 8:25 p.m. UTC 
From: Matt Roper <matthew.d.roper@intel.com>

Although ADL-P and DG2 both use the same general power well setup, the
DC5/DC6 requirements are slightly different which means each platform
should have its own "DC off" power well.

DG2 (i.e., Xe_HPD IP) requires that DC5 be disabled whenever PG2 is
active.  However ADL-P (i.e., Xe_LPD IP) only requires DC5/DC6 to be
disabled when the PGC or PGD subwells are active; we should be able to
remain in these DC states when PGB and general PG2 functionality is in
use.

Bspec: 49193
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 .../i915/display/intel_display_power_map.c    | 41 +++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

Comments

Imre Deak March 17, 2023, 5:42 p.m. UTC | #1 
On Thu, Mar 16, 2023 at 01:25:45PM -0700, Radhakrishna Sripada wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> Although ADL-P and DG2 both use the same general power well setup, the
> DC5/DC6 requirements are slightly different which means each platform
> should have its own "DC off" power well.
> 
> DG2 (i.e., Xe_HPD IP) requires that DC5 be disabled whenever PG2 is
> active.  However ADL-P (i.e., Xe_LPD IP) only requires DC5/DC6 to be
> disabled when the PGC or PGD subwells are active; we should be able to
> remain in these DC states when PGB and general PG2 functionality is in
> use.
> 
> Bspec: 49193
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  .../i915/display/intel_display_power_map.c    | 41 +++++++++++++++++--
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> index 6645eb1911d8..d9e02cc9cf3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> @@ -1301,7 +1301,8 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_2,
>   */
>  
>  I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
> -	XELPD_PW_2_POWER_DOMAINS,
> +	XELPD_PW_C_POWER_DOMAINS,
> +	XELPD_PW_D_POWER_DOMAINS,
>  	POWER_DOMAIN_PORT_DSI,
>  	POWER_DOMAIN_AUDIO_MMIO,
>  	POWER_DOMAIN_AUX_A,
> @@ -1310,14 +1311,38 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
>  	POWER_DOMAIN_DC_OFF,
>  	POWER_DOMAIN_INIT);
>  
> -static const struct i915_power_well_desc xelpd_power_wells_main[] = {
> +static const struct i915_power_well_desc xelpd_power_wells_dcoff[] = {

Nit: Here and in the xehpd defintion, s/dcoff/dc_off/ to match other platforms.

>  	{
>  		.instances = &I915_PW_INSTANCES(
>  			I915_PW("DC_off", &xelpd_pwdoms_dc_off,
>  				.id = SKL_DISP_DC_OFF),
>  		),
>  		.ops = &gen9_dc_off_power_well_ops,
> -	}, {
> +	}
> +};
> +
> +I915_DECL_PW_DOMAINS(xehpd_pwdoms_dc_off,
> +	XELPD_PW_2_POWER_DOMAINS,
> +	POWER_DOMAIN_PORT_DSI,
> +	POWER_DOMAIN_AUDIO_MMIO,
> +	POWER_DOMAIN_AUX_A,
> +	POWER_DOMAIN_AUX_B,
> +	POWER_DOMAIN_MODESET,
> +	POWER_DOMAIN_DC_OFF,
> +	POWER_DOMAIN_INIT);
> +
> +static const struct i915_power_well_desc xehpd_power_wells_dcoff[] = {
> +	{
> +		.instances = &I915_PW_INSTANCES(
> +			I915_PW("DC_off", &xehpd_pwdoms_dc_off,
> +				.id = SKL_DISP_DC_OFF),
> +		),
> +		.ops = &gen9_dc_off_power_well_ops,
> +	}
> +};

Could you move the xehpd definitions to precede xehpd_power_wells[]?

Patches 1, 2 look ok to me:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +
> +static const struct i915_power_well_desc xelpd_power_wells_main[] = {
> +	{
>  		.instances = &I915_PW_INSTANCES(
>  			I915_PW("PW_2", &xelpd_pwdoms_pw_2,
>  				.hsw.idx = ICL_PW_CTL_IDX_PW_2,
> @@ -1400,6 +1425,14 @@ static const struct i915_power_well_desc xelpd_power_wells_main[] = {
>  static const struct i915_power_well_desc_list xelpd_power_wells[] = {
>  	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
>  	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> +	I915_PW_DESCRIPTORS(xelpd_power_wells_dcoff),
> +	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
> +};
> +
> +static const struct i915_power_well_desc_list xehpd_power_wells[] = {
> +	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
> +	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> +	I915_PW_DESCRIPTORS(xehpd_power_wells_dcoff),
>  	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
>  };
>  
> @@ -1624,6 +1657,8 @@ int intel_display_power_map_init(struct i915_power_domains *power_domains)
>  
>  	if (DISPLAY_VER(i915) >= 14)
>  		return set_power_wells(power_domains, xelpdp_power_wells);
> +	else if (IS_DG2(i915))
> +		return set_power_wells(power_domains, xehpd_power_wells);
>  	else if (DISPLAY_VER(i915) >= 13)
>  		return set_power_wells(power_domains, xelpd_power_wells);
>  	else if (IS_DG1(i915))
> -- 
> 2.34.1
>
Sripada, Radhakrishna April 20, 2023, 3:28 a.m. UTC | #2 
> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Friday, March 17, 2023 10:43 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; De Marchi, Lucas
> <lucas.demarchi@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915: Use separate "DC off" power well
> for ADL-P and DG2
> 
> On Thu, Mar 16, 2023 at 01:25:45PM -0700, Radhakrishna Sripada wrote:
> > From: Matt Roper <matthew.d.roper@intel.com>
> >
> > Although ADL-P and DG2 both use the same general power well setup, the
> > DC5/DC6 requirements are slightly different which means each platform
> > should have its own "DC off" power well.
> >
> > DG2 (i.e., Xe_HPD IP) requires that DC5 be disabled whenever PG2 is
> > active.  However ADL-P (i.e., Xe_LPD IP) only requires DC5/DC6 to be
> > disabled when the PGC or PGD subwells are active; we should be able to
> > remain in these DC states when PGB and general PG2 functionality is in
> > use.
> >
> > Bspec: 49193
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  .../i915/display/intel_display_power_map.c    | 41 +++++++++++++++++--
> >  1 file changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > index 6645eb1911d8..d9e02cc9cf3c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > @@ -1301,7 +1301,8 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_2,
> >   */
> >
> >  I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
> > -	XELPD_PW_2_POWER_DOMAINS,
> > +	XELPD_PW_C_POWER_DOMAINS,
> > +	XELPD_PW_D_POWER_DOMAINS,
> >  	POWER_DOMAIN_PORT_DSI,
> >  	POWER_DOMAIN_AUDIO_MMIO,
> >  	POWER_DOMAIN_AUX_A,
> > @@ -1310,14 +1311,38 @@
> I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
> >  	POWER_DOMAIN_DC_OFF,
> >  	POWER_DOMAIN_INIT);
> >
> > -static const struct i915_power_well_desc xelpd_power_wells_main[] = {
> > +static const struct i915_power_well_desc xelpd_power_wells_dcoff[] = {
> 
> Nit: Here and in the xehpd defintion, s/dcoff/dc_off/ to match other platforms.
> 
> >  	{
> >  		.instances = &I915_PW_INSTANCES(
> >  			I915_PW("DC_off", &xelpd_pwdoms_dc_off,
> >  				.id = SKL_DISP_DC_OFF),
> >  		),
> >  		.ops = &gen9_dc_off_power_well_ops,
> > -	}, {
> > +	}
> > +};
> > +
> > +I915_DECL_PW_DOMAINS(xehpd_pwdoms_dc_off,
> > +	XELPD_PW_2_POWER_DOMAINS,
> > +	POWER_DOMAIN_PORT_DSI,
> > +	POWER_DOMAIN_AUDIO_MMIO,
> > +	POWER_DOMAIN_AUX_A,
> > +	POWER_DOMAIN_AUX_B,
> > +	POWER_DOMAIN_MODESET,
> > +	POWER_DOMAIN_DC_OFF,
> > +	POWER_DOMAIN_INIT);
> > +
> > +static const struct i915_power_well_desc xehpd_power_wells_dcoff[] = {
> > +	{
> > +		.instances = &I915_PW_INSTANCES(
> > +			I915_PW("DC_off", &xehpd_pwdoms_dc_off,
> > +				.id = SKL_DISP_DC_OFF),
> > +		),
> > +		.ops = &gen9_dc_off_power_well_ops,
> > +	}
> > +};
> 
> Could you move the xehpd definitions to precede xehpd_power_wells[]?
> 
> Patches 1, 2 look ok to me:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
Thank you for the R-b merged Patches 1, 2

- Radhakrishna(RK) Sripada
> 
> > +
> > +static const struct i915_power_well_desc xelpd_power_wells_main[] = {
> > +	{
> >  		.instances = &I915_PW_INSTANCES(
> >  			I915_PW("PW_2", &xelpd_pwdoms_pw_2,
> >  				.hsw.idx = ICL_PW_CTL_IDX_PW_2,
> > @@ -1400,6 +1425,14 @@ static const struct i915_power_well_desc
> xelpd_power_wells_main[] = {
> >  static const struct i915_power_well_desc_list xelpd_power_wells[] = {
> >  	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
> >  	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> > +	I915_PW_DESCRIPTORS(xelpd_power_wells_dcoff),
> > +	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
> > +};
> > +
> > +static const struct i915_power_well_desc_list xehpd_power_wells[] = {
> > +	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
> > +	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> > +	I915_PW_DESCRIPTORS(xehpd_power_wells_dcoff),
> >  	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
> >  };
> >
> > @@ -1624,6 +1657,8 @@ int intel_display_power_map_init(struct
> i915_power_domains *power_domains)
> >
> >  	if (DISPLAY_VER(i915) >= 14)
> >  		return set_power_wells(power_domains, xelpdp_power_wells);
> > +	else if (IS_DG2(i915))
> > +		return set_power_wells(power_domains, xehpd_power_wells);
> >  	else if (DISPLAY_VER(i915) >= 13)
> >  		return set_power_wells(power_domains, xelpd_power_wells);
> >  	else if (IS_DG1(i915))
> > --
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index 6645eb1911d8..d9e02cc9cf3c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -1301,7 +1301,8 @@  I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_2,
  */
 
 I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
-	XELPD_PW_2_POWER_DOMAINS,
+	XELPD_PW_C_POWER_DOMAINS,
+	XELPD_PW_D_POWER_DOMAINS,
 	POWER_DOMAIN_PORT_DSI,
 	POWER_DOMAIN_AUDIO_MMIO,
 	POWER_DOMAIN_AUX_A,
@@ -1310,14 +1311,38 @@  I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
 	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
-static const struct i915_power_well_desc xelpd_power_wells_main[] = {
+static const struct i915_power_well_desc xelpd_power_wells_dcoff[] = {
 	{
 		.instances = &I915_PW_INSTANCES(
 			I915_PW("DC_off", &xelpd_pwdoms_dc_off,
 				.id = SKL_DISP_DC_OFF),
 		),
 		.ops = &gen9_dc_off_power_well_ops,
-	}, {
+	}
+};
+
+I915_DECL_PW_DOMAINS(xehpd_pwdoms_dc_off,
+	XELPD_PW_2_POWER_DOMAINS,
+	POWER_DOMAIN_PORT_DSI,
+	POWER_DOMAIN_AUDIO_MMIO,
+	POWER_DOMAIN_AUX_A,
+	POWER_DOMAIN_AUX_B,
+	POWER_DOMAIN_MODESET,
+	POWER_DOMAIN_DC_OFF,
+	POWER_DOMAIN_INIT);
+
+static const struct i915_power_well_desc xehpd_power_wells_dcoff[] = {
+	{
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("DC_off", &xehpd_pwdoms_dc_off,
+				.id = SKL_DISP_DC_OFF),
+		),
+		.ops = &gen9_dc_off_power_well_ops,
+	}
+};
+
+static const struct i915_power_well_desc xelpd_power_wells_main[] = {
+	{
 		.instances = &I915_PW_INSTANCES(
 			I915_PW("PW_2", &xelpd_pwdoms_pw_2,
 				.hsw.idx = ICL_PW_CTL_IDX_PW_2,
@@ -1400,6 +1425,14 @@  static const struct i915_power_well_desc xelpd_power_wells_main[] = {
 static const struct i915_power_well_desc_list xelpd_power_wells[] = {
 	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
 	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
+	I915_PW_DESCRIPTORS(xelpd_power_wells_dcoff),
+	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
+};
+
+static const struct i915_power_well_desc_list xehpd_power_wells[] = {
+	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
+	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
+	I915_PW_DESCRIPTORS(xehpd_power_wells_dcoff),
 	I915_PW_DESCRIPTORS(xelpd_power_wells_main),
 };
 
@@ -1624,6 +1657,8 @@  int intel_display_power_map_init(struct i915_power_domains *power_domains)
 
 	if (DISPLAY_VER(i915) >= 14)
 		return set_power_wells(power_domains, xelpdp_power_wells);
+	else if (IS_DG2(i915))
+		return set_power_wells(power_domains, xehpd_power_wells);
 	else if (DISPLAY_VER(i915) >= 13)
 		return set_power_wells(power_domains, xelpd_power_wells);
 	else if (IS_DG1(i915))