* [PATCH] drm/i915: remove check for aux irq
@ 2018-04-25 21:55 Lucas De Marchi
2018-04-25 22:11 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
` (11 more replies)
0 siblings, 12 replies; 28+ messages in thread
From: Lucas De Marchi @ 2018-04-25 21:55 UTC (permalink / raw)
To: intel-gfx
This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
GMBUS and AUX interrupts on gen4/g4x").
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_psr.c | 2 +-
4 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8444ca8d5aa3..09e1c2289ea1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
/*
- * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
+ * gmbus irq on gen4 seems to be able to generate legacy interrupts
* even when in MSI mode. This results in spurious interrupt warnings if the
* legacy irq no. is shared with another device. The kernel then disables that
* interrupt source and so prevents the other device from working properly.
@@ -2553,7 +2553,6 @@ intel_info(const struct drm_i915_private *dev_priv)
* Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
* interrupts.
*/
-#define HAS_AUX_IRQ(dev_priv) true
#define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
/* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4298ac..fd417473b6a9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
}
static uint32_t
-intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
+intel_dp_aux_wait_done(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
@@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
bool done;
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
- if (has_aux_irq)
- done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
- msecs_to_jiffies_timeout(10));
- else
- done = wait_for(C, 10) == 0;
+ done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
+ msecs_to_jiffies_timeout(10));
if (!done)
- DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
- has_aux_irq);
+ DRM_ERROR("dp aux hw did not signal timeout!\n");
#undef C
return status;
@@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
}
static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
- bool has_aux_irq,
int send_bytes,
uint32_t aux_clock_divider)
{
@@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
return DP_AUX_CH_CTL_SEND_BUSY |
DP_AUX_CH_CTL_DONE |
- (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+ DP_AUX_CH_CTL_INTERRUPT |
DP_AUX_CH_CTL_TIME_OUT_ERROR |
timeout |
DP_AUX_CH_CTL_RECEIVE_ERROR |
@@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
}
static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
- bool has_aux_irq,
int send_bytes,
uint32_t unused)
{
return DP_AUX_CH_CTL_SEND_BUSY |
DP_AUX_CH_CTL_DONE |
- (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+ DP_AUX_CH_CTL_INTERRUPT |
DP_AUX_CH_CTL_TIME_OUT_ERROR |
DP_AUX_CH_CTL_TIME_OUT_MAX |
DP_AUX_CH_CTL_RECEIVE_ERROR |
@@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
int i, ret, recv_bytes;
uint32_t status;
int try, clock = 0;
- bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
bool vdd;
ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
@@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
- has_aux_irq,
send_bytes,
aux_clock_divider);
@@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
/* Send the command and wait for it to complete */
I915_WRITE(ch_ctl, send_ctl);
- status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+ status = intel_dp_aux_wait_done(intel_dp);
/* Clear done status and any errors */
I915_WRITE(ch_ctl,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9bba0354ccd3..7cc5deed9511 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1135,7 +1135,6 @@ struct intel_dp {
* register with to kick off an AUX transaction.
*/
uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
- bool has_aux_irq,
int send_bytes,
uint32_t aux_clock_divider);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0d548292dd09..fd348f1b53ef 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -328,7 +328,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
/* Start with bits set for DDI_AUX_CTL register */
- aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
+ aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
aux_clock_divider);
/* Select only valid bits for SRD_AUX_CTL */
--
2.14.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
@ 2018-04-25 22:11 ` Patchwork
2018-04-25 22:28 ` ✓ Fi.CI.BAT: success " Patchwork
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-25 22:11 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq
URL : https://patchwork.freedesktop.org/series/42305/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm/i915: remove check for aux irq
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3659:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3658:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: remove check for aux irq
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
2018-04-25 22:11 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
@ 2018-04-25 22:28 ` Patchwork
2018-04-25 23:24 ` [PATCH] " Rodrigo Vivi
` (9 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-25 22:28 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4099 -> Patchwork_8803 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_8803 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_mmap_gtt@basic-small-bo-tiledx:
fi-gdg-551: PASS -> FAIL (fdo#102575)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-ivb-3520m: PASS -> DMESG-WARN (fdo#106084)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
== Participating hosts (38 -> 35) ==
Missing (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510
== Build changes ==
* Linux: CI_DRM_4099 -> Patchwork_8803
CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8803: e1dc08fa1c44a40c406ef757a2b85f4c41b6fb64 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit
== Linux commits ==
e1dc08fa1c44 drm/i915: remove check for aux irq
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8803/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
2018-04-25 22:11 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
2018-04-25 22:28 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-25 23:24 ` Rodrigo Vivi
2018-04-26 2:45 ` ✓ Fi.CI.IGT: success for " Patchwork
` (8 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Rodrigo Vivi @ 2018-04-25 23:24 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> GMBUS and AUX interrupts on gen4/g4x").
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_psr.c | 2 +-
> 4 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8444ca8d5aa3..09e1c2289ea1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>
> /*
> - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> * even when in MSI mode. This results in spurious interrupt warnings if the
> * legacy irq no. is shared with another device. The kernel then disables that
> * interrupt source and so prevents the other device from working properly.
> @@ -2553,7 +2553,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> * interrupts.
> */
> -#define HAS_AUX_IRQ(dev_priv) true
> #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
>
> /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 62f82c4298ac..fd417473b6a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> }
>
> static uint32_t
> -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> bool done;
>
> #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> - if (has_aux_irq)
> - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> - msecs_to_jiffies_timeout(10));
> - else
> - done = wait_for(C, 10) == 0;
> + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> + msecs_to_jiffies_timeout(10));
> if (!done)
> - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> - has_aux_irq);
> + DRM_ERROR("dp aux hw did not signal timeout!\n");
> #undef C
>
> return status;
> @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> }
>
> static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider)
> {
> @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> timeout |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> }
>
> static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t unused)
> {
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> DP_AUX_CH_CTL_TIME_OUT_MAX |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> int i, ret, recv_bytes;
> uint32_t status;
> int try, clock = 0;
> - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> bool vdd;
>
> ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> - has_aux_irq,
> send_bytes,
> aux_clock_divider);
>
> @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> /* Send the command and wait for it to complete */
> I915_WRITE(ch_ctl, send_ctl);
>
> - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> + status = intel_dp_aux_wait_done(intel_dp);
>
> /* Clear done status and any errors */
> I915_WRITE(ch_ctl,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9bba0354ccd3..7cc5deed9511 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1135,7 +1135,6 @@ struct intel_dp {
> * register with to kick off an AUX transaction.
> */
> uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider);
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0d548292dd09..fd348f1b53ef 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -328,7 +328,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>
> /* Start with bits set for DDI_AUX_CTL register */
> - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> aux_clock_divider);
>
> /* Select only valid bits for SRD_AUX_CTL */
> --
> 2.14.3
>
> _______________________________________________
> 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] 28+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: remove check for aux irq
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (2 preceding siblings ...)
2018-04-25 23:24 ` [PATCH] " Rodrigo Vivi
@ 2018-04-26 2:45 ` Patchwork
2018-04-26 13:43 ` [PATCH] " Ville Syrjälä
` (7 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-26 2:45 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4099_full -> Patchwork_8803_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8803_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8803_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8803_full:
=== IGT changes ===
==== Warnings ====
igt@gem_mocs_settings@mocs-rc6-vebox:
shard-kbl: SKIP -> PASS +1
== Known issues ==
Here are the changes found in Patchwork_8803_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665)
igt@kms_cursor_legacy@flip-vs-cursor-atomic:
shard-hsw: PASS -> FAIL (fdo#102670)
igt@kms_flip@flip-vs-wf_vblank-interruptible:
shard-apl: PASS -> FAIL (fdo#100368)
igt@kms_flip@modeset-vs-vblank-race-interruptible:
shard-glk: PASS -> FAIL (fdo#103060)
igt@kms_flip@wf_vblank-ts-check-interruptible:
shard-glk: PASS -> FAIL (fdo#100368)
==== Possible fixes ====
igt@kms_atomic@plane_invalid_params:
shard-kbl: DMESG-WARN -> PASS
igt@kms_flip@2x-flip-vs-wf_vblank:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@absolute-wf_vblank-interruptible:
shard-glk: FAIL (fdo#106087) -> PASS
igt@kms_flip@flip-vs-expired-vblank:
shard-hsw: FAIL (fdo#105707) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@flip-vs-wf_vblank-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_vblank@pipe-c-accuracy-idle:
shard-hsw: FAIL (fdo#102583) -> PASS
igt@perf@buffer-fill:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
== Participating hosts (6 -> 5) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4099 -> Patchwork_8803
CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8803: e1dc08fa1c44a40c406ef757a2b85f4c41b6fb64 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8803/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (3 preceding siblings ...)
2018-04-26 2:45 ` ✓ Fi.CI.IGT: success for " Patchwork
@ 2018-04-26 13:43 ` Ville Syrjälä
2018-04-26 15:22 ` Lucas De Marchi
2018-04-30 22:17 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: remove check for aux irq (rev2) Patchwork
` (6 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-04-26 13:43 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> GMBUS and AUX interrupts on gen4/g4x").
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_psr.c | 2 +-
> 4 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8444ca8d5aa3..09e1c2289ea1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>
> /*
> - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> + * gmbus irq on gen4 seems to be able to generate legacy interrupts
Why are you removing vital information from the comment?
> * even when in MSI mode. This results in spurious interrupt warnings if the
> * legacy irq no. is shared with another device. The kernel then disables that
> * interrupt source and so prevents the other device from working properly.
> @@ -2553,7 +2553,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> * interrupts.
> */
> -#define HAS_AUX_IRQ(dev_priv) true
> #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
>
> /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 62f82c4298ac..fd417473b6a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> }
>
> static uint32_t
> -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> bool done;
>
> #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> - if (has_aux_irq)
> - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> - msecs_to_jiffies_timeout(10));
> - else
> - done = wait_for(C, 10) == 0;
> + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> + msecs_to_jiffies_timeout(10));
> if (!done)
> - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> - has_aux_irq);
> + DRM_ERROR("dp aux hw did not signal timeout!\n");
> #undef C
>
> return status;
> @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> }
>
> static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider)
> {
> @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> timeout |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> }
>
> static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t unused)
> {
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> DP_AUX_CH_CTL_TIME_OUT_MAX |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> int i, ret, recv_bytes;
> uint32_t status;
> int try, clock = 0;
> - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> bool vdd;
>
> ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> - has_aux_irq,
> send_bytes,
> aux_clock_divider);
>
> @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> /* Send the command and wait for it to complete */
> I915_WRITE(ch_ctl, send_ctl);
>
> - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> + status = intel_dp_aux_wait_done(intel_dp);
>
> /* Clear done status and any errors */
> I915_WRITE(ch_ctl,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9bba0354ccd3..7cc5deed9511 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1135,7 +1135,6 @@ struct intel_dp {
> * register with to kick off an AUX transaction.
> */
> uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider);
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0d548292dd09..fd348f1b53ef 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -328,7 +328,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>
> /* Start with bits set for DDI_AUX_CTL register */
> - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> aux_clock_divider);
>
> /* Select only valid bits for SRD_AUX_CTL */
> --
> 2.14.3
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-26 13:43 ` [PATCH] " Ville Syrjälä
@ 2018-04-26 15:22 ` Lucas De Marchi
2018-04-26 15:27 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-04-26 15:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > GMBUS and AUX interrupts on gen4/g4x").
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > 4 files changed, 9 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8444ca8d5aa3..09e1c2289ea1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> >
> > /*
> > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
>
> Why are you removing vital information from the comment?
Because it wouldn't match the code anymore. We always use aux irq.
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-26 15:22 ` Lucas De Marchi
@ 2018-04-26 15:27 ` Ville Syrjälä
2018-04-26 15:42 ` Lucas De Marchi
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-04-26 15:27 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Thu, Apr 26, 2018 at 08:22:12AM -0700, Lucas De Marchi wrote:
> On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > GMBUS and AUX interrupts on gen4/g4x").
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > 4 files changed, 9 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 8444ca8d5aa3..09e1c2289ea1 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > >
> > > /*
> > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> >
> > Why are you removing vital information from the comment?
>
> Because it wouldn't match the code anymore. We always use aux irq.
The comment is documenting the hardware behaviour. We don't want to lose
that information.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-26 15:27 ` Ville Syrjälä
@ 2018-04-26 15:42 ` Lucas De Marchi
2018-04-26 15:50 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-04-26 15:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Thu, Apr 26, 2018 at 06:27:26PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 26, 2018 at 08:22:12AM -0700, Lucas De Marchi wrote:
> > On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > GMBUS and AUX interrupts on gen4/g4x").
> > > >
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > 4 files changed, 9 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 8444ca8d5aa3..09e1c2289ea1 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > >
> > > > /*
> > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> > >
> > > Why are you removing vital information from the comment?
> >
> > Because it wouldn't match the code anymore. We always use aux irq.
>
> The comment is documenting the hardware behaviour. We don't want to lose
> that information.
IMO it's confusing to have the comment saying one thing and then code
not following it. Reading it again I see the second paragraph you added
actually document the code and the first the HW behavior. Maybe starting
the second paragraph with a "However" would make it clearer. Or I can just
drop this change in the comment.
thanks
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-26 15:42 ` Lucas De Marchi
@ 2018-04-26 15:50 ` Ville Syrjälä
2018-04-30 22:05 ` Lucas De Marchi
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-04-26 15:50 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Thu, Apr 26, 2018 at 08:42:54AM -0700, Lucas De Marchi wrote:
> On Thu, Apr 26, 2018 at 06:27:26PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 26, 2018 at 08:22:12AM -0700, Lucas De Marchi wrote:
> > > On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > >
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > 4 files changed, 9 insertions(+), 19 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index 8444ca8d5aa3..09e1c2289ea1 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > >
> > > > > /*
> > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > >
> > > > Why are you removing vital information from the comment?
> > >
> > > Because it wouldn't match the code anymore. We always use aux irq.
> >
> > The comment is documenting the hardware behaviour. We don't want to lose
> > that information.
>
> IMO it's confusing to have the comment saying one thing and then code
> not following it. Reading it again I see the second paragraph you added
> actually document the code and the first the HW behavior. Maybe starting
> the second paragraph with a "However" would make it clearer. Or I can just
> drop this change in the comment.
Or you can move the relevant parts of the comment to the place where
we do the "MSI or not to MSI" decision.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-26 15:50 ` Ville Syrjälä
@ 2018-04-30 22:05 ` Lucas De Marchi
2018-05-02 12:07 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-04-30 22:05 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Thu, Apr 26, 2018 at 06:50:05PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 26, 2018 at 08:42:54AM -0700, Lucas De Marchi wrote:
> > On Thu, Apr 26, 2018 at 06:27:26PM +0300, Ville Syrjälä wrote:
> > > On Thu, Apr 26, 2018 at 08:22:12AM -0700, Lucas De Marchi wrote:
> > > > On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > > >
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > > 4 files changed, 9 insertions(+), 19 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > index 8444ca8d5aa3..09e1c2289ea1 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > > >
> > > > > > /*
> > > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > > > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > >
> > > > > Why are you removing vital information from the comment?
> > > >
> > > > Because it wouldn't match the code anymore. We always use aux irq.
> > >
> > > The comment is documenting the hardware behaviour. We don't want to lose
> > > that information.
> >
> > IMO it's confusing to have the comment saying one thing and then code
> > not following it. Reading it again I see the second paragraph you added
> > actually document the code and the first the HW behavior. Maybe starting
> > the second paragraph with a "However" would make it clearer. Or I can just
> > drop this change in the comment.
>
> Or you can move the relevant parts of the comment to the place where
> we do the "MSI or not to MSI" decision.
Humn, not sure I fully understand what you mean by relevant part. Do you
mean something like this?
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b7dbeba72dec..3fc6b915dac1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1183,6 +1183,9 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
* get lost on g4x as well, and interrupt delivery seems to stay
* properly dead afterwards. So we'll just disable them for all
* pre-gen5 chipsets.
+ *
+ * Since we don't enable MSI on gen <= 4 we can always use GMBUS/AUX
+ * interrupts.
*/
if (INTEL_GEN(dev_priv) >= 5) {
if (pci_enable_msi(pdev) < 0)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09e1c2289ea1..5fd47227da23 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2545,13 +2545,10 @@ intel_info(const struct drm_i915_private *dev_priv)
IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
/*
- * gmbus irq on gen4 seems to be able to generate legacy interrupts
+ * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
* even when in MSI mode. This results in spurious interrupt warnings if the
* legacy irq no. is shared with another device. The kernel then disables that
* interrupt source and so prevents the other device from working properly.
- *
- * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
- * interrupts.
*/
#define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
-------
It doesn't seem clearer to me.
Lucas De Marchi
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: remove check for aux irq (rev2)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (4 preceding siblings ...)
2018-04-26 13:43 ` [PATCH] " Ville Syrjälä
@ 2018-04-30 22:17 ` Patchwork
2018-04-30 22:18 ` ✗ Fi.CI.SPARSE: " Patchwork
` (5 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-30 22:17 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev2)
URL : https://patchwork.freedesktop.org/series/42305/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
83b37e765db4 drm/i915: remove check for aux irq
-:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15:
> > > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 309bd8ed464f ("drm/i915: Reinstate GMBUS and AUX interrupts on gen4/g4x")'
#15:
> > > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
-:95: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
total: 2 errors, 1 warnings, 0 checks, 18 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq (rev2)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (5 preceding siblings ...)
2018-04-30 22:17 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: remove check for aux irq (rev2) Patchwork
@ 2018-04-30 22:18 ` Patchwork
2018-04-30 22:35 ` ✓ Fi.CI.BAT: success " Patchwork
` (4 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-30 22:18 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev2)
URL : https://patchwork.freedesktop.org/series/42305/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm/i915: remove check for aux irq
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3663:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3660:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: remove check for aux irq (rev2)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (6 preceding siblings ...)
2018-04-30 22:18 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-04-30 22:35 ` Patchwork
2018-05-01 0:54 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-04-30 22:35 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev2)
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4115 -> Patchwork_8848 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8848 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8848, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8848:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_8848 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_mmap_gtt@basic-small-bo-tiledx:
fi-gdg-551: PASS -> FAIL (fdo#102575)
igt@kms_chamelium@dp-edid-read:
fi-kbl-7500u: PASS -> FAIL (fdo#103841)
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: PASS -> FAIL (fdo#100368)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-cnl-psr: PASS -> DMESG-WARN (fdo#104951)
==== Possible fixes ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-ivb-3520m: DMESG-WARN (fdo#106084) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
== Participating hosts (38 -> 35) ==
Missing (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4115 -> Patchwork_8848
CI_DRM_4115: 348182ddb984ca22a8005e93f18d55bbebb1702e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8848: 83b37e765db46e83ae2f781fb52ed21f66311357 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit
== Linux commits ==
83b37e765db4 drm/i915: remove check for aux irq
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8848/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: remove check for aux irq (rev2)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (7 preceding siblings ...)
2018-04-30 22:35 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-01 0:54 ` Patchwork
2018-05-23 18:13 ` ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq (rev3) Patchwork
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-05-01 0:54 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev2)
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4115_full -> Patchwork_8848_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8848_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8848_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8848_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-vebox:
shard-kbl: PASS -> SKIP +2
igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
shard-kbl: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_8848_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_mmap_gtt@coherency:
shard-glk: NOTRUN -> FAIL (fdo#100587)
igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge:
shard-apl: PASS -> FAIL (fdo#104671)
igt@kms_cursor_legacy@cursor-vs-flip-toggle:
shard-hsw: PASS -> FAIL (fdo#103355)
igt@kms_flip@2x-plain-flip-fb-recreate:
shard-hsw: PASS -> FAIL (fdo#100368)
igt@kms_flip@dpms-vs-vblank-race-interruptible:
shard-hsw: PASS -> FAIL (fdo#103060)
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-hsw: PASS -> FAIL (fdo#105707)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-apl: PASS -> WARN (fdo#100047)
==== Possible fixes ====
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-glk: INCOMPLETE (k.org#198133, fdo#103359) -> PASS
igt@kms_cursor_crc@cursor-64x64-dpms:
shard-apl: FAIL (fdo#103232, fdo#104645) -> PASS
igt@kms_flip@flip-vs-wf_vblank-interruptible:
shard-glk: FAIL (fdo#105312) -> PASS
igt@kms_flip@modeset-vs-vblank-race:
shard-glk: FAIL (fdo#103060) -> PASS +1
igt@kms_flip@plain-flip-fb-recreate:
shard-hsw: FAIL (fdo#100368) -> PASS +1
igt@kms_flip@plain-flip-ts-check-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +1
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#100587 https://bugs.freedesktop.org/show_bug.cgi?id=100587
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#104645 https://bugs.freedesktop.org/show_bug.cgi?id=104645
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312
fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (9 -> 8) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4115 -> Patchwork_8848
CI_DRM_4115: 348182ddb984ca22a8005e93f18d55bbebb1702e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4453: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8848: 83b37e765db46e83ae2f781fb52ed21f66311357 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4453: d0a0bca2194a673c4d9a70a2256837c59213c64b @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8848/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] drm/i915: remove check for aux irq
2018-04-30 22:05 ` Lucas De Marchi
@ 2018-05-02 12:07 ` Ville Syrjälä
2018-05-23 18:04 ` [PATCH v2] " Lucas De Marchi
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-05-02 12:07 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Mon, Apr 30, 2018 at 03:05:01PM -0700, Lucas De Marchi wrote:
> On Thu, Apr 26, 2018 at 06:50:05PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 26, 2018 at 08:42:54AM -0700, Lucas De Marchi wrote:
> > > On Thu, Apr 26, 2018 at 06:27:26PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Apr 26, 2018 at 08:22:12AM -0700, Lucas De Marchi wrote:
> > > > > On Thu, Apr 26, 2018 at 04:43:38PM +0300, Ville Syrjälä wrote:
> > > > > > On Wed, Apr 25, 2018 at 02:55:24PM -0700, Lucas De Marchi wrote:
> > > > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > > > >
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/i915_drv.h | 3 +--
> > > > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > > > 4 files changed, 9 insertions(+), 19 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > index 8444ca8d5aa3..09e1c2289ea1 100644
> > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > @@ -2545,7 +2545,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > > > >
> > > > > > > /*
> > > > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > > > > + * gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > > >
> > > > > > Why are you removing vital information from the comment?
> > > > >
> > > > > Because it wouldn't match the code anymore. We always use aux irq.
> > > >
> > > > The comment is documenting the hardware behaviour. We don't want to lose
> > > > that information.
> > >
> > > IMO it's confusing to have the comment saying one thing and then code
> > > not following it. Reading it again I see the second paragraph you added
> > > actually document the code and the first the HW behavior. Maybe starting
> > > the second paragraph with a "However" would make it clearer. Or I can just
> > > drop this change in the comment.
> >
> > Or you can move the relevant parts of the comment to the place where
> > we do the "MSI or not to MSI" decision.
>
> Humn, not sure I fully understand what you mean by relevant part. Do you
> mean something like this?
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b7dbeba72dec..3fc6b915dac1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1183,6 +1183,9 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> * get lost on g4x as well, and interrupt delivery seems to stay
> * properly dead afterwards. So we'll just disable them for all
> * pre-gen5 chipsets.
> + *
> + * Since we don't enable MSI on gen <= 4 we can always use GMBUS/AUX
> + * interrupts.
> */
> if (INTEL_GEN(dev_priv) >= 5) {
> if (pci_enable_msi(pdev) < 0)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 09e1c2289ea1..5fd47227da23 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2545,13 +2545,10 @@ intel_info(const struct drm_i915_private *dev_priv)
> IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>
> /*
> - * gmbus irq on gen4 seems to be able to generate legacy interrupts
> + * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> * even when in MSI mode. This results in spurious interrupt warnings if the
> * legacy irq no. is shared with another device. The kernel then disables that
> * interrupt source and so prevents the other device from working properly.
I mean all of that ^
> - *
> - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> - * interrupts.
> */
> #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
>
> -------
>
> It doesn't seem clearer to me.
>
> Lucas De Marchi
>
>
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] drm/i915: remove check for aux irq
2018-05-02 12:07 ` Ville Syrjälä
@ 2018-05-23 18:04 ` Lucas De Marchi
2018-06-15 17:58 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-05-23 18:04 UTC (permalink / raw)
To: intel-gfx
This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
GMBUS and AUX interrupts on gen4/g4x").
v2: Move comment about HW behavior to where decision is made to enable
MSI (Ville).
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
drivers/gpu/drm/i915/i915_drv.h | 10 ----------
drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_psr.c | 2 +-
5 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..a1461de20472 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
* get lost on g4x as well, and interrupt delivery seems to stay
* properly dead afterwards. So we'll just disable them for all
* pre-gen5 chipsets.
+ *
+ * dp aux and gmbus irq on gen4 seems to be able to generate legacy
+ * interrupts even when in MSI mode. This results in spurious
+ * interrupt warnings if the legacy irq no. is shared with another
+ * device. The kernel then disables that interrupt source and so
+ * prevents the other device from working properly.
*/
if (INTEL_GEN(dev_priv) >= 5) {
if (pci_enable_msi(pdev) < 0)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b86ed6401120..c5e1f648c47c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
(IS_CANNONLAKE(dev_priv) || \
IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
-/*
- * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
- * even when in MSI mode. This results in spurious interrupt warnings if the
- * legacy irq no. is shared with another device. The kernel then disables that
- * interrupt source and so prevents the other device from working properly.
- *
- * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
- * interrupts.
- */
-#define HAS_AUX_IRQ(dev_priv) true
#define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
/* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ce07bd794aed..1dab40056df7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
}
static uint32_t
-intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
+intel_dp_aux_wait_done(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
@@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
bool done;
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
- if (has_aux_irq)
- done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
- msecs_to_jiffies_timeout(10));
- else
- done = wait_for(C, 10) == 0;
+ done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
+ msecs_to_jiffies_timeout(10));
if (!done)
- DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
- has_aux_irq);
+ DRM_ERROR("dp aux hw did not signal timeout!\n");
#undef C
return status;
@@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
}
static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
- bool has_aux_irq,
int send_bytes,
uint32_t aux_clock_divider)
{
@@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
return DP_AUX_CH_CTL_SEND_BUSY |
DP_AUX_CH_CTL_DONE |
- (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+ DP_AUX_CH_CTL_INTERRUPT |
DP_AUX_CH_CTL_TIME_OUT_ERROR |
timeout |
DP_AUX_CH_CTL_RECEIVE_ERROR |
@@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
}
static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
- bool has_aux_irq,
int send_bytes,
uint32_t unused)
{
return DP_AUX_CH_CTL_SEND_BUSY |
DP_AUX_CH_CTL_DONE |
- (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+ DP_AUX_CH_CTL_INTERRUPT |
DP_AUX_CH_CTL_TIME_OUT_ERROR |
DP_AUX_CH_CTL_TIME_OUT_MAX |
DP_AUX_CH_CTL_RECEIVE_ERROR |
@@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
int i, ret, recv_bytes;
uint32_t status;
int try, clock = 0;
- bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
bool vdd;
ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
@@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
- has_aux_irq,
send_bytes,
aux_clock_divider);
@@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
/* Send the command and wait for it to complete */
I915_WRITE(ch_ctl, send_ctl);
- status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+ status = intel_dp_aux_wait_done(intel_dp);
/* Clear done status and any errors */
I915_WRITE(ch_ctl,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2855363f5b6c..7ac91d5ee3fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1138,7 +1138,6 @@ struct intel_dp {
* register with to kick off an AUX transaction.
*/
uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
- bool has_aux_irq,
int send_bytes,
uint32_t aux_clock_divider);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index db27f2faa1de..ee26f5f33b87 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
/* Start with bits set for DDI_AUX_CTL register */
- aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
+ aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
aux_clock_divider);
/* Select only valid bits for SRD_AUX_CTL */
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq (rev3)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (8 preceding siblings ...)
2018-05-01 0:54 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-05-23 18:13 ` Patchwork
2018-05-23 18:28 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-23 22:38 ` ✓ Fi.CI.IGT: " Patchwork
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-05-23 18:13 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev3)
URL : https://patchwork.freedesktop.org/series/42305/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm/i915: remove check for aux irq
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3663:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: remove check for aux irq (rev3)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (9 preceding siblings ...)
2018-05-23 18:13 ` ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq (rev3) Patchwork
@ 2018-05-23 18:28 ` Patchwork
2018-05-23 22:38 ` ✓ Fi.CI.IGT: " Patchwork
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-05-23 18:28 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev3)
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4227 -> Patchwork_9099 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9099 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9099, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/3/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9099:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_9099 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-hsw-4770: FAIL (fdo#100368) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
== Participating hosts (44 -> 38) ==
Missing (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4227 -> Patchwork_9099
CI_DRM_4227: a8727d3fe03770e4d523468dfbc487dfe01597d3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4495: 71c7a5740913d2618f44bca252669efe8a84f4c9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9099: 7217baea500cd2700296eb294dcdc66bcfc8494a @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4495: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Linux commits ==
7217baea500c drm/i915: remove check for aux irq
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9099/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: remove check for aux irq (rev3)
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
` (10 preceding siblings ...)
2018-05-23 18:28 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-23 22:38 ` Patchwork
11 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2018-05-23 22:38 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: remove check for aux irq (rev3)
URL : https://patchwork.freedesktop.org/series/42305/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4227_full -> Patchwork_9099_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9099_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9099_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42305/revisions/3/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9099_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-vebox:
shard-kbl: PASS -> SKIP
igt@perf_pmu@rc6:
shard-kbl: SKIP -> PASS
igt@pm_rc6_residency@rc6-accuracy:
shard-snb: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_9099_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_gtt:
shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@drv_selftest@live_hangcheck:
shard-kbl: PASS -> DMESG-FAIL (fdo#106560)
shard-apl: PASS -> DMESG-FAIL (fdo#106560)
igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
shard-hsw: PASS -> FAIL (fdo#104873)
igt@kms_flip@2x-flip-vs-wf_vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#103928)
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#102887)
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-hsw: PASS -> FAIL (fdo#100368)
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: PASS -> FAIL (fdo#103822, fdo#104724)
==== Possible fixes ====
igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
shard-glk: FAIL (fdo#105703) -> PASS
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: FAIL (fdo#105707) -> PASS
igt@kms_flip@2x-plain-flip-fb-recreate:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
shard-hsw: FAIL (fdo#103928) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +1
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: FAIL (fdo#103822, fdo#104724) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4227 -> Patchwork_9099
CI_DRM_4227: a8727d3fe03770e4d523468dfbc487dfe01597d3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4495: 71c7a5740913d2618f44bca252669efe8a84f4c9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9099: 7217baea500cd2700296eb294dcdc66bcfc8494a @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4495: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9099/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-05-23 18:04 ` [PATCH v2] " Lucas De Marchi
@ 2018-06-15 17:58 ` Ville Syrjälä
2018-06-15 21:51 ` Lucas De Marchi
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-06-15 17:58 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi wrote:
> This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> GMBUS and AUX interrupts on gen4/g4x").
>
> v2: Move comment about HW behavior to where decision is made to enable
> MSI (Ville).
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_psr.c | 2 +-
> 5 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9c449b8d8eab..a1461de20472 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> * get lost on g4x as well, and interrupt delivery seems to stay
> * properly dead afterwards. So we'll just disable them for all
> * pre-gen5 chipsets.
> + *
> + * dp aux and gmbus irq on gen4 seems to be able to generate legacy
> + * interrupts even when in MSI mode. This results in spurious
> + * interrupt warnings if the legacy irq no. is shared with another
> + * device. The kernel then disables that interrupt source and so
> + * prevents the other device from working properly.
> */
> if (INTEL_GEN(dev_priv) >= 5) {
> if (pci_enable_msi(pdev) < 0)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b86ed6401120..c5e1f648c47c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> (IS_CANNONLAKE(dev_priv) || \
> IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>
> -/*
> - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> - * even when in MSI mode. This results in spurious interrupt warnings if the
> - * legacy irq no. is shared with another device. The kernel then disables that
> - * interrupt source and so prevents the other device from working properly.
> - *
> - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> - * interrupts.
> - */
> -#define HAS_AUX_IRQ(dev_priv) true
> #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
>
> /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ce07bd794aed..1dab40056df7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> }
>
> static uint32_t
> -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> bool done;
>
> #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> - if (has_aux_irq)
> - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> - msecs_to_jiffies_timeout(10));
> - else
> - done = wait_for(C, 10) == 0;
> + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> + msecs_to_jiffies_timeout(10));
> if (!done)
> - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> - has_aux_irq);
> + DRM_ERROR("dp aux hw did not signal timeout!\n");
> #undef C
>
> return status;
> @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> }
>
> static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider)
> {
> @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> timeout |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> }
>
> static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t unused)
> {
> return DP_AUX_CH_CTL_SEND_BUSY |
> DP_AUX_CH_CTL_DONE |
> - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> + DP_AUX_CH_CTL_INTERRUPT |
> DP_AUX_CH_CTL_TIME_OUT_ERROR |
> DP_AUX_CH_CTL_TIME_OUT_MAX |
> DP_AUX_CH_CTL_RECEIVE_ERROR |
> @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> int i, ret, recv_bytes;
> uint32_t status;
> int try, clock = 0;
> - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> bool vdd;
>
> ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> - has_aux_irq,
> send_bytes,
> aux_clock_divider);
>
> @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> /* Send the command and wait for it to complete */
> I915_WRITE(ch_ctl, send_ctl);
>
> - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> + status = intel_dp_aux_wait_done(intel_dp);
>
> /* Clear done status and any errors */
> I915_WRITE(ch_ctl,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2855363f5b6c..7ac91d5ee3fa 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1138,7 +1138,6 @@ struct intel_dp {
> * register with to kick off an AUX transaction.
> */
> uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> - bool has_aux_irq,
> int send_bytes,
> uint32_t aux_clock_divider);
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index db27f2faa1de..ee26f5f33b87 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>
> /* Start with bits set for DDI_AUX_CTL register */
> - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> aux_clock_divider);
Hmm. What's the situation with the PSR stuff here?
>
> /* Select only valid bits for SRD_AUX_CTL */
> --
> 2.17.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-15 17:58 ` Ville Syrjälä
@ 2018-06-15 21:51 ` Lucas De Marchi
2018-06-19 14:02 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-06-15 21:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: daniel.vetter, intel-gfx, dhinakaran.pandiyan
On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi wrote:
> > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > GMBUS and AUX interrupts on gen4/g4x").
> >
> > v2: Move comment about HW behavior to where decision is made to enable
> > MSI (Ville).
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > 5 files changed, 14 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 9c449b8d8eab..a1461de20472 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> > * get lost on g4x as well, and interrupt delivery seems to stay
> > * properly dead afterwards. So we'll just disable them for all
> > * pre-gen5 chipsets.
> > + *
> > + * dp aux and gmbus irq on gen4 seems to be able to generate legacy
> > + * interrupts even when in MSI mode. This results in spurious
> > + * interrupt warnings if the legacy irq no. is shared with another
> > + * device. The kernel then disables that interrupt source and so
> > + * prevents the other device from working properly.
> > */
> > if (INTEL_GEN(dev_priv) >= 5) {
> > if (pci_enable_msi(pdev) < 0)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index b86ed6401120..c5e1f648c47c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > (IS_CANNONLAKE(dev_priv) || \
> > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> >
> > -/*
> > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > - * even when in MSI mode. This results in spurious interrupt warnings if the
> > - * legacy irq no. is shared with another device. The kernel then disables that
> > - * interrupt source and so prevents the other device from working properly.
> > - *
> > - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> > - * interrupts.
> > - */
> > -#define HAS_AUX_IRQ(dev_priv) true
> > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> >
> > /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index ce07bd794aed..1dab40056df7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > }
> >
> > static uint32_t
> > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > bool done;
> >
> > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > - if (has_aux_irq)
> > - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > - msecs_to_jiffies_timeout(10));
> > - else
> > - done = wait_for(C, 10) == 0;
> > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > + msecs_to_jiffies_timeout(10));
> > if (!done)
> > - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> > - has_aux_irq);
> > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > #undef C
> >
> > return status;
> > @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > }
> >
> > static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > - bool has_aux_irq,
> > int send_bytes,
> > uint32_t aux_clock_divider)
> > {
> > @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> >
> > return DP_AUX_CH_CTL_SEND_BUSY |
> > DP_AUX_CH_CTL_DONE |
> > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > + DP_AUX_CH_CTL_INTERRUPT |
> > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > timeout |
> > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > }
> >
> > static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > - bool has_aux_irq,
> > int send_bytes,
> > uint32_t unused)
> > {
> > return DP_AUX_CH_CTL_SEND_BUSY |
> > DP_AUX_CH_CTL_DONE |
> > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > + DP_AUX_CH_CTL_INTERRUPT |
> > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > int i, ret, recv_bytes;
> > uint32_t status;
> > int try, clock = 0;
> > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > bool vdd;
> >
> > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >
> > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > - has_aux_irq,
> > send_bytes,
> > aux_clock_divider);
> >
> > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > /* Send the command and wait for it to complete */
> > I915_WRITE(ch_ctl, send_ctl);
> >
> > - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> > + status = intel_dp_aux_wait_done(intel_dp);
> >
> > /* Clear done status and any errors */
> > I915_WRITE(ch_ctl,
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 2855363f5b6c..7ac91d5ee3fa 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > * register with to kick off an AUX transaction.
> > */
> > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > - bool has_aux_irq,
> > int send_bytes,
> > uint32_t aux_clock_divider);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index db27f2faa1de..ee26f5f33b87 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
> >
> > /* Start with bits set for DDI_AUX_CTL register */
> > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> > aux_clock_divider);
>
> Hmm. What's the situation with the PSR stuff here?
I hadn't notice this, thanks.
I don't see why it would actually be correct though. It seems to be a
bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
functions") when the rmw was removed (so whatever was on the register
before get overwritten).
CC'ing Daniel who wrote that patch and José/DK who worked recently on
psr stuff.
Lucas De Marchi
>
> >
> > /* Select only valid bits for SRD_AUX_CTL */
> > --
> > 2.17.0
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-15 21:51 ` Lucas De Marchi
@ 2018-06-19 14:02 ` Ville Syrjälä
2018-06-19 15:24 ` Lucas De Marchi
0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2018-06-19 14:02 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: daniel.vetter, intel-gfx, dhinakaran.pandiyan
On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi wrote:
> On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi wrote:
> > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > GMBUS and AUX interrupts on gen4/g4x").
> > >
> > > v2: Move comment about HW behavior to where decision is made to enable
> > > MSI (Ville).
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 9c449b8d8eab..a1461de20472 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> > > * get lost on g4x as well, and interrupt delivery seems to stay
> > > * properly dead afterwards. So we'll just disable them for all
> > > * pre-gen5 chipsets.
> > > + *
> > > + * dp aux and gmbus irq on gen4 seems to be able to generate legacy
> > > + * interrupts even when in MSI mode. This results in spurious
> > > + * interrupt warnings if the legacy irq no. is shared with another
> > > + * device. The kernel then disables that interrupt source and so
> > > + * prevents the other device from working properly.
> > > */
> > > if (INTEL_GEN(dev_priv) >= 5) {
> > > if (pci_enable_msi(pdev) < 0)
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index b86ed6401120..c5e1f648c47c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > (IS_CANNONLAKE(dev_priv) || \
> > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > >
> > > -/*
> > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > - * even when in MSI mode. This results in spurious interrupt warnings if the
> > > - * legacy irq no. is shared with another device. The kernel then disables that
> > > - * interrupt source and so prevents the other device from working properly.
> > > - *
> > > - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> > > - * interrupts.
> > > - */
> > > -#define HAS_AUX_IRQ(dev_priv) true
> > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> > >
> > > /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index ce07bd794aed..1dab40056df7 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > }
> > >
> > > static uint32_t
> > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > bool done;
> > >
> > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > - if (has_aux_irq)
> > > - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > - msecs_to_jiffies_timeout(10));
> > > - else
> > > - done = wait_for(C, 10) == 0;
> > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > + msecs_to_jiffies_timeout(10));
> > > if (!done)
> > > - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> > > - has_aux_irq);
> > > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > > #undef C
> > >
> > > return status;
> > > @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > > }
> > >
> > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > - bool has_aux_irq,
> > > int send_bytes,
> > > uint32_t aux_clock_divider)
> > > {
> > > @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > >
> > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > DP_AUX_CH_CTL_DONE |
> > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > + DP_AUX_CH_CTL_INTERRUPT |
> > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > timeout |
> > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > }
> > >
> > > static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > - bool has_aux_irq,
> > > int send_bytes,
> > > uint32_t unused)
> > > {
> > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > DP_AUX_CH_CTL_DONE |
> > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > + DP_AUX_CH_CTL_INTERRUPT |
> > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > int i, ret, recv_bytes;
> > > uint32_t status;
> > > int try, clock = 0;
> > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > bool vdd;
> > >
> > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > >
> > > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > - has_aux_irq,
> > > send_bytes,
> > > aux_clock_divider);
> > >
> > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > /* Send the command and wait for it to complete */
> > > I915_WRITE(ch_ctl, send_ctl);
> > >
> > > - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> > > + status = intel_dp_aux_wait_done(intel_dp);
> > >
> > > /* Clear done status and any errors */
> > > I915_WRITE(ch_ctl,
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > * register with to kick off an AUX transaction.
> > > */
> > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > - bool has_aux_irq,
> > > int send_bytes,
> > > uint32_t aux_clock_divider);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index db27f2faa1de..ee26f5f33b87 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> > > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
> > >
> > > /* Start with bits set for DDI_AUX_CTL register */
> > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> > > aux_clock_divider);
> >
> > Hmm. What's the situation with the PSR stuff here?
>
> I hadn't notice this, thanks.
>
> I don't see why it would actually be correct though. It seems to be a
> bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> functions") when the rmw was removed (so whatever was on the register
> before get overwritten).
Dunno why you think there's a bug. The problem I see with this patch is
that you're now setting the irq enable bit on the psr aux register.
Does that bit even exist? If so does it do what we want it to do?
>
> CC'ing Daniel who wrote that patch and José/DK who worked recently on
> psr stuff.
>
>
> Lucas De Marchi
> >
> > >
> > > /* Select only valid bits for SRD_AUX_CTL */
> > > --
> > > 2.17.0
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-19 14:02 ` Ville Syrjälä
@ 2018-06-19 15:24 ` Lucas De Marchi
2018-06-19 20:17 ` Dhinakaran Pandiyan
2018-06-21 16:01 ` Ville Syrjälä
0 siblings, 2 replies; 28+ messages in thread
From: Lucas De Marchi @ 2018-06-19 15:24 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, intel-gfx, Lucas De Marchi, Dhinakaran Pandiyan
On Tue, Jun 19, 2018 at 7:06 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi wrote:
> > On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> > > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi wrote:
> > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > GMBUS and AUX interrupts on gen4/g4x").
> > > >
> > > > v2: Move comment about HW behavior to where decision is made to enable
> > > > MSI (Ville).
> > > >
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 9c449b8d8eab..a1461de20472 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> > > > * get lost on g4x as well, and interrupt delivery seems to stay
> > > > * properly dead afterwards. So we'll just disable them for all
> > > > * pre-gen5 chipsets.
> > > > + *
> > > > + * dp aux and gmbus irq on gen4 seems to be able to generate legacy
> > > > + * interrupts even when in MSI mode. This results in spurious
> > > > + * interrupt warnings if the legacy irq no. is shared with another
> > > > + * device. The kernel then disables that interrupt source and so
> > > > + * prevents the other device from working properly.
> > > > */
> > > > if (INTEL_GEN(dev_priv) >= 5) {
> > > > if (pci_enable_msi(pdev) < 0)
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index b86ed6401120..c5e1f648c47c 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > (IS_CANNONLAKE(dev_priv) || \
> > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > >
> > > > -/*
> > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > - * even when in MSI mode. This results in spurious interrupt warnings if the
> > > > - * legacy irq no. is shared with another device. The kernel then disables that
> > > > - * interrupt source and so prevents the other device from working properly.
> > > > - *
> > > > - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> > > > - * interrupts.
> > > > - */
> > > > -#define HAS_AUX_IRQ(dev_priv) true
> > > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> > > >
> > > > /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index ce07bd794aed..1dab40056df7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > }
> > > >
> > > > static uint32_t
> > > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > > bool done;
> > > >
> > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > - if (has_aux_irq)
> > > > - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > - msecs_to_jiffies_timeout(10));
> > > > - else
> > > > - done = wait_for(C, 10) == 0;
> > > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > + msecs_to_jiffies_timeout(10));
> > > > if (!done)
> > > > - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> > > > - has_aux_irq);
> > > > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > > > #undef C
> > > >
> > > > return status;
> > > > @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > > > }
> > > >
> > > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > - bool has_aux_irq,
> > > > int send_bytes,
> > > > uint32_t aux_clock_divider)
> > > > {
> > > > @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > >
> > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > DP_AUX_CH_CTL_DONE |
> > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > timeout |
> > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > }
> > > >
> > > > static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > - bool has_aux_irq,
> > > > int send_bytes,
> > > > uint32_t unused)
> > > > {
> > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > DP_AUX_CH_CTL_DONE |
> > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > int i, ret, recv_bytes;
> > > > uint32_t status;
> > > > int try, clock = 0;
> > > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > > bool vdd;
> > > >
> > > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > >
> > > > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > > > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > - has_aux_irq,
> > > > send_bytes,
> > > > aux_clock_divider);
> > > >
> > > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > /* Send the command and wait for it to complete */
> > > > I915_WRITE(ch_ctl, send_ctl);
> > > >
> > > > - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> > > > + status = intel_dp_aux_wait_done(intel_dp);
> > > >
> > > > /* Clear done status and any errors */
> > > > I915_WRITE(ch_ctl,
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > > * register with to kick off an AUX transaction.
> > > > */
> > > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > > - bool has_aux_irq,
> > > > int send_bytes,
> > > > uint32_t aux_clock_divider);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > index db27f2faa1de..ee26f5f33b87 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> > > > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
> > > >
> > > > /* Start with bits set for DDI_AUX_CTL register */
> > > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> > > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> > > > aux_clock_divider);
> > >
> > > Hmm. What's the situation with the PSR stuff here?
> >
> > I hadn't notice this, thanks.
> >
> > I don't see why it would actually be correct though. It seems to be a
> > bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> > functions") when the rmw was removed (so whatever was on the register
> > before get overwritten).
>
> Dunno why you think there's a bug. The problem I see with this patch is
> that you're now setting the irq enable bit on the psr aux register.
> Does that bit even exist? If so does it do what we want it to do?
I was assuming they should be the same. Now I see the psr code path
gets the bits for
DDI_AUX_CTL and mask to use only the valid bits for EDP_PSR_AUX_CTL. Bit 28 will
be left out regardless the return of get_aux_send_ctl()
Lucas De Marchi
>
> >
> > CC'ing Daniel who wrote that patch and José/DK who worked recently on
> > psr stuff.
> >
> >
> > Lucas De Marchi
> > >
> > > >
> > > > /* Select only valid bits for SRD_AUX_CTL */
> > > > --
> > > > 2.17.0
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-19 15:24 ` Lucas De Marchi
@ 2018-06-19 20:17 ` Dhinakaran Pandiyan
2018-06-21 17:04 ` Lucas De Marchi
2018-06-21 16:01 ` Ville Syrjälä
1 sibling, 1 reply; 28+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-19 20:17 UTC (permalink / raw)
To: Lucas De Marchi, Ville Syrjälä
Cc: Daniel Vetter, intel-gfx, Lucas De Marchi
On Tue, 2018-06-19 at 08:24 -0700, Lucas De Marchi wrote:
> On Tue, Jun 19, 2018 at 7:06 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> >
> > On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi wrote:
> > >
> > > On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> > > >
> > > > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi
> > > > wrote:
> > > > >
> > > > > This became dead code with commit 309bd8ed464f ("drm/i915:
> > > > > Reinstate
> > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > >
> > > > > v2: Move comment about HW behavior to where decision is made
> > > > > to enable
> > > > > MSI (Ville).
> > > > >
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > > index 9c449b8d8eab..a1461de20472 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct
> > > > > drm_i915_private *dev_priv)
> > > > > * get lost on g4x as well, and interrupt delivery seems to
> > > > > stay
> > > > > * properly dead afterwards. So we'll just disable them for
> > > > > all
> > > > > * pre-gen5 chipsets.
> > > > > + *
> > > > > + * dp aux and gmbus irq on gen4 seems to be able to
> > > > > generate legacy
> > > > > + * interrupts even when in MSI mode. This results in
> > > > > spurious
> > > > > + * interrupt warnings if the legacy irq no. is shared with
> > > > > another
> > > > > + * device. The kernel then disables that interrupt source
> > > > > and so
> > > > > + * prevents the other device from working properly.
> > > > > */
> > > > > if (INTEL_GEN(dev_priv) >= 5) {
> > > > > if (pci_enable_msi(pdev) < 0)
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index b86ed6401120..c5e1f648c47c 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -2558,16 +2558,6 @@ intel_info(const struct
> > > > > drm_i915_private *dev_priv)
> > > > > (IS_CANNONLAKE(dev_priv) || \
> > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > >
> > > > > -/*
> > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate
> > > > > legacy interrupts
> > > > > - * even when in MSI mode. This results in spurious interrupt
> > > > > warnings if the
> > > > > - * legacy irq no. is shared with another device. The kernel
> > > > > then disables that
> > > > > - * interrupt source and so prevents the other device from
> > > > > working properly.
> > > > > - *
> > > > > - * Since we don't enable MSI anymore on gen4, we can always
> > > > > use GMBUS/AUX
> > > > > - * interrupts.
> > > > > - */
> > > > > -#define HAS_AUX_IRQ(dev_priv) true
> > > > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> > > > >
> > > > > /* With the 945 and later, Y tiling got adjusted so that it
> > > > > was 32 128-byte
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index ce07bd794aed..1dab40056df7 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp
> > > > > *intel_dp)
> > > > > }
> > > > >
> > > > > static uint32_t
> > > > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
> > > > > has_aux_irq)
> > > > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > {
> > > > > struct drm_i915_private *dev_priv =
> > > > > to_i915(intel_dp_to_dev(intel_dp));
> > > > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp
> > > > > *intel_dp, bool has_aux_irq)
> > > > > bool done;
> > > > >
> > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) &
> > > > > DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > - if (has_aux_irq)
> > > > > - done = wait_event_timeout(dev_priv-
> > > > > >gmbus_wait_queue, C,
> > > > > - msecs_to_jiffies_timeout(
> > > > > 10));
> > > > > - else
> > > > > - done = wait_for(C, 10) == 0;
> > > > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > + msecs_to_jiffies_timeout(10));
> > > > > if (!done)
> > > > > - DRM_ERROR("dp aux hw did not signal timeout (has
> > > > > irq: %i)!\n",
> > > > > - has_aux_irq);
> > > > > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > > > > #undef C
> > > > >
> > > > > return status;
> > > > > @@ -1016,7 +1012,6 @@ static uint32_t
> > > > > skl_get_aux_clock_divider(struct intel_dp *intel_dp, int
> > > > > index)
> > > > > }
> > > > >
> > > > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp
> > > > > *intel_dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t aux_clock_divider)
> > > > > {
> > > > > @@ -1037,7 +1032,7 @@ static uint32_t
> > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > >
> > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > DP_AUX_CH_CTL_DONE |
> > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > timeout |
> > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > @@ -1047,13 +1042,12 @@ static uint32_t
> > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > }
> > > > >
> > > > > static uint32_t skl_get_aux_send_ctl(struct intel_dp
> > > > > *intel_dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t unused)
> > > > > {
> > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > DP_AUX_CH_CTL_DONE |
> > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > *intel_dp,
> > > > > int i, ret, recv_bytes;
> > > > > uint32_t status;
> > > > > int try, clock = 0;
> > > > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > > > bool vdd;
> > > > >
> > > > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > *intel_dp,
> > > > >
> > > > > while ((aux_clock_divider = intel_dp-
> > > > > >get_aux_clock_divider(intel_dp, clock++))) {
> > > > > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > - has_aux_i
> > > > > rq,
> > > > > send_byte
> > > > > s,
> > > > > aux_clock
> > > > > _divider);
> > > > >
> > > > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > *intel_dp,
> > > > > /* Send the command and wait for it to
> > > > > complete */
> > > > > I915_WRITE(ch_ctl, send_ctl);
> > > > >
> > > > > - status = intel_dp_aux_wait_done(intel_dp,
> > > > > has_aux_irq);
> > > > > + status = intel_dp_aux_wait_done(intel_dp);
> > > > >
> > > > > /* Clear done status and any errors */
> > > > > I915_WRITE(ch_ctl,
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > > > * register with to kick off an AUX transaction.
> > > > > */
> > > > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t aux_clock_divider);
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index db27f2faa1de..ee26f5f33b87 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct
> > > > > intel_dp *intel_dp)
> > > > > aux_clock_divider = intel_dp-
> > > > > >get_aux_clock_divider(intel_dp, 0);
> > > > >
> > > > > /* Start with bits set for DDI_AUX_CTL register */
> > > > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0,
> > > > > sizeof(aux_msg),
> > > > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > sizeof(aux_msg),
> > > > > aux_clock_divider);
> > > > Hmm. What's the situation with the PSR stuff here?
> > > I hadn't notice this, thanks.
> > >
> > > I don't see why it would actually be correct though. It seems to
> > > be a
> > > bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> > > functions") when the rmw was removed (so whatever was on the
> > > register
> > > before get overwritten).
> > Dunno why you think there's a bug. The problem I see with this
> > patch is
> > that you're now setting the irq enable bit on the psr aux register.
> > Does that bit even exist? If so does it do what we want it to do?
> I was assuming they should be the same. Now I see the psr code path
> gets the bits for
> DDI_AUX_CTL and mask to use only the valid bits for EDP_PSR_AUX_CTL.
> Bit 28 will
> be left out regardless the return of get_aux_send_ctl()
Right.
Looking at this code, I realized we should enable the error interrupt
(bit-11) for PSR_AUX_CTL too.
>
> Lucas De Marchi
>
> >
> >
> > >
> > >
> > > CC'ing Daniel who wrote that patch and José/DK who worked
> > > recently on
> > > psr stuff.
> > >
> > >
> > > Lucas De Marchi
> > > >
> > > >
> > > > >
> > > > >
> > > > > /* Select only valid bits for SRD_AUX_CTL */
> > > > > --
> > > > > 2.17.0
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> > --
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > 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] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-19 15:24 ` Lucas De Marchi
2018-06-19 20:17 ` Dhinakaran Pandiyan
@ 2018-06-21 16:01 ` Ville Syrjälä
1 sibling, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2018-06-21 16:01 UTC (permalink / raw)
To: Lucas De Marchi
Cc: Daniel Vetter, intel-gfx, Lucas De Marchi, Dhinakaran Pandiyan
On Tue, Jun 19, 2018 at 08:24:33AM -0700, Lucas De Marchi wrote:
> On Tue, Jun 19, 2018 at 7:06 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi wrote:
> > > On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> > > > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi wrote:
> > > > > This became dead code with commit 309bd8ed464f ("drm/i915: Reinstate
> > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > >
> > > > > v2: Move comment about HW behavior to where decision is made to enable
> > > > > MSI (Ville).
> > > > >
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > > index 9c449b8d8eab..a1461de20472 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> > > > > * get lost on g4x as well, and interrupt delivery seems to stay
> > > > > * properly dead afterwards. So we'll just disable them for all
> > > > > * pre-gen5 chipsets.
> > > > > + *
> > > > > + * dp aux and gmbus irq on gen4 seems to be able to generate legacy
> > > > > + * interrupts even when in MSI mode. This results in spurious
> > > > > + * interrupt warnings if the legacy irq no. is shared with another
> > > > > + * device. The kernel then disables that interrupt source and so
> > > > > + * prevents the other device from working properly.
> > > > > */
> > > > > if (INTEL_GEN(dev_priv) >= 5) {
> > > > > if (pci_enable_msi(pdev) < 0)
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index b86ed6401120..c5e1f648c47c 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -2558,16 +2558,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > > > (IS_CANNONLAKE(dev_priv) || \
> > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > >
> > > > > -/*
> > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
> > > > > - * even when in MSI mode. This results in spurious interrupt warnings if the
> > > > > - * legacy irq no. is shared with another device. The kernel then disables that
> > > > > - * interrupt source and so prevents the other device from working properly.
> > > > > - *
> > > > > - * Since we don't enable MSI anymore on gen4, we can always use GMBUS/AUX
> > > > > - * interrupts.
> > > > > - */
> > > > > -#define HAS_AUX_IRQ(dev_priv) true
> > > > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> > > > >
> > > > > /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index ce07bd794aed..1dab40056df7 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > > }
> > > > >
> > > > > static uint32_t
> > > > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > {
> > > > > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > > > > bool done;
> > > > >
> > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > - if (has_aux_irq)
> > > > > - done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > - msecs_to_jiffies_timeout(10));
> > > > > - else
> > > > > - done = wait_for(C, 10) == 0;
> > > > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > + msecs_to_jiffies_timeout(10));
> > > > > if (!done)
> > > > > - DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
> > > > > - has_aux_irq);
> > > > > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > > > > #undef C
> > > > >
> > > > > return status;
> > > > > @@ -1016,7 +1012,6 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > > > > }
> > > > >
> > > > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t aux_clock_divider)
> > > > > {
> > > > > @@ -1037,7 +1032,7 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > >
> > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > DP_AUX_CH_CTL_DONE |
> > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > timeout |
> > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > @@ -1047,13 +1042,12 @@ static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > }
> > > > >
> > > > > static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t unused)
> > > > > {
> > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > DP_AUX_CH_CTL_DONE |
> > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > > int i, ret, recv_bytes;
> > > > > uint32_t status;
> > > > > int try, clock = 0;
> > > > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > > > bool vdd;
> > > > >
> > > > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > >
> > > > > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > > > > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > - has_aux_irq,
> > > > > send_bytes,
> > > > > aux_clock_divider);
> > > > >
> > > > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> > > > > /* Send the command and wait for it to complete */
> > > > > I915_WRITE(ch_ctl, send_ctl);
> > > > >
> > > > > - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> > > > > + status = intel_dp_aux_wait_done(intel_dp);
> > > > >
> > > > > /* Clear done status and any errors */
> > > > > I915_WRITE(ch_ctl,
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > > > * register with to kick off an AUX transaction.
> > > > > */
> > > > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > > > - bool has_aux_irq,
> > > > > int send_bytes,
> > > > > uint32_t aux_clock_divider);
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index db27f2faa1de..ee26f5f33b87 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> > > > > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
> > > > >
> > > > > /* Start with bits set for DDI_AUX_CTL register */
> > > > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> > > > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, sizeof(aux_msg),
> > > > > aux_clock_divider);
> > > >
> > > > Hmm. What's the situation with the PSR stuff here?
> > >
> > > I hadn't notice this, thanks.
> > >
> > > I don't see why it would actually be correct though. It seems to be a
> > > bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> > > functions") when the rmw was removed (so whatever was on the register
> > > before get overwritten).
> >
> > Dunno why you think there's a bug. The problem I see with this patch is
> > that you're now setting the irq enable bit on the psr aux register.
> > Does that bit even exist? If so does it do what we want it to do?
>
> I was assuming they should be the same. Now I see the psr code path
> gets the bits for
> DDI_AUX_CTL and mask to use only the valid bits for EDP_PSR_AUX_CTL. Bit 28 will
> be left out regardless the return of get_aux_send_ctl()
OK. Seems to all check out. Pushed to dinq. Thanks for the patch.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-19 20:17 ` Dhinakaran Pandiyan
@ 2018-06-21 17:04 ` Lucas De Marchi
2018-06-21 19:37 ` Dhinakaran Pandiyan
0 siblings, 1 reply; 28+ messages in thread
From: Lucas De Marchi @ 2018-06-21 17:04 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: Daniel Vetter, intel-gfx, Lucas De Marchi
On Tue, Jun 19, 2018 at 01:17:10PM -0700, Dhinakaran Pandiyan wrote:
> On Tue, 2018-06-19 at 08:24 -0700, Lucas De Marchi wrote:
> > On Tue, Jun 19, 2018 at 7:06 AM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > >
> > > On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi wrote:
> > > >
> > > > On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä wrote:
> > > > >
> > > > > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi
> > > > > wrote:
> > > > > >
> > > > > > This became dead code with commit 309bd8ed464f ("drm/i915:
> > > > > > Reinstate
> > > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > > >
> > > > > > v2: Move comment about HW behavior to where decision is made
> > > > > > to enable
> > > > > > MSI (Ville).
> > > > > >
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > > > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++---------------
> > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > > > index 9c449b8d8eab..a1461de20472 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > > @@ -1189,6 +1189,12 @@ static int i915_driver_init_hw(struct
> > > > > > drm_i915_private *dev_priv)
> > > > > > * get lost on g4x as well, and interrupt delivery seems to
> > > > > > stay
> > > > > > * properly dead afterwards. So we'll just disable them for
> > > > > > all
> > > > > > * pre-gen5 chipsets.
> > > > > > + *
> > > > > > + * dp aux and gmbus irq on gen4 seems to be able to
> > > > > > generate legacy
> > > > > > + * interrupts even when in MSI mode. This results in
> > > > > > spurious
> > > > > > + * interrupt warnings if the legacy irq no. is shared with
> > > > > > another
> > > > > > + * device. The kernel then disables that interrupt source
> > > > > > and so
> > > > > > + * prevents the other device from working properly.
> > > > > > */
> > > > > > if (INTEL_GEN(dev_priv) >= 5) {
> > > > > > if (pci_enable_msi(pdev) < 0)
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > index b86ed6401120..c5e1f648c47c 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > @@ -2558,16 +2558,6 @@ intel_info(const struct
> > > > > > drm_i915_private *dev_priv)
> > > > > > (IS_CANNONLAKE(dev_priv) || \
> > > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > > >
> > > > > > -/*
> > > > > > - * dp aux and gmbus irq on gen4 seems to be able to generate
> > > > > > legacy interrupts
> > > > > > - * even when in MSI mode. This results in spurious interrupt
> > > > > > warnings if the
> > > > > > - * legacy irq no. is shared with another device. The kernel
> > > > > > then disables that
> > > > > > - * interrupt source and so prevents the other device from
> > > > > > working properly.
> > > > > > - *
> > > > > > - * Since we don't enable MSI anymore on gen4, we can always
> > > > > > use GMBUS/AUX
> > > > > > - * interrupts.
> > > > > > - */
> > > > > > -#define HAS_AUX_IRQ(dev_priv) true
> > > > > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> > > > > >
> > > > > > /* With the 945 and later, Y tiling got adjusted so that it
> > > > > > was 32 128-byte
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index ce07bd794aed..1dab40056df7 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp
> > > > > > *intel_dp)
> > > > > > }
> > > > > >
> > > > > > static uint32_t
> > > > > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
> > > > > > has_aux_irq)
> > > > > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > > {
> > > > > > struct drm_i915_private *dev_priv =
> > > > > > to_i915(intel_dp_to_dev(intel_dp));
> > > > > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct intel_dp
> > > > > > *intel_dp, bool has_aux_irq)
> > > > > > bool done;
> > > > > >
> > > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) &
> > > > > > DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > > - if (has_aux_irq)
> > > > > > - done = wait_event_timeout(dev_priv-
> > > > > > >gmbus_wait_queue, C,
> > > > > > - msecs_to_jiffies_timeout(
> > > > > > 10));
> > > > > > - else
> > > > > > - done = wait_for(C, 10) == 0;
> > > > > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > > + msecs_to_jiffies_timeout(10));
> > > > > > if (!done)
> > > > > > - DRM_ERROR("dp aux hw did not signal timeout (has
> > > > > > irq: %i)!\n",
> > > > > > - has_aux_irq);
> > > > > > + DRM_ERROR("dp aux hw did not signal timeout!\n");
> > > > > > #undef C
> > > > > >
> > > > > > return status;
> > > > > > @@ -1016,7 +1012,6 @@ static uint32_t
> > > > > > skl_get_aux_clock_divider(struct intel_dp *intel_dp, int
> > > > > > index)
> > > > > > }
> > > > > >
> > > > > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp
> > > > > > *intel_dp,
> > > > > > - bool has_aux_irq,
> > > > > > int send_bytes,
> > > > > > uint32_t aux_clock_divider)
> > > > > > {
> > > > > > @@ -1037,7 +1032,7 @@ static uint32_t
> > > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > >
> > > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > DP_AUX_CH_CTL_DONE |
> > > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > > timeout |
> > > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > > @@ -1047,13 +1042,12 @@ static uint32_t
> > > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > > }
> > > > > >
> > > > > > static uint32_t skl_get_aux_send_ctl(struct intel_dp
> > > > > > *intel_dp,
> > > > > > - bool has_aux_irq,
> > > > > > int send_bytes,
> > > > > > uint32_t unused)
> > > > > > {
> > > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > DP_AUX_CH_CTL_DONE |
> > > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > *intel_dp,
> > > > > > int i, ret, recv_bytes;
> > > > > > uint32_t status;
> > > > > > int try, clock = 0;
> > > > > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > > > > bool vdd;
> > > > > >
> > > > > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > *intel_dp,
> > > > > >
> > > > > > while ((aux_clock_divider = intel_dp-
> > > > > > >get_aux_clock_divider(intel_dp, clock++))) {
> > > > > > u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > > - has_aux_i
> > > > > > rq,
> > > > > > send_byte
> > > > > > s,
> > > > > > aux_clock
> > > > > > _divider);
> > > > > >
> > > > > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > *intel_dp,
> > > > > > /* Send the command and wait for it to
> > > > > > complete */
> > > > > > I915_WRITE(ch_ctl, send_ctl);
> > > > > >
> > > > > > - status = intel_dp_aux_wait_done(intel_dp,
> > > > > > has_aux_irq);
> > > > > > + status = intel_dp_aux_wait_done(intel_dp);
> > > > > >
> > > > > > /* Clear done status and any errors */
> > > > > > I915_WRITE(ch_ctl,
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > > > > * register with to kick off an AUX transaction.
> > > > > > */
> > > > > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > > > > - bool has_aux_irq,
> > > > > > int send_bytes,
> > > > > > uint32_t aux_clock_divider);
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > index db27f2faa1de..ee26f5f33b87 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct
> > > > > > intel_dp *intel_dp)
> > > > > > aux_clock_divider = intel_dp-
> > > > > > >get_aux_clock_divider(intel_dp, 0);
> > > > > >
> > > > > > /* Start with bits set for DDI_AUX_CTL register */
> > > > > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0,
> > > > > > sizeof(aux_msg),
> > > > > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > > sizeof(aux_msg),
> > > > > > aux_clock_divider);
> > > > > Hmm. What's the situation with the PSR stuff here?
> > > > I hadn't notice this, thanks.
> > > >
> > > > I don't see why it would actually be correct though. It seems to
> > > > be a
> > > > bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> > > > functions") when the rmw was removed (so whatever was on the
> > > > register
> > > > before get overwritten).
> > > Dunno why you think there's a bug. The problem I see with this
> > > patch is
> > > that you're now setting the irq enable bit on the psr aux register.
> > > Does that bit even exist? If so does it do what we want it to do?
> > I was assuming they should be the same. Now I see the psr code path
> > gets the bits for
> > DDI_AUX_CTL and mask to use only the valid bits for EDP_PSR_AUX_CTL.
> > Bit 28 will
> > be left out regardless the return of get_aux_send_ctl()
> Right.
>
> Looking at this code, I realized we should enable the error interrupt
> (bit-11) for PSR_AUX_CTL too.
You mean enable it and handle the interrupt, right? Because it's
currently not being handled.
Lucas De Marchi
>
>
> >
> > Lucas De Marchi
> >
> > >
> > >
> > > >
> > > >
> > > > CC'ing Daniel who wrote that patch and José/DK who worked
> > > > recently on
> > > > psr stuff.
> > > >
> > > >
> > > > Lucas De Marchi
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > /* Select only valid bits for SRD_AUX_CTL */
> > > > > > --
> > > > > > 2.17.0
> > > > > --
> > > > > Ville Syrjälä
> > > > > Intel
> > > --
> > > Ville Syrjälä
> > > Intel
> > > _______________________________________________
> > > 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] 28+ messages in thread
* Re: [PATCH v2] drm/i915: remove check for aux irq
2018-06-21 17:04 ` Lucas De Marchi
@ 2018-06-21 19:37 ` Dhinakaran Pandiyan
0 siblings, 0 replies; 28+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-21 19:37 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Daniel Vetter, intel-gfx, Lucas De Marchi
On Thu, 2018-06-21 at 10:04 -0700, Lucas De Marchi wrote:
> On Tue, Jun 19, 2018 at 01:17:10PM -0700, Dhinakaran Pandiyan wrote:
> >
> > On Tue, 2018-06-19 at 08:24 -0700, Lucas De Marchi wrote:
> > >
> > > On Tue, Jun 19, 2018 at 7:06 AM Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> wrote:
> > > >
> > > >
> > > >
> > > > On Fri, Jun 15, 2018 at 02:51:06PM -0700, Lucas De Marchi
> > > > wrote:
> > > > >
> > > > >
> > > > > On Fri, Jun 15, 2018 at 08:58:28PM +0300, Ville Syrjälä
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > On Wed, May 23, 2018 at 11:04:35AM -0700, Lucas De Marchi
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > This became dead code with commit 309bd8ed464f
> > > > > > > ("drm/i915:
> > > > > > > Reinstate
> > > > > > > GMBUS and AUX interrupts on gen4/g4x").
> > > > > > >
> > > > > > > v2: Move comment about HW behavior to where decision is
> > > > > > > made
> > > > > > > to enable
> > > > > > > MSI (Ville).
> > > > > > >
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > > > > > drivers/gpu/drm/i915/i915_drv.h | 10 ----------
> > > > > > > drivers/gpu/drm/i915/intel_dp.c | 22 +++++++-----------
> > > > > > > ----
> > > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > > > > > drivers/gpu/drm/i915/intel_psr.c | 2 +-
> > > > > > > 5 files changed, 14 insertions(+), 27 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > > > > index 9c449b8d8eab..a1461de20472 100644
> > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > > > @@ -1189,6 +1189,12 @@ static int
> > > > > > > i915_driver_init_hw(struct
> > > > > > > drm_i915_private *dev_priv)
> > > > > > > * get lost on g4x as well, and interrupt delivery
> > > > > > > seems to
> > > > > > > stay
> > > > > > > * properly dead afterwards. So we'll just disable them
> > > > > > > for
> > > > > > > all
> > > > > > > * pre-gen5 chipsets.
> > > > > > > + *
> > > > > > > + * dp aux and gmbus irq on gen4 seems to be able to
> > > > > > > generate legacy
> > > > > > > + * interrupts even when in MSI mode. This results in
> > > > > > > spurious
> > > > > > > + * interrupt warnings if the legacy irq no. is shared
> > > > > > > with
> > > > > > > another
> > > > > > > + * device. The kernel then disables that interrupt
> > > > > > > source
> > > > > > > and so
> > > > > > > + * prevents the other device from working properly.
> > > > > > > */
> > > > > > > if (INTEL_GEN(dev_priv) >= 5) {
> > > > > > > if (pci_enable_msi(pdev) < 0)
> > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > index b86ed6401120..c5e1f648c47c 100644
> > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > > @@ -2558,16 +2558,6 @@ intel_info(const struct
> > > > > > > drm_i915_private *dev_priv)
> > > > > > > (IS_CANNONLAKE(dev_priv) || \
> > > > > > > IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> > > > > > >
> > > > > > > -/*
> > > > > > > - * dp aux and gmbus irq on gen4 seems to be able to
> > > > > > > generate
> > > > > > > legacy interrupts
> > > > > > > - * even when in MSI mode. This results in spurious
> > > > > > > interrupt
> > > > > > > warnings if the
> > > > > > > - * legacy irq no. is shared with another device. The
> > > > > > > kernel
> > > > > > > then disables that
> > > > > > > - * interrupt source and so prevents the other device
> > > > > > > from
> > > > > > > working properly.
> > > > > > > - *
> > > > > > > - * Since we don't enable MSI anymore on gen4, we can
> > > > > > > always
> > > > > > > use GMBUS/AUX
> > > > > > > - * interrupts.
> > > > > > > - */
> > > > > > > -#define HAS_AUX_IRQ(dev_priv) true
> > > > > > > #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >=
> > > > > > > 4)
> > > > > > >
> > > > > > > /* With the 945 and later, Y tiling got adjusted so that
> > > > > > > it
> > > > > > > was 32 128-byte
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > index ce07bd794aed..1dab40056df7 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > @@ -936,7 +936,7 @@ intel_dp_check_edp(struct intel_dp
> > > > > > > *intel_dp)
> > > > > > > }
> > > > > > >
> > > > > > > static uint32_t
> > > > > > > -intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
> > > > > > > has_aux_irq)
> > > > > > > +intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > > > {
> > > > > > > struct drm_i915_private *dev_priv =
> > > > > > > to_i915(intel_dp_to_dev(intel_dp));
> > > > > > > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > > > @@ -944,14 +944,10 @@ intel_dp_aux_wait_done(struct
> > > > > > > intel_dp
> > > > > > > *intel_dp, bool has_aux_irq)
> > > > > > > bool done;
> > > > > > >
> > > > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) &
> > > > > > > DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > > > - if (has_aux_irq)
> > > > > > > - done = wait_event_timeout(dev_priv-
> > > > > > > >
> > > > > > > > gmbus_wait_queue, C,
> > > > > > > - msecs_to_jiffies_time
> > > > > > > out(
> > > > > > > 10));
> > > > > > > - else
> > > > > > > - done = wait_for(C, 10) == 0;
> > > > > > > + done = wait_event_timeout(dev_priv->gmbus_wait_queue,
> > > > > > > C,
> > > > > > > + msecs_to_jiffies_timeout(10))
> > > > > > > ;
> > > > > > > if (!done)
> > > > > > > - DRM_ERROR("dp aux hw did not signal timeout
> > > > > > > (has
> > > > > > > irq: %i)!\n",
> > > > > > > - has_aux_irq);
> > > > > > > + DRM_ERROR("dp aux hw did not signal
> > > > > > > timeout!\n");
> > > > > > > #undef C
> > > > > > >
> > > > > > > return status;
> > > > > > > @@ -1016,7 +1012,6 @@ static uint32_t
> > > > > > > skl_get_aux_clock_divider(struct intel_dp *intel_dp, int
> > > > > > > index)
> > > > > > > }
> > > > > > >
> > > > > > > static uint32_t g4x_get_aux_send_ctl(struct intel_dp
> > > > > > > *intel_dp,
> > > > > > > - bool has_aux_irq,
> > > > > > > int send_bytes,
> > > > > > > uint32_t
> > > > > > > aux_clock_divider)
> > > > > > > {
> > > > > > > @@ -1037,7 +1032,7 @@ static uint32_t
> > > > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > > >
> > > > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > DP_AUX_CH_CTL_DONE |
> > > > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > > > timeout |
> > > > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > > > @@ -1047,13 +1042,12 @@ static uint32_t
> > > > > > > g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > > > > > > }
> > > > > > >
> > > > > > > static uint32_t skl_get_aux_send_ctl(struct intel_dp
> > > > > > > *intel_dp,
> > > > > > > - bool has_aux_irq,
> > > > > > > int send_bytes,
> > > > > > > uint32_t unused)
> > > > > > > {
> > > > > > > return DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > DP_AUX_CH_CTL_DONE |
> > > > > > > - (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> > > > > > > + DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > DP_AUX_CH_CTL_TIME_OUT_ERROR |
> > > > > > > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > > > > > > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > > > > > > @@ -1076,7 +1070,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > > *intel_dp,
> > > > > > > int i, ret, recv_bytes;
> > > > > > > uint32_t status;
> > > > > > > int try, clock = 0;
> > > > > > > - bool has_aux_irq = HAS_AUX_IRQ(dev_priv);
> > > > > > > bool vdd;
> > > > > > >
> > > > > > > ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
> > > > > > > @@ -1131,7 +1124,6 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > > *intel_dp,
> > > > > > >
> > > > > > > while ((aux_clock_divider = intel_dp-
> > > > > > > >
> > > > > > > > get_aux_clock_divider(intel_dp, clock++))) {
> > > > > > > u32 send_ctl = intel_dp-
> > > > > > > >get_aux_send_ctl(intel_dp,
> > > > > > > - has_a
> > > > > > > ux_i
> > > > > > > rq,
> > > > > > > send_
> > > > > > > byte
> > > > > > > s,
> > > > > > > aux_c
> > > > > > > lock
> > > > > > > _divider);
> > > > > > >
> > > > > > > @@ -1148,7 +1140,7 @@ intel_dp_aux_xfer(struct intel_dp
> > > > > > > *intel_dp,
> > > > > > > /* Send the command and wait for it to
> > > > > > > complete */
> > > > > > > I915_WRITE(ch_ctl, send_ctl);
> > > > > > >
> > > > > > > - status =
> > > > > > > intel_dp_aux_wait_done(intel_dp,
> > > > > > > has_aux_irq);
> > > > > > > + status =
> > > > > > > intel_dp_aux_wait_done(intel_dp);
> > > > > > >
> > > > > > > /* Clear done status and any errors */
> > > > > > > I915_WRITE(ch_ctl,
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > index 2855363f5b6c..7ac91d5ee3fa 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > > > @@ -1138,7 +1138,6 @@ struct intel_dp {
> > > > > > > * register with to kick off an AUX transaction.
> > > > > > > */
> > > > > > > uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> > > > > > > - bool has_aux_irq,
> > > > > > > int send_bytes,
> > > > > > > uint32_t
> > > > > > > aux_clock_divider);
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > index db27f2faa1de..ee26f5f33b87 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > > @@ -373,7 +373,7 @@ static void hsw_psr_setup_aux(struct
> > > > > > > intel_dp *intel_dp)
> > > > > > > aux_clock_divider = intel_dp-
> > > > > > > >
> > > > > > > > get_aux_clock_divider(intel_dp, 0);
> > > > > > > /* Start with bits set for DDI_AUX_CTL register */
> > > > > > > - aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0,
> > > > > > > sizeof(aux_msg),
> > > > > > > + aux_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > > > > > > sizeof(aux_msg),
> > > > > > > aux_clock_divider)
> > > > > > > ;
> > > > > > Hmm. What's the situation with the PSR stuff here?
> > > > > I hadn't notice this, thanks.
> > > > >
> > > > > I don't see why it would actually be correct though. It seems
> > > > > to
> > > > > be a
> > > > > bug since d4dcbdceabe4 ("drm/i915/psr: Use ->get_aux_send_ctl
> > > > > functions") when the rmw was removed (so whatever was on the
> > > > > register
> > > > > before get overwritten).
> > > > Dunno why you think there's a bug. The problem I see with this
> > > > patch is
> > > > that you're now setting the irq enable bit on the psr aux
> > > > register.
> > > > Does that bit even exist? If so does it do what we want it to
> > > > do?
> > > I was assuming they should be the same. Now I see the psr code
> > > path
> > > gets the bits for
> > > DDI_AUX_CTL and mask to use only the valid bits for
> > > EDP_PSR_AUX_CTL.
> > > Bit 28 will
> > > be left out regardless the return of get_aux_send_ctl()
> > Right.
> >
> > Looking at this code, I realized we should enable the error
> > interrupt
> > (bit-11) for PSR_AUX_CTL too.
> You mean enable it and handle the interrupt, right? Because it's
> currently not being handled.
I see that psr_irq_handler() gets called when the PSR bit is set
in DEIIR/GEN8_DE_MISC_IIR, although we don't do much after the
interrupt is handled.
Do you mean something else?
>
>
> Lucas De Marchi
>
>
> >
> >
> >
> > >
> > >
> > > Lucas De Marchi
> > >
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > CC'ing Daniel who wrote that patch and José/DK who worked
> > > > > recently on
> > > > > psr stuff.
> > > > >
> > > > >
> > > > > Lucas De Marchi
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > /* Select only valid bits for SRD_AUX_CTL */
> > > > > > > --
> > > > > > > 2.17.0
> > > > > > --
> > > > > > Ville Syrjälä
> > > > > > Intel
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> > > > _______________________________________________
> > > > 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] 28+ messages in thread
end of thread, other threads:[~2018-06-21 19:11 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-25 21:55 [PATCH] drm/i915: remove check for aux irq Lucas De Marchi
2018-04-25 22:11 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
2018-04-25 22:28 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-25 23:24 ` [PATCH] " Rodrigo Vivi
2018-04-26 2:45 ` ✓ Fi.CI.IGT: success for " Patchwork
2018-04-26 13:43 ` [PATCH] " Ville Syrjälä
2018-04-26 15:22 ` Lucas De Marchi
2018-04-26 15:27 ` Ville Syrjälä
2018-04-26 15:42 ` Lucas De Marchi
2018-04-26 15:50 ` Ville Syrjälä
2018-04-30 22:05 ` Lucas De Marchi
2018-05-02 12:07 ` Ville Syrjälä
2018-05-23 18:04 ` [PATCH v2] " Lucas De Marchi
2018-06-15 17:58 ` Ville Syrjälä
2018-06-15 21:51 ` Lucas De Marchi
2018-06-19 14:02 ` Ville Syrjälä
2018-06-19 15:24 ` Lucas De Marchi
2018-06-19 20:17 ` Dhinakaran Pandiyan
2018-06-21 17:04 ` Lucas De Marchi
2018-06-21 19:37 ` Dhinakaran Pandiyan
2018-06-21 16:01 ` Ville Syrjälä
2018-04-30 22:17 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: remove check for aux irq (rev2) Patchwork
2018-04-30 22:18 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-04-30 22:35 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-01 0:54 ` ✓ Fi.CI.IGT: " Patchwork
2018-05-23 18:13 ` ✗ Fi.CI.SPARSE: warning for drm/i915: remove check for aux irq (rev3) Patchwork
2018-05-23 18:28 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-23 22:38 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.