* [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw
@ 2018-03-27 1:16 Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-27 1:16 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 da2f6c623ab2..af6e2b1dd6ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3845,6 +3845,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)
@@ -6651,6 +6658,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] 17+ messages in thread
* [PATCH v2 2/4] drm/i915: Enable edp psr error interrupts on bdw+
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
@ 2018-03-27 1:16 ` Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-27 1:16 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 af6e2b1dd6ba..5c838c6f3b40 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3848,9 +3848,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)
@@ -6783,6 +6783,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] 17+ messages in thread
* [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
@ 2018-03-27 1:16 ` Dhinakaran Pandiyan
2018-03-27 10:24 ` Ville Syrjälä
2018-03-27 10:26 ` [PATCH v2 3/4] " Chris Wilson
2018-03-27 1:16 ` [PATCH v2 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
` (4 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-27 1:16 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
v2: Unroll loops (Ville)
Avoid resetting error mask bits.
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 | 55 +++++++++++--------------------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
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 7816cd53100a..6fd801ef7cbb 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;
+
+ if (val < 0 || val > 1)
+ return -EINVAL;
+
+ DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
+ intel_psr_debug_control(dev_priv, val);
+
+ 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);
@@ -4811,7 +4844,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 c9c3b2ba6a86..c0224a86344e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -608,6 +608,7 @@ struct i915_psr {
bool colorimetry_support;
bool alpm;
bool has_hw_tracking;
+ 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..e5aaf805c6a8 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);
+ I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -3845,6 +3818,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 +3843,10 @@ 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);
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 a215aa78b0be..8be75cc5bf24 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1887,6 +1887,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 b8e083e10029..fdc5e1bf198a 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,6 +93,60 @@ 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 mask;
+
+ /* No PSR interrupts on VLV/CHV */
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ return;
+
+ mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
+ EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
+
+ if (INTEL_GEN(dev_priv) >= 8)
+ 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, I915_READ(EDP_PSR_IMR) & ~mask);
+ } else {
+ I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 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_cord_status(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] 17+ messages in thread
* [PATCH v2 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
@ 2018-03-27 1:16 ` Dhinakaran Pandiyan
2018-03-27 1:49 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-27 1:16 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 6fd801ef7cbb..c69384d17226 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 c0224a86344e..41c3268b5de5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -609,6 +609,8 @@ struct i915_psr {
bool alpm;
bool has_hw_tracking;
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 fdc5e1bf198a..0d338632c2bb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -125,6 +125,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) |
@@ -137,13 +138,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] 17+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (2 preceding siblings ...)
2018-03-27 1:16 ` [PATCH v2 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
@ 2018-03-27 1:49 ` Patchwork
2018-03-27 2:04 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-03-27 1:49 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
URL : https://patchwork.freedesktop.org/series/40704/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e0e155ae5044 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:4011:
+#define EDP_PSR_ERROR (1<<2)
^
-:110: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#110: FILE: drivers/gpu/drm/i915/i915_reg.h:4012:
+#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:4013:
+#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:6821:
+#define DE_EDP_PSR_INT_HSW (1<<19)
^
total: 0 errors, 0 warnings, 4 checks, 78 lines checked
d27113364c8d 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
07e5a8224b22 drm/i915/psr: Control PSR interrupts via debugfs
f4ea75bae00f 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] 17+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (3 preceding siblings ...)
2018-03-27 1:49 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
@ 2018-03-27 2:04 ` Patchwork
2018-03-27 3:53 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-30 21:24 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-03-27 2:04 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
URL : https://patchwork.freedesktop.org/series/40704/
State : success
== Summary ==
Series 40704v1 series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
https://patchwork.freedesktop.org/api/1.0/series/40704/revisions/1/mbox/
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-cnl-y3) fdo#104951
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:435s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:442s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:546s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:510s
fi-cnl-y3 total:285 pass:258 dwarn:1 dfail:0 fail:0 skip:26 time:584s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:429s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:318s
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:431s
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:471s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:514s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:659s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:442s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:542s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:507s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:431s
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:575s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:401s
Blacklisted hosts:
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:571s
fi-cnl-psr total:224 pass:198 dwarn:0 dfail:0 fail:1 skip:24
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:487s
d439f4eca05fe48c26bf9c3863e56ee19ac1c50b drm-tip: 2018y-03m-27d-00h-15m-56s UTC integration manifest
f4ea75bae00f drm/i915/psr: Timestamps for PSR entry and exit interrupts.
07e5a8224b22 drm/i915/psr: Control PSR interrupts via debugfs
d27113364c8d drm/i915: Enable edp psr error interrupts on bdw+
e0e155ae5044 drm/i915: Enable edp psr error interrupts on hsw
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8497/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (4 preceding siblings ...)
2018-03-27 2:04 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-27 3:53 ` Patchwork
2018-03-30 21:24 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-03-27 3:53 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw
URL : https://patchwork.freedesktop.org/series/40704/
State : success
== Summary ==
---- Known issues:
Test kms_flip:
Subgroup blocking-wf_vblank:
pass -> FAIL (shard-hsw) fdo#100368
Test kms_rotation_crc:
Subgroup sprite-rotation-180:
fail -> PASS (shard-apl) fdo#103925
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
shard-apl total:3495 pass:1831 dwarn:1 dfail:0 fail:7 skip:1655 time:12899s
shard-hsw total:3495 pass:1782 dwarn:1 dfail:0 fail:2 skip:1709 time:11695s
shard-snb total:3495 pass:1374 dwarn:1 dfail:0 fail:3 skip:2117 time:6976s
Blacklisted hosts:
shard-kbl total:3495 pass:1956 dwarn:1 dfail:1 fail:9 skip:1528 time:9652s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8497/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 1:16 ` [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
@ 2018-03-27 10:24 ` Ville Syrjälä
2018-03-27 18:33 ` Pandiyan, Dhinakaran
2018-03-27 10:26 ` [PATCH v2 3/4] " Chris Wilson
1 sibling, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2018-03-27 10:24 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, Rodrigo Vivi
On Mon, Mar 26, 2018 at 06:16:22PM -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
>
> v2: Unroll loops (Ville)
> Avoid resetting error mask bits.
>
> 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 | 55 +++++++++++--------------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> 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 7816cd53100a..6fd801ef7cbb 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;
> +
> + if (val < 0 || val > 1)
> + return -EINVAL;
Can't be < 0.
> +
> + DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
==1 seems pointless
enableddisabled() could be used perhaps.
> + intel_psr_debug_control(dev_priv, val);
> +
> + 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);
> @@ -4811,7 +4844,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 c9c3b2ba6a86..c0224a86344e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -608,6 +608,7 @@ struct i915_psr {
> bool colorimetry_support;
> bool alpm;
> bool has_hw_tracking;
> + 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..e5aaf805c6a8 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);
> + I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
>
> @@ -3845,6 +3818,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 +3843,10 @@ 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);
>
> 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 a215aa78b0be..8be75cc5bf24 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1887,6 +1887,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 b8e083e10029..fdc5e1bf198a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -93,6 +93,60 @@ 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 mask;
> +
> + /* No PSR interrupts on VLV/CHV */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + return;
> +
> + mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> +
> + if (INTEL_GEN(dev_priv) >= 8)
> + 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, I915_READ(EDP_PSR_IMR) & ~mask);
Why RMW?
> + } else {
> + I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 when this happens. */
Should this maybe say "retrain the link 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_cord_status(struct intel_dp *intel_dp)
> {
> uint8_t psr_caps = 0;
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 1:16 ` [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
2018-03-27 10:24 ` Ville Syrjälä
@ 2018-03-27 10:26 ` Chris Wilson
2018-03-27 21:17 ` Pandiyan, Dhinakaran
1 sibling, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2018-03-27 10:26 UTC (permalink / raw)
To: Dhinakaran Pandiyan, intel-gfx; +Cc: Dhinakaran
Quoting Dhinakaran Pandiyan (2018-03-27 02:16:22)
> 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
>
> v2: Unroll loops (Ville)
> Avoid resetting error mask bits.
>
> 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 | 55 +++++++++++--------------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> 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 7816cd53100a..6fd801ef7cbb 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;
> +
> + if (val < 0 || val > 1)
> + return -EINVAL;
> +
> + DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
> + intel_psr_debug_control(dev_priv, val);
> +
> + return 0;
> +}
A really useful trick for features like this (that we think should be
used wherever it can) is just to enable debug while the debugfs file is
open. igt need only do something like
openat(debugfs_dir, "i915_edp_psr_debug", O_RDONLY);
and then it will be automatically released and the system returned to
default when terminated. You can then enforce exclusive access or keep an
open count.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 10:24 ` Ville Syrjälä
@ 2018-03-27 18:33 ` Pandiyan, Dhinakaran
2018-03-28 10:28 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-27 18:33 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Vivi, Rodrigo
On Tue, 2018-03-27 at 13:24 +0300, Ville Syrjälä wrote:
> On Mon, Mar 26, 2018 at 06:16:22PM -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
> >
> > v2: Unroll loops (Ville)
> > Avoid resetting error mask bits.
> >
> > 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 | 55 +++++++++++--------------------------
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> > 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 7816cd53100a..6fd801ef7cbb 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;
> > +
> > + if (val < 0 || val > 1)
> > + return -EINVAL;
>
> Can't be < 0.
>
> > +
> > + DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
>
> ==1 seems pointless
> enableddisabled() could be used perhaps.
>
Will do.
>
> > + intel_psr_debug_control(dev_priv, val);
> > +
> > + 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);
> > @@ -4811,7 +4844,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 c9c3b2ba6a86..c0224a86344e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -608,6 +608,7 @@ struct i915_psr {
> > bool colorimetry_support;
> > bool alpm;
> > bool has_hw_tracking;
> > + 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..e5aaf805c6a8 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);
> > + I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
> > display_mask |= DE_EDP_PSR_INT_HSW;
> > }
> >
> > @@ -3845,6 +3818,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 +3843,10 @@ 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);
> >
> > 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 a215aa78b0be..8be75cc5bf24 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1887,6 +1887,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 b8e083e10029..fdc5e1bf198a 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -93,6 +93,60 @@ 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 mask;
> > +
> > + /* No PSR interrupts on VLV/CHV */
> > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > + return;
> > +
> > + mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> > + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> > +
> > + if (INTEL_GEN(dev_priv) >= 8)
> > + 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, I915_READ(EDP_PSR_IMR) & ~mask);
>
> Why RMW?
>
Avoids updating this function when new PSR error bits are added in
i915_irq.c
Would you prefer
mask |= EDP_PSR_ERROR(TRANCODER_A) | ... here?
I think this has started to look ugly already. The loop was concise IMO.
The other option is to
#define HSW_PSR_INTR_DBG_MASK = 0x7
#define BDW_PSR_INTR_DBG_MASK = 0x07070707
> > + } else {
> > + I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 when this happens. */
>
> Should this maybe say "retrain the link manually when this happens"?
>
Yeah, we should do both in fact. Makes sense to exit PSR, link train
manually and keep it disabled.
> > + 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_cord_status(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 [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 10:26 ` [PATCH v2 3/4] " Chris Wilson
@ 2018-03-27 21:17 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-27 21:17 UTC (permalink / raw)
To: chris; +Cc: intel-gfx
On Tue, 2018-03-27 at 11:26 +0100, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-03-27 02:16:22)
> > 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
> >
> > v2: Unroll loops (Ville)
> > Avoid resetting error mask bits.
> >
> > 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 | 55 +++++++++++--------------------------
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> > 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 7816cd53100a..6fd801ef7cbb 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;
> > +
> > + if (val < 0 || val > 1)
> > + return -EINVAL;
> > +
> > + DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
> > + intel_psr_debug_control(dev_priv, val);
> > +
> > + return 0;
> > +}
>
> A really useful trick for features like this (that we think should be
> used wherever it can) is just to enable debug while the debugfs file is
> open. igt need only do something like
> openat(debugfs_dir, "i915_edp_psr_debug", O_RDONLY);
That is neat.
We also want to support user enabling/disabling PSR debug via shell. The
semantics get slightly confusing taking that into consideration.
For example,
1) should cat $debugfs/i915_edp_psr_debug
also enable debug or just print the current status of debug
2) if the file is already open (debug enabled), should
echo 0 > $debugfs/i915_edp_psr_debug disable debug or check for the
refcount to drop to zero.
Choosing the correct solution and implementing it correctly feels like
we'd be over-engineering.
> and then it will be automatically released and the system returned to
> default when terminated. You can then enforce exclusive access or keep an
> open count.
> -Chris
> _______________________________________________
> 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] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-27 18:33 ` Pandiyan, Dhinakaran
@ 2018-03-28 10:28 ` Ville Syrjälä
2018-03-28 17:37 ` Pandiyan, Dhinakaran
2018-03-30 21:15 ` [PATCH v3] " Dhinakaran Pandiyan
0 siblings, 2 replies; 17+ messages in thread
From: Ville Syrjälä @ 2018-03-28 10:28 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo
On Tue, Mar 27, 2018 at 06:33:11PM +0000, Pandiyan, Dhinakaran wrote:
> On Tue, 2018-03-27 at 13:24 +0300, Ville Syrjälä wrote:
> > On Mon, Mar 26, 2018 at 06:16:22PM -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
> > >
> > > v2: Unroll loops (Ville)
> > > Avoid resetting error mask bits.
> > >
> > > 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 | 55 +++++++++++--------------------------
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> > > 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 7816cd53100a..6fd801ef7cbb 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;
> > > +
> > > + if (val < 0 || val > 1)
> > > + return -EINVAL;
> >
> > Can't be < 0.
> >
> > > +
> > > + DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
> >
> > ==1 seems pointless
> > enableddisabled() could be used perhaps.
> >
>
> Will do.
>
> >
> > > + intel_psr_debug_control(dev_priv, val);
> > > +
> > > + 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);
> > > @@ -4811,7 +4844,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 c9c3b2ba6a86..c0224a86344e 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -608,6 +608,7 @@ struct i915_psr {
> > > bool colorimetry_support;
> > > bool alpm;
> > > bool has_hw_tracking;
> > > + 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..e5aaf805c6a8 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);
> > > + I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
> > > display_mask |= DE_EDP_PSR_INT_HSW;
> > > }
> > >
> > > @@ -3845,6 +3818,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 +3843,10 @@ 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);
> > >
> > > 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 a215aa78b0be..8be75cc5bf24 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1887,6 +1887,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 b8e083e10029..fdc5e1bf198a 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -93,6 +93,60 @@ 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 mask;
> > > +
> > > + /* No PSR interrupts on VLV/CHV */
> > > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > + return;
> > > +
> > > + mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> > > + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> > > +
> > > + if (INTEL_GEN(dev_priv) >= 8)
> > > + 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, I915_READ(EDP_PSR_IMR) & ~mask);
> >
> > Why RMW?
> >
> Avoids updating this function when new PSR error bits are added in
> i915_irq.c
The usual rule is "avoid RMW unles there is a really compelling reason
for it". There is none in this case AFAICS.
>
> Would you prefer
>
> mask |= EDP_PSR_ERROR(TRANCODER_A) | ... here?
>
> I think this has started to look ugly already. The loop was concise IMO.
>
> The other option is to
>
> #define HSW_PSR_INTR_DBG_MASK = 0x7
> #define BDW_PSR_INTR_DBG_MASK = 0x07070707
Not sure what these have to do with the RMW.
>
>
>
> > > + } else {
> > > + I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 when this happens. */
> >
> > Should this maybe say "retrain the link manually when this happens"?
> >
>
> Yeah, we should do both in fact. Makes sense to exit PSR, link train
> manually and keep it disabled.
BTW is there a hardware mechnism to inject this failure? Would be nice
for testing it.
If there is no way, maybe we could provide garbage settings for the AUX
stuff and that would cause the failure?
>
>
>
> > > + 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_cord_status(struct intel_dp *intel_dp)
> > > {
> > > uint8_t psr_caps = 0;
> > > --
> > > 2.14.1
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-28 10:28 ` Ville Syrjälä
@ 2018-03-28 17:37 ` Pandiyan, Dhinakaran
2018-03-28 23:44 ` Pandiyan, Dhinakaran
2018-03-30 21:15 ` [PATCH v3] " Dhinakaran Pandiyan
1 sibling, 1 reply; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-28 17:37 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Vivi, Rodrigo
On Wed, 2018-03-28 at 13:28 +0300, Ville Syrjälä wrote:
> On Tue, Mar 27, 2018 at 06:33:11PM +0000, Pandiyan, Dhinakaran wrote:
> > On Tue, 2018-03-27 at 13:24 +0300, Ville Syrjälä wrote:
> > > On Mon, Mar 26, 2018 at 06:16:22PM -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
> > > >
> > > > v2: Unroll loops (Ville)
> > > > Avoid resetting error mask bits.
> > > >
> > > > 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 | 55 +++++++++++--------------------------
> > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > > drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> > > > 5 files changed, 108 insertions(+), 40 deletions(-)
> > > >
<snip>
> > > >
> > > > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable)
> > > > +{
> > > > + u32 mask;
> > > > +
> > > > + /* No PSR interrupts on VLV/CHV */
> > > > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > > + return;
> > > > +
> > > > + mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> > > > + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> > > > +
> > > > + if (INTEL_GEN(dev_priv) >= 8)
> > > > + 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, I915_READ(EDP_PSR_IMR) & ~mask);
> > >
> > > Why RMW?
> > >
> > Avoids updating this function when new PSR error bits are added in
> > i915_irq.c
>
> The usual rule is "avoid RMW unles there is a really compelling reason
> for it". There is none in this case AFAICS.
>
> >
> > Would you prefer
> >
> > mask |= EDP_PSR_ERROR(TRANCODER_A) | ... here?
> >
> > I think this has started to look ugly already. The loop was concise IMO.
> >
> > The other option is to
> >
> > #define HSW_PSR_INTR_DBG_MASK = 0x7
> > #define BDW_PSR_INTR_DBG_MASK = 0x07070707
>
> Not sure what these have to do with the RMW.
>
They don't, that was an alternative to the series of OR's we have to do
to derive the mask.
0x07070707 is the mask of all the relevant bits (error + debug). The
above definitions could be used as
if (INTEL_GEN(dev_priv) > 8)
mask = BDW_PSR_INTR_DBG_MASK;
else
mask = HSW_PSR_INTR_DBG_MASK
if (enable)
I915_WRITE(EDP_PSR_IMR, ~mask)
Anyway, I am not sure how you want this to be written at this point. Can
you suggest in code what you like to see here?
> >
> >
> >
> > > > + } else {
> > > > + I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 when this happens. */
> > >
> > > Should this maybe say "retrain the link manually when this happens"?
> > >
> >
> > Yeah, we should do both in fact. Makes sense to exit PSR, link train
> > manually and keep it disabled.
>
> BTW is there a hardware mechnism to inject this failure? Would be nice
> for testing it.
>
Haven't seen anything like that in bspec.
> If there is no way, maybe we could provide garbage settings for the AUX
> stuff and that would cause the failure?
>
Probably, are you aware of a way that aux failures can be triggered in
general? We could add it as an IGT test to make sure we do get
interrupts and handle it correctly.
> >
> >
> >
> > > > + 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_cord_status(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 [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-28 17:37 ` Pandiyan, Dhinakaran
@ 2018-03-28 23:44 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-28 23:44 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Vivi, Rodrigo
On Wed, 2018-03-28 at 17:37 +0000, Pandiyan, Dhinakaran wrote:
>
>
> On Wed, 2018-03-28 at 13:28 +0300, Ville Syrjälä wrote:
> > On Tue, Mar 27, 2018 at 06:33:11PM +0000, Pandiyan, Dhinakaran wrote:
> > > On Tue, 2018-03-27 at 13:24 +0300, Ville Syrjälä wrote:
> > > > On Mon, Mar 26, 2018 at 06:16:22PM -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
> > > > >
> > > > > v2: Unroll loops (Ville)
> > > > > Avoid resetting error mask bits.
> > > > >
> > > > > 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 | 55 +++++++++++--------------------------
> > > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > > > drivers/gpu/drm/i915/intel_psr.c | 54 ++++++++++++++++++++++++++++++++++++
> > > > > 5 files changed, 108 insertions(+), 40 deletions(-)
> > > > >
> <snip>
> > > > >
> > > > > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable)
> > > > > +{
> > > > > + u32 mask;
> > > > > +
> > > > > + /* No PSR interrupts on VLV/CHV */
> > > > > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > > > + return;
> > > > > +
> > > > > + mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> > > > > + EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) >= 8)
> > > > > + 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, I915_READ(EDP_PSR_IMR) & ~mask);
> > > >
> > > > Why RMW?
> > > >
> > > Avoids updating this function when new PSR error bits are added in
> > > i915_irq.c
> >
> > The usual rule is "avoid RMW unles there is a really compelling reason
> > for it". There is none in this case AFAICS.
> >
> > >
> > > Would you prefer
> > >
> > > mask |= EDP_PSR_ERROR(TRANCODER_A) | ... here?
> > >
> > > I think this has started to look ugly already. The loop was concise IMO.
> > >
> > > The other option is to
> > >
> > > #define HSW_PSR_INTR_DBG_MASK = 0x7
> > > #define BDW_PSR_INTR_DBG_MASK = 0x07070707
> >
> > Not sure what these have to do with the RMW.
> >
> They don't, that was an alternative to the series of OR's we have to do
> to derive the mask.
>
> 0x07070707 is the mask of all the relevant bits (error + debug). The
> above definitions could be used as
>
> if (INTEL_GEN(dev_priv) > 8)
> mask = BDW_PSR_INTR_DBG_MASK;
> else
> mask = HSW_PSR_INTR_DBG_MASK
>
>
> if (enable)
> I915_WRITE(EDP_PSR_IMR, ~mask)
>
> Anyway, I am not sure how you want this to be written at this point. Can
> you suggest in code what you like to see here?
>
+ 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, I915_READ(EDP_PSR_IMR) & ~mask);
+ I915_WRITE(EDP_PSR_IMR, ~debug_mask);
} else {
- I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | mask);
+ I915_WRITE(EDP_PSR_IMR, ~error_mask);
WRITE_ONCE(dev_priv->psr.debug, false);
}
Let me know if this is what you had in mind. Also,
dev->driver->irq_uninstall() will disable the debug interrupts. Does it
make sense to re-enable the interrupts in intel_psr_enable() if
psr.debug was still set?
> > >
> > >
> > >
> > > > > + } else {
> > > > > + I915_WRITE(EDP_PSR_IMR, I915_READ(EDP_PSR_IMR) | 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 when this happens. */
> > > >
> > > > Should this maybe say "retrain the link manually when this happens"?
> > > >
> > >
> > > Yeah, we should do both in fact. Makes sense to exit PSR, link train
> > > manually and keep it disabled.
> >
> > BTW is there a hardware mechnism to inject this failure? Would be nice
> > for testing it.
> >
> Haven't seen anything like that in bspec.
>
> > If there is no way, maybe we could provide garbage settings for the AUX
> > stuff and that would cause the failure?
> >
> Probably, are you aware of a way that aux failures can be triggered in
> general? We could add it as an IGT test to make sure we do get
> interrupts and handle it correctly.
>
> > >
> > >
> > >
> > > > > + 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_cord_status(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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-28 10:28 ` Ville Syrjälä
2018-03-28 17:37 ` Pandiyan, Dhinakaran
@ 2018-03-30 21:15 ` Dhinakaran Pandiyan
2018-03-30 21:22 ` Pandiyan, Dhinakaran
1 sibling, 1 reply; 17+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-30 21:15 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 | 33 +++++++++++++++++++-
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, 113 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ff90577da450..5e697867a8ca 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2690,6 +2690,36 @@ 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_psr_debug_control(dev_priv, !!val);
+
+ 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 +4842,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 800230ba1c3b..71577ebf6a60 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 colorimetry_support;
bool alpm;
bool has_hw_tracking;
+ 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 b8e083e10029..0d9305f98d33 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_cord_status(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] 17+ messages in thread
* Re: [PATCH v3] drm/i915/psr: Control PSR interrupts via debugfs
2018-03-30 21:15 ` [PATCH v3] " Dhinakaran Pandiyan
@ 2018-03-30 21:22 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 17+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-30 21:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Vivi, Rodrigo
On Fri, 2018-03-30 at 14:15 -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.
Let me know what you think, I am not completely sure about this.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
` (5 preceding siblings ...)
2018-03-27 3:53 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-30 21:24 ` Patchwork
6 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-03-30 21:24 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2)
URL : https://patchwork.freedesktop.org/series/40704/
State : failure
== Summary ==
Applying: drm/i915: Enable edp psr error interrupts on hsw
Applying: drm/i915: Enable edp psr error interrupts on bdw+
Applying: drm/i915/psr: Control PSR interrupts via debugfs
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_debugfs.c
M drivers/gpu/drm/i915/i915_drv.h
M drivers/gpu/drm/i915/intel_psr.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_psr.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_psr.c
Auto-merging drivers/gpu/drm/i915/i915_drv.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_drv.h
Auto-merging drivers/gpu/drm/i915/i915_debugfs.c
Patch failed at 0003 drm/i915/psr: Control PSR interrupts via debugfs
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-03-30 21:24 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 1:16 [PATCH v2 1/4] drm/i915: Enable edp psr error interrupts on hsw Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 2/4] drm/i915: Enable edp psr error interrupts on bdw+ Dhinakaran Pandiyan
2018-03-27 1:16 ` [PATCH v2 3/4] drm/i915/psr: Control PSR interrupts via debugfs Dhinakaran Pandiyan
2018-03-27 10:24 ` Ville Syrjälä
2018-03-27 18:33 ` Pandiyan, Dhinakaran
2018-03-28 10:28 ` Ville Syrjälä
2018-03-28 17:37 ` Pandiyan, Dhinakaran
2018-03-28 23:44 ` Pandiyan, Dhinakaran
2018-03-30 21:15 ` [PATCH v3] " Dhinakaran Pandiyan
2018-03-30 21:22 ` Pandiyan, Dhinakaran
2018-03-27 10:26 ` [PATCH v2 3/4] " Chris Wilson
2018-03-27 21:17 ` Pandiyan, Dhinakaran
2018-03-27 1:16 ` [PATCH v2 4/4] drm/i915/psr: Timestamps for PSR entry and exit interrupts Dhinakaran Pandiyan
2018-03-27 1:49 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw Patchwork
2018-03-27 2:04 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-27 3:53 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-30 21:24 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/4] drm/i915: Enable edp psr error interrupts on hsw (rev2) 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.