Re: [PATCH 08/12] drm/i915/psr: Re-org Activate after enable

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 12 Jul 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's move the activation calls together after enable is done.
>
> No real functional change should be expected here. Just an attempt
> to get it clear when we are really activating PSR after enabling it.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 422033a6f9f6..750df0172e8b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -543,9 +543,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  		hsw_psr_enable_sink(intel_dp);
>  
>  		hsw_psr_enable_source(intel_dp);
> -
> -		if (INTEL_GEN(dev_priv) >= 9)
> -			intel_psr_activate(intel_dp);
>  	} else {
>  		vlv_psr_setup_vsc(intel_dp);
>  
> @@ -555,20 +552,24 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  		vlv_psr_enable_source(intel_dp);
>  	}
>  
> -	/*
> -	 * FIXME: Activation should happen immediately since this function
> -	 * is just called after pipe is fully trained and enabled.
> -	 * However on every platform we face issues when first activation
> -	 * follows a modeset so quickly.
> -	 *     - On VLV/CHV we get bank screen on first activation
> -	 *     - On HSW/BDW we get a recoverable frozen screen until next
> -	 *       exit-activate sequence.
> -	 */
> -	if (INTEL_GEN(dev_priv) < 9)
> +	dev_priv->psr.enabled = intel_dp;
> +
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		intel_psr_activate(intel_dp);
> +	else
> +		/*
> +		 * FIXME: Activation should happen immediately since this
> +		 * function is just called after pipe is fully trained and
> +		 * enabled.
> +		 * However on some platforms we face issues when first
> +		 * activation follows a modeset so quickly.
> +		 *     - On VLV/CHV we get bank screen on first activation
> +		 *     - On HSW/BDW we get a recoverable frozen screen until
> +		 *       next exit-activate sequence.
> +		 */
>  		schedule_delayed_work(&dev_priv->psr.work,
>  				      msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));

While technically not required, I'd put curly braces around the branches
here for readability, because the comment is so long.

BR,
Jani.

>  
> -	dev_priv->psr.enabled = intel_dp;
>  unlock:
>  	mutex_unlock(&dev_priv->psr.lock);
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx