On Wed, 2020-11-11 at 13:12 -0800, Lucas De Marchi wrote: > On Wed, Nov 11, 2020 at 08:24:07AM -0800, Jose Souza wrote: > > DC9 has a separate HW flow from the rest of the DC states and it is > > available in GEN9 LP platforms and on GEN11 and newer, so here > > moving the assignment of the mask to a single conditional block to > > simplifly code. > > > > Cc: Lucas De Marchi <[email protected]> > > Cc: Anusha Srivatsa <[email protected]> > > Signed-off-by: José Roberto de Souza <[email protected]> > > --- > > .../gpu/drm/i915/display/intel_display_power.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 689922480661..48d41a43fbb2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -4497,26 +4497,24 @@ static u32 get_allowed_dc_mask(const struct > > drm_i915_private *dev_priv, > > max_dc = 3; > > else > > max_dc = 4; > > - /* > > - * DC9 has a separate HW flow from the rest of the DC states, > > - * not depending on the DMC firmware. It's needed by system > > - * suspend/resume, so allow it unconditionally. > > - */ > > - mask = DC_STATE_EN_DC9; > > } else if (IS_GEN(dev_priv, 11)) { > > max_dc = 2; > > - mask = DC_STATE_EN_DC9; > > } else if (IS_GEN(dev_priv, 10) || IS_GEN9_BC(dev_priv)) { > > max_dc = 2; > > - mask = 0; > > } else if (IS_GEN9_LP(dev_priv)) { > > max_dc = 1; > > - mask = DC_STATE_EN_DC9; > > } else { > > max_dc = 0; > > - mask = 0; > > } > > > > + /* > > + * DC9 has a separate HW flow from the rest of the DC states, > > + * not depending on the DMC firmware. It's needed by system > > + * suspend/resume, so allow it unconditionally. > > + */ > > + mask = IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 11 ? > > + DC_STATE_EN_DC9 : 0; > > humn... these 2 conditions here in a ternary operator is something that > will probably get even harder to read if we have to add more conditions. > Maybe just move the default value to the declaration (mask = 0) and here > you do: > > if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 11) > mask = DC_STATE_EN_DC9; > > ? > > Up to you. Change looks correct
Will keep the way this patch is as I believe that all new platforms will support DC9 so this block will not change but if it happens we should do like you suggested. > > > Reviewed-by: Lucas De Marchi <[email protected]> thanks > > Lucas De Marchi > > > + > > if (!dev_priv->params.disable_power_well) > > max_dc = 0; > > > > -- > > 2.29.2 > > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
