> -----Original Message----- > From: Deak, Imre <[email protected]> > Sent: Friday, March 17, 2023 10:43 AM > To: Sripada, Radhakrishna <[email protected]> > Cc: [email protected]; De Marchi, Lucas > <[email protected]> > 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 <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Radhakrishna Sripada <[email protected]> > > --- > > .../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 <[email protected]> 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 > >
