* [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw
@ 2018-04-03 21:24 Dhinakaran Pandiyan
2018-04-03 21:24 ` [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
` (14 more replies)
0 siblings, 15 replies; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-03 21:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Daniel Vetter, Dhinakaran Pandiyan, Rodrigo Vivi
From: Daniel Vetter <daniel.vetter@ffwll.ch>
The definitions for the error register should be valid on bdw/skl too,
but there we haven't even enabled DE_MISC handling yet.
Somewhat confusing the the moved register offset on bdw is only for
the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
on bdw.
v2: Fixes from Ville.
v3: From DK
* Rebased on drm-tip
* Removed BDW IIR bit definition, looks like an unintentional change that
should be in the following patch.
v4: From DK
* Don't mask REG_WRITE.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 34 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 27aee25429b7..c2d3f30778ee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
ironlake_rps_change_irq_handler(dev_priv);
}
+static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
+{
+ u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+
+ if (edp_psr_iir & EDP_PSR_ERROR)
+ DRM_DEBUG_KMS("PSR error\n");
+
+ if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
+ DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
+ I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
+ }
+
+ if (edp_psr_iir & EDP_PSR_POST_EXIT) {
+ DRM_DEBUG_KMS("PSR exit completed\n");
+ I915_WRITE(EDP_PSR_IMR, 0);
+ }
+
+ I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
+}
+
static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
u32 de_iir)
{
@@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev_priv);
+ if (de_iir & DE_EDP_PSR_INT_HSW)
+ hsw_edp_psr_irq_handler(dev_priv);
+
if (de_iir & DE_AUX_CHANNEL_A_IVB)
dp_aux_irq_handler(dev_priv);
@@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
if (IS_GEN7(dev_priv))
I915_WRITE(GEN7_ERR_INT, 0xffffffff);
+ if (IS_HASWELL(dev_priv)) {
+ I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+ I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+ }
+
gen5_gt_irq_reset(dev_priv);
ibx_irq_reset(dev_priv);
@@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
DE_DP_A_HOTPLUG);
}
+ if (IS_HASWELL(dev_priv)) {
+ gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+ I915_WRITE(EDP_PSR_IMR, 0);
+ display_mask |= DE_EDP_PSR_INT_HSW;
+ }
+
dev_priv->irq_mask = ~display_mask;
ibx_irq_pre_postinstall(dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 176dca6554f4..f5783d6db614 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4011,6 +4011,13 @@ enum {
#define EDP_PSR_TP1_TIME_0us (3<<4)
#define EDP_PSR_IDLE_FRAME_SHIFT 0
+/* Bspec claims those aren't shifted but stay at 0x64800 */
+#define EDP_PSR_IMR _MMIO(0x64834)
+#define EDP_PSR_IIR _MMIO(0x64838)
+#define EDP_PSR_ERROR (1<<2)
+#define EDP_PSR_POST_EXIT (1<<1)
+#define EDP_PSR_PRE_ENTRY (1<<0)
+
#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10)
#define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
#define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
@@ -6820,6 +6827,7 @@ enum {
#define DE_PCH_EVENT_IVB (1<<28)
#define DE_DP_A_HOTPLUG_IVB (1<<27)
#define DE_AUX_CHANNEL_A_IVB (1<<26)
+#define DE_EDP_PSR_INT_HSW (1<<19)
#define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
#define DE_PLANEC_FLIP_DONE_IVB (1<<13)
#define DE_PIPEC_VBLANK_IVB (1<<10)
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
@ 2018-04-03 21:24 ` Dhinakaran Pandiyan
2018-04-05 20:38 ` Souza, Jose
2018-04-03 21:24 ` [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
` (13 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-03 21:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Dhinakaran Pandiyan, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr on
any transcoder in theory, though the we don't currenty enable PSR except
on the EDP transcoder.
v2: From DK
* Rebased on drm-tip
v3: Switched author to Ville based on IRC discussion.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 57 ++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/i915_reg.h | 7 +++--
drivers/gpu/drm/i915/intel_display.h | 4 +++
3 files changed, 52 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c2d3f30778ee..8a894adf2ca1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2394,20 +2394,34 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
{
u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+ u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
+ u32 mask = BIT(TRANSCODER_EDP);
+ enum transcoder cpu_transcoder;
- if (edp_psr_iir & EDP_PSR_ERROR)
- DRM_DEBUG_KMS("PSR error\n");
-
- if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
- DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
- I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
- }
+ if (INTEL_GEN(dev_priv) >= 8)
+ mask |= BIT(TRANSCODER_A) |
+ BIT(TRANSCODER_B) |
+ BIT(TRANSCODER_C);
+
+ for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
+ if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+ DRM_DEBUG_KMS("Transcoder %s PSR error\n",
+ transcoder_name(cpu_transcoder));
+
+ if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+ DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 vblanks\n",
+ transcoder_name(cpu_transcoder));
+ edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
+ }
- if (edp_psr_iir & EDP_PSR_POST_EXIT) {
- DRM_DEBUG_KMS("PSR exit completed\n");
- I915_WRITE(EDP_PSR_IMR, 0);
+ if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+ DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
+ transcoder_name(cpu_transcoder));
+ edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
+ }
}
+ I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
}
@@ -2555,11 +2569,22 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
if (master_ctl & GEN8_DE_MISC_IRQ) {
iir = I915_READ(GEN8_DE_MISC_IIR);
if (iir) {
+ bool found = false;
+
I915_WRITE(GEN8_DE_MISC_IIR, iir);
ret = IRQ_HANDLED;
- if (iir & GEN8_DE_MISC_GSE)
+
+ if (iir & GEN8_DE_MISC_GSE) {
intel_opregion_asle_intr(dev_priv);
- else
+ found = true;
+ }
+
+ if (iir & GEN8_DE_EDP_PSR) {
+ hsw_edp_psr_irq_handler(dev_priv);
+ found = true;
+ }
+
+ if (!found)
DRM_ERROR("Unexpected DE Misc interrupt\n");
}
else
@@ -3326,6 +3351,9 @@ static void gen8_irq_reset(struct drm_device *dev)
gen8_gt_irq_reset(dev_priv);
+ I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+ I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+
for_each_pipe(dev_priv, pipe)
if (intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe)))
@@ -3815,7 +3843,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
uint32_t de_pipe_enables;
u32 de_port_masked = GEN8_AUX_CHANNEL_A;
u32 de_port_enables;
- u32 de_misc_masked = GEN8_DE_MISC_GSE;
+ u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
enum pipe pipe;
if (INTEL_GEN(dev_priv) >= 9) {
@@ -3840,6 +3868,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
else if (IS_BROADWELL(dev_priv))
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
+ gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+ I915_WRITE(EDP_PSR_IMR, 0);
+
for_each_pipe(dev_priv, pipe) {
dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f5783d6db614..6eb351177a68 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4014,9 +4014,9 @@ enum {
/* Bspec claims those aren't shifted but stay at 0x64800 */
#define EDP_PSR_IMR _MMIO(0x64834)
#define EDP_PSR_IIR _MMIO(0x64838)
-#define EDP_PSR_ERROR (1<<2)
-#define EDP_PSR_POST_EXIT (1<<1)
-#define EDP_PSR_PRE_ENTRY (1<<0)
+#define EDP_PSR_ERROR(trans) (1 << (((trans) * 8 + 10) & 31))
+#define EDP_PSR_POST_EXIT(trans) (1 << (((trans) * 8 + 9) & 31))
+#define EDP_PSR_PRE_ENTRY(trans) (1 << (((trans) * 8 + 8) & 31))
#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10)
#define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
@@ -6952,6 +6952,7 @@ enum {
#define GEN8_DE_MISC_IIR _MMIO(0x44468)
#define GEN8_DE_MISC_IER _MMIO(0x4446c)
#define GEN8_DE_MISC_GSE (1 << 27)
+#define GEN8_DE_EDP_PSR (1 << 19)
#define GEN8_PCU_ISR _MMIO(0x444e0)
#define GEN8_PCU_IMR _MMIO(0x444e4)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 4e7418b345bc..2ef31617614a 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -218,6 +218,10 @@ struct intel_link_m_n {
for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++) \
for_each_if((__mask) & BIT(__p))
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
+
#define for_each_universal_plane(__dev_priv, __pipe, __p) \
for ((__p) = 0; \
(__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-04-03 21:24 ` [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
@ 2018-04-03 21:24 ` Dhinakaran Pandiyan
2018-04-04 0:54 ` Souza, Jose
2018-04-03 21:24 ` [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
` (12 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-03 21:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Interrupts other than the one for AUX errors are required only for debug,
so unmask them via debugfs when the user requests debug.
User can make such a request with
echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
There are no locks to serialize PSR debug enabling from
irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
called only during module initialization/resume and IGT subtests
aren't expected to modify PSR debug at those times, we should be safe.
v2: Unroll loops (Ville)
Avoid resetting error mask bits.
v3: Unmask interrupts in postinstall() if debug was still enabled.
Avoid RMW (Ville)
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 36 +++++++++++++++++++++-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 57 +++++++++++------------------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 60 +++++++++++++++++++++++++++++++++++++
5 files changed, 116 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1dba2c451255..28f91df5b401 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
return 0;
}
+static int
+i915_edp_psr_debug_set(void *data, u64 val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ if (!CAN_PSR(dev_priv))
+ return -ENODEV;
+
+ DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
+
+ intel_runtime_pm_get(dev_priv);
+ intel_psr_debug_control(dev_priv, !!val);
+ intel_runtime_pm_put(dev_priv);
+
+ return 0;
+}
+
+static int
+i915_edp_psr_debug_get(void *data, u64 *val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ if (!CAN_PSR(dev_priv))
+ return -ENODEV;
+
+ *val = READ_ONCE(dev_priv->psr.debug);
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
+ i915_edp_psr_debug_get, i915_edp_psr_debug_set,
+ "%llu\n");
+
static int i915_sink_crc(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4812,7 +4845,8 @@ static const struct i915_debugfs_files {
{"i915_guc_log_relay", &i915_guc_log_relay_fops},
{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
{"i915_ipc_status", &i915_ipc_status_fops},
- {"i915_drrs_ctl", &i915_drrs_ctl_fops}
+ {"i915_drrs_ctl", &i915_drrs_ctl_fops},
+ {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
};
int i915_debugfs_register(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5373b171bb96..b97ed0cd4ca9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -609,6 +609,7 @@ struct i915_psr {
bool has_hw_tracking;
bool psr2_enabled;
u8 sink_sync_latency;
+ bool debug;
void (*enable_source)(struct intel_dp *,
const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8a894adf2ca1..714570955196 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,40 +2391,6 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
ironlake_rps_change_irq_handler(dev_priv);
}
-static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
-{
- u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
- u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
- u32 mask = BIT(TRANSCODER_EDP);
- enum transcoder cpu_transcoder;
-
- if (INTEL_GEN(dev_priv) >= 8)
- mask |= BIT(TRANSCODER_A) |
- BIT(TRANSCODER_B) |
- BIT(TRANSCODER_C);
-
- for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
- if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
- DRM_DEBUG_KMS("Transcoder %s PSR error\n",
- transcoder_name(cpu_transcoder));
-
- if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
- DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 vblanks\n",
- transcoder_name(cpu_transcoder));
- edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
- }
-
- if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
- DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
- transcoder_name(cpu_transcoder));
- edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
- }
- }
-
- I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
- I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
-}
-
static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
u32 de_iir)
{
@@ -2437,8 +2403,12 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev_priv);
- if (de_iir & DE_EDP_PSR_INT_HSW)
- hsw_edp_psr_irq_handler(dev_priv);
+ if (de_iir & DE_EDP_PSR_INT_HSW) {
+ u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+ intel_psr_irq_handler(dev_priv, psr_iir);
+ I915_WRITE(EDP_PSR_IIR, psr_iir);
+ }
if (de_iir & DE_AUX_CHANNEL_A_IVB)
dp_aux_irq_handler(dev_priv);
@@ -2580,7 +2550,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
}
if (iir & GEN8_DE_EDP_PSR) {
- hsw_edp_psr_irq_handler(dev_priv);
+ u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+ intel_psr_irq_handler(dev_priv, psr_iir);
+ I915_WRITE(EDP_PSR_IIR, psr_iir);
found = true;
}
@@ -3729,7 +3702,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
if (IS_HASWELL(dev_priv)) {
gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
- I915_WRITE(EDP_PSR_IMR, 0);
+ I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
+ intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -3845,6 +3819,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
u32 de_port_enables;
u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
enum pipe pipe;
+ u32 psr_masked;
if (INTEL_GEN(dev_priv) >= 9) {
de_pipe_masked |= GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
@@ -3869,7 +3844,11 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
- I915_WRITE(EDP_PSR_IMR, 0);
+ psr_masked = EDP_PSR_ERROR(TRANSCODER_EDP) |
+ EDP_PSR_ERROR(TRANSCODER_A) | EDP_PSR_ERROR(TRANSCODER_B) |
+ EDP_PSR_ERROR(TRANSCODER_C);
+ I915_WRITE(EDP_PSR_IMR, ~psr_masked);
+ intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
for_each_pipe(dev_priv, pipe) {
dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1452fd2a58d..53cb76b920e6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
unsigned frontbuffer_bits);
void intel_psr_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state);
+void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable);
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2d53f7398a6d..56ff2d7691a1 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,6 +93,66 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
}
+void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable)
+{
+ u32 debug_mask, error_mask;
+
+ /* No PSR interrupts on VLV/CHV */
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ return;
+
+ error_mask = EDP_PSR_ERROR(TRANSCODER_EDP);
+ debug_mask = error_mask | EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
+
+ if (INTEL_GEN(dev_priv) >= 8) {
+ error_mask |= EDP_PSR_ERROR(TRANSCODER_A) |
+ EDP_PSR_ERROR(TRANSCODER_B) |
+ EDP_PSR_ERROR(TRANSCODER_C);
+
+ debug_mask |= error_mask | EDP_PSR_POST_EXIT(TRANSCODER_A) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
+ EDP_PSR_POST_EXIT(TRANSCODER_B) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
+ EDP_PSR_POST_EXIT(TRANSCODER_C) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+ }
+
+ if (enable) {
+ WRITE_ONCE(dev_priv->psr.debug, true);
+ I915_WRITE(EDP_PSR_IMR, ~debug_mask);
+ } else {
+ I915_WRITE(EDP_PSR_IMR, ~error_mask);
+ WRITE_ONCE(dev_priv->psr.debug, false);
+ }
+}
+
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
+{
+ u32 transcoders = BIT(TRANSCODER_EDP);
+ enum transcoder cpu_transcoder;
+
+ if (INTEL_GEN(dev_priv) >= 8)
+ transcoders |= BIT(TRANSCODER_A) |
+ BIT(TRANSCODER_B) |
+ BIT(TRANSCODER_C);
+
+ for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+ /* FIXME: Exit PSR and link train manually when this happens. */
+ if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
+ transcoder_name(cpu_transcoder));
+
+ if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
+ transcoder_name(cpu_transcoder));
+
+ if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
+ transcoder_name(cpu_transcoder));
+ }
+}
+
static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
{
uint8_t psr_caps = 0;
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-04-03 21:24 ` [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
2018-04-03 21:24 ` [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
@ 2018-04-03 21:24 ` Dhinakaran Pandiyan
2018-04-04 0:56 ` Souza, Jose
2018-04-03 21:50 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
` (11 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-03 21:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Timestamps are useful for IGT tests that trigger PSR exit and/or wait for
PSR entry.
v2: Removed seqlock (Ville)
Removed erroneous warning in irq loop (Chris)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 9 +++++++--
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 28f91df5b401..b378fa013054 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2686,6 +2686,13 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
}
mutex_unlock(&dev_priv->psr.lock);
+ if (READ_ONCE(dev_priv->psr.debug)) {
+ seq_printf(m, "Last attempted entry at: %lld\n",
+ dev_priv->psr.last_entry_attempt);
+ seq_printf(m, "Last exit at: %lld\n",
+ dev_priv->psr.last_exit);
+ }
+
intel_runtime_pm_put(dev_priv);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b97ed0cd4ca9..2124a795d10c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -610,6 +610,8 @@ struct i915_psr {
bool psr2_enabled;
u8 sink_sync_latency;
bool debug;
+ ktime_t last_entry_attempt;
+ ktime_t last_exit;
void (*enable_source)(struct intel_dp *,
const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 56ff2d7691a1..a11a6d940203 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -131,6 +131,7 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
{
u32 transcoders = BIT(TRANSCODER_EDP);
enum transcoder cpu_transcoder;
+ ktime_t time_ns = ktime_get();
if (INTEL_GEN(dev_priv) >= 8)
transcoders |= BIT(TRANSCODER_A) |
@@ -143,13 +144,17 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
transcoder_name(cpu_transcoder));
- if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
+ if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+ dev_priv->psr.last_entry_attempt = time_ns;
DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
transcoder_name(cpu_transcoder));
+ }
- if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
+ if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+ dev_priv->psr.last_exit = time_ns;
DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
transcoder_name(cpu_transcoder));
+ }
}
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (2 preceding siblings ...)
2018-04-03 21:24 ` [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
@ 2018-04-03 21:50 ` Patchwork
2018-04-03 22:06 ` ✗ Fi.CI.BAT: failure " Patchwork
` (10 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-03 21:50 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
URL : https://patchwork.freedesktop.org/series/41095/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
7f73ae199d28 drm/i915: Enable edp psr error interrupts on hsw
-:109: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#109: FILE: drivers/gpu/drm/i915/i915_reg.h:4017:
+#define EDP_PSR_ERROR (1<<2)
^
-:110: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#110: FILE: drivers/gpu/drm/i915/i915_reg.h:4018:
+#define EDP_PSR_POST_EXIT (1<<1)
^
-:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#111: FILE: drivers/gpu/drm/i915/i915_reg.h:4019:
+#define EDP_PSR_PRE_ENTRY (1<<0)
^
-:120: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#120: FILE: drivers/gpu/drm/i915/i915_reg.h:6830:
+#define DE_EDP_PSR_INT_HSW (1<<19)
^
total: 0 errors, 0 warnings, 4 checks, 78 lines checked
bf91538c6b8d drm/i915: Enable edp psr error interrupts on bdw+
-:158: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#158: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:158: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
#158: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:159: CHECK:SPACING: No space is necessary after a cast
#159: FILE: drivers/gpu/drm/i915/intel_display.h:222:
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
-:160: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#160: FILE: drivers/gpu/drm/i915/intel_display.h:223:
+ for_each_if ((__mask) & (1 << (__t)))
total: 1 errors, 1 warnings, 2 checks, 123 lines checked
eb2120118831 drm/i915/psr: Control PSR interrupts via debugfs
9fde80e28074 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (3 preceding siblings ...)
2018-04-03 21:50 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
@ 2018-04-03 22:06 ` Patchwork
2018-04-05 2:03 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) Patchwork
` (9 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-03 22:06 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
URL : https://patchwork.freedesktop.org/series/41095/
State : failure
== Summary ==
Series 41095v1 series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
https://patchwork.freedesktop.org/api/1.0/series/41095/revisions/1/mbox/
---- Possible new issues:
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-a:
pass -> INCOMPLETE (fi-elk-e7500)
---- Known issues:
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
Test kms_chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#103841
Subgroup hdmi-hpd-fast:
skip -> FAIL (fi-kbl-7500u) fdo#102672
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
fail -> PASS (fi-skl-6700k2) fdo#103191
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:428s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:443s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:383s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s
fi-bxt-dsi total:216 pass:193 dwarn:0 dfail:0 fail:0 skip:22
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:516s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:507s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:558s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:514s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:575s
fi-elk-e7500 total:229 pass:180 dwarn:1 dfail:0 fail:0 skip:47
fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:316s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:537s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:408s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:421s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:463s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:430s
fi-kbl-7500u total:285 pass:259 dwarn:1 dfail:0 fail:2 skip:23 time:472s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:464s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:510s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:663s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:439s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:539s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:506s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:499s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:435s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:445s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:575s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:402s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:526s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:490s
29940f138482ff38047287ad288cea1fcf1f73b4 drm-tip: 2018y-04m-03d-13h-23m-36s UTC integration manifest
9fde80e28074 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
eb2120118831 drm/i915/psr: Control PSR interrupts via debugfs
bf91538c6b8d drm/i915: Enable edp psr error interrupts on bdw+
7f73ae199d28 drm/i915: Enable edp psr error interrupts on hsw
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8574/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-04-03 21:24 ` [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
@ 2018-04-04 0:54 ` Souza, Jose
2018-04-04 23:58 ` Pandiyan, Dhinakaran
2018-04-05 1:37 ` [PATCH v4 " Dhinakaran Pandiyan
0 siblings, 2 replies; 32+ messages in thread
From: Souza, Jose @ 2018-04-04 0:54 UTC (permalink / raw)
To: intel-gfx, Pandiyan, Dhinakaran; +Cc: Vivi, Rodrigo
On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> Interrupts other than the one for AUX errors are required only for
> debug,
> so unmask them via debugfs when the user requests debug.
>
> User can make such a request with
> echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
>
> There are no locks to serialize PSR debug enabling from
> irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
> called only during module initialization/resume and IGT subtests
> aren't expected to modify PSR debug at those times, we should be
> safe.
>
> v2: Unroll loops (Ville)
> Avoid resetting error mask bits.
>
> v3: Unmask interrupts in postinstall() if debug was still enabled.
> Avoid RMW (Ville)
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 36 +++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 57 +++++++++++--------------
> ----------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 60
> +++++++++++++++++++++++++++++++++++++
> 5 files changed, 116 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1dba2c451255..28f91df5b401 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file
> *m, void *data)
> return 0;
> }
>
> +static int
> +i915_edp_psr_debug_set(void *data, u64 val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + if (!CAN_PSR(dev_priv))
> + return -ENODEV;
> +
> + DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
> +
> + intel_runtime_pm_get(dev_priv);
> + intel_psr_debug_control(dev_priv, !!val);
> + intel_runtime_pm_put(dev_priv);
> +
> + return 0;
> +}
> +
> +static int
> +i915_edp_psr_debug_get(void *data, u64 *val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + if (!CAN_PSR(dev_priv))
> + return -ENODEV;
> +
> + *val = READ_ONCE(dev_priv->psr.debug);
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
> + i915_edp_psr_debug_get,
> i915_edp_psr_debug_set,
> + "%llu\n");
> +
> static int i915_sink_crc(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m-
> >private);
> @@ -4812,7 +4845,8 @@ static const struct i915_debugfs_files {
> {"i915_guc_log_relay", &i915_guc_log_relay_fops},
> {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> {"i915_ipc_status", &i915_ipc_status_fops},
> - {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> + {"i915_drrs_ctl", &i915_drrs_ctl_fops},
> + {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
same as bellow, why not i915_edp_psr_int_debug?
> };
>
> int i915_debugfs_register(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 5373b171bb96..b97ed0cd4ca9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -609,6 +609,7 @@ struct i915_psr {
> bool has_hw_tracking;
> bool psr2_enabled;
> u8 sink_sync_latency;
> + bool debug;
maybe change to a name that gives more information about the use of
this flag? like int_debug?
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 8a894adf2ca1..714570955196 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,40 +2391,6 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
> ironlake_rps_change_irq_handler(dev_priv);
> }
>
> -static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> -{
> - u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> - u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
> - u32 mask = BIT(TRANSCODER_EDP);
> - enum transcoder cpu_transcoder;
> -
> - if (INTEL_GEN(dev_priv) >= 8)
> - mask |= BIT(TRANSCODER_A) |
> - BIT(TRANSCODER_B) |
> - BIT(TRANSCODER_C);
> -
> - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> mask) {
> - if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> - DRM_DEBUG_KMS("Transcoder %s PSR error\n",
> - transcoder_name(cpu_transcoder
> ));
> -
> - if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> {
> - DRM_DEBUG_KMS("Transcoder %s PSR prepare
> entry in 2 vblanks\n",
> - transcoder_name(cpu_transcoder
> ));
> - edp_psr_imr |=
> EDP_PSR_PRE_ENTRY(cpu_transcoder);
> - }
> -
> - if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> {
> - DRM_DEBUG_KMS("Transcoder %s PSR exit
> completed\n",
> - transcoder_name(cpu_transcoder
> ));
> - edp_psr_imr &=
> ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
> - }
> - }
> -
> - I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
> - I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> -}
> -
> static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
> u32 de_iir)
> {
> @@ -2437,8 +2403,12 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev_priv);
>
> - if (de_iir & DE_EDP_PSR_INT_HSW)
> - hsw_edp_psr_irq_handler(dev_priv);
> + if (de_iir & DE_EDP_PSR_INT_HSW) {
> + u32 psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + intel_psr_irq_handler(dev_priv, psr_iir);
> + I915_WRITE(EDP_PSR_IIR, psr_iir);
> + }
>
> if (de_iir & DE_AUX_CHANNEL_A_IVB)
> dp_aux_irq_handler(dev_priv);
> @@ -2580,7 +2550,10 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
> }
>
> if (iir & GEN8_DE_EDP_PSR) {
> - hsw_edp_psr_irq_handler(dev_priv);
> + u32 psr_iir =
> I915_READ(EDP_PSR_IIR);
> +
> + intel_psr_irq_handler(dev_priv,
> psr_iir);
> + I915_WRITE(EDP_PSR_IIR, psr_iir);
> found = true;
> }
>
> @@ -3729,7 +3702,8 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
>
> if (IS_HASWELL(dev_priv)) {
> gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> - I915_WRITE(EDP_PSR_IMR, 0);
> + I915_WRITE(EDP_PSR_IMR,
> ~EDP_PSR_ERROR(TRANSCODER_EDP));
> + intel_psr_debug_control(dev_priv, dev_priv-
> >psr.debug);
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
>
> @@ -3845,6 +3819,7 @@ static void gen8_de_irq_postinstall(struct
> drm_i915_private *dev_priv)
> u32 de_port_enables;
> u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
> enum pipe pipe;
> + u32 psr_masked;
>
> if (INTEL_GEN(dev_priv) >= 9) {
> de_pipe_masked |= GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
> @@ -3869,7 +3844,11 @@ static void gen8_de_irq_postinstall(struct
> drm_i915_private *dev_priv)
> de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
>
> gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> - I915_WRITE(EDP_PSR_IMR, 0);
> + psr_masked = EDP_PSR_ERROR(TRANSCODER_EDP) |
> + EDP_PSR_ERROR(TRANSCODER_A) |
> EDP_PSR_ERROR(TRANSCODER_B) |
> + EDP_PSR_ERROR(TRANSCODER_C);
> + I915_WRITE(EDP_PSR_IMR, ~psr_masked);
intel_psr_debug_control() will already write to EDP_PSR_IMR with the
right value, why write twice?
> + intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
>
> for_each_pipe(dev_priv, pipe) {
> dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d1452fd2a58d..53cb76b920e6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct
> drm_i915_private *dev_priv,
> unsigned frontbuffer_bits);
> void intel_psr_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool
> enable);
> +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> psr_iir);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 2d53f7398a6d..56ff2d7691a1 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -93,6 +93,66 @@ static void psr_aux_io_power_put(struct intel_dp
> *intel_dp)
> intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> }
>
> +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool
> enable)
> +{
> + u32 debug_mask, error_mask;
> +
> + /* No PSR interrupts on VLV/CHV */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + return;
> +
> + error_mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> + debug_mask = error_mask | EDP_PSR_POST_EXIT(TRANSCODER_EDP)
> |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> +
> + if (INTEL_GEN(dev_priv) >= 8) {
> + error_mask |= EDP_PSR_ERROR(TRANSCODER_A) |
> + EDP_PSR_ERROR(TRANSCODER_B) |
> + EDP_PSR_ERROR(TRANSCODER_C);
> +
> + debug_mask |= error_mask |
> EDP_PSR_POST_EXIT(TRANSCODER_A) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
> + EDP_PSR_POST_EXIT(TRANSCODER_B) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
> + EDP_PSR_POST_EXIT(TRANSCODER_C) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> + }
> +
> + if (enable) {
> + WRITE_ONCE(dev_priv->psr.debug, true);
> + I915_WRITE(EDP_PSR_IMR, ~debug_mask);
> + } else {
> + I915_WRITE(EDP_PSR_IMR, ~error_mask);
> + WRITE_ONCE(dev_priv->psr.debug, false);
> + }
nit pick but why not something like?
u32 debug_mask, mask;
mask = EDP_PSR_ERROR(TRANSCODER_EDP);
debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP)
...
if (enable)
mask |= debug_mask;
I915_WRITE(EDP_PSR_IMR, ~mask)
WRITE_ONCE(dev_priv->psr.debug, enable);
> +}
> +
> +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> psr_iir)
two minnor things here:
- why you moved the function here? it adds more noise to the diff.
- the rename from hsw_edp_psr_irq_handler to intel_psr_irq_handler
should have being done in 'drm/i915: Enable edp psr error interrupts on
bdw+'
> +{
> + u32 transcoders = BIT(TRANSCODER_EDP);
> + enum transcoder cpu_transcoder;
> +
> + if (INTEL_GEN(dev_priv) >= 8)
> + transcoders |= BIT(TRANSCODER_A) |
> + BIT(TRANSCODER_B) |
> + BIT(TRANSCODER_C);
> +
> + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> transcoders) {
> + /* FIXME: Exit PSR and link train manually when this
> happens. */
> + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR aux
> error\n",
> + transcoder_name(cpu_transcoder
> ));
> +
> + if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR entry
> attempt in 2 vblanks\n",
> + transcoder_name(cpu_transcoder
> ));
> +
> + if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR exit
> completed\n",
> + transcoder_name(cpu_transcoder
> ));
> + }
> +}
> +
> static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
> {
> uint8_t psr_caps = 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
2018-04-03 21:24 ` [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
@ 2018-04-04 0:56 ` Souza, Jose
0 siblings, 0 replies; 32+ messages in thread
From: Souza, Jose @ 2018-04-04 0:56 UTC (permalink / raw)
To: intel-gfx, Pandiyan, Dhinakaran; +Cc: Vivi, Rodrigo
On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> Timestamps are useful for IGT tests that trigger PSR exit and/or wait
> for
> PSR entry.
>
> v2: Removed seqlock (Ville)
> Removed erroneous warning in irq loop (Chris)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++++
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 9 +++++++--
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 28f91df5b401..b378fa013054 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2686,6 +2686,13 @@ static int i915_edp_psr_status(struct seq_file
> *m, void *data)
> }
> mutex_unlock(&dev_priv->psr.lock);
>
> + if (READ_ONCE(dev_priv->psr.debug)) {
> + seq_printf(m, "Last attempted entry at: %lld\n",
> + dev_priv->psr.last_entry_attempt);
> + seq_printf(m, "Last exit at: %lld\n",
> + dev_priv->psr.last_exit);
> + }
> +
> intel_runtime_pm_put(dev_priv);
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index b97ed0cd4ca9..2124a795d10c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -610,6 +610,8 @@ struct i915_psr {
> bool psr2_enabled;
> u8 sink_sync_latency;
> bool debug;
> + ktime_t last_entry_attempt;
> + ktime_t last_exit;
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 56ff2d7691a1..a11a6d940203 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -131,6 +131,7 @@ void intel_psr_irq_handler(struct
> drm_i915_private *dev_priv, u32 psr_iir)
> {
> u32 transcoders = BIT(TRANSCODER_EDP);
> enum transcoder cpu_transcoder;
> + ktime_t time_ns = ktime_get();
>
> if (INTEL_GEN(dev_priv) >= 8)
> transcoders |= BIT(TRANSCODER_A) |
> @@ -143,13 +144,17 @@ void intel_psr_irq_handler(struct
> drm_i915_private *dev_priv, u32 psr_iir)
> DRM_DEBUG_KMS("[transcoder %s] PSR aux
> error\n",
> transcoder_name(cpu_transcoder
> ));
>
> - if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> + if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> + dev_priv->psr.last_entry_attempt = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR entry
> attempt in 2 vblanks\n",
> transcoder_name(cpu_transcoder
> ));
> + }
>
> - if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> + if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> + dev_priv->psr.last_exit = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR exit
> completed\n",
> transcoder_name(cpu_transcoder
> ));
> + }
> }
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-04-04 0:54 ` Souza, Jose
@ 2018-04-04 23:58 ` Pandiyan, Dhinakaran
2018-04-05 1:37 ` [PATCH v4 " Dhinakaran Pandiyan
1 sibling, 0 replies; 32+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-04-04 23:58 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx, Vivi, Rodrigo
On Wed, 2018-04-04 at 00:54 +0000, Souza, Jose wrote:
> On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> > Interrupts other than the one for AUX errors are required only for
> > debug,
> > so unmask them via debugfs when the user requests debug.
> >
> > User can make such a request with
> > echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
> >
> > There are no locks to serialize PSR debug enabling from
> > irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
> > called only during module initialization/resume and IGT subtests
> > aren't expected to modify PSR debug at those times, we should be
> > safe.
> >
> > v2: Unroll loops (Ville)
> > Avoid resetting error mask bits.
> >
> > v3: Unmask interrupts in postinstall() if debug was still enabled.
> > Avoid RMW (Ville)
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 36 +++++++++++++++++++++-
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_irq.c | 57 +++++++++++--------------
> > ----------
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 60
> > +++++++++++++++++++++++++++++++++++++
> > 5 files changed, 116 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 1dba2c451255..28f91df5b401 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file
> > *m, void *data)
> > return 0;
> > }
> >
> > +static int
> > +i915_edp_psr_debug_set(void *data, u64 val)
> > +{
> > + struct drm_i915_private *dev_priv = data;
> > +
> > + if (!CAN_PSR(dev_priv))
> > + return -ENODEV;
> > +
> > + DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
> > +
> > + intel_runtime_pm_get(dev_priv);
> > + intel_psr_debug_control(dev_priv, !!val);
> > + intel_runtime_pm_put(dev_priv);
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +i915_edp_psr_debug_get(void *data, u64 *val)
> > +{
> > + struct drm_i915_private *dev_priv = data;
> > +
> > + if (!CAN_PSR(dev_priv))
> > + return -ENODEV;
> > +
> > + *val = READ_ONCE(dev_priv->psr.debug);
> > + return 0;
> > +}
> > +
> > +DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
> > + i915_edp_psr_debug_get,
> > i915_edp_psr_debug_set,
> > + "%llu\n");
> > +
> > static int i915_sink_crc(struct seq_file *m, void *data)
> > {
> > struct drm_i915_private *dev_priv = node_to_i915(m-
> > >private);
> > @@ -4812,7 +4845,8 @@ static const struct i915_debugfs_files {
> > {"i915_guc_log_relay", &i915_guc_log_relay_fops},
> > {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> > {"i915_ipc_status", &i915_ipc_status_fops},
> > - {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> > + {"i915_drrs_ctl", &i915_drrs_ctl_fops},
> > + {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
>
>
> same as bellow, why not i915_edp_psr_int_debug?
>
> > };
> >
> > int i915_debugfs_register(struct drm_i915_private *dev_priv)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5373b171bb96..b97ed0cd4ca9 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -609,6 +609,7 @@ struct i915_psr {
> > bool has_hw_tracking;
> > bool psr2_enabled;
> > u8 sink_sync_latency;
> > + bool debug;
>
> maybe change to a name that gives more information about the use of
> this flag? like int_debug?
>
Enabling interrupts is one part of enabling PSR debug, that is how I
think of it. We'll probably use the same control to enable other debug
mechanisms as well. At the least to update timestamps from psr_activate
and psr_exit for VLV/CHV.
> >
> > void (*enable_source)(struct intel_dp *,
> > const struct intel_crtc_state *);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 8a894adf2ca1..714570955196 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2391,40 +2391,6 @@ static void ilk_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > ironlake_rps_change_irq_handler(dev_priv);
> > }
> >
> > -static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > *dev_priv)
> > -{
> > - u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > - u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
> > - u32 mask = BIT(TRANSCODER_EDP);
> > - enum transcoder cpu_transcoder;
> > -
> > - if (INTEL_GEN(dev_priv) >= 8)
> > - mask |= BIT(TRANSCODER_A) |
> > - BIT(TRANSCODER_B) |
> > - BIT(TRANSCODER_C);
> > -
> > - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > mask) {
> > - if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > - DRM_DEBUG_KMS("Transcoder %s PSR error\n",
> > - transcoder_name(cpu_transcoder
> > ));
> > -
> > - if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> > {
> > - DRM_DEBUG_KMS("Transcoder %s PSR prepare
> > entry in 2 vblanks\n",
> > - transcoder_name(cpu_transcoder
> > ));
> > - edp_psr_imr |=
> > EDP_PSR_PRE_ENTRY(cpu_transcoder);
> > - }
> > -
> > - if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> > {
> > - DRM_DEBUG_KMS("Transcoder %s PSR exit
> > completed\n",
> > - transcoder_name(cpu_transcoder
> > ));
> > - edp_psr_imr &=
> > ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
> > - }
> > - }
> > -
> > - I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
> > - I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > -}
> > -
> > static void ivb_display_irq_handler(struct drm_i915_private
> > *dev_priv,
> > u32 de_iir)
> > {
> > @@ -2437,8 +2403,12 @@ static void ivb_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > if (de_iir & DE_ERR_INT_IVB)
> > ivb_err_int_handler(dev_priv);
> >
> > - if (de_iir & DE_EDP_PSR_INT_HSW)
> > - hsw_edp_psr_irq_handler(dev_priv);
> > + if (de_iir & DE_EDP_PSR_INT_HSW) {
> > + u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > + intel_psr_irq_handler(dev_priv, psr_iir);
> > + I915_WRITE(EDP_PSR_IIR, psr_iir);
> > + }
> >
> > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > dp_aux_irq_handler(dev_priv);
> > @@ -2580,7 +2550,10 @@ gen8_de_irq_handler(struct drm_i915_private
> > *dev_priv, u32 master_ctl)
> > }
> >
> > if (iir & GEN8_DE_EDP_PSR) {
> > - hsw_edp_psr_irq_handler(dev_priv);
> > + u32 psr_iir =
> > I915_READ(EDP_PSR_IIR);
> > +
> > + intel_psr_irq_handler(dev_priv,
> > psr_iir);
> > + I915_WRITE(EDP_PSR_IIR, psr_iir);
> > found = true;
> > }
> >
> > @@ -3729,7 +3702,8 @@ static int ironlake_irq_postinstall(struct
> > drm_device *dev)
> >
> > if (IS_HASWELL(dev_priv)) {
> > gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > - I915_WRITE(EDP_PSR_IMR, 0);
> > + I915_WRITE(EDP_PSR_IMR,
> > ~EDP_PSR_ERROR(TRANSCODER_EDP));
> > + intel_psr_debug_control(dev_priv, dev_priv-
> > >psr.debug);
> > display_mask |= DE_EDP_PSR_INT_HSW;
> > }
> >
> > @@ -3845,6 +3819,7 @@ static void gen8_de_irq_postinstall(struct
> > drm_i915_private *dev_priv)
> > u32 de_port_enables;
> > u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
> > enum pipe pipe;
> > + u32 psr_masked;
> >
> > if (INTEL_GEN(dev_priv) >= 9) {
> > de_pipe_masked |= GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
> > @@ -3869,7 +3844,11 @@ static void gen8_de_irq_postinstall(struct
> > drm_i915_private *dev_priv)
> > de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
> >
> > gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > - I915_WRITE(EDP_PSR_IMR, 0);
> > + psr_masked = EDP_PSR_ERROR(TRANSCODER_EDP) |
> > + EDP_PSR_ERROR(TRANSCODER_A) |
> > EDP_PSR_ERROR(TRANSCODER_B) |
> > + EDP_PSR_ERROR(TRANSCODER_C);
> > + I915_WRITE(EDP_PSR_IMR, ~psr_masked);
>
> intel_psr_debug_control() will already write to EDP_PSR_IMR with the
> right value, why write twice?
>
Good point. The benefit is it keeps whatever happens in psr_debug_control()
separate from what irq_postinstall should always do. Doesn't outweigh the
benefit of avoiding an additional MMIO write, I think. I will remove the IMR
write from here.
> > + intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
> >
> > for_each_pipe(dev_priv, pipe) {
> > dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index d1452fd2a58d..53cb76b920e6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct
> > drm_i915_private *dev_priv,
> > unsigned frontbuffer_bits);
> > void intel_psr_compute_config(struct intel_dp *intel_dp,
> > struct intel_crtc_state *crtc_state);
> > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool
> > enable);
> > +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > psr_iir);
> >
> > /* intel_runtime_pm.c */
> > int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 2d53f7398a6d..56ff2d7691a1 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -93,6 +93,66 @@ static void psr_aux_io_power_put(struct intel_dp
> > *intel_dp)
> > intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> > }
> >
> > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool
> > enable)
> > +{
> > + u32 debug_mask, error_mask;
> > +
> > + /* No PSR interrupts on VLV/CHV */
> > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > + return;
> > +
> > + error_mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> > + debug_mask = error_mask | EDP_PSR_POST_EXIT(TRANSCODER_EDP)
> > |
> > + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> > +
> > + if (INTEL_GEN(dev_priv) >= 8) {
> > + error_mask |= EDP_PSR_ERROR(TRANSCODER_A) |
> > + EDP_PSR_ERROR(TRANSCODER_B) |
> > + EDP_PSR_ERROR(TRANSCODER_C);
> > +
> > + debug_mask |= error_mask |
> > EDP_PSR_POST_EXIT(TRANSCODER_A) |
> > + EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
> > + EDP_PSR_POST_EXIT(TRANSCODER_B) |
> > + EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
> > + EDP_PSR_POST_EXIT(TRANSCODER_C) |
> > + EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> > + }
> > +
> > + if (enable) {
> > + WRITE_ONCE(dev_priv->psr.debug, true);
> > + I915_WRITE(EDP_PSR_IMR, ~debug_mask);
> > + } else {
> > + I915_WRITE(EDP_PSR_IMR, ~error_mask);
> > + WRITE_ONCE(dev_priv->psr.debug, false);
> > + }
>
>
> nit pick but why not something like?
>
> u32 debug_mask, mask;
>
> mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP)
> ...
>
> if (enable)
> mask |= debug_mask;
>
> I915_WRITE(EDP_PSR_IMR, ~mask)
> WRITE_ONCE(dev_priv->psr.debug, enable);
>
>
Don't see much advantage in writing it one way or the other. Having said
that I have no issues in making the change.
> > +}
> > +
> > +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > psr_iir)
>
> two minnor things here:
> - why you moved the function here? it adds more noise to the diff.
Wanted the PSR irq_handler to be close to the rest of PSR code. We'll
eventually want to call other functions in this file to handle errors.
> - the rename from hsw_edp_psr_irq_handler to intel_psr_irq_handler
> should have being done in 'drm/i915: Enable edp psr error interrupts on
> bdw+'
>
I wanted to make only minimal rebase changes for the previous two
patches since I did not write them originally.
> > +{
> > + u32 transcoders = BIT(TRANSCODER_EDP);
> > + enum transcoder cpu_transcoder;
> > +
> > + if (INTEL_GEN(dev_priv) >= 8)
> > + transcoders |= BIT(TRANSCODER_A) |
> > + BIT(TRANSCODER_B) |
> > + BIT(TRANSCODER_C);
> > +
> > + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > transcoders) {
> > + /* FIXME: Exit PSR and link train manually when this
> > happens. */
> > + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > + DRM_DEBUG_KMS("[transcoder %s] PSR aux
> > error\n",
> > + transcoder_name(cpu_transcoder
> > ));
> > +
> > + if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> > + DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > attempt in 2 vblanks\n",
> > + transcoder_name(cpu_transcoder
> > ));
> > +
> > + if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> > + DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > completed\n",
> > + transcoder_name(cpu_transcoder
> > ));
> > + }
> > +}
> > +
> > static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
> > {
> > uint8_t psr_caps = 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v4 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-04-04 0:54 ` Souza, Jose
2018-04-04 23:58 ` Pandiyan, Dhinakaran
@ 2018-04-05 1:37 ` Dhinakaran Pandiyan
2018-04-05 19:42 ` Souza, Jose
1 sibling, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-05 1:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Interrupts other than the one for AUX errors are required only for debug,
so unmask them via debugfs when the user requests debug.
User can make such a request with
echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
There are no locks to serialize PSR debug enabling from
irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
called only during module initialization/resume and IGT subtests
aren't expected to modify PSR debug at those times, we should be safe.
v2: Unroll loops (Ville)
Avoid resetting error mask bits.
v3: Unmask interrupts in postinstall() if debug was still enabled.
Avoid RMW (Ville)
v4: Avoid extra IMR write introduced in the previous version.(Jose)
Style changes, renames (Jose).
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 51 ++++++++------------------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 58 +++++++++++++++++++++++++++++++++++++
5 files changed, 108 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1dba2c451255..025410a08786 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
return 0;
}
+static int
+i915_edp_psr_debug_set(void *data, u64 val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ if (!CAN_PSR(dev_priv))
+ return -ENODEV;
+
+ DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
+
+ intel_runtime_pm_get(dev_priv);
+ intel_psr_irq_control(dev_priv, !!val);
+ intel_runtime_pm_put(dev_priv);
+
+ return 0;
+}
+
+static int
+i915_edp_psr_debug_get(void *data, u64 *val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ if (!CAN_PSR(dev_priv))
+ return -ENODEV;
+
+ *val = READ_ONCE(dev_priv->psr.debug);
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
+ i915_edp_psr_debug_get, i915_edp_psr_debug_set,
+ "%llu\n");
+
static int i915_sink_crc(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4812,7 +4845,8 @@ static const struct i915_debugfs_files {
{"i915_guc_log_relay", &i915_guc_log_relay_fops},
{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
{"i915_ipc_status", &i915_ipc_status_fops},
- {"i915_drrs_ctl", &i915_drrs_ctl_fops}
+ {"i915_drrs_ctl", &i915_drrs_ctl_fops},
+ {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
};
int i915_debugfs_register(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5373b171bb96..b97ed0cd4ca9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -609,6 +609,7 @@ struct i915_psr {
bool has_hw_tracking;
bool psr2_enabled;
u8 sink_sync_latency;
+ bool debug;
void (*enable_source)(struct intel_dp *,
const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8a894adf2ca1..91b66a52cae5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,40 +2391,6 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
ironlake_rps_change_irq_handler(dev_priv);
}
-static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
-{
- u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
- u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
- u32 mask = BIT(TRANSCODER_EDP);
- enum transcoder cpu_transcoder;
-
- if (INTEL_GEN(dev_priv) >= 8)
- mask |= BIT(TRANSCODER_A) |
- BIT(TRANSCODER_B) |
- BIT(TRANSCODER_C);
-
- for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
- if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
- DRM_DEBUG_KMS("Transcoder %s PSR error\n",
- transcoder_name(cpu_transcoder));
-
- if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
- DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 vblanks\n",
- transcoder_name(cpu_transcoder));
- edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
- }
-
- if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
- DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
- transcoder_name(cpu_transcoder));
- edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
- }
- }
-
- I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
- I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
-}
-
static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
u32 de_iir)
{
@@ -2437,8 +2403,12 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev_priv);
- if (de_iir & DE_EDP_PSR_INT_HSW)
- hsw_edp_psr_irq_handler(dev_priv);
+ if (de_iir & DE_EDP_PSR_INT_HSW) {
+ u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+ intel_psr_irq_handler(dev_priv, psr_iir);
+ I915_WRITE(EDP_PSR_IIR, psr_iir);
+ }
if (de_iir & DE_AUX_CHANNEL_A_IVB)
dp_aux_irq_handler(dev_priv);
@@ -2580,7 +2550,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
}
if (iir & GEN8_DE_EDP_PSR) {
- hsw_edp_psr_irq_handler(dev_priv);
+ u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+ intel_psr_irq_handler(dev_priv, psr_iir);
+ I915_WRITE(EDP_PSR_IIR, psr_iir);
found = true;
}
@@ -3729,7 +3702,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
if (IS_HASWELL(dev_priv)) {
gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
- I915_WRITE(EDP_PSR_IMR, 0);
+ intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -3869,7 +3842,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
- I915_WRITE(EDP_PSR_IMR, 0);
+ intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
for_each_pipe(dev_priv, pipe) {
dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1452fd2a58d..3227f2af5294 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
unsigned frontbuffer_bits);
void intel_psr_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state);
+void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2d53f7398a6d..9ebe9e8a87bb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,6 +93,64 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
}
+void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
+{
+ u32 debug_mask, mask;
+
+ /* No PSR interrupts on VLV/CHV */
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ return;
+
+ mask = EDP_PSR_ERROR(TRANSCODER_EDP);
+ debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
+
+ if (INTEL_GEN(dev_priv) >= 8) {
+ mask |= EDP_PSR_ERROR(TRANSCODER_A) |
+ EDP_PSR_ERROR(TRANSCODER_B) |
+ EDP_PSR_ERROR(TRANSCODER_C);
+
+ debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
+ EDP_PSR_POST_EXIT(TRANSCODER_B) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
+ EDP_PSR_POST_EXIT(TRANSCODER_C) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+ }
+
+ if (debug)
+ mask |= debug_mask;
+
+ WRITE_ONCE(dev_priv->psr.debug, debug);
+ I915_WRITE(EDP_PSR_IMR, ~mask);
+}
+
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
+{
+ u32 transcoders = BIT(TRANSCODER_EDP);
+ enum transcoder cpu_transcoder;
+
+ if (INTEL_GEN(dev_priv) >= 8)
+ transcoders |= BIT(TRANSCODER_A) |
+ BIT(TRANSCODER_B) |
+ BIT(TRANSCODER_C);
+
+ for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+ /* FIXME: Exit PSR and link train manually when this happens. */
+ if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
+ transcoder_name(cpu_transcoder));
+
+ if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
+ transcoder_name(cpu_transcoder));
+
+ if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
+ DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
+ transcoder_name(cpu_transcoder));
+ }
+}
+
static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
{
uint8_t psr_caps = 0;
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (4 preceding siblings ...)
2018-04-03 22:06 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-04-05 2:03 ` Patchwork
2018-04-05 2:18 ` ✓ Fi.CI.BAT: success " Patchwork
` (8 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-05 2:03 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
URL : https://patchwork.freedesktop.org/series/41095/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
f3b80889ad58 drm/i915: Enable edp psr error interrupts on hsw
-:109: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#109: FILE: drivers/gpu/drm/i915/i915_reg.h:4017:
+#define EDP_PSR_ERROR (1<<2)
^
-:110: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#110: FILE: drivers/gpu/drm/i915/i915_reg.h:4018:
+#define EDP_PSR_POST_EXIT (1<<1)
^
-:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#111: FILE: drivers/gpu/drm/i915/i915_reg.h:4019:
+#define EDP_PSR_PRE_ENTRY (1<<0)
^
-:120: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#120: FILE: drivers/gpu/drm/i915/i915_reg.h:6830:
+#define DE_EDP_PSR_INT_HSW (1<<19)
^
total: 0 errors, 0 warnings, 4 checks, 78 lines checked
a5cf241c72c3 drm/i915: Enable edp psr error interrupts on bdw+
-:158: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#158: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:158: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
#158: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:159: CHECK:SPACING: No space is necessary after a cast
#159: FILE: drivers/gpu/drm/i915/intel_display.h:222:
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
-:160: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#160: FILE: drivers/gpu/drm/i915/intel_display.h:223:
+ for_each_if ((__mask) & (1 << (__t)))
total: 1 errors, 1 warnings, 2 checks, 123 lines checked
d117c36f789e drm/i915/psr: Control PSR interrupts via debugfs
19bb9639e1a7 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (5 preceding siblings ...)
2018-04-05 2:03 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) Patchwork
@ 2018-04-05 2:18 ` Patchwork
2018-04-05 3:18 ` ✗ Fi.CI.IGT: failure " Patchwork
` (7 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-05 2:18 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
URL : https://patchwork.freedesktop.org/series/41095/
State : success
== Summary ==
Series 41095v2 series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw
https://patchwork.freedesktop.org/api/1.0/series/41095/revisions/2/mbox/
---- Known issues:
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (fi-glk-j4005) fdo#103359
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (fi-cfl-s3) fdo#100368
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:431s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:440s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:386s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:542s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:296s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:517s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:511s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:507s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s
fi-cfl-s3 total:285 pass:258 dwarn:0 dfail:0 fail:1 skip:26 time:549s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:514s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:583s
fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:426s
fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:317s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:537s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:484s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:422s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:469s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:437s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:473s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:464s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:512s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:440s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:535s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:503s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:502s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:429s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:448s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:565s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:400s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:533s
e29a10513429cca404e9847a399efbdbb4bdd4bf drm-tip: 2018y-04m-04d-20h-47m-24s UTC integration manifest
19bb9639e1a7 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
d117c36f789e drm/i915/psr: Control PSR interrupts via debugfs
a5cf241c72c3 drm/i915: Enable edp psr error interrupts on bdw+
f3b80889ad58 drm/i915: Enable edp psr error interrupts on hsw
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8584/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (6 preceding siblings ...)
2018-04-05 2:18 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-05 3:18 ` Patchwork
2018-04-05 20:40 ` [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Souza, Jose
` (6 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-05 3:18 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
URL : https://patchwork.freedesktop.org/series/41095/
State : failure
== Summary ==
---- Possible new issues:
Test gem_cs_tlb:
Subgroup render:
pass -> INCOMPLETE (shard-snb)
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-offscren-pri-shrfb-draw-pwrite:
dmesg-fail -> PASS (shard-apl)
Subgroup fbc-modesetfrombusy:
pass -> FAIL (shard-apl)
---- Known issues:
Test kms_atomic_transition:
Subgroup 1x-modeset-transitions-nonblocking:
pass -> FAIL (shard-apl) fdo#103207
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-toggle:
pass -> FAIL (shard-hsw) fdo#102670 +1
Test kms_flip:
Subgroup 2x-flip-vs-expired-vblank:
fail -> PASS (shard-hsw) fdo#102887 +1
Subgroup plain-flip-ts-check-interruptible:
fail -> PASS (shard-hsw) fdo#100368 +2
Test kms_plane_multiple:
Subgroup atomic-pipe-a-tiling-x:
pass -> FAIL (shard-snb) fdo#103166
Test kms_rotation_crc:
Subgroup sprite-rotation-180:
pass -> FAIL (shard-snb) fdo#103925
Test kms_setmode:
Subgroup basic:
fail -> PASS (shard-hsw) fdo#99912
Test kms_sysfs_edid_timing:
warn -> PASS (shard-apl) fdo#100047
fdo#103207 https://bugs.freedesktop.org/show_bug.cgi?id=103207
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
shard-apl total:3499 pass:1834 dwarn:1 dfail:0 fail:9 skip:1655 time:12886s
shard-hsw total:3499 pass:1784 dwarn:1 dfail:0 fail:3 skip:1710 time:11560s
shard-snb total:3429 pass:1350 dwarn:1 dfail:0 fail:4 skip:2073 time:6880s
Blacklisted hosts:
shard-kbl total:3374 pass:1893 dwarn:1 dfail:1 fail:6 skip:1471 time:8906s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8584/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-04-05 1:37 ` [PATCH v4 " Dhinakaran Pandiyan
@ 2018-04-05 19:42 ` Souza, Jose
0 siblings, 0 replies; 32+ messages in thread
From: Souza, Jose @ 2018-04-05 19:42 UTC (permalink / raw)
To: intel-gfx, Pandiyan, Dhinakaran; +Cc: Vivi, Rodrigo
On Wed, 2018-04-04 at 18:37 -0700, Dhinakaran Pandiyan wrote:
> Interrupts other than the one for AUX errors are required only for
> debug,
> so unmask them via debugfs when the user requests debug.
>
> User can make such a request with
> echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
>
> There are no locks to serialize PSR debug enabling from
> irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
> called only during module initialization/resume and IGT subtests
> aren't expected to modify PSR debug at those times, we should be
> safe.
>
> v2: Unroll loops (Ville)
> Avoid resetting error mask bits.
>
> v3: Unmask interrupts in postinstall() if debug was still enabled.
> Avoid RMW (Ville)
>
> v4: Avoid extra IMR write introduced in the previous version.(Jose)
> Style changes, renames (Jose).
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 51 ++++++++-------------------
> -----
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 58
> +++++++++++++++++++++++++++++++++++++
> 5 files changed, 108 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1dba2c451255..025410a08786 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file
> *m, void *data)
> return 0;
> }
>
> +static int
> +i915_edp_psr_debug_set(void *data, u64 val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + if (!CAN_PSR(dev_priv))
> + return -ENODEV;
> +
> + DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
> +
> + intel_runtime_pm_get(dev_priv);
> + intel_psr_irq_control(dev_priv, !!val);
> + intel_runtime_pm_put(dev_priv);
> +
> + return 0;
> +}
> +
> +static int
> +i915_edp_psr_debug_get(void *data, u64 *val)
> +{
> + struct drm_i915_private *dev_priv = data;
> +
> + if (!CAN_PSR(dev_priv))
> + return -ENODEV;
> +
> + *val = READ_ONCE(dev_priv->psr.debug);
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
> + i915_edp_psr_debug_get,
> i915_edp_psr_debug_set,
> + "%llu\n");
> +
> static int i915_sink_crc(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m-
> >private);
> @@ -4812,7 +4845,8 @@ static const struct i915_debugfs_files {
> {"i915_guc_log_relay", &i915_guc_log_relay_fops},
> {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> {"i915_ipc_status", &i915_ipc_status_fops},
> - {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> + {"i915_drrs_ctl", &i915_drrs_ctl_fops},
> + {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
> };
>
> int i915_debugfs_register(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 5373b171bb96..b97ed0cd4ca9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -609,6 +609,7 @@ struct i915_psr {
> bool has_hw_tracking;
> bool psr2_enabled;
> u8 sink_sync_latency;
> + bool debug;
>
> void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 8a894adf2ca1..91b66a52cae5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,40 +2391,6 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
> ironlake_rps_change_irq_handler(dev_priv);
> }
>
> -static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> -{
> - u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> - u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
> - u32 mask = BIT(TRANSCODER_EDP);
> - enum transcoder cpu_transcoder;
> -
> - if (INTEL_GEN(dev_priv) >= 8)
> - mask |= BIT(TRANSCODER_A) |
> - BIT(TRANSCODER_B) |
> - BIT(TRANSCODER_C);
> -
> - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> mask) {
> - if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> - DRM_DEBUG_KMS("Transcoder %s PSR error\n",
> - transcoder_name(cpu_transcoder
> ));
> -
> - if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> {
> - DRM_DEBUG_KMS("Transcoder %s PSR prepare
> entry in 2 vblanks\n",
> - transcoder_name(cpu_transcoder
> ));
> - edp_psr_imr |=
> EDP_PSR_PRE_ENTRY(cpu_transcoder);
> - }
> -
> - if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> {
> - DRM_DEBUG_KMS("Transcoder %s PSR exit
> completed\n",
> - transcoder_name(cpu_transcoder
> ));
> - edp_psr_imr &=
> ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
> - }
> - }
> -
> - I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
> - I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> -}
> -
> static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
> u32 de_iir)
> {
> @@ -2437,8 +2403,12 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev_priv);
>
> - if (de_iir & DE_EDP_PSR_INT_HSW)
> - hsw_edp_psr_irq_handler(dev_priv);
> + if (de_iir & DE_EDP_PSR_INT_HSW) {
> + u32 psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + intel_psr_irq_handler(dev_priv, psr_iir);
> + I915_WRITE(EDP_PSR_IIR, psr_iir);
> + }
>
> if (de_iir & DE_AUX_CHANNEL_A_IVB)
> dp_aux_irq_handler(dev_priv);
> @@ -2580,7 +2550,10 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
> }
>
> if (iir & GEN8_DE_EDP_PSR) {
> - hsw_edp_psr_irq_handler(dev_priv);
> + u32 psr_iir =
> I915_READ(EDP_PSR_IIR);
> +
> + intel_psr_irq_handler(dev_priv,
> psr_iir);
> + I915_WRITE(EDP_PSR_IIR, psr_iir);
> found = true;
> }
>
> @@ -3729,7 +3702,7 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
>
> if (IS_HASWELL(dev_priv)) {
> gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> - I915_WRITE(EDP_PSR_IMR, 0);
> + intel_psr_irq_control(dev_priv, dev_priv-
> >psr.debug);
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
>
> @@ -3869,7 +3842,7 @@ static void gen8_de_irq_postinstall(struct
> drm_i915_private *dev_priv)
> de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
>
> gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> - I915_WRITE(EDP_PSR_IMR, 0);
> + intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
>
> for_each_pipe(dev_priv, pipe) {
> dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d1452fd2a58d..3227f2af5294 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1891,6 +1891,8 @@ void intel_psr_single_frame_update(struct
> drm_i915_private *dev_priv,
> unsigned frontbuffer_bits);
> void intel_psr_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> +void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool
> debug);
> +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> psr_iir);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 2d53f7398a6d..9ebe9e8a87bb 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -93,6 +93,64 @@ static void psr_aux_io_power_put(struct intel_dp
> *intel_dp)
> intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> }
>
> +void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool
> debug)
> +{
> + u32 debug_mask, mask;
> +
> + /* No PSR interrupts on VLV/CHV */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + return;
> +
> + mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> + debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> +
> + if (INTEL_GEN(dev_priv) >= 8) {
> + mask |= EDP_PSR_ERROR(TRANSCODER_A) |
> + EDP_PSR_ERROR(TRANSCODER_B) |
> + EDP_PSR_ERROR(TRANSCODER_C);
> +
> + debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
> + EDP_PSR_POST_EXIT(TRANSCODER_B) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
> + EDP_PSR_POST_EXIT(TRANSCODER_C) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> + }
> +
> + if (debug)
> + mask |= debug_mask;
> +
> + WRITE_ONCE(dev_priv->psr.debug, debug);
> + I915_WRITE(EDP_PSR_IMR, ~mask);
> +}
> +
> +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> psr_iir)
> +{
> + u32 transcoders = BIT(TRANSCODER_EDP);
> + enum transcoder cpu_transcoder;
> +
> + if (INTEL_GEN(dev_priv) >= 8)
> + transcoders |= BIT(TRANSCODER_A) |
> + BIT(TRANSCODER_B) |
> + BIT(TRANSCODER_C);
> +
> + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> transcoders) {
> + /* FIXME: Exit PSR and link train manually when this
> happens. */
> + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR aux
> error\n",
> + transcoder_name(cpu_transcoder
> ));
> +
> + if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR entry
> attempt in 2 vblanks\n",
> + transcoder_name(cpu_transcoder
> ));
> +
> + if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> + DRM_DEBUG_KMS("[transcoder %s] PSR exit
> completed\n",
> + transcoder_name(cpu_transcoder
> ));
> + }
> +}
> +
> static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
> {
> uint8_t psr_caps = 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+
2018-04-03 21:24 ` [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
@ 2018-04-05 20:38 ` Souza, Jose
2018-04-05 21:25 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 32+ messages in thread
From: Souza, Jose @ 2018-04-05 20:38 UTC (permalink / raw)
To: intel-gfx, Pandiyan, Dhinakaran; +Cc: daniel.vetter, Vivi, Rodrigo
On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr
> on
> any transcoder in theory, though the we don't currenty enable PSR
> except
> on the EDP transcoder.
>
> v2: From DK
> * Rebased on drm-tip
> v3: Switched author to Ville based on IRC discussion.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 57
> ++++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/i915_reg.h | 7 +++--
> drivers/gpu/drm/i915/intel_display.h | 4 +++
> 3 files changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index c2d3f30778ee..8a894adf2ca1 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2394,20 +2394,34 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
> static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
nitpick: Like I said in the other patch I would like to have this
function renamed here.
> {
> u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> + u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
> + u32 mask = BIT(TRANSCODER_EDP);
> + enum transcoder cpu_transcoder;
>
> - if (edp_psr_iir & EDP_PSR_ERROR)
> - DRM_DEBUG_KMS("PSR error\n");
> -
> - if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> - DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> - I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> - }
> + if (INTEL_GEN(dev_priv) >= 8)
> + mask |= BIT(TRANSCODER_A) |
> + BIT(TRANSCODER_B) |
> + BIT(TRANSCODER_C);
> +
> + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> mask) {
> + if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> + DRM_DEBUG_KMS("Transcoder %s PSR error\n",
> + transcoder_name(cpu_transcoder
> ));
> +
> + if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
> {
> + DRM_DEBUG_KMS("Transcoder %s PSR prepare
> entry in 2 vblanks\n",
> + transcoder_name(cpu_transcoder
> ));
> + edp_psr_imr |=
> EDP_PSR_PRE_ENTRY(cpu_transcoder);
> + }
>
> - if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> - DRM_DEBUG_KMS("PSR exit completed\n");
> - I915_WRITE(EDP_PSR_IMR, 0);
> + if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
> {
> + DRM_DEBUG_KMS("Transcoder %s PSR exit
> completed\n",
> + transcoder_name(cpu_transcoder
> ));
> + edp_psr_imr &=
> ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
> + }
> }
>
> + I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
> I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> }
>
> @@ -2555,11 +2569,22 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
> if (master_ctl & GEN8_DE_MISC_IRQ) {
> iir = I915_READ(GEN8_DE_MISC_IIR);
> if (iir) {
> + bool found = false;
> +
> I915_WRITE(GEN8_DE_MISC_IIR, iir);
> ret = IRQ_HANDLED;
> - if (iir & GEN8_DE_MISC_GSE)
> +
> + if (iir & GEN8_DE_MISC_GSE) {
> intel_opregion_asle_intr(dev_priv);
> - else
> + found = true;
> + }
> +
> + if (iir & GEN8_DE_EDP_PSR) {
> + hsw_edp_psr_irq_handler(dev_priv);
> + found = true;
> + }
> +
> + if (!found)
> DRM_ERROR("Unexpected DE Misc
> interrupt\n");
> }
> else
> @@ -3326,6 +3351,9 @@ static void gen8_irq_reset(struct drm_device
> *dev)
>
> gen8_gt_irq_reset(dev_priv);
>
> + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> +
> for_each_pipe(dev_priv, pipe)
> if (intel_display_power_is_enabled(dev_priv,
> POWER_DOMAIN_PIPE
> (pipe)))
> @@ -3815,7 +3843,7 @@ static void gen8_de_irq_postinstall(struct
> drm_i915_private *dev_priv)
> uint32_t de_pipe_enables;
> u32 de_port_masked = GEN8_AUX_CHANNEL_A;
> u32 de_port_enables;
> - u32 de_misc_masked = GEN8_DE_MISC_GSE;
> + u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
> enum pipe pipe;
>
> if (INTEL_GEN(dev_priv) >= 9) {
> @@ -3840,6 +3868,9 @@ static void gen8_de_irq_postinstall(struct
> drm_i915_private *dev_priv)
> else if (IS_BROADWELL(dev_priv))
> de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
>
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IMR, 0);
> +
> for_each_pipe(dev_priv, pipe) {
> dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index f5783d6db614..6eb351177a68 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4014,9 +4014,9 @@ enum {
> /* Bspec claims those aren't shifted but stay at 0x64800 */
> #define EDP_PSR_IMR _MMIO(0x64834)
> #define EDP_PSR_IIR _MMIO(0x64838)
> -#define EDP_PSR_ERROR (1<<2)
> -#define EDP_PSR_POST_EXIT (1<<1)
> -#define EDP_PSR_PRE_ENTRY (1<<0)
> +#define EDP_PSR_ERROR(trans) (1 <<
> (((trans) * 8 + 10) & 31))
> +#define EDP_PSR_POST_EXIT(trans) (1 << (((trans) *
> 8 + 9) & 31))
> +#define EDP_PSR_PRE_ENTRY(trans) (1 << (((trans) *
> 8 + 8) & 31))
>
Clever macros
> #define EDP_PSR_AUX_CTL _MMIO(dev_pri
> v->psr_mmio_base + 0x10)
> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> @@ -6952,6 +6952,7 @@ enum {
> #define GEN8_DE_MISC_IIR _MMIO(0x44468)
> #define GEN8_DE_MISC_IER _MMIO(0x4446c)
> #define GEN8_DE_MISC_GSE (1 << 27)
> +#define GEN8_DE_EDP_PSR (1 << 19)
>
> #define GEN8_PCU_ISR _MMIO(0x444e0)
> #define GEN8_PCU_IMR _MMIO(0x444e4)
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index 4e7418b345bc..2ef31617614a 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -218,6 +218,10 @@ struct intel_link_m_n {
> for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;
> (__p)++) \
> for_each_if((__mask) & BIT(__p))
>
> +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++)
> \
> + for_each_if ((__mask) & (1 << (__t)))
> +
> #define for_each_universal_plane(__dev_priv, __pipe, __p)
> \
> for ((__p) = 0;
> \
> (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> 1; \
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (7 preceding siblings ...)
2018-04-05 3:18 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-04-05 20:40 ` Souza, Jose
2018-04-05 21:42 ` Dhinakaran Pandiyan
2018-04-05 22:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3) Patchwork
` (5 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Souza, Jose @ 2018-04-05 20:40 UTC (permalink / raw)
To: intel-gfx, Pandiyan, Dhinakaran
Cc: daniel.vetter, Vetter, Daniel, Vivi, Rodrigo
On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> The definitions for the error register should be valid on bdw/skl
> too,
> but there we haven't even enabled DE_MISC handling yet.
>
> Somewhat confusing the the moved register offset on bdw is only for
> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> on bdw.
>
> v2: Fixes from Ville.
>
> v3: From DK
> * Rebased on drm-tip
> * Removed BDW IIR bit definition, looks like an unintentional change
> that
> should be in the following patch.
>
> v4: From DK
> * Don't mask REG_WRITE.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
With bspec id and comment about why are you masking interruptions in
hsw_edp_psr_irq_handler() feel free to add:
Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 34
> ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 27aee25429b7..c2d3f30778ee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
> ironlake_rps_change_irq_handler(dev_priv);
> }
>
> +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> +{
> + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + if (edp_psr_iir & EDP_PSR_ERROR)
> + DRM_DEBUG_KMS("PSR error\n");
> +
> + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
Just to know... you need to mask this one otherwise it will keep
triggering EDP_PSR_PRE_ENTRY interruptions? Would be nice to have a
comment explaning why you are masking it.
> + }
> +
> + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> + DRM_DEBUG_KMS("PSR exit completed\n");
> + I915_WRITE(EDP_PSR_IMR, 0);
> + }
> +
> + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> +}
> +
> static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
> u32 de_iir)
> {
> @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev_priv);
>
> + if (de_iir & DE_EDP_PSR_INT_HSW)
> + hsw_edp_psr_irq_handler(dev_priv);
> +
> if (de_iir & DE_AUX_CHANNEL_A_IVB)
> dp_aux_irq_handler(dev_priv);
>
> @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> drm_device *dev)
> if (IS_GEN7(dev_priv))
> I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>
> + if (IS_HASWELL(dev_priv)) {
> + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
No need to do a POSTING_READ(EDP_PSR_IMR);? Like is done in:
GEN3_IRQ_RESET()
> + }
> +
> gen5_gt_irq_reset(dev_priv);
>
> ibx_irq_reset(dev_priv);
> @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
> DE_DP_A_HOTPLUG);
> }
>
> + if (IS_HASWELL(dev_priv)) {
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IMR, 0);
> + display_mask |= DE_EDP_PSR_INT_HSW;
> + }
> +
> dev_priv->irq_mask = ~display_mask;
>
> ibx_irq_pre_postinstall(dev);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 176dca6554f4..f5783d6db614 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4011,6 +4011,13 @@ enum {
> #define EDP_PSR_TP1_TIME_0us (3<<4)
> #define EDP_PSR_IDLE_FRAME_SHIFT 0
>
> +/* Bspec claims those aren't shifted but stay at 0x64800 */
> +#define EDP_PSR_IMR _MMIO(0x64834)
> +#define EDP_PSR_IIR _MMIO(0x64838)
> +#define EDP_PSR_ERROR (1<<2)
> +#define EDP_PSR_POST_EXIT (1<<1)
> +#define EDP_PSR_PRE_ENTRY (1<<0)
Could you add the bspec id of where did you got this?
The hsw spec that I'm reading don't have the bits, skl have but don't
have the bits of the other transcoders.
> +
> #define EDP_PSR_AUX_CTL _MMIO(dev_pri
> v->psr_mmio_base + 0x10)
> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
> @@ -6820,6 +6827,7 @@ enum {
> #define DE_PCH_EVENT_IVB (1<<28)
> #define DE_DP_A_HOTPLUG_IVB (1<<27)
> #define DE_AUX_CHANNEL_A_IVB (1<<26)
> +#define DE_EDP_PSR_INT_HSW (1<<19)
> #define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
> #define DE_PLANEC_FLIP_DONE_IVB (1<<13)
> #define DE_PIPEC_VBLANK_IVB (1<<10)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+
2018-04-05 20:38 ` Souza, Jose
@ 2018-04-05 21:25 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 32+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-04-05 21:25 UTC (permalink / raw)
To: Souza, Jose; +Cc: daniel.vetter, intel-gfx, Vivi, Rodrigo
On Thu, 2018-04-05 at 20:38 +0000, Souza, Jose wrote:
> On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr
> > on
> > any transcoder in theory, though the we don't currenty enable PSR
> > except
> > on the EDP transcoder.
> >
> > v2: From DK
> > * Rebased on drm-tip
> > v3: Switched author to Ville based on IRC discussion.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 57
> > ++++++++++++++++++++++++++++--------
> > drivers/gpu/drm/i915/i915_reg.h | 7 +++--
> > drivers/gpu/drm/i915/intel_display.h | 4 +++
> > 3 files changed, 52 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index c2d3f30778ee..8a894adf2ca1 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2394,20 +2394,34 @@ static void ilk_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > *dev_priv)
>
> nitpick: Like I said in the other patch I would like to have this
> function renamed here.
To intel_psr_irq_handler? I renamed it in patch 3 because the function
was moved out of the file. I would like to leave this patch as it is, as
the original author intended.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-05 20:40 ` [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Souza, Jose
@ 2018-04-05 21:42 ` Dhinakaran Pandiyan
2018-04-05 21:44 ` Souza, Jose
0 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-05 21:42 UTC (permalink / raw)
To: Souza, Jose; +Cc: daniel.vetter, Vetter, Daniel, intel-gfx, Vivi, Rodrigo
On Thu, 2018-04-05 at 20:40 +0000, Souza, Jose wrote:
> On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > The definitions for the error register should be valid on bdw/skl
> > too,
> > but there we haven't even enabled DE_MISC handling yet.
> >
> > Somewhat confusing the the moved register offset on bdw is only for
> > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > on bdw.
> >
> > v2: Fixes from Ville.
> >
> > v3: From DK
> > * Rebased on drm-tip
> > * Removed BDW IIR bit definition, looks like an unintentional change
> > that
> > should be in the following patch.
> >
> > v4: From DK
> > * Don't mask REG_WRITE.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> With bspec id and comment about why are you masking interruptions in
> hsw_edp_psr_irq_handler() feel free to add:
>
> Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 34
> > ++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 27aee25429b7..c2d3f30778ee 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > ironlake_rps_change_irq_handler(dev_priv);
> > }
> >
> > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > *dev_priv)
> > +{
> > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > + if (edp_psr_iir & EDP_PSR_ERROR)
> > + DRM_DEBUG_KMS("PSR error\n");
> > +
> > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
>
> Just to know... you need to mask this one otherwise it will keep
> triggering EDP_PSR_PRE_ENTRY interruptions? Would be nice to have a
> comment explaning why you are masking it.
>
The final implementation in patch 3/4 doesn't do that. Adding a comment
here and removing will be pointless IMO.
> > + }
> > +
> > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > + DRM_DEBUG_KMS("PSR exit completed\n");
> > + I915_WRITE(EDP_PSR_IMR, 0);
> > + }
> > +
> > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > +}
> > +
> > static void ivb_display_irq_handler(struct drm_i915_private
> > *dev_priv,
> > u32 de_iir)
> > {
> > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > if (de_iir & DE_ERR_INT_IVB)
> > ivb_err_int_handler(dev_priv);
> >
> > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > + hsw_edp_psr_irq_handler(dev_priv);
> > +
> > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > dp_aux_irq_handler(dev_priv);
> >
> > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > drm_device *dev)
> > if (IS_GEN7(dev_priv))
> > I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> >
> > + if (IS_HASWELL(dev_priv)) {
> > + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> > + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
>
> No need to do a POSTING_READ(EDP_PSR_IMR);? Like is done in:
> GEN3_IRQ_RESET()
>
We should be fine without that. From what I was told a while ago, some
of these POSTING_READS are cargo culted.
> > + }
> > +
> > gen5_gt_irq_reset(dev_priv);
> >
> > ibx_irq_reset(dev_priv);
> > @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> > drm_device *dev)
> > DE_DP_A_HOTPLUG);
> > }
> >
> > + if (IS_HASWELL(dev_priv)) {
> > + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > + I915_WRITE(EDP_PSR_IMR, 0);
> > + display_mask |= DE_EDP_PSR_INT_HSW;
> > + }
> > +
> > dev_priv->irq_mask = ~display_mask;
> >
> > ibx_irq_pre_postinstall(dev);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 176dca6554f4..f5783d6db614 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4011,6 +4011,13 @@ enum {
> > #define EDP_PSR_TP1_TIME_0us (3<<4)
> > #define EDP_PSR_IDLE_FRAME_SHIFT 0
> >
> > +/* Bspec claims those aren't shifted but stay at 0x64800 */
> > +#define EDP_PSR_IMR _MMIO(0x64834)
> > +#define EDP_PSR_IIR _MMIO(0x64838)
> > +#define EDP_PSR_ERROR (1<<2)
> > +#define EDP_PSR_POST_EXIT (1<<1)
> > +#define EDP_PSR_PRE_ENTRY (1<<0)
>
> Could you add the bspec id of where did you got this?
> The hsw spec that I'm reading don't have the bits, skl have but don't
> have the bits of the other transcoders.
>
I understand it is not easy to find, but are we going to add bspec
references for all new register definitions? :) From what I have seen, a
bspec reference is typically added only for workarounds. I'll update
this patch though since I've waited too long to get this patch in. Would
adding the bspec reference in the commit message suffice?
> > +
> > #define EDP_PSR_AUX_CTL _MMIO(dev_pri
> > v->psr_mmio_base + 0x10)
> > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
> > @@ -6820,6 +6827,7 @@ enum {
> > #define DE_PCH_EVENT_IVB (1<<28)
> > #define DE_DP_A_HOTPLUG_IVB (1<<27)
> > #define DE_AUX_CHANNEL_A_IVB (1<<26)
> > +#define DE_EDP_PSR_INT_HSW (1<<19)
> > #define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
> > #define DE_PLANEC_FLIP_DONE_IVB (1<<13)
> > #define DE_PIPEC_VBLANK_IVB (1<<10)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-05 21:42 ` Dhinakaran Pandiyan
@ 2018-04-05 21:44 ` Souza, Jose
2018-04-05 22:00 ` [PATCH v4] " Dhinakaran Pandiyan
0 siblings, 1 reply; 32+ messages in thread
From: Souza, Jose @ 2018-04-05 21:44 UTC (permalink / raw)
To: Pandiyan, Dhinakaran
Cc: daniel.vetter, Vetter, Daniel, intel-gfx, Vivi, Rodrigo
On Thu, 2018-04-05 at 14:42 -0700, Dhinakaran Pandiyan wrote:
>
>
> On Thu, 2018-04-05 at 20:40 +0000, Souza, Jose wrote:
> > On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> > > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >
> > > The definitions for the error register should be valid on bdw/skl
> > > too,
> > > but there we haven't even enabled DE_MISC handling yet.
> > >
> > > Somewhat confusing the the moved register offset on bdw is only
> > > for
> > > the _CTL/_AUX register, and that _IIR/IMR stayed where they have
> > > been
> > > on bdw.
> > >
> > > v2: Fixes from Ville.
> > >
> > > v3: From DK
> > > * Rebased on drm-tip
> > > * Removed BDW IIR bit definition, looks like an unintentional
> > > change
> > > that
> > > should be in the following patch.
> > >
> > > v4: From DK
> > > * Don't mask REG_WRITE.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> >
> > With bspec id and comment about why are you masking interruptions
> > in
> > hsw_edp_psr_irq_handler() feel free to add:
> >
> > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> > > >
> > > ---
> > > drivers/gpu/drm/i915/i915_irq.c | 34
> > > ++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> > > 2 files changed, 42 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index 27aee25429b7..c2d3f30778ee 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > > ironlake_rps_change_irq_handler(dev_priv);
> > > }
> > >
> > > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > > +
> > > + if (edp_psr_iir & EDP_PSR_ERROR)
> > > + DRM_DEBUG_KMS("PSR error\n");
> > > +
> > > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > > + DRM_DEBUG_KMS("PSR prepare entry in 2
> > > vblanks\n");
> > > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> >
> > Just to know... you need to mask this one otherwise it will keep
> > triggering EDP_PSR_PRE_ENTRY interruptions? Would be nice to have a
> > comment explaning why you are masking it.
> >
>
> The final implementation in patch 3/4 doesn't do that. Adding a
> comment
> here and removing will be pointless IMO.
Okay
>
> > > + }
> > > +
> > > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > > + DRM_DEBUG_KMS("PSR exit completed\n");
> > > + I915_WRITE(EDP_PSR_IMR, 0);
> > > + }
> > > +
> > > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > > +}
> > > +
> > > static void ivb_display_irq_handler(struct drm_i915_private
> > > *dev_priv,
> > > u32 de_iir)
> > > {
> > > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > > if (de_iir & DE_ERR_INT_IVB)
> > > ivb_err_int_handler(dev_priv);
> > >
> > > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > > + hsw_edp_psr_irq_handler(dev_priv);
> > > +
> > > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > > dp_aux_irq_handler(dev_priv);
> > >
> > > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > > drm_device *dev)
> > > if (IS_GEN7(dev_priv))
> > > I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> > >
> > > + if (IS_HASWELL(dev_priv)) {
> > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> >
> > No need to do a POSTING_READ(EDP_PSR_IMR);? Like is done in:
> > GEN3_IRQ_RESET()
> >
>
> We should be fine without that. From what I was told a while ago,
> some
> of these POSTING_READS are cargo culted.
Ack
>
> > > + }
> > > +
> > > gen5_gt_irq_reset(dev_priv);
> > >
> > > ibx_irq_reset(dev_priv);
> > > @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> > > drm_device *dev)
> > > DE_DP_A_HOTPLUG);
> > > }
> > >
> > > + if (IS_HASWELL(dev_priv)) {
> > > + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > > + I915_WRITE(EDP_PSR_IMR, 0);
> > > + display_mask |= DE_EDP_PSR_INT_HSW;
> > > + }
> > > +
> > > dev_priv->irq_mask = ~display_mask;
> > >
> > > ibx_irq_pre_postinstall(dev);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 176dca6554f4..f5783d6db614 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4011,6 +4011,13 @@ enum {
> > > #define EDP_PSR_TP1_TIME_0us (3<<4)
> > > #define EDP_PSR_IDLE_FRAME_SHIFT 0
> > >
> > > +/* Bspec claims those aren't shifted but stay at 0x64800 */
> > > +#define EDP_PSR_IMR _MMIO(0x64834
> > > )
> > > +#define EDP_PSR_IIR _MMIO(0x64838
> > > )
> > > +#define EDP_PSR_ERROR (1<<2)
> > > +#define EDP_PSR_POST_EXIT (1<<1)
> > > +#define EDP_PSR_PRE_ENTRY (1<<0)
> >
> > Could you add the bspec id of where did you got this?
> > The hsw spec that I'm reading don't have the bits, skl have but
> > don't
> > have the bits of the other transcoders.
> >
>
> I understand it is not easy to find, but are we going to add bspec
> references for all new register definitions? :) From what I have
> seen, a
> bspec reference is typically added only for workarounds. I'll update
> this patch though since I've waited too long to get this patch in.
> Would
> adding the bspec reference in the commit message suffice?
Yeah in the commit message please.
>
>
> > > +
> > > #define EDP_PSR_AUX_CTL _MMIO(dev
> > > _pri
> > > v->psr_mmio_base + 0x10)
> > > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> > > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
> > > @@ -6820,6 +6827,7 @@ enum {
> > > #define DE_PCH_EVENT_IVB (1<<28)
> > > #define DE_DP_A_HOTPLUG_IVB (1<<27)
> > > #define DE_AUX_CHANNEL_A_IVB (1<<26)
> > > +#define DE_EDP_PSR_INT_HSW (1<<19)
> > > #define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
> > > #define DE_PLANEC_FLIP_DONE_IVB (1<<13)
> > > #define DE_PIPEC_VBLANK_IVB (1<<10)
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-05 21:44 ` Souza, Jose
@ 2018-04-05 22:00 ` Dhinakaran Pandiyan
2018-04-17 0:43 ` Paulo Zanoni
0 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-05 22:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Dhinakaran Pandiyan, Rodrigo Vivi, Daniel Vetter
From: Daniel Vetter <daniel.vetter@ffwll.ch>
The definitions for the error register should be valid on bdw/skl too,
but there we haven't even enabled DE_MISC handling yet.
Somewhat confusing the the moved register offset on bdw is only for
the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
on bdw.
v2: Fixes from Ville.
v3: From DK
* Rebased on drm-tip
* Removed BDW IIR bit definition, looks like an unintentional change that
should be in the following patch.
v4: From DK
* Don't mask REG_WRITE.
References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 34 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 27aee25429b7..c2d3f30778ee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
ironlake_rps_change_irq_handler(dev_priv);
}
+static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
+{
+ u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+
+ if (edp_psr_iir & EDP_PSR_ERROR)
+ DRM_DEBUG_KMS("PSR error\n");
+
+ if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
+ DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
+ I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
+ }
+
+ if (edp_psr_iir & EDP_PSR_POST_EXIT) {
+ DRM_DEBUG_KMS("PSR exit completed\n");
+ I915_WRITE(EDP_PSR_IMR, 0);
+ }
+
+ I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
+}
+
static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
u32 de_iir)
{
@@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev_priv);
+ if (de_iir & DE_EDP_PSR_INT_HSW)
+ hsw_edp_psr_irq_handler(dev_priv);
+
if (de_iir & DE_AUX_CHANNEL_A_IVB)
dp_aux_irq_handler(dev_priv);
@@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
if (IS_GEN7(dev_priv))
I915_WRITE(GEN7_ERR_INT, 0xffffffff);
+ if (IS_HASWELL(dev_priv)) {
+ I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+ I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+ }
+
gen5_gt_irq_reset(dev_priv);
ibx_irq_reset(dev_priv);
@@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
DE_DP_A_HOTPLUG);
}
+ if (IS_HASWELL(dev_priv)) {
+ gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+ I915_WRITE(EDP_PSR_IMR, 0);
+ display_mask |= DE_EDP_PSR_INT_HSW;
+ }
+
dev_priv->irq_mask = ~display_mask;
ibx_irq_pre_postinstall(dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 176dca6554f4..f5783d6db614 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4011,6 +4011,13 @@ enum {
#define EDP_PSR_TP1_TIME_0us (3<<4)
#define EDP_PSR_IDLE_FRAME_SHIFT 0
+/* Bspec claims those aren't shifted but stay at 0x64800 */
+#define EDP_PSR_IMR _MMIO(0x64834)
+#define EDP_PSR_IIR _MMIO(0x64838)
+#define EDP_PSR_ERROR (1<<2)
+#define EDP_PSR_POST_EXIT (1<<1)
+#define EDP_PSR_PRE_ENTRY (1<<0)
+
#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10)
#define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
#define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
@@ -6820,6 +6827,7 @@ enum {
#define DE_PCH_EVENT_IVB (1<<28)
#define DE_DP_A_HOTPLUG_IVB (1<<27)
#define DE_AUX_CHANNEL_A_IVB (1<<26)
+#define DE_EDP_PSR_INT_HSW (1<<19)
#define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
#define DE_PLANEC_FLIP_DONE_IVB (1<<13)
#define DE_PIPEC_VBLANK_IVB (1<<10)
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (8 preceding siblings ...)
2018-04-05 20:40 ` [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Souza, Jose
@ 2018-04-05 22:34 ` Patchwork
2018-04-05 22:51 ` ✓ Fi.CI.BAT: success " Patchwork
` (4 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-05 22:34 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
6494853a48be drm/i915: Enable edp psr error interrupts on hsw
-:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#111: FILE: drivers/gpu/drm/i915/i915_reg.h:4017:
+#define EDP_PSR_ERROR (1<<2)
^
-:112: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#112: FILE: drivers/gpu/drm/i915/i915_reg.h:4018:
+#define EDP_PSR_POST_EXIT (1<<1)
^
-:113: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#113: FILE: drivers/gpu/drm/i915/i915_reg.h:4019:
+#define EDP_PSR_PRE_ENTRY (1<<0)
^
-:122: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#122: FILE: drivers/gpu/drm/i915/i915_reg.h:6830:
+#define DE_EDP_PSR_INT_HSW (1<<19)
^
total: 0 errors, 0 warnings, 4 checks, 78 lines checked
536a6705099e drm/i915: Enable edp psr error interrupts on bdw+
-:159: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:159: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
#159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:160: CHECK:SPACING: No space is necessary after a cast
#160: FILE: drivers/gpu/drm/i915/intel_display.h:222:
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
-:161: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#161: FILE: drivers/gpu/drm/i915/intel_display.h:223:
+ for_each_if ((__mask) & (1 << (__t)))
total: 1 errors, 1 warnings, 2 checks, 123 lines checked
8bfb9d8526bd drm/i915/psr: Control PSR interrupts via debugfs
443c59203be8 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (9 preceding siblings ...)
2018-04-05 22:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3) Patchwork
@ 2018-04-05 22:51 ` Patchwork
2018-04-06 1:50 ` ✗ Fi.CI.IGT: warning " Patchwork
` (3 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-05 22:51 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : success
== Summary ==
Series 41095v3 series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw
https://patchwork.freedesktop.org/api/1.0/series/41095/revisions/3/mbox/
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (fi-cnl-y3) fdo#104951
Test prime_vgem:
Subgroup basic-fence-flip:
fail -> PASS (fi-ilk-650) fdo#104008
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:429s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:450s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:384s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:516s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:511s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:508s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:566s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:582s
fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:422s
fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:316s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:540s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:402s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:421s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:471s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:433s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:472s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:460s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:512s
fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:669s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:441s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:534s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:513s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:494s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:432s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:447s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:562s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:411s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:522s
fcaf73c13c14d6bfd64c4f37089bf5437fb32221 drm-tip: 2018y-04m-05d-15h-53m-18s UTC integration manifest
443c59203be8 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
8bfb9d8526bd drm/i915/psr: Control PSR interrupts via debugfs
536a6705099e drm/i915: Enable edp psr error interrupts on bdw+
6494853a48be drm/i915: Enable edp psr error interrupts on hsw
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8609/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✗ Fi.CI.IGT: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (10 preceding siblings ...)
2018-04-05 22:51 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-06 1:50 ` Patchwork
2018-04-10 0:49 ` ✗ Fi.CI.CHECKPATCH: " Patchwork
` (2 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-06 1:50 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : warning
== Summary ==
---- Possible new issues:
Test kms_flip:
Subgroup 2x-plain-flip:
pass -> DMESG-WARN (shard-hsw)
Test kms_frontbuffer_tracking:
Subgroup fbcpsr-1p-primscrn-shrfb-msflip-blt:
fail -> SKIP (shard-snb)
---- Known issues:
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-varying-size:
pass -> FAIL (shard-hsw) fdo#102670
Test kms_flip:
Subgroup 2x-flip-vs-expired-vblank:
fail -> PASS (shard-hsw) fdo#102887
Subgroup plain-flip-ts-check:
fail -> PASS (shard-hsw) fdo#100368 +1
Test kms_plane_multiple:
Subgroup atomic-pipe-a-tiling-x:
fail -> PASS (shard-snb) fdo#103166
Test kms_rotation_crc:
Subgroup primary-rotation-180:
fail -> PASS (shard-snb) fdo#103925
Test kms_setmode:
Subgroup basic:
pass -> FAIL (shard-apl) fdo#99912
Test kms_vblank:
Subgroup pipe-b-accuracy-idle:
pass -> FAIL (shard-hsw) fdo#102583
Test perf:
Subgroup blocking:
fail -> PASS (shard-hsw) fdo#102252
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-apl total:2680 pass:1835 dwarn:1 dfail:0 fail:7 skip:836 time:12628s
shard-hsw total:2680 pass:1782 dwarn:2 dfail:0 fail:4 skip:891 time:11444s
shard-snb total:2680 pass:1378 dwarn:1 dfail:0 fail:2 skip:1299 time:6949s
Blacklisted hosts:
shard-kbl total:1955 pass:1414 dwarn:0 dfail:0 fail:6 skip:534 time:6174s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8609/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (11 preceding siblings ...)
2018-04-06 1:50 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-04-10 0:49 ` Patchwork
[not found] ` <20180410175934.GA2303@intel.com>
2018-04-10 1:05 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-10 2:25 ` ✓ Fi.CI.IGT: " Patchwork
14 siblings, 1 reply; 32+ messages in thread
From: Patchwork @ 2018-04-10 0:49 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
0a22dbbae8f8 drm/i915: Enable edp psr error interrupts on hsw
-:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#111: FILE: drivers/gpu/drm/i915/i915_reg.h:4032:
+#define EDP_PSR_ERROR (1<<2)
^
-:112: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#112: FILE: drivers/gpu/drm/i915/i915_reg.h:4033:
+#define EDP_PSR_POST_EXIT (1<<1)
^
-:113: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#113: FILE: drivers/gpu/drm/i915/i915_reg.h:4034:
+#define EDP_PSR_PRE_ENTRY (1<<0)
^
-:122: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#122: FILE: drivers/gpu/drm/i915/i915_reg.h:6847:
+#define DE_EDP_PSR_INT_HSW (1<<19)
^
total: 0 errors, 0 warnings, 4 checks, 78 lines checked
7fdf2eed9ed4 drm/i915: Enable edp psr error interrupts on bdw+
-:159: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:159: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
#159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
+ for_each_if ((__mask) & (1 << (__t)))
-:160: CHECK:SPACING: No space is necessary after a cast
#160: FILE: drivers/gpu/drm/i915/intel_display.h:222:
+ for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
-:161: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#161: FILE: drivers/gpu/drm/i915/intel_display.h:223:
+ for_each_if ((__mask) & (1 << (__t)))
total: 1 errors, 1 warnings, 2 checks, 123 lines checked
003ec0005027 drm/i915/psr: Control PSR interrupts via debugfs
d8186b823b62 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (12 preceding siblings ...)
2018-04-10 0:49 ` ✗ Fi.CI.CHECKPATCH: " Patchwork
@ 2018-04-10 1:05 ` Patchwork
2018-04-10 2:25 ` ✓ Fi.CI.IGT: " Patchwork
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-10 1:05 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : success
== Summary ==
Series 41095v3 series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw
https://patchwork.freedesktop.org/api/1.0/series/41095/revisions/3/mbox/
---- Known issues:
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (fi-glk-j4005) fdo#103359
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (fi-glk-j4005) fdo#100368
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:432s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:445s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:382s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:543s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:519s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:519s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:522s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:509s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:559s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:510s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:584s
fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:424s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:543s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:406s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:471s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:443s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:477s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:465s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:511s
fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:671s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:444s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:534s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:508s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:511s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:427s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:443s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:586s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:399s
617cdf0bd4fd2cb0dcc64ddf07fbb56572ba800a drm-tip: 2018y-04m-09d-19h-55m-54s UTC integration manifest
d8186b823b62 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
003ec0005027 drm/i915/psr: Control PSR interrupts via debugfs
7fdf2eed9ed4 drm/i915: Enable edp psr error interrupts on bdw+
0a22dbbae8f8 drm/i915: Enable edp psr error interrupts on hsw
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8648/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (13 preceding siblings ...)
2018-04-10 1:05 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-10 2:25 ` Patchwork
14 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2018-04-10 2:25 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
URL : https://patchwork.freedesktop.org/series/41095/
State : success
== Summary ==
---- Possible new issues:
Test kms_cursor_legacy:
Subgroup pipe-c-torture-move:
dmesg-warn -> PASS (shard-hsw)
Test kms_plane:
Subgroup pixel-format-pipe-b-planes:
dmesg-warn -> PASS (shard-hsw)
---- Known issues:
Test kms_flip:
Subgroup 2x-plain-flip-ts-check:
pass -> FAIL (shard-hsw) fdo#100368
Subgroup dpms-vs-vblank-race:
fail -> PASS (shard-hsw) fdo#103060
Test kms_rotation_crc:
Subgroup sprite-rotation-270:
pass -> FAIL (shard-apl) fdo#103356
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103356 https://bugs.freedesktop.org/show_bug.cgi?id=103356
shard-apl total:2680 pass:1834 dwarn:1 dfail:0 fail:8 skip:836 time:12726s
shard-hsw total:2680 pass:1785 dwarn:1 dfail:0 fail:2 skip:891 time:11285s
Blacklisted hosts:
shard-kbl total:2680 pass:1963 dwarn:1 dfail:0 fail:6 skip:710 time:9201s
shard-snb total:2680 pass:1377 dwarn:1 dfail:0 fail:3 skip:1299 time:6884s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8648/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
[not found] ` <20180410175934.GA2303@intel.com>
@ 2018-04-10 18:29 ` Dhinakaran Pandiyan
2018-04-20 21:33 ` Rodrigo Vivi
0 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-10 18:29 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, 2018-04-10 at 10:59 -0700, Rodrigo Vivi wrote:
> On Tue, Apr 10, 2018 at 12:49:25AM -0000, Patchwork wrote:
> > == Series Details ==
> >
> > Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
> > URL : https://patchwork.freedesktop.org/series/41095/
> > State : warning
> >
> > == Summary ==
> >
> > $ dim checkpatch origin/drm-tip
> > 0a22dbbae8f8 drm/i915: Enable edp psr error interrupts on hsw
> > -:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > #111: FILE: drivers/gpu/drm/i915/i915_reg.h:4032:
> > +#define EDP_PSR_ERROR (1<<2)
> > ^
> >
> > -:112: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > #112: FILE: drivers/gpu/drm/i915/i915_reg.h:4033:
> > +#define EDP_PSR_POST_EXIT (1<<1)
> > ^
> >
> > -:113: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > #113: FILE: drivers/gpu/drm/i915/i915_reg.h:4034:
> > +#define EDP_PSR_PRE_ENTRY (1<<0)
> > ^
> >
> > -:122: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > #122: FILE: drivers/gpu/drm/i915/i915_reg.h:6847:
> > +#define DE_EDP_PSR_INT_HSW (1<<19)
> > ^
> >
> > total: 0 errors, 0 warnings, 4 checks, 78 lines checked
> > 7fdf2eed9ed4 drm/i915: Enable edp psr error interrupts on bdw+
> > -:159: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
> > #159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
> > +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> > + for_each_if ((__mask) & (1 << (__t)))
>
> This showed up on red on dim when I was going to push here...
>
> DK, could you please address this one here before we can push?
>
The macros look correct to me, that is how other macros are written too.
check_patch is confused?
> Thanks,
> Rodrigo.
>
> >
> > -:159: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
> > #159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
> > +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> > + for_each_if ((__mask) & (1 << (__t)))
> >
> > -:160: CHECK:SPACING: No space is necessary after a cast
> > #160: FILE: drivers/gpu/drm/i915/intel_display.h:222:
> > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> >
> > -:161: WARNING:SPACING: space prohibited between function name and open parenthesis '('
> > #161: FILE: drivers/gpu/drm/i915/intel_display.h:223:
> > + for_each_if ((__mask) & (1 << (__t)))
> >
> > total: 1 errors, 1 warnings, 2 checks, 123 lines checked
> > 003ec0005027 drm/i915/psr: Control PSR interrupts via debugfs
> > d8186b823b62 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
> >
> > _______________________________________________
> > 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] 32+ messages in thread
* Re: [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-05 22:00 ` [PATCH v4] " Dhinakaran Pandiyan
@ 2018-04-17 0:43 ` Paulo Zanoni
2018-04-17 17:41 ` Ville Syrjälä
0 siblings, 1 reply; 32+ messages in thread
From: Paulo Zanoni @ 2018-04-17 0:43 UTC (permalink / raw)
To: Dhinakaran Pandiyan, intel-gfx; +Cc: Daniel Vetter, Daniel Vetter, Rodrigo Vivi
Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> The definitions for the error register should be valid on bdw/skl
> too,
> but there we haven't even enabled DE_MISC handling yet.
>
> Somewhat confusing the the moved register offset on bdw is only for
> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> on bdw.
>
> v2: Fixes from Ville.
>
> v3: From DK
> * Rebased on drm-tip
> * Removed BDW IIR bit definition, looks like an unintentional change
> that
> should be in the following patch.
>
> v4: From DK
> * Don't mask REG_WRITE.
>
> References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 34
> ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 27aee25429b7..c2d3f30778ee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
> ironlake_rps_change_irq_handler(dev_priv);
> }
>
> +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> +{
> + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + if (edp_psr_iir & EDP_PSR_ERROR)
> + DRM_DEBUG_KMS("PSR error\n");
> +
> + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
Why are we masking it here? During these 2 vblanks it's possible that
something will happen (e.g., frontbuffer writing, cursor moving, page
flipping). Are we guaranteed to get a POST_EXIT interrupt even if we
give up entering PSR before it is actually entered?
> + }
> +
> + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> + DRM_DEBUG_KMS("PSR exit completed\n");
> + I915_WRITE(EDP_PSR_IMR, 0);
> + }
> +
> + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> +}
> +
> static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
> u32 de_iir)
> {
> @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev_priv);
>
> + if (de_iir & DE_EDP_PSR_INT_HSW)
> + hsw_edp_psr_irq_handler(dev_priv);
> +
> if (de_iir & DE_AUX_CHANNEL_A_IVB)
> dp_aux_irq_handler(dev_priv);
>
> @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> drm_device *dev)
> if (IS_GEN7(dev_priv))
> I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>
> + if (IS_HASWELL(dev_priv)) {
> + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> + }
We need another IIR write we do for the other platforms. This is not
cargo cult (as mentioned in previous review emails), this is required
since our hardware is able to store more than one IIR interrupt. Please
do it like we do for the other interrupts.
Do git-blame in the relevant lines and read the commit messages for
more information on why stuff is the way it is. If you still think this
is unnecessary, feel free to write a patch removing the unnecessary
stuff for every interrupt instead of of making just one register be
not-cargo-culted.
> +
> gen5_gt_irq_reset(dev_priv);
>
> ibx_irq_reset(dev_priv);
> @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
> DE_DP_A_HOTPLUG);
> }
>
> + if (IS_HASWELL(dev_priv)) {
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IMR, 0);
> + display_mask |= DE_EDP_PSR_INT_HSW;
> + }
> +
> dev_priv->irq_mask = ~display_mask;
>
> ibx_irq_pre_postinstall(dev);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 176dca6554f4..f5783d6db614 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4011,6 +4011,13 @@ enum {
> #define EDP_PSR_TP1_TIME_0us (3<<4)
> #define EDP_PSR_IDLE_FRAME_SHIFT 0
>
> +/* Bspec claims those aren't shifted but stay at 0x64800 */
> +#define EDP_PSR_IMR _MMIO(0x64834)
> +#define EDP_PSR_IIR _MMIO(0x64838)
> +#define EDP_PSR_ERROR (1<<2)
> +#define EDP_PSR_POST_EXIT (1<<1)
> +#define EDP_PSR_PRE_ENTRY (1<<0)
> +
I know you'll remove these definitions in the next patch, but please
make sure you add the definitions following the new standard described
at the top of the file: (1 << 1) instead of (1<<1).
> #define EDP_PSR_AUX_CTL _MMIO(dev_pri
> v->psr_mmio_base + 0x10)
> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
> @@ -6820,6 +6827,7 @@ enum {
> #define DE_PCH_EVENT_IVB (1<<28)
> #define DE_DP_A_HOTPLUG_IVB (1<<27)
> #define DE_AUX_CHANNEL_A_IVB (1<<26)
> +#define DE_EDP_PSR_INT_HSW (1<<19)
For this one you're allowed to not put the spaces since the other bits
are like this. But you also have the option of drive-by fixing the bits
of this register if you want since you're touching it.
> #define DE_SPRITEC_FLIP_DONE_IVB (1<<14)
> #define DE_PLANEC_FLIP_DONE_IVB (1<<13)
> #define DE_PIPEC_VBLANK_IVB (1<<10)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-17 0:43 ` Paulo Zanoni
@ 2018-04-17 17:41 ` Ville Syrjälä
2018-04-17 20:01 ` Dhinakaran Pandiyan
0 siblings, 1 reply; 32+ messages in thread
From: Ville Syrjälä @ 2018-04-17 17:41 UTC (permalink / raw)
To: Paulo Zanoni
Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan,
Daniel Vetter
On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > The definitions for the error register should be valid on bdw/skl
> > too,
> > but there we haven't even enabled DE_MISC handling yet.
> >
> > Somewhat confusing the the moved register offset on bdw is only for
> > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > on bdw.
> >
> > v2: Fixes from Ville.
> >
> > v3: From DK
> > * Rebased on drm-tip
> > * Removed BDW IIR bit definition, looks like an unintentional change
> > that
> > should be in the following patch.
> >
> > v4: From DK
> > * Don't mask REG_WRITE.
> >
> > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 34
> > ++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 27aee25429b7..c2d3f30778ee 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > ironlake_rps_change_irq_handler(dev_priv);
> > }
> >
> > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > *dev_priv)
> > +{
> > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > + if (edp_psr_iir & EDP_PSR_ERROR)
> > + DRM_DEBUG_KMS("PSR error\n");
> > +
> > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
>
> Why are we masking it here? During these 2 vblanks it's possible that
> something will happen (e.g., frontbuffer writing, cursor moving, page
> flipping).
The masking was here to avoid seeing a big flood of entry interrupts.
> Are we guaranteed to get a POST_EXIT interrupt even if we
> give up entering PSR before it is actually entered?
No. The exit interrupt only happens if you actually reached PSR.
>
>
> > + }
> > +
> > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > + DRM_DEBUG_KMS("PSR exit completed\n");
> > + I915_WRITE(EDP_PSR_IMR, 0);
> > + }
> > +
> > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > +}
> > +
> > static void ivb_display_irq_handler(struct drm_i915_private
> > *dev_priv,
> > u32 de_iir)
> > {
> > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > if (de_iir & DE_ERR_INT_IVB)
> > ivb_err_int_handler(dev_priv);
> >
> > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > + hsw_edp_psr_irq_handler(dev_priv);
> > +
> > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > dp_aux_irq_handler(dev_priv);
> >
> > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > drm_device *dev)
> > if (IS_GEN7(dev_priv))
> > I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> >
> > + if (IS_HASWELL(dev_priv)) {
> > + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> > + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> > + }
>
> We need another IIR write we do for the other platforms. This is not
> cargo cult (as mentioned in previous review emails), this is required
> since our hardware is able to store more than one IIR interrupt. Please
> do it like we do for the other interrupts.
I don't remember if the PSR IIR is double buffered or not. I would
assumeme it is. It is super easy to verify though if you want to be
sure:
1. unmask the interrupt
2. generate at least two interrupts
3. mask the interrupt
4. clear IIR
5. read IIR
6. clear IIR
7. read IIR
Step 5 should still show the IIR bit set if the IIR is double buffered.
And step 7 it should show it to be clear.
--
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] 32+ messages in thread
* Re: [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-17 17:41 ` Ville Syrjälä
@ 2018-04-17 20:01 ` Dhinakaran Pandiyan
2018-04-20 22:22 ` Rodrigo Vivi
0 siblings, 1 reply; 32+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-17 20:01 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi, Paulo Zanoni, Daniel Vetter
On Tue, 2018-04-17 at 20:41 +0300, Ville Syrjälä wrote:
> On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> > Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >
> > > The definitions for the error register should be valid on bdw/skl
> > > too,
> > > but there we haven't even enabled DE_MISC handling yet.
> > >
> > > Somewhat confusing the the moved register offset on bdw is only for
> > > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > > on bdw.
> > >
> > > v2: Fixes from Ville.
> > >
> > > v3: From DK
> > > * Rebased on drm-tip
> > > * Removed BDW IIR bit definition, looks like an unintentional change
> > > that
> > > should be in the following patch.
> > >
> > > v4: From DK
> > > * Don't mask REG_WRITE.
> > >
> > > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_irq.c | 34
> > > ++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> > > 2 files changed, 42 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index 27aee25429b7..c2d3f30778ee 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > > ironlake_rps_change_irq_handler(dev_priv);
> > > }
> > >
> > > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > > +
> > > + if (edp_psr_iir & EDP_PSR_ERROR)
> > > + DRM_DEBUG_KMS("PSR error\n");
> > > +
> > > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > > + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> >
> > Why are we masking it here? During these 2 vblanks it's possible that
> > something will happen (e.g., frontbuffer writing, cursor moving, page
> > flipping).
>
> The masking was here to avoid seeing a big flood of entry interrupts.
>
> > Are we guaranteed to get a POST_EXIT interrupt even if we
> > give up entering PSR before it is actually entered?
>
> No. The exit interrupt only happens if you actually reached PSR.
>
> >
> >
> > > + }
> > > +
> > > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > > + DRM_DEBUG_KMS("PSR exit completed\n");
> > > + I915_WRITE(EDP_PSR_IMR, 0);
> > > + }
> > > +
> > > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > > +}
> > > +
> > > static void ivb_display_irq_handler(struct drm_i915_private
> > > *dev_priv,
> > > u32 de_iir)
> > > {
> > > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > > if (de_iir & DE_ERR_INT_IVB)
> > > ivb_err_int_handler(dev_priv);
> > >
> > > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > > + hsw_edp_psr_irq_handler(dev_priv);
> > > +
> > > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > > dp_aux_irq_handler(dev_priv);
> > >
> > > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > > drm_device *dev)
> > > if (IS_GEN7(dev_priv))
> > > I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> > >
> > > + if (IS_HASWELL(dev_priv)) {
> > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> > > + }
> >
> > We need another IIR write we do for the other platforms. This is not
> > cargo cult (as mentioned in previous review emails), this is required
> > since our hardware is able to store more than one IIR interrupt. Please
> > do it like we do for the other interrupts.
>
My reply and the review were specifically about the POSTING_READ(). I
agree on the second IIR write, but that was not what the original review
comment was about.
I'm still skeptical about inserting the POSTING_READ() between write's
There are at commit messages and emails that indicate the
POSTING_READS() are indeed cargo culted. Let me find more data about
this and get back.
> I don't remember if the PSR IIR is double buffered or not. I would
> assumeme it is. It is super easy to verify though if you want to be
> sure:
> 1. unmask the interrupt
> 2. generate at least two interrupts
> 3. mask the interrupt
> 4. clear IIR
> 5. read IIR
> 6. clear IIR
> 7. read IIR
>
> Step 5 should still show the IIR bit set if the IIR is double buffered.
> And step 7 it should show it to be clear.
>
I'll give it a try, thanks!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
2018-04-10 18:29 ` Dhinakaran Pandiyan
@ 2018-04-20 21:33 ` Rodrigo Vivi
0 siblings, 0 replies; 32+ messages in thread
From: Rodrigo Vivi @ 2018-04-20 21:33 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
On Tue, Apr 10, 2018 at 11:29:09AM -0700, Dhinakaran Pandiyan wrote:
>
>
>
> On Tue, 2018-04-10 at 10:59 -0700, Rodrigo Vivi wrote:
> > On Tue, Apr 10, 2018 at 12:49:25AM -0000, Patchwork wrote:
> > > == Series Details ==
> > >
> > > Series: series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3)
> > > URL : https://patchwork.freedesktop.org/series/41095/
> > > State : warning
> > >
> > > == Summary ==
> > >
> > > $ dim checkpatch origin/drm-tip
> > > 0a22dbbae8f8 drm/i915: Enable edp psr error interrupts on hsw
> > > -:111: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > > #111: FILE: drivers/gpu/drm/i915/i915_reg.h:4032:
> > > +#define EDP_PSR_ERROR (1<<2)
> > > ^
> > >
> > > -:112: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > > #112: FILE: drivers/gpu/drm/i915/i915_reg.h:4033:
> > > +#define EDP_PSR_POST_EXIT (1<<1)
> > > ^
> > >
> > > -:113: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > > #113: FILE: drivers/gpu/drm/i915/i915_reg.h:4034:
> > > +#define EDP_PSR_PRE_ENTRY (1<<0)
> > > ^
> > >
> > > -:122: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
> > > #122: FILE: drivers/gpu/drm/i915/i915_reg.h:6847:
> > > +#define DE_EDP_PSR_INT_HSW (1<<19)
> > > ^
> > >
> > > total: 0 errors, 0 warnings, 4 checks, 78 lines checked
> > > 7fdf2eed9ed4 drm/i915: Enable edp psr error interrupts on bdw+
> > > -:159: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
> > > #159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
> > > +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> > > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> > > + for_each_if ((__mask) & (1 << (__t)))
> >
> > This showed up on red on dim when I was going to push here...
> >
> > DK, could you please address this one here before we can push?
> >
>
> The macros look correct to me, that is how other macros are written too.
> check_patch is confused?
Well, you are right. New macro is just like the other for_each_*
If we need a clean we can do that later, or silent dim on that...
Anyways, pushed this patches to dinq.
Thanks for patches, reviews, patience and extra checks ;)
>
> > Thanks,
> > Rodrigo.
> >
> > >
> > > -:159: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__t' - possible side-effects?
> > > #159: FILE: drivers/gpu/drm/i915/intel_display.h:221:
> > > +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> > > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> > > + for_each_if ((__mask) & (1 << (__t)))
> > >
> > > -:160: CHECK:SPACING: No space is necessary after a cast
> > > #160: FILE: drivers/gpu/drm/i915/intel_display.h:222:
> > > + for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++) \
> > >
> > > -:161: WARNING:SPACING: space prohibited between function name and open parenthesis '('
> > > #161: FILE: drivers/gpu/drm/i915/intel_display.h:223:
> > > + for_each_if ((__mask) & (1 << (__t)))
> > >
> > > total: 1 errors, 1 warnings, 2 checks, 123 lines checked
> > > 003ec0005027 drm/i915/psr: Control PSR interrupts via debugfs
> > > d8186b823b62 drm/i915/psr: Timestamps for PSR entry and exit interrupts.
> > >
> > > _______________________________________________
> > > 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] 32+ messages in thread
* Re: [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw
2018-04-17 20:01 ` Dhinakaran Pandiyan
@ 2018-04-20 22:22 ` Rodrigo Vivi
0 siblings, 0 replies; 32+ messages in thread
From: Rodrigo Vivi @ 2018-04-20 22:22 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: Daniel Vetter, intel-gfx, Paulo Zanoni, Daniel Vetter
On Tue, Apr 17, 2018 at 01:01:39PM -0700, Dhinakaran Pandiyan wrote:
>
>
>
> On Tue, 2018-04-17 at 20:41 +0300, Ville Syrjälä wrote:
> > On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> > > Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > > > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > >
> > > > The definitions for the error register should be valid on bdw/skl
> > > > too,
> > > > but there we haven't even enabled DE_MISC handling yet.
> > > >
> > > > Somewhat confusing the the moved register offset on bdw is only for
> > > > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > > > on bdw.
> > > >
> > > > v2: Fixes from Ville.
> > > >
> > > > v3: From DK
> > > > * Rebased on drm-tip
> > > > * Removed BDW IIR bit definition, looks like an unintentional change
> > > > that
> > > > should be in the following patch.
> > > >
> > > > v4: From DK
> > > > * Don't mask REG_WRITE.
> > > >
> > > > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_irq.c | 34
> > > > ++++++++++++++++++++++++++++++++++
> > > > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> > > > 2 files changed, 42 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > index 27aee25429b7..c2d3f30778ee 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > > > drm_i915_private *dev_priv,
> > > > ironlake_rps_change_irq_handler(dev_priv);
> > > > }
> > > >
> > > > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > > > *dev_priv)
> > > > +{
> > > > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > > > +
> > > > + if (edp_psr_iir & EDP_PSR_ERROR)
> > > > + DRM_DEBUG_KMS("PSR error\n");
> > > > +
> > > > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > > > + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > > > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> > >
> > > Why are we masking it here? During these 2 vblanks it's possible that
> > > something will happen (e.g., frontbuffer writing, cursor moving, page
> > > flipping).
> >
> > The masking was here to avoid seeing a big flood of entry interrupts.
> >
> > > Are we guaranteed to get a POST_EXIT interrupt even if we
> > > give up entering PSR before it is actually entered?
> >
> > No. The exit interrupt only happens if you actually reached PSR.
> >
> > >
> > >
> > > > + }
> > > > +
> > > > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > > > + DRM_DEBUG_KMS("PSR exit completed\n");
> > > > + I915_WRITE(EDP_PSR_IMR, 0);
> > > > + }
> > > > +
> > > > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > > > +}
> > > > +
> > > > static void ivb_display_irq_handler(struct drm_i915_private
> > > > *dev_priv,
> > > > u32 de_iir)
> > > > {
> > > > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > > > drm_i915_private *dev_priv,
> > > > if (de_iir & DE_ERR_INT_IVB)
> > > > ivb_err_int_handler(dev_priv);
> > > >
> > > > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > > > + hsw_edp_psr_irq_handler(dev_priv);
> > > > +
> > > > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > > > dp_aux_irq_handler(dev_priv);
> > > >
> > > > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > > > drm_device *dev)
> > > > if (IS_GEN7(dev_priv))
> > > > I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> > > >
> > > > + if (IS_HASWELL(dev_priv)) {
> > > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> > > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> > > > + }
> > >
> > > We need another IIR write we do for the other platforms. This is not
> > > cargo cult (as mentioned in previous review emails), this is required
> > > since our hardware is able to store more than one IIR interrupt. Please
> > > do it like we do for the other interrupts.
> >
>
> My reply and the review were specifically about the POSTING_READ(). I
> agree on the second IIR write, but that was not what the original review
> comment was about.
>
> I'm still skeptical about inserting the POSTING_READ() between write's
> There are at commit messages and emails that indicate the
> POSTING_READS() are indeed cargo culted. Let me find more data about
> this and get back.
>
> > I don't remember if the PSR IIR is double buffered or not. I would
> > assumeme it is. It is super easy to verify though if you want to be
> > sure:
> > 1. unmask the interrupt
> > 2. generate at least two interrupts
> > 3. mask the interrupt
> > 4. clear IIR
> > 5. read IIR
> > 6. clear IIR
> > 7. read IIR
> >
> > Step 5 should still show the IIR bit set if the IIR is double buffered.
> > And step 7 it should show it to be clear.
> >
>
> I'll give it a try, thanks!
I'm far behind this week and merging the series was on top of my todo list...
:( sorry for that.
I know that you guys already talked about it and know how to do the follow-up ;)
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2018-04-20 22:22 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 21:24 [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-04-03 21:24 ` [PATCH v3 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
2018-04-05 20:38 ` Souza, Jose
2018-04-05 21:25 ` Pandiyan, Dhinakaran
2018-04-03 21:24 ` [PATCH v3 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
2018-04-04 0:54 ` Souza, Jose
2018-04-04 23:58 ` Pandiyan, Dhinakaran
2018-04-05 1:37 ` [PATCH v4 " Dhinakaran Pandiyan
2018-04-05 19:42 ` Souza, Jose
2018-04-03 21:24 ` [PATCH v3 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
2018-04-04 0:56 ` Souza, Jose
2018-04-03 21:50 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
2018-04-03 22:06 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-04-05 2:03 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) Patchwork
2018-04-05 2:18 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-05 3:18 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-04-05 20:40 ` [PATCH v3 1/4] drm/i915: Enable edp psr error interrupts on hsw Souza, Jose
2018-04-05 21:42 ` Dhinakaran Pandiyan
2018-04-05 21:44 ` Souza, Jose
2018-04-05 22:00 ` [PATCH v4] " Dhinakaran Pandiyan
2018-04-17 0:43 ` Paulo Zanoni
2018-04-17 17:41 ` Ville Syrjälä
2018-04-17 20:01 ` Dhinakaran Pandiyan
2018-04-20 22:22 ` Rodrigo Vivi
2018-04-05 22:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4] drm/i915: Enable edp psr error interrupts on hsw (rev3) Patchwork
2018-04-05 22:51 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-06 1:50 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-04-10 0:49 ` ✗ Fi.CI.CHECKPATCH: " Patchwork
[not found] ` <20180410175934.GA2303@intel.com>
2018-04-10 18:29 ` Dhinakaran Pandiyan
2018-04-20 21:33 ` Rodrigo Vivi
2018-04-10 1:05 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-10 2:25 ` ✓ 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.