[Intel-gfx] [PATCH 1/2] drm/i915/display: Group DC9 mask set

[Lucas De Marchi]

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 <lucas.demarchi at intel.com>
>Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
>---
> .../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


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>+
> 	if (!dev_priv->params.disable_power_well)
> 		max_dc = 0;
>
>-- 
>2.29.2
>