* [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link
2017-10-05 21:38 [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Paulo Zanoni
@ 2017-10-05 21:38 ` Paulo Zanoni
2017-10-05 21:50 ` Rodrigo Vivi
2017-10-05 21:49 ` [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Rodrigo Vivi
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2017-10-05 21:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link
2017-10-05 21:38 ` [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link Paulo Zanoni
@ 2017-10-05 21:50 ` Rodrigo Vivi
0 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Vivi @ 2017-10-05 21:50 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-05 21:38 [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Paulo Zanoni
2017-10-05 21:38 ` [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link Paulo Zanoni
@ 2017-10-05 21:49 ` Rodrigo Vivi
2017-10-06 12:56 ` Paulo Zanoni
2017-10-06 10:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-10-05 21:49 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-05 21:49 ` [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Rodrigo Vivi
@ 2017-10-06 12:56 ` Paulo Zanoni
2017-10-09 12:31 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2017-10-06 12:56 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-06 12:56 ` Paulo Zanoni
@ 2017-10-09 12:31 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-10-09 12:31 UTC (permalink / raw)
To: Paulo Zanoni, Rodrigo Vivi; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-05 21:38 [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Paulo Zanoni
2017-10-05 21:38 ` [PATCH 2/2] drm/i915: avoid division by zero on cnl_calc_wrpll_link Paulo Zanoni
2017-10-05 21:49 ` [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Rodrigo Vivi
@ 2017-10-06 10:45 ` Patchwork
2017-10-06 21:19 ` Paulo Zanoni
2017-10-09 12:29 ` [PATCH 1/2] " Jani Nikula
2017-10-09 13:25 ` Ville Syrjälä
4 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2017-10-06 10:45 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-06 10:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2017-10-06 21:19 ` Paulo Zanoni
2017-10-06 21:26 ` Manasi Navare
2017-10-09 10:14 ` Martin Peres
0 siblings, 2 replies; 12+ messages in thread
From: Paulo Zanoni @ 2017-10-06 21:19 UTC (permalink / raw)
To: intel-gfx; +Cc: martin.peres
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-06 21:19 ` Paulo Zanoni
@ 2017-10-06 21:26 ` Manasi Navare
2017-10-09 10:14 ` Martin Peres
1 sibling, 0 replies; 12+ messages in thread
From: Manasi Navare @ 2017-10-06 21:26 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, martin.peres
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-06 21:19 ` Paulo Zanoni
2017-10-06 21:26 ` Manasi Navare
@ 2017-10-09 10:14 ` Martin Peres
1 sibling, 0 replies; 12+ messages in thread
From: Martin Peres @ 2017-10-09 10:14 UTC (permalink / raw)
To: Paulo Zanoni, intel-gfx; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-05 21:38 [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Paulo Zanoni
` (2 preceding siblings ...)
2017-10-06 10:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2017-10-09 12:29 ` Jani Nikula
2017-10-09 13:25 ` Ville Syrjälä
4 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-10-09 12:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port
2017-10-05 21:38 [PATCH 1/2] drm/i915: avoid unnecessary call to intel_hpd_pin_to_port Paulo Zanoni
` (3 preceding siblings ...)
2017-10-09 12:29 ` [PATCH 1/2] " Jani Nikula
@ 2017-10-09 13:25 ` Ville Syrjälä
4 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2017-10-09 13:25 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 12+ messages in thread