Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Souza, Jose" <jose.souza@intel.com>
To: "Singh, Gaurav K" <gaurav.k.singh@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Enable PSR2 in next iteration of suspend-resume/S0ix cycling
Date: Mon, 25 Nov 2019 21:59:47 +0000	[thread overview]
Message-ID: <a05814026af61db3825deb847fcd7f4941ae3965.camel@intel.com> (raw)
In-Reply-To: <1574611468-3319-1-git-send-email-gaurav.k.singh@intel.com>

Hi Gaurav

As we already talked, on upstream for now we do not want to recovery of
PSR runtime errors, so not merging this patch.

But if you want comments in this patch to merge in your kernel tree...I
would change the commit title to: "Do not mark as sink as not reliable
to PSR runtime errors" or something similar as the changes here will
not only affect during suspend-resume cycling.

Also I would change it to only do not set sink_not_reliable in case of
CRC mismatch leaving the other errors setting sink_not_reliable, if
doing so changing the commit message suggested above.

On Sun, 2019-11-24 at 21:34 +0530, Gaurav K Singh wrote:
> In case of CRC mismatch, panel generates IRQ_HD and
> PSR2 gets disabled by i915 driver. Due to this, PSR2 will
> only be enabled back only if system is rebooted or cold boot.
> So, in cases of suspend resume stress test and S0ix stress test,
> when we encounter this CRC issue on a particular iteration,
> once PSR2 is disabled,it remains disabled throughout all the
> cycling iterations until the system is rebooted.
> 
> Keeping this in mind, many times users do not reboot their system and
> they just keep lid off/on or suspend/resume. In these scenarios
> in case of CRC issue, panel will become non-PSR2 which will
> eventually
> drain out battery.
> 
> In order to fix this behavior, did not set the "sink_not_reliable"
> flag
> to be true, so that intel_psr_compute_config() can pass in case of a
> normal modeset which will lead to enabling PSR2 again in next
> iteration
> of suspend/resume or S0ix cycle(without reboot).
> 
> Tested this patch and works fine on Gen9 Intel chromebook, PSR2 was
> enabled back in next iteration, no other sideeffects observed.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index c1d133362b76..8465d0fc2214 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1420,10 +1420,9 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  	if (val & ~errors)
>  		DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
>  			  val & ~errors);
> -	if (val & errors) {
> +	if (val & errors)
>  		intel_psr_disable_locked(intel_dp);
> -		psr->sink_not_reliable = true;
> -	}
> +
>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
>  exit:
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

WARNING: multiple messages have this Message-ID (diff)
From: "Souza, Jose" <jose.souza@intel.com>
To: "Singh, Gaurav K" <gaurav.k.singh@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Enable PSR2 in next iteration of suspend-resume/S0ix cycling
Date: Mon, 25 Nov 2019 21:59:47 +0000	[thread overview]
Message-ID: <a05814026af61db3825deb847fcd7f4941ae3965.camel@intel.com> (raw)
Message-ID: <20191125215947.-2ix9vEsIt6KEWQeipQEApPtI8MBPPs341PLmgh3TpM@z> (raw)
In-Reply-To: <1574611468-3319-1-git-send-email-gaurav.k.singh@intel.com>

Hi Gaurav

As we already talked, on upstream for now we do not want to recovery of
PSR runtime errors, so not merging this patch.

But if you want comments in this patch to merge in your kernel tree...I
would change the commit title to: "Do not mark as sink as not reliable
to PSR runtime errors" or something similar as the changes here will
not only affect during suspend-resume cycling.

Also I would change it to only do not set sink_not_reliable in case of
CRC mismatch leaving the other errors setting sink_not_reliable, if
doing so changing the commit message suggested above.

On Sun, 2019-11-24 at 21:34 +0530, Gaurav K Singh wrote:
> In case of CRC mismatch, panel generates IRQ_HD and
> PSR2 gets disabled by i915 driver. Due to this, PSR2 will
> only be enabled back only if system is rebooted or cold boot.
> So, in cases of suspend resume stress test and S0ix stress test,
> when we encounter this CRC issue on a particular iteration,
> once PSR2 is disabled,it remains disabled throughout all the
> cycling iterations until the system is rebooted.
> 
> Keeping this in mind, many times users do not reboot their system and
> they just keep lid off/on or suspend/resume. In these scenarios
> in case of CRC issue, panel will become non-PSR2 which will
> eventually
> drain out battery.
> 
> In order to fix this behavior, did not set the "sink_not_reliable"
> flag
> to be true, so that intel_psr_compute_config() can pass in case of a
> normal modeset which will lead to enabling PSR2 again in next
> iteration
> of suspend/resume or S0ix cycle(without reboot).
> 
> Tested this patch and works fine on Gen9 Intel chromebook, PSR2 was
> enabled back in next iteration, no other sideeffects observed.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index c1d133362b76..8465d0fc2214 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1420,10 +1420,9 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  	if (val & ~errors)
>  		DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
>  			  val & ~errors);
> -	if (val & errors) {
> +	if (val & errors)
>  		intel_psr_disable_locked(intel_dp);
> -		psr->sink_not_reliable = true;
> -	}
> +
>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
>  exit:
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2019-11-25 21:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-24 16:04 [PATCH] drm/i915: Enable PSR2 in next iteration of suspend-resume/S0ix cycling Gaurav K Singh
2019-11-24 16:04 ` [Intel-gfx] " Gaurav K Singh
2019-11-24 17:02 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-11-24 17:02   ` [Intel-gfx] " Patchwork
2019-11-24 21:35 ` ✓ Fi.CI.IGT: " Patchwork
2019-11-24 21:35   ` [Intel-gfx] " Patchwork
2019-11-25 21:59 ` Souza, Jose [this message]
2019-11-25 21:59   ` [Intel-gfx] [PATCH] " Souza, Jose
2019-12-05  5:52 ` [Intel-gfx] [PATCH] [v2] drm/i915: Do not mark as sink as not reliable to PSR runtime errors Gaurav K Singh
2019-12-05  6:00 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Enable PSR2 in next iteration of suspend-resume/S0ix cycling (rev2) Patchwork
2019-12-05  6:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a05814026af61db3825deb847fcd7f4941ae3965.camel@intel.com \
    --to=jose.souza@intel.com \
    --cc=gaurav.k.singh@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox