diff mbox series

[v5,5/5] drm/i915/display: Increase number of fast wake precharge pulses

Message ID 20240308110039.3900838-6-jouni.hogander@intel.com (mailing list archive)
State New, archived
Headers show
Series IO and fast wake lines calculation and increase fw sync length | expand

Commit Message

Hogander, Jouni March 8, 2024, 11 a.m. UTC
Increasing number of fast wake sync pulses seem to fix problems with
certain PSR panels. This should be ok for other panels as well as the eDP
specification allows 10...16 precharge pulses and we are still within that
range.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9739
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ville Syrjälä March 12, 2024, 4:44 p.m. UTC | #1
On Fri, Mar 08, 2024 at 01:00:39PM +0200, Jouni Högander wrote:
> Increasing number of fast wake sync pulses seem to fix problems with
> certain PSR panels. This should be ok for other panels as well as the eDP
> specification allows 10...16 precharge pulses and we are still within that
> range.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9739
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index 7e69be100d90..5dff1bc85d61 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -145,7 +145,7 @@ static int intel_dp_aux_sync_len(void)
>  
>  int intel_dp_aux_fw_sync_len(void)
>  {
> -	int precharge = 10; /* 10-16 */
> +	int precharge = 12; /* 10-16 */

This is still giving me allergies because Windows doesn't have
anything like this. So the mystery is how does Windows work?
This was an actual production machine I take it?

Did we have look at the error bits in PSR2_DEBUG to see if there
is some difference between the working and non-working values?

Anyways, this at least needs a proper comment to explain why
we're not usign the standard value.

>  	int preamble = 8;
>  
>  	return precharge + preamble;
> -- 
> 2.34.1
Hogander, Jouni March 13, 2024, 6:20 a.m. UTC | #2
On Tue, 2024-03-12 at 18:44 +0200, Ville Syrjälä wrote:
> On Fri, Mar 08, 2024 at 01:00:39PM +0200, Jouni Högander wrote:
> > Increasing number of fast wake sync pulses seem to fix problems
> > with
> > certain PSR panels. This should be ok for other panels as well as
> > the eDP
> > specification allows 10...16 precharge pulses and we are still
> > within that
> > range.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9739
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_aux.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > index 7e69be100d90..5dff1bc85d61 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > @@ -145,7 +145,7 @@ static int intel_dp_aux_sync_len(void)
> >  
> >  int intel_dp_aux_fw_sync_len(void)
> >  {
> > -       int precharge = 10; /* 10-16 */
> > +       int precharge = 12; /* 10-16 */
> 
> This is still giving me allergies because Windows doesn't have
> anything like this. So the mystery is how does Windows work?
> This was an actual production machine I take it?

Not sure if it's already on market. To my understanding it's production
machine.

The problematic panel here is successfully used on older platform (RPL)
but now we are seeing glitches when used on MTL. More discussion about
the issue : https://gitlab.freedesktop.org/drm/intel/-/issues/9739

> 
> Did we have look at the error bits in PSR2_DEBUG to see if there
> is some difference between the working and non-working values?

There is no error bits in PSR2_DEBUG. Just bit 13 indicating "FastWake
Done"
> 
> Anyways, this at least needs a proper comment to explain why
> we're not usign the standard value.

Ok, I will add that comment and send a new version.

BR,

Jouni Högander

> 
> >         int preamble = 8;
> >  
> >         return precharge + preamble;
> > -- 
> > 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index 7e69be100d90..5dff1bc85d61 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -145,7 +145,7 @@  static int intel_dp_aux_sync_len(void)
 
 int intel_dp_aux_fw_sync_len(void)
 {
-	int precharge = 10; /* 10-16 */
+	int precharge = 12; /* 10-16 */
 	int preamble = 8;
 
 	return precharge + preamble;