Re: [Intel-gfx] [PATCH rebased 2/3] drm/i915/dc3co: Check for DC3C0 exit state instead of sleep

From: Anshuman Gupta <anshuman.gupta@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH rebased 2/3] drm/i915/dc3co: Check for DC3C0 exit state instead of sleep
Date: Mon, 20 Jan 2020 09:42:59 +0530	[thread overview]
Message-ID: <20200120041258.GA20564@intel.com> (raw)
In-Reply-To: <20200116185421.66585-2-jose.souza@intel.com>

On 2020-01-16 at 10:54:20 -0800, José Roberto de Souza wrote:
> DC3C0 could have already exit so no need to always sleep, so lets
> read the register with the state.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 21561acfa3ac..1075e4edd2b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -807,7 +807,7 @@ static void tgl_disable_dc3co(struct drm_i915_private *dev_priv)
>  	/*
>  	 * Delay of 200us DC3CO Exit time B.Spec 49196
>  	 */
> -	usleep_range(200, 210);
> +	intel_de_wait_for_set(dev_priv, DC_STATE_EN, DC_STATE_DC3CO_STATUS, 1);
It is not documented in B.Specs to wait for DC3CO status, and this DC3CO status bit is
just for DEBUG purpose.
what if, it have never reached DC3CO due to PSR2 edp link is never gets to sleep
i.e 9th bit of PSR2_STATUS, it is going to wait for entire 1ms with this patch.
Lets say if DC3CO is already exited and we land here in disallow sequence, B.Specs
sequence first clear the DC3CO status bit before disabling it, so then also
it is going to wait for entire 1ms with this patch.
I think wait_for_us() will be better choice if, we want to wait for DC3CO exit status
rahter then plain sleep.
And while reviewing B.Specs there is recent change that we also need to disable 
PSR2 in DC3CO dsiallow sequence (Not related to this patch).
Thanks,
Anshuman Gupta.

>  }
>  
>  static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> -- 
> 2.25.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx