[PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Paulo Zanoni]

Don't call it when we can do like the other functions and just look at
port->port. Also rename the intel_digital_port variable to make it
look like the other functions.

My main goal here is to prevent the copy-pasters from propagating the
call to other parts of the code.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ca48bce..6fb90fa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
 }
 
 static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *intel_dig_port)
+				       struct intel_digital_port *port)
 {
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
-	enum port port;
 	u32 bit;
 
-	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
-	switch (port) {
+	switch (port->port) {
 	case PORT_A:
 		bit = BXT_DE_PORT_HP_DDIA;
 		break;
@@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
 		bit = BXT_DE_PORT_HP_DDIC;
 		break;
 	default:
-		MISSING_CASE(port);
+		MISSING_CASE(port->port);
 		return false;
 	}
 
-- 
2.9.5

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

[PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link

[Paulo Zanoni]

If for some unexpected reason the registers all read zero it's better
to WARN and return instead of dividing by zero and completely freezing
the machine.

I don't expect this to happen in the wild with the current code, but I
accidentally triggered the division by zero while doing some debugging
in an unusual environment.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f7c91bb..be86eb6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1220,6 +1220,9 @@ static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
 	dco_freq += (((cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
 		      DPLL_CFGCR0_DCO_FRACTION_SHIFT) * ref_clock) / 0x8000;
 
+	if (WARN_ON(p0 == 0 || p1 == 0 || p2 == 0))
+		return 0;
+
 	return dco_freq / (p0 * p1 * p2 * 5);
 }
 
-- 
2.9.5

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

Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Rodrigo Vivi]

On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote:
> Don't call it when we can do like the other functions and just look at
> port->port. Also rename the intel_digital_port variable to make it
> look like the other functions.

what other functions?

Most of our functions use intel_dig_port. Also dport and less used is port.

$ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/ | wc -l
93
$ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l
11
$ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l
18

> 
> My main goal here is to prevent the copy-pasters from propagating the
> call to other parts of the code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ca48bce..6fb90fa 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
>  }
>  
>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> -				       struct intel_digital_port *intel_dig_port)
> +				       struct intel_digital_port *port)
>  {
> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> -	enum port port;
>  	u32 bit;
>  
> -	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
> -	switch (port) {
> +	switch (port->port) {
>  	case PORT_A:
>  		bit = BXT_DE_PORT_HP_DDIA;
>  		break;
> @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
>  		bit = BXT_DE_PORT_HP_DDIC;
>  		break;
>  	default:
> -		MISSING_CASE(port);
> +		MISSING_CASE(port->port);
>  		return false;
>  	}
>  
> -- 
> 2.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link

[Rodrigo Vivi]

On Thu, Oct 05, 2017 at 09:38:42PM +0000, Paulo Zanoni wrote:
> If for some unexpected reason the registers all read zero it's better
> to WARN and return instead of dividing by zero and completely freezing
> the machine.
> 
> I don't expect this to happen in the wild with the current code, but I
> accidentally triggered the division by zero while doing some debugging
> in an unusual environment.

Good idea

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f7c91bb..be86eb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1220,6 +1220,9 @@ static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>  	dco_freq += (((cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
>  		      DPLL_CFGCR0_DCO_FRACTION_SHIFT) * ref_clock) / 0x8000;
>  
> +	if (WARN_ON(p0 == 0 || p1 == 0 || p2 == 0))
> +		return 0;
> +
>  	return dco_freq / (p0 * p1 * p2 * 5);
>  }
>  
> -- 
> 2.9.5
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
URL   : https://patchwork.freedesktop.org/series/31459/
State : warning

== Summary ==

Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-cnl-y) fdo#103027
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-cfl-s) fdo#103026
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7500u)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103027 https://bugs.freedesktop.org/show_bug.cgi?id=103027
fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:461s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:467s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:390s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:573s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:288s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:526s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:537s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:538s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:525s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:553s
fi-cnl-y         total:31   pass:21   dwarn:0   dfail:0   fail:0   skip:9  
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:437s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:599s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:416s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:468s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-kbl-7500u     total:289  pass:262  dwarn:2   dfail:0   fail:1   skip:24  time:501s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:584s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:499s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:598s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:661s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:663s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:534s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:521s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:473s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:578s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:437s

97c9e99b242fe40bbda48ba2bcaed07c47fba085 drm-tip: 2017y-10m-06d-09h-07m-21s UTC integration manifest
d0674fac8e07 drm/i915: avoid division by zero on cnl_calc_wrpll_link
7d8046f85adf drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5919/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Paulo Zanoni]

Em Qui, 2017-10-05 às 14:49 -0700, Rodrigo Vivi escreveu:
> On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote:
> > Don't call it when we can do like the other functions and just look
> > at
> > port->port. Also rename the intel_digital_port variable to make it
> > look like the other functions.
> 
> what other functions?

All the _digital_port_connected() functions from intel_dp.c.

> 
> Most of our functions use intel_dig_port. Also dport and less used is
> port.
> 
> $ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/
> | wc -l
> 93
> $ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l
> 11
> $ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l
> 18
> 
> > 
> > My main goal here is to prevent the copy-pasters from propagating
> > the
> > call to other parts of the code.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index ca48bce..6fb90fa 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4614,14 +4614,11 @@ static bool
> > bdw_digital_port_connected(struct drm_i915_private *dev_priv,
> >  }
> >  
> >  static bool bxt_digital_port_connected(struct drm_i915_private
> > *dev_priv,
> > -				       struct intel_digital_port
> > *intel_dig_port)
> > +				       struct intel_digital_port
> > *port)
> >  {
> > -	struct intel_encoder *intel_encoder = &intel_dig_port-
> > >base;
> > -	enum port port;
> >  	u32 bit;
> >  
> > -	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
> > -	switch (port) {
> > +	switch (port->port) {
> >  	case PORT_A:
> >  		bit = BXT_DE_PORT_HP_DDIA;
> >  		break;
> > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct
> > drm_i915_private *dev_priv,
> >  		bit = BXT_DE_PORT_HP_DDIC;
> >  		break;
> >  	default:
> > -		MISSING_CASE(port);
> > +		MISSING_CASE(port->port);
> >  		return false;
> >  	}
> >  
> > -- 
> > 2.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Paulo Zanoni]

Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu:
> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915: avoid unnecessary call
> to intel_hpd_pin_to_port
> URL   : https://patchwork.freedesktop.org/series/31459/
> State : warning
> 
> == Summary ==
> 
> Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary
> call to intel_hpd_pin_to_port
> https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb
> ox/
> 
> Test chamelium:
>         Subgroup dp-crc-fast:
>                 pass       -> FAIL       (fi-kbl-7500u) fdo#102514
> Test gem_ctx_switch:
>         Subgroup basic-default:
>                 pass       -> INCOMPLETE (fi-cnl-y) fdo#103027
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 pass       -> DMESG-WARN (fi-cfl-s) fdo#103026
>         Subgroup basic-s4-devices:
>                 pass       -> DMESG-WARN (fi-kbl-7500u)

[  242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not
signal timeout (has irq: 1)!

I do not believe this is caused by my patches. This test on this
machine is failing in many other recent patch series, but with
different error messages. Looks very unstable.


> 
> fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
> fdo#103027 https://bugs.freedesktop.org/show_bug.cgi?id=103027
> fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026
> 
> fi-bdw-
> 5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  
> time:461s
> fi-bdw-
> gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  
> time:467s
> fi-blb-
> e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  
> time:390s
> fi-bsw-
> n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  
> time:573s
> fi-bwr-
> 2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106
> time:288s
> fi-bxt-
> dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  
> time:526s
> fi-bxt-
> j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> time:537s
> fi-byt-
> j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  
> time:538s
> fi-byt-
> n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  
> time:525s
> fi-cfl-
> s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  
> time:553s
> fi-cnl-
> y         total:31   pass:21   dwarn:0   dfail:0   fail:0   skip:9  
> fi-elk-
> e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  
> time:437s
> fi-glk-
> 1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  
> time:599s
> fi-hsw-
> 4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> time:439s
> fi-hsw-
> 4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> time:416s
> fi-ilk-
> 650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  
> time:468s
> fi-ivb-
> 3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> time:508s
> fi-ivb-
> 3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> time:479s
> fi-kbl-
> 7500u     total:289  pass:262  dwarn:2   dfail:0   fail:1   skip:24  
> time:501s
> fi-kbl-
> 7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  
> time:584s
> fi-kbl-
> 7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  
> time:499s
> fi-kbl-
> r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> time:598s
> fi-pnv-
> d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  
> time:661s
> fi-skl-
> 6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  
> time:477s
> fi-skl-
> 6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  
> time:663s
> fi-skl-
> 6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  
> time:534s
> fi-skl-
> 6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  
> time:521s
> fi-skl-
> gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  
> time:473s
> fi-snb-
> 2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  
> time:578s
> fi-snb-
> 2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  
> time:437s
> 
> 97c9e99b242fe40bbda48ba2bcaed07c47fba085 drm-tip: 2017y-10m-06d-09h-
> 07m-21s UTC integration manifest
> d0674fac8e07 drm/i915: avoid division by zero on cnl_calc_wrpll_link
> 7d8046f85adf drm/i915: avoid unnecessary call to
> intel_hpd_pin_to_port
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchw
> ork_5919/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Manasi Navare]

On Fri, Oct 06, 2017 at 06:19:12PM -0300, Paulo Zanoni wrote:
> Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu:
> > == Series Details ==
> > 
> > Series: series starting with [1/2] drm/i915: avoid unnecessary call
> > to intel_hpd_pin_to_port
> > URL   : https://patchwork.freedesktop.org/series/31459/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary
> > call to intel_hpd_pin_to_port
> > https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb
> > ox/
> > 
> > Test chamelium:
> >         Subgroup dp-crc-fast:
> >                 pass       -> FAIL       (fi-kbl-7500u) fdo#102514
> > Test gem_ctx_switch:
> >         Subgroup basic-default:
> >                 pass       -> INCOMPLETE (fi-cnl-y) fdo#103027
> > Test gem_exec_suspend:
> >         Subgroup basic-s3:
> >                 pass       -> DMESG-WARN (fi-cfl-s) fdo#103026
> >         Subgroup basic-s4-devices:
> >                 pass       -> DMESG-WARN (fi-kbl-7500u)
> 
> [  242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not
> signal timeout (has irq: 1)!
> 
> I do not believe this is caused by my patches. This test on this
> machine is failing in many other recent patch series, but with
> different error messages. Looks very unstable.
> 
>

Yes this is the system where we have had these messages due to
LSPCON issue recently.

Manasi
 
> > 
> > fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
> > fdo#103027 https://bugs.freedesktop.org/show_bug.cgi?id=103027
> > fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026
> > 
> > fi-bdw-
> > 5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  
> > time:461s
> > fi-bdw-
> > gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  
> > time:467s
> > fi-blb-
> > e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  
> > time:390s
> > fi-bsw-
> > n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  
> > time:573s
> > fi-bwr-
> > 2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106
> > time:288s
> > fi-bxt-
> > dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  
> > time:526s
> > fi-bxt-
> > j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> > time:537s
> > fi-byt-
> > j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  
> > time:538s
> > fi-byt-
> > n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  
> > time:525s
> > fi-cfl-
> > s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  
> > time:553s
> > fi-cnl-
> > y         total:31   pass:21   dwarn:0   dfail:0   fail:0   skip:9  
> > fi-elk-
> > e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  
> > time:437s
> > fi-glk-
> > 1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  
> > time:599s
> > fi-hsw-
> > 4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> > time:439s
> > fi-hsw-
> > 4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> > time:416s
> > fi-ilk-
> > 650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  
> > time:468s
> > fi-ivb-
> > 3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> > time:508s
> > fi-ivb-
> > 3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  
> > time:479s
> > fi-kbl-
> > 7500u     total:289  pass:262  dwarn:2   dfail:0   fail:1   skip:24  
> > time:501s
> > fi-kbl-
> > 7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  
> > time:584s
> > fi-kbl-
> > 7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  
> > time:499s
> > fi-kbl-
> > r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  
> > time:598s
> > fi-pnv-
> > d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  
> > time:661s
> > fi-skl-
> > 6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  
> > time:477s
> > fi-skl-
> > 6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  
> > time:663s
> > fi-skl-
> > 6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  
> > time:534s
> > fi-skl-
> > 6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  
> > time:521s
> > fi-skl-
> > gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  
> > time:473s
> > fi-snb-
> > 2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  
> > time:578s
> > fi-snb-
> > 2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  
> > time:437s
> > 
> > 97c9e99b242fe40bbda48ba2bcaed07c47fba085 drm-tip: 2017y-10m-06d-09h-
> > 07m-21s UTC integration manifest
> > d0674fac8e07 drm/i915: avoid division by zero on cnl_calc_wrpll_link
> > 7d8046f85adf drm/i915: avoid unnecessary call to
> > intel_hpd_pin_to_port
> > 
> > == Logs ==
> > 
> > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchw
> > ork_5919/
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Martin Peres]

On 07/10/17 00:19, Paulo Zanoni wrote:
> Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu:
>> == Series Details ==
>>
>> Series: series starting with [1/2] drm/i915: avoid unnecessary call
>> to intel_hpd_pin_to_port
>> URL   : https://patchwork.freedesktop.org/series/31459/
>> State : warning
>>
>> == Summary ==
>>
>> Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary
>> call to intel_hpd_pin_to_port
>> https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb
>> ox/
>>
>> Test chamelium:
>>          Subgroup dp-crc-fast:
>>                  pass       -> FAIL       (fi-kbl-7500u) fdo#102514
>> Test gem_ctx_switch:
>>          Subgroup basic-default:
>>                  pass       -> INCOMPLETE (fi-cnl-y) fdo#103027
>> Test gem_exec_suspend:
>>          Subgroup basic-s3:
>>                  pass       -> DMESG-WARN (fi-cfl-s) fdo#103026
>>          Subgroup basic-s4-devices:
>>                  pass       -> DMESG-WARN (fi-kbl-7500u)
> 
> [  242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not
> signal timeout (has irq: 1)!
> 
> I do not believe this is caused by my patches. This test on this
> machine is failing in many other recent patch series, but with
> different error messages. Looks very unstable.

Thanks for letting us know! We need more feedback from developers when 
we get these false positives in pre-merge :)

Marta has been taking over the bug filing for a month now. She will know 
better what to do than me on this. I've Cc:ed her and will let her 
handle this.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Jani Nikula]

On Thu, 05 Oct 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Don't call it when we can do like the other functions and just look at
> port->port. Also rename the intel_digital_port variable to make it
> look like the other functions.

I guess the question is, *which* other functions. A quick rough grep
gives me:

$ grep -ho "intel_digital_port[ *]*[a-zA-Z0-9_]*" $(git ls-files -- drivers/gpu/drm/i915) | sort | uniq -c | sort -rn

     93 intel_digital_port *intel_dig_port
     22 intel_digital_port *dig_port
     18 intel_digital_port *dport
     11 intel_digital_port *port
      5 intel_digital_port_connected
      5 intel_digital_port
      4 intel_digital_port *
      1 intel_digital_port *primary
      1 intel_digital_port *irq_port
      1 intel_digital_port 

I think it's more common to use port for enum port. I'm fine with any of
the top three for struct intel_dig_port.

BR,
Jani.

>
> My main goal here is to prevent the copy-pasters from propagating the
> call to other parts of the code.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ca48bce..6fb90fa 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
>  }
>  
>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> -				       struct intel_digital_port *intel_dig_port)
> +				       struct intel_digital_port *port)
>  {
> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> -	enum port port;
>  	u32 bit;
>  
> -	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
> -	switch (port) {
> +	switch (port->port) {
>  	case PORT_A:
>  		bit = BXT_DE_PORT_HP_DDIA;
>  		break;
> @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
>  		bit = BXT_DE_PORT_HP_DDIC;
>  		break;
>  	default:
> -		MISSING_CASE(port);
> +		MISSING_CASE(port->port);
>  		return false;
>  	}

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

Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Jani Nikula]

On Fri, 06 Oct 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Em Qui, 2017-10-05 às 14:49 -0700, Rodrigo Vivi escreveu:
>> On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote:
>> > Don't call it when we can do like the other functions and just look
>> > at
>> > port->port. Also rename the intel_digital_port variable to make it
>> > look like the other functions.
>> 
>> what other functions?
>
> All the _digital_port_connected() functions from intel_dp.c.

Wah, I should read all of the thread before replying...

Anyway, I'd rather move away from port for struct intel_digital_port.

BR,
Jani.


>
>> 
>> Most of our functions use intel_dig_port. Also dport and less used is
>> port.
>> 
>> $ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/
>> | wc -l
>> 93
>> $ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l
>> 11
>> $ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l
>> 18
>> 
>> > 
>> > My main goal here is to prevent the copy-pasters from propagating
>> > the
>> > call to other parts of the code.
>> > 
>> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp.c | 9 +++------
>> >  1 file changed, 3 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > b/drivers/gpu/drm/i915/intel_dp.c
>> > index ca48bce..6fb90fa 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -4614,14 +4614,11 @@ static bool
>> > bdw_digital_port_connected(struct drm_i915_private *dev_priv,
>> >  }
>> >  
>> >  static bool bxt_digital_port_connected(struct drm_i915_private
>> > *dev_priv,
>> > -				       struct intel_digital_port
>> > *intel_dig_port)
>> > +				       struct intel_digital_port
>> > *port)
>> >  {
>> > -	struct intel_encoder *intel_encoder = &intel_dig_port-
>> > >base;
>> > -	enum port port;
>> >  	u32 bit;
>> >  
>> > -	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
>> > -	switch (port) {
>> > +	switch (port->port) {
>> >  	case PORT_A:
>> >  		bit = BXT_DE_PORT_HP_DDIA;
>> >  		break;
>> > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct
>> > drm_i915_private *dev_priv,
>> >  		bit = BXT_DE_PORT_HP_DDIC;
>> >  		break;
>> >  	default:
>> > -		MISSING_CASE(port);
>> > +		MISSING_CASE(port->port);
>> >  		return false;
>> >  	}
>> >  
>> > -- 
>> > 2.9.5
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port

[Ville Syrjälä]

On Thu, Oct 05, 2017 at 06:38:41PM -0300, Paulo Zanoni wrote:
> Don't call it when we can do like the other functions and just look at
> port->port. Also rename the intel_digital_port variable to make it
> look like the other functions.
> 
> My main goal here is to prevent the copy-pasters from propagating the
> call to other parts of the code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ca48bce..6fb90fa 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
>  }
>  
>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> -				       struct intel_digital_port *intel_dig_port)
> +				       struct intel_digital_port *port)
>  {
> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> -	enum port port;
>  	u32 bit;
>  
> -	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
> -	switch (port) {
> +	switch (port->port) {

Hmm. I'm thinking we might want to go the other way and change everyone
else to use hpd_pin instead. IIRC we'll need to start considering mixed
hpd_pin<->port mappings with port F. We already had to do this with
the BXT A step w/a (though it was just crudely hacked in) which I suppose
is the reason the BXT code is still doing this.

Though I see no reason why we would have to call intel_hpd_pin_to_port()
here. Instead we could just 's/port->port/encoder->hpd_pin/' everywhere.


>  	case PORT_A:
>  		bit = BXT_DE_PORT_HP_DDIA;
>  		break;
> @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
>  		bit = BXT_DE_PORT_HP_DDIC;
>  		break;
>  	default:
> -		MISSING_CASE(port);
> +		MISSING_CASE(port->port);
>  		return false;
>  	}
>  
> -- 
> 2.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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