Re: [PATCH 3/3] drm/i915: Remove special case for power well 1/MISC_IO state verification

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 09, 2018 at 04:58:22PM +0200, Imre Deak wrote:
> Even though PW#1 and the MISC_IO power wells are managed by the
> DMC firmware (toggled dynamically if conditions allow it) from the
> driver's POV they are always on if the display core is initialized
> (always restored by DMC to the enabled state after exiting from DC5/6
> for instance b/c of MMIO access). Accordingly we can just mark them as
> always-on and remove the special casing for them during state
> verification (thus enabling verification for these power wells too).
> 
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 44a77de439f2..6b1576ae778f 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2358,6 +2358,7 @@ static const struct i915_power_well_desc skl_power_wells[] = {
>  	{
>  		.name = "power well 1",
>  		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,

First I was wondering if this somehow changes the behaviour of
__intel_display_power_is_enabled(), but it won't since all these
wells have domains==0 and so would be skipped there anyway.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_1,
> @@ -2370,6 +2371,7 @@ static const struct i915_power_well_desc skl_power_wells[] = {
>  	{
>  		.name = "MISC IO power well",
>  		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_MISC_IO,
> @@ -2449,6 +2451,8 @@ static const struct i915_power_well_desc bxt_power_wells[] = {
>  	},
>  	{
>  		.name = "power well 1",
> +		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_1,
> @@ -2508,6 +2512,7 @@ static const struct i915_power_well_desc glk_power_wells[] = {
>  	{
>  		.name = "power well 1",
>  		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_1,
> @@ -2636,6 +2641,7 @@ static const struct i915_power_well_desc cnl_power_wells[] = {
>  	{
>  		.name = "power well 1",
>  		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_1,
> @@ -2803,6 +2809,7 @@ static const struct i915_power_well_desc icl_power_wells[] = {
>  	{
>  		.name = "power well 1",
>  		/* Handled by the DMC firmware */
> +		.always_on = true,
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
>  		.id = SKL_DISP_PW_1,
> @@ -3936,14 +3943,6 @@ static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
>  		int domains_count;
>  		bool enabled;
>  
> -		/*
> -		 * Power wells not belonging to any domain (like the MISC_IO
> -		 * and PW1 power wells) are under FW control, so ignore them,
> -		 * since their state can change asynchronously.
> -		 */
> -		if (!power_well->desc->domains)
> -			continue;
> -
>  		enabled = power_well->desc->ops->is_enabled(dev_priv,
>  							    power_well);
>  		if ((power_well->count || power_well->desc->always_on) !=
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx