* [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
@ 2019-06-19 23:02 José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 2/4] drm/i915: Add _TRANS2() José Roberto de Souza
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: José Roberto de Souza @ 2019-06-19 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
Just moving it to reduce the tabs and avoid break code lines.
No behavior changes intended here.
v2:
- Reading misc display IRQ outside of gen8_de_misc_irq_handler() as
other irq handlers (Dhinakaran)
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 45 ++++++++++++++++++---------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b2e27b5b0df9..14b0933809d8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2826,6 +2826,28 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
return mask;
}
+static void
+gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
+{
+ bool found = false;
+
+ if (iir & GEN8_DE_MISC_GSE) {
+ intel_opregion_asle_intr(dev_priv);
+ found = true;
+ }
+
+ if (iir & GEN8_DE_EDP_PSR) {
+ 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;
+ }
+
+ if (!found)
+ DRM_ERROR("Unexpected DE Misc interrupt\n");
+}
+
static irqreturn_t
gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
{
@@ -2836,29 +2858,12 @@ 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) {
- intel_opregion_asle_intr(dev_priv);
- found = true;
- }
-
- if (iir & GEN8_DE_EDP_PSR) {
- 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;
- }
-
- if (!found)
- DRM_ERROR("Unexpected DE Misc interrupt\n");
- }
- else
+ gen8_de_misc_irq_handler(dev_priv, iir);
+ } else {
DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
+ }
}
if (INTEL_GEN(dev_priv) >= 11 && (master_ctl & GEN11_DE_HPD_IRQ)) {
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v6 2/4] drm/i915: Add _TRANS2()
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
@ 2019-06-19 23:02 ` José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders José Roberto de Souza
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: José Roberto de Souza @ 2019-06-19 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
A new macro that is going to be added in a further patch will need to
adjust the offset returned by _MMIO_TRANS2(), so here adding
_TRANS2() and moving most of the implementation of _MMIO_TRANS2() to
it and while at it taking the opportunity to rename pipe to trans.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiya@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiya@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d98142940c38..4fc8dc083766 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -250,9 +250,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _MMIO_PIPE2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)->pipe_offsets[pipe] - \
INTEL_INFO(dev_priv)->pipe_offsets[PIPE_A] + (reg) + \
DISPLAY_MMIO_BASE(dev_priv))
-#define _MMIO_TRANS2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)->trans_offsets[(pipe)] - \
- INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \
- DISPLAY_MMIO_BASE(dev_priv))
+#define _TRANS2(tran, reg) (INTEL_INFO(dev_priv)->trans_offsets[(tran)] - \
+ INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \
+ DISPLAY_MMIO_BASE(dev_priv))
+#define _MMIO_TRANS2(tran, reg) _MMIO(_TRANS2(tran, reg))
#define _CURSOR2(pipe, reg) _MMIO(INTEL_INFO(dev_priv)->cursor_offsets[(pipe)] - \
INTEL_INFO(dev_priv)->cursor_offsets[PIPE_A] + (reg) + \
DISPLAY_MMIO_BASE(dev_priv))
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 2/4] drm/i915: Add _TRANS2() José Roberto de Souza
@ 2019-06-19 23:02 ` José Roberto de Souza
2019-06-20 8:13 ` Jani Nikula
2019-06-19 23:02 ` [PATCH v6 4/4] drm/i915: Add transcoder restriction to PSR2 José Roberto de Souza
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: José Roberto de Souza @ 2019-06-19 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
PSR registers are a mess, some have the full address while others just
have the additional offset from psr_mmio_base.
For BDW+ psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET +
0x800 and using it makes more difficult for people with an PSR
register address or PSR register name from from BSpec as i915 also
don't match the BSpec names.
For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR registers are
only available in DDIA.
Other reason to make relative to transcoder is that since BDW every
transcoder have PSR registers, so in theory it should be possible to
have PSR enabled in a non-eDP transcoder.
So for BDW+ we can use _TRANS2() to get the register offset of any
PSR register in any transcoder while for HSW we have _HSW_PSR_ADJ
that will calculate the register offset for the single PSR instance,
noting that we are already guarded about trying to enable PSR in other
port than DDIA on HSW by the 'if (dig_port->base.port != PORT_A)' in
intel_psr_compute_config(), this check should only be valid for HSW
and will be changed in future.
PSR2 registers and PSR_EVENT was added after Haswell so that is why
_PSR_ADJ() is not used in some macros.
The only registers that can not be relative to transcoder are
PSR_IMR and PSR_IIR that are not relative to anything, so keeping it
hardcoded.
Also removing BDW_EDP_PSR_BASE from GVT because it is not used as it
is the only PSR register that GVT have.
v6:
- Checking for interruption errors after module reload in the
transcoder that will be used (Dhinakaran)
- Using lowercase to the registers offsets
v5:
- Macros changed to be more explicit about HSW (Dhinakaran)
- Squashed with the patch that added the tran parameter to the
macros (Dhinakaran)
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 89 +++++++++++++-----------
drivers/gpu/drm/i915/gvt/handlers.c | 1 -
drivers/gpu/drm/i915/i915_debugfs.c | 18 +++--
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_reg.h | 58 ++++++++++-----
5 files changed, 100 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 69d908e6a050..ed422b788d88 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
BUILD_BUG_ON(sizeof(aux_msg) > 20);
for (i = 0; i < sizeof(aux_msg); i += 4)
- I915_WRITE(EDP_PSR_AUX_DATA(i >> 2),
+ I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i >> 2),
intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
@@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
/* Select only valid bits for SRD_AUX_CTL */
aux_ctl &= psr_aux_mask;
- I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl);
+ I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl);
}
static void intel_psr_enable_sink(struct intel_dp *intel_dp)
@@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
if (INTEL_GEN(dev_priv) >= 8)
val |= EDP_PSR_CRC_ENABLE;
- val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
- I915_WRITE(EDP_PSR_CTL, val);
+ val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) &
+ EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK);
+ I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val);
}
static void hsw_activate_psr2(struct intel_dp *intel_dp)
@@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
* PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
* recommending keep this bit unset while PSR2 is enabled.
*/
- I915_WRITE(EDP_PSR_CTL, 0);
+ I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0);
- I915_WRITE(EDP_PSR2_CTL, val);
+ I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
}
static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
@@ -649,8 +650,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
if (INTEL_GEN(dev_priv) >= 9)
- WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
- WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+ WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)) & EDP_PSR2_ENABLE);
+ WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & EDP_PSR_ENABLE);
WARN_ON(dev_priv->psr.active);
lockdep_assert_held(&dev_priv->psr.lock);
@@ -720,19 +721,37 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
if (INTEL_GEN(dev_priv) < 11)
mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
- I915_WRITE(EDP_PSR_DEBUG, mask);
+ I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask);
}
static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
const struct intel_crtc_state *crtc_state)
{
struct intel_dp *intel_dp = dev_priv->psr.dp;
+ u32 val;
WARN_ON(dev_priv->psr.enabled);
dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
dev_priv->psr.busy_frontbuffer_bits = 0;
dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
+
+ /*
+ * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
+ * will still keep the error set even after the reset done in the
+ * irq_preinstall and irq_uninstall hooks.
+ * And enabling in this situation cause the screen to freeze in the
+ * first time that PSR HW tries to activate so lets keep PSR disabled
+ * to avoid any rendering problems.
+ */
+ val = I915_READ(EDP_PSR_IIR);
+ val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv->psr.transcoder));
+ if (val) {
+ dev_priv->psr.sink_not_reliable = true;
+ DRM_DEBUG_KMS("PSR interruption error set, not enabling PSR\n");
+ return;
+ }
DRM_DEBUG_KMS("Enabling PSR%s\n",
dev_priv->psr.psr2_enabled ? "2" : "1");
@@ -782,20 +801,27 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
u32 val;
if (!dev_priv->psr.active) {
- if (INTEL_GEN(dev_priv) >= 9)
- WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
- WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+ if (INTEL_GEN(dev_priv) >= 9) {
+ val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
+ WARN_ON(val & EDP_PSR2_ENABLE);
+ }
+
+ val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
+ WARN_ON(val & EDP_PSR_ENABLE);
+
return;
}
if (dev_priv->psr.psr2_enabled) {
- val = I915_READ(EDP_PSR2_CTL);
+ val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
WARN_ON(!(val & EDP_PSR2_ENABLE));
- I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
+ val &= ~EDP_PSR2_ENABLE;
+ I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
} else {
- val = I915_READ(EDP_PSR_CTL);
+ val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
WARN_ON(!(val & EDP_PSR_ENABLE));
- I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
+ val &= ~EDP_PSR_ENABLE;
+ I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val);
}
dev_priv->psr.active = false;
}
@@ -817,10 +843,10 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
intel_psr_exit(dev_priv);
if (dev_priv->psr.psr2_enabled) {
- psr_status = EDP_PSR2_STATUS;
+ psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
} else {
- psr_status = EDP_PSR_STATUS;
+ psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder);
psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
}
@@ -963,7 +989,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
* defensive enough to cover everything.
*/
- return __intel_wait_for_register(&dev_priv->uncore, EDP_PSR_STATUS,
+ return __intel_wait_for_register(&dev_priv->uncore,
+ EDP_PSR_STATUS(dev_priv->psr.transcoder),
EDP_PSR_STATUS_STATE_MASK,
EDP_PSR_STATUS_STATE_IDLE, 2, 50,
out_value);
@@ -979,10 +1006,10 @@ static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv)
return false;
if (dev_priv->psr.psr2_enabled) {
- reg = EDP_PSR2_STATUS;
+ reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
mask = EDP_PSR2_STATUS_STATE_MASK;
} else {
- reg = EDP_PSR_STATUS;
+ reg = EDP_PSR_STATUS(dev_priv->psr.transcoder);
mask = EDP_PSR_STATUS_STATE_MASK;
}
@@ -1208,14 +1235,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
*/
void intel_psr_init(struct drm_i915_private *dev_priv)
{
- u32 val;
-
if (!HAS_PSR(dev_priv))
return;
- dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
- HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
-
if (!dev_priv->psr.sink_support)
return;
@@ -1223,21 +1245,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable)
i915_modparams.enable_psr = 0;
- /*
- * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
- * will still keep the error set even after the reset done in the
- * irq_preinstall and irq_uninstall hooks.
- * And enabling in this situation cause the screen to freeze in the
- * first time that PSR HW tries to activate so lets keep PSR disabled
- * to avoid any rendering problems.
- */
- val = I915_READ(EDP_PSR_IIR);
- val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
- if (val) {
- DRM_DEBUG_KMS("PSR interruption error set\n");
- dev_priv->psr.sink_not_reliable = true;
- }
-
/* Set link_standby x link_off defaults */
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
/* HSW and BDW require workarounds that we don't implement. */
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index 25f78196b964..50164fdf914c 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2796,7 +2796,6 @@ static int init_broadwell_mmio_info(struct intel_gvt *gvt)
MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS);
MMIO_D(WM_MISC, D_BDW);
- MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW);
MMIO_D(_MMIO(0x6671c), D_BDW_PLUS);
MMIO_D(_MMIO(0x66c00), D_BDW_PLUS);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 62cf34db9280..659f1d3d9162 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2258,7 +2258,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
"BUF_ON",
"TG_ON"
};
- val = I915_READ(EDP_PSR2_STATUS);
+ val = I915_READ(EDP_PSR2_STATUS(dev_priv->psr.transcoder));
status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
EDP_PSR2_STATUS_STATE_SHIFT;
if (status_val < ARRAY_SIZE(live_status))
@@ -2274,7 +2274,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
"SRDOFFACK",
"SRDENT_ON",
};
- val = I915_READ(EDP_PSR_STATUS);
+ val = I915_READ(EDP_PSR_STATUS(dev_priv->psr.transcoder));
status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
EDP_PSR_STATUS_STATE_SHIFT;
if (status_val < ARRAY_SIZE(live_status))
@@ -2317,10 +2317,10 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
goto unlock;
if (psr->psr2_enabled) {
- val = I915_READ(EDP_PSR2_CTL);
+ val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
enabled = val & EDP_PSR2_ENABLE;
} else {
- val = I915_READ(EDP_PSR_CTL);
+ val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
enabled = val & EDP_PSR_ENABLE;
}
seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
@@ -2333,7 +2333,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
* SKL+ Perf counter is reset to 0 everytime DC state is entered
*/
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
- val = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK;
+ val = I915_READ(EDP_PSR_PERF_CNT(dev_priv->psr.transcoder));
+ val &= EDP_PSR_PERF_CNT_MASK;
seq_printf(m, "Performance counter: %u\n", val);
}
@@ -2351,8 +2352,11 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
* Reading all 3 registers before hand to minimize crossing a
* frame boundary between register reads
*/
- for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3)
- su_frames_val[frame / 3] = I915_READ(PSR2_SU_STATUS(frame));
+ for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) {
+ val = I915_READ(PSR2_SU_STATUS(dev_priv->psr.transcoder,
+ frame));
+ su_frames_val[frame / 3] = val;
+ }
seq_puts(m, "Frame:\tPSR2 SU blocks:\n");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc909ec5d9c3..2a8a8036facb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -504,6 +504,7 @@ struct i915_psr {
bool enabled;
struct intel_dp *dp;
enum pipe pipe;
+ enum transcoder transcoder;
bool active;
struct work_struct work;
unsigned busy_frontbuffer_bits;
@@ -1367,8 +1368,6 @@ struct drm_i915_private {
/* MMIO base address for MIPI regs */
u32 mipi_mmio_base;
- u32 psr_mmio_base;
-
u32 pps_mmio_base;
wait_queue_head_t gmbus_wait_queue;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4fc8dc083766..31fb4de5081c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4225,10 +4225,18 @@ enum {
#define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
#define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
-/* HSW+ eDP PSR registers */
-#define HSW_EDP_PSR_BASE 0x64800
-#define BDW_EDP_PSR_BASE 0x6f800
-#define EDP_PSR_CTL _MMIO(dev_priv->psr_mmio_base + 0)
+/*
+ * HSW+ eDP PSR registers
+ *
+ * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) with just one
+ * instance of it
+ */
+#define _HSW_EDP_PSR_BASE 0x64800
+#define _SRD_CTL_A 0x60800
+#define _SRD_CTL_EDP 0x6f800
+#define _HSW_PSR_ADJ(reg) ((reg) - _SRD_CTL_A + _HSW_EDP_PSR_BASE)
+#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev_priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
+#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_CTL_A))
#define EDP_PSR_ENABLE (1 << 31)
#define BDW_PSR_SINGLE_FRAME (1 << 30)
#define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW can't modify */
@@ -4265,16 +4273,22 @@ enum {
#define EDP_PSR_TRANSCODER_A_SHIFT 8
#define EDP_PSR_TRANSCODER_EDP_SHIFT 0
-#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10)
+#define _SRD_AUX_CTL_A 0x60810
+#define _SRD_AUX_CTL_EDP 0x6f810
+#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_AUX_CTL_A))
#define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
#define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20)
#define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16)
#define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11)
#define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff)
-#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
+#define _SRD_AUX_DATA_A 0x60814
+#define _SRD_AUX_DATA_EDP 0x6f814
+#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */
-#define EDP_PSR_STATUS _MMIO(dev_priv->psr_mmio_base + 0x40)
+#define _SRD_STATUS_A 0x60840
+#define _SRD_STATUS_EDP 0x6f840
+#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ(tran, _SRD_STATUS_A))
#define EDP_PSR_STATUS_STATE_MASK (7 << 29)
#define EDP_PSR_STATUS_STATE_SHIFT 29
#define EDP_PSR_STATUS_STATE_IDLE (0 << 29)
@@ -4299,10 +4313,15 @@ enum {
#define EDP_PSR_STATUS_SENDING_TP1 (1 << 4)
#define EDP_PSR_STATUS_IDLE_MASK 0xf
-#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + 0x44)
+#define _SRD_PERF_CNT_A 0x60844
+#define _SRD_PERF_CNT_EDP 0x6f844
+#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, _SRD_PERF_CNT_A))
#define EDP_PSR_PERF_CNT_MASK 0xffffff
-#define EDP_PSR_DEBUG _MMIO(dev_priv->psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */
+/* PSR_MASK on SKL+ */
+#define _SRD_DEBUG_A 0x60860
+#define _SRD_DEBUG_EDP 0x6f860
+#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ(tran, _SRD_DEBUG_A))
#define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28)
#define EDP_PSR_DEBUG_MASK_LPSP (1 << 27)
#define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26)
@@ -4310,7 +4329,9 @@ enum {
#define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */
#define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
-#define EDP_PSR2_CTL _MMIO(0x6f900)
+#define _PSR2_CTL_A 0x60900
+#define _PSR2_CTL_EDP 0x6f900
+#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, _PSR2_CTL_A)
#define EDP_PSR2_ENABLE (1 << 31)
#define EDP_SU_TRACK_ENABLE (1 << 30)
#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */
@@ -4332,8 +4353,8 @@ enum {
#define _PSR_EVENT_TRANS_B 0x61848
#define _PSR_EVENT_TRANS_C 0x62848
#define _PSR_EVENT_TRANS_D 0x63848
-#define _PSR_EVENT_TRANS_EDP 0x6F848
-#define PSR_EVENT(trans) _MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
+#define _PSR_EVENT_TRANS_EDP 0x6f848
+#define PSR_EVENT(tran) _MMIO_TRANS2(tran, _PSR_EVENT_TRANS_A)
#define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
#define PSR_EVENT_PSR2_DISABLED (1 << 16)
#define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
@@ -4351,15 +4372,16 @@ enum {
#define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
#define PSR_EVENT_PSR_DISABLE (1 << 0)
-#define EDP_PSR2_STATUS _MMIO(0x6f940)
+#define _PSR2_STATUS_A 0x60940
+#define _PSR2_STATUS_EDP 0x6f940
+#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, _PSR2_STATUS_A)
#define EDP_PSR2_STATUS_STATE_MASK (0xf << 28)
#define EDP_PSR2_STATUS_STATE_SHIFT 28
-#define _PSR2_SU_STATUS_0 0x6F914
-#define _PSR2_SU_STATUS_1 0x6F918
-#define _PSR2_SU_STATUS_2 0x6F91C
-#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
-#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame) / 3))
+#define _PSR2_SU_STATUS_A 0x60914
+#define _PSR2_SU_STATUS_EDP 0x6f914
+#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, _PSR2_SU_STATUS_A) + (index) * 4)
+#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, (frame) / 3))
#define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10)
#define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame))
#define PSR2_SU_STATUS_FRAMES 8
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v6 4/4] drm/i915: Add transcoder restriction to PSR2
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 2/4] drm/i915: Add _TRANS2() José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders José Roberto de Souza
@ 2019-06-19 23:02 ` José Roberto de Souza
2019-06-19 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: José Roberto de Souza @ 2019-06-19 23:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
According to PSR2_CTL definition on BSpec there is only one instance
of PSR2_CTL also ICL display overview state that PSR2 is only
supported in EDP transcoder, so now that is possible to have PSR in
any transcoder lets add this hardware restriction.
BSpec: 7713
BSpec: 20584
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index ed422b788d88..da56b8b1b9b9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -545,6 +545,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
if (!dev_priv->psr.sink_psr2_support)
return false;
+ if (crtc_state->cpu_transcoder != TRANSCODER_EDP) {
+ DRM_DEBUG_KMS("PSR2 is only supported in EDP transcoder\n");
+ return false;
+ }
+
/*
* DSC and PSR2 cannot be enabled simultaneously. If a requested
* resolution requires DSC to be enabled, priority is given to DSC
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
` (2 preceding siblings ...)
2019-06-19 23:02 ` [PATCH v6 4/4] drm/i915: Add transcoder restriction to PSR2 José Roberto de Souza
@ 2019-06-19 23:36 ` Patchwork
2019-06-19 23:56 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-20 15:15 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-06-19 23:36 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL : https://patchwork.freedesktop.org/series/62416/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
2622a5f202e3 drm/i915/bdw+: Move misc display IRQ handling to it own function
1ecf05a786ab drm/i915: Add _TRANS2()
-:31: WARNING:LONG_LINE: line over 100 characters
#31: FILE: drivers/gpu/drm/i915/i915_reg.h:254:
+ INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \
total: 0 errors, 1 warnings, 0 checks, 13 lines checked
52333122c0fb drm/i915/psr: Make PSR registers relative to transcoders
-:375: WARNING:LONG_LINE: line over 100 characters
#375: FILE: drivers/gpu/drm/i915/i915_reg.h:4238:
+#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev_priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
-:375: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'reg' - possible side-effects?
#375: FILE: drivers/gpu/drm/i915/i915_reg.h:4238:
+#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev_priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
-:397: WARNING:LONG_LINE_COMMENT: line over 100 characters
#397: FILE: drivers/gpu/drm/i915/i915_reg.h:4287:
+#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */
total: 0 errors, 2 warnings, 1 checks, 366 lines checked
413c4f5206bb drm/i915: Add transcoder restriction to PSR2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
` (3 preceding siblings ...)
2019-06-19 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function Patchwork
@ 2019-06-19 23:56 ` Patchwork
2019-06-20 15:15 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-06-19 23:56 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL : https://patchwork.freedesktop.org/series/62416/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6312 -> Patchwork_13355
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/
Known issues
------------
Here are the changes found in Patchwork_13355 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_busy@busy-all:
- fi-icl-u3: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/fi-icl-u3/igt@gem_busy@busy-all.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/fi-icl-u3/igt@gem_busy@busy-all.html
* igt@gem_ctx_create@basic-files:
- fi-icl-guc: [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#109100])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/fi-icl-guc/igt@gem_ctx_create@basic-files.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/fi-icl-guc/igt@gem_ctx_create@basic-files.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][5] -> [DMESG-WARN][6] ([fdo#102614])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
#### Possible fixes ####
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][7] ([fdo#109485]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
Participating hosts (49 -> 42)
------------------------------
Additional (4): fi-cml-u2 fi-bxt-j4205 fi-gdg-551 fi-cml-u
Missing (11): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6312 -> Patchwork_13355
CI_DRM_6312: 034e3ac6a2d180d188da927388b60c7e62c5655b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5061: c88ced79a7b71aec58f1d9c5c599ac2f431bcf7a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13355: 413c4f5206bbad8c1ec87fbee44d6013659e600d @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
413c4f5206bb drm/i915: Add transcoder restriction to PSR2
52333122c0fb drm/i915/psr: Make PSR registers relative to transcoders
1ecf05a786ab drm/i915: Add _TRANS2()
2622a5f202e3 drm/i915/bdw+: Move misc display IRQ handling to it own function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-19 23:02 ` [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders José Roberto de Souza
@ 2019-06-20 8:13 ` Jani Nikula
2019-06-20 17:36 ` Souza, Jose
0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2019-06-20 8:13 UTC (permalink / raw)
To: José Roberto de Souza, intel-gfx; +Cc: Dhinakaran Pandiyan
On Wed, 19 Jun 2019, José Roberto de Souza <jose.souza@intel.com> wrote:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4fc8dc083766..31fb4de5081c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4225,10 +4225,18 @@ enum {
> #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
> #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
>
> -/* HSW+ eDP PSR registers */
> -#define HSW_EDP_PSR_BASE 0x64800
> -#define BDW_EDP_PSR_BASE 0x6f800
> -#define EDP_PSR_CTL _MMIO(dev_priv->psr_mmio_base + 0)
> +/*
> + * HSW+ eDP PSR registers
> + *
> + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) with just one
> + * instance of it
> + */
> +#define _HSW_EDP_PSR_BASE 0x64800
> +#define _SRD_CTL_A 0x60800
> +#define _SRD_CTL_EDP 0x6f800
> +#define _HSW_PSR_ADJ(reg) ((reg) - _SRD_CTL_A + _HSW_EDP_PSR_BASE)
> +#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev_priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_CTL_A))
There are currently three instances of platform/gen checks in
i915_reg.h. They are the exception, and they're in individual macros
that aren't even register offset definitions let alone helpers that get
proliferated to several other macros.
This change here is quite a big precedent in that regard, and not to be
done lightly. Usually the case is others will follow suit, so this is
not just about this one instance. It's about deciding whether this is
the direction we want to take. How far are we prepared to go and how do
we stop there?
There's really no way to set the psr->transcoder such on HSW that it
would work with MMIO_TRANS2()?
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
` (4 preceding siblings ...)
2019-06-19 23:56 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-06-20 15:15 ` Patchwork
5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-06-20 15:15 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL : https://patchwork.freedesktop.org/series/62416/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6312_full -> Patchwork_13355_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_13355_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@in-flight-contexts-1us:
- shard-kbl: [PASS][1] -> [DMESG-WARN][2] ([fdo#110913 ]) +2 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-kbl6/igt@gem_eio@in-flight-contexts-1us.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-kbl6/igt@gem_eio@in-flight-contexts-1us.html
* igt@gem_eio@in-flight-suspend:
- shard-kbl: [PASS][3] -> [FAIL][4] ([fdo#110667])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-kbl3/igt@gem_eio@in-flight-suspend.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-kbl2/igt@gem_eio@in-flight-suspend.html
* igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
- shard-snb: [PASS][5] -> [DMESG-WARN][6] ([fdo#110789] / [fdo#110913 ])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-snb6/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-snb5/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
* igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#110913 ])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-apl8/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-apl6/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
* igt@gem_tiled_swapping@non-threaded:
- shard-hsw: [PASS][9] -> [FAIL][10] ([fdo#108686])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-hsw8/igt@gem_tiled_swapping@non-threaded.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-hsw8/igt@gem_tiled_swapping@non-threaded.html
* igt@i915_pm_rc6_residency@rc6-accuracy:
- shard-snb: [PASS][11] -> [SKIP][12] ([fdo#109271])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-snb1/igt@i915_pm_rc6_residency@rc6-accuracy.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-snb6/igt@i915_pm_rc6_residency@rc6-accuracy.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw: [PASS][13] -> [FAIL][14] ([fdo#105767])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
* igt@kms_flip@2x-plain-flip-ts-check-interruptible:
- shard-hsw: [PASS][15] -> [SKIP][16] ([fdo#109271]) +38 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-hsw6/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-hsw1/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
- shard-skl: [PASS][17] -> [FAIL][18] ([fdo#108040])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: [PASS][19] -> [INCOMPLETE][20] ([fdo#104108] / [fdo#106978])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl5/igt@kms_frontbuffer_tracking@psr-suspend.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl2/igt@kms_frontbuffer_tracking@psr-suspend.html
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
- shard-skl: [PASS][21] -> [FAIL][22] ([fdo#103191])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl3/igt@kms_pipe_crc_basic@hang-read-crc-pipe-c.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl7/igt@kms_pipe_crc_basic@hang-read-crc-pipe-c.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- shard-apl: [PASS][23] -> [DMESG-WARN][24] ([fdo#108566]) +3 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
* igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: [PASS][25] -> [FAIL][26] ([fdo#108145]) +2 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][27] -> [FAIL][28] ([fdo#99912])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-apl6/igt@kms_setmode@basic.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-apl8/igt@kms_setmode@basic.html
#### Possible fixes ####
* igt@gem_eio@wait-10ms:
- shard-apl: [DMESG-WARN][29] ([fdo#110913 ]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-apl8/igt@gem_eio@wait-10ms.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-apl5/igt@gem_eio@wait-10ms.html
* igt@gem_persistent_relocs@forked-interruptible-thrashing:
- shard-snb: [DMESG-WARN][31] ([fdo#110789] / [fdo#110913 ]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-snb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
* igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
- shard-kbl: [DMESG-WARN][33] ([fdo#110913 ]) -> [PASS][34] +1 similar issue
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-kbl1/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-kbl7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
* igt@kms_cursor_edge_walk@pipe-b-128x128-top-edge:
- shard-snb: [SKIP][35] ([fdo#109271] / [fdo#109278]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-snb2/igt@kms_cursor_edge_walk@pipe-b-128x128-top-edge.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-snb2/igt@kms_cursor_edge_walk@pipe-b-128x128-top-edge.html
* igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
- shard-hsw: [SKIP][37] ([fdo#109271]) -> [PASS][38] +33 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-hsw1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-hsw8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-apl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
- shard-skl: [INCOMPLETE][41] ([fdo#104108]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
* igt@kms_plane@plane-panning-bottom-right-pipe-b-planes:
- shard-snb: [SKIP][43] ([fdo#109271]) -> [PASS][44] +1 similar issue
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-snb2/igt@kms_plane@plane-panning-bottom-right-pipe-b-planes.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-snb2/igt@kms_plane@plane-panning-bottom-right-pipe-b-planes.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][45] ([fdo#99912]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-kbl1/igt@kms_setmode@basic.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-kbl2/igt@kms_setmode@basic.html
#### Warnings ####
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt:
- shard-skl: [FAIL][47] ([fdo#103167]) -> [FAIL][48] ([fdo#108040])
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6312/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
[fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
[fdo#110913 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110913
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-iclb
Build changes
-------------
* Linux: CI_DRM_6312 -> Patchwork_13355
CI_DRM_6312: 034e3ac6a2d180d188da927388b60c7e62c5655b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5061: c88ced79a7b71aec58f1d9c5c599ac2f431bcf7a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13355: 413c4f5206bbad8c1ec87fbee44d6013659e600d @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13355/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-20 8:13 ` Jani Nikula
@ 2019-06-20 17:36 ` Souza, Jose
2019-06-24 21:11 ` Souza, Jose
0 siblings, 1 reply; 13+ messages in thread
From: Souza, Jose @ 2019-06-20 17:36 UTC (permalink / raw)
To: intel-gfx, jani.nikula; +Cc: Pandiyan, Dhinakaran
On Thu, 2019-06-20 at 11:13 +0300, Jani Nikula wrote:
> On Wed, 19 Jun 2019, José Roberto de Souza <jose.souza@intel.com>
> wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 4fc8dc083766..31fb4de5081c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4225,10 +4225,18 @@ enum {
> > #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
> > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
> >
> > -/* HSW+ eDP PSR registers */
> > -#define HSW_EDP_PSR_BASE 0x64800
> > -#define BDW_EDP_PSR_BASE 0x6f800
> > -#define EDP_PSR_CTL _MMIO(dev_priv-
> > >psr_mmio_base + 0)
> > +/*
> > + * HSW+ eDP PSR registers
> > + *
> > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800)
> > with just one
> > + * instance of it
> > + */
> > +#define _HSW_EDP_PSR_BASE 0x64800
> > +#define _SRD_CTL_A 0x60800
> > +#define _SRD_CTL_EDP 0x6f800
> > +#define _HSW_PSR_ADJ(reg) ((reg) - _SRD_CTL_A +
> > _HSW_EDP_PSR_BASE)
> > +#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev
> > _priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran,
> > _SRD_CTL_A))
>
> There are currently three instances of platform/gen checks in
> i915_reg.h. They are the exception, and they're in individual macros
> that aren't even register offset definitions let alone helpers that
> get
> proliferated to several other macros.
>
> This change here is quite a big precedent in that regard, and not to
> be
> done lightly. Usually the case is others will follow suit, so this is
> not just about this one instance. It's about deciding whether this is
> the direction we want to take. How far are we prepared to go and how
> do
> we stop there?
>
> There's really no way to set the psr->transcoder such on HSW that it
> would work with MMIO_TRANS2()?
I'm going to think about but right now the only other option that comes
in my mind is to have the transcoder offset as macro parameter:
#define _SRD_CTL 0x800
#define EDP_PSR_CTL(trans) _MMIO(trans + _SRD_CTL)
But we would lose the full offset address of PSR registers.
>
> BR,
> Jani.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-20 17:36 ` Souza, Jose
@ 2019-06-24 21:11 ` Souza, Jose
2019-06-29 2:25 ` Dhinakaran Pandiyan
0 siblings, 1 reply; 13+ messages in thread
From: Souza, Jose @ 2019-06-24 21:11 UTC (permalink / raw)
To: intel-gfx, jani.nikula; +Cc: Pandiyan, Dhinakaran
On Thu, 2019-06-20 at 17:36 +0000, Souza, Jose wrote:
> On Thu, 2019-06-20 at 11:13 +0300, Jani Nikula wrote:
> > On Wed, 19 Jun 2019, José Roberto de Souza <jose.souza@intel.com>
> > wrote:
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 4fc8dc083766..31fb4de5081c 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4225,10 +4225,18 @@ enum {
> > > #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
> > > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
> > >
> > > -/* HSW+ eDP PSR registers */
> > > -#define HSW_EDP_PSR_BASE 0x64800
> > > -#define BDW_EDP_PSR_BASE 0x6f800
> > > -#define EDP_PSR_CTL _MMIO(dev_priv-
> > > > psr_mmio_base + 0)
> > > +/*
> > > + * HSW+ eDP PSR registers
> > > + *
> > > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A +
> > > 0x800)
> > > with just one
> > > + * instance of it
> > > + */
> > > +#define _HSW_EDP_PSR_BASE 0x64800
> > > +#define _SRD_CTL_A 0x60800
> > > +#define _SRD_CTL_EDP 0x6f800
> > > +#define _HSW_PSR_ADJ(reg) ((reg) -
> > > _SRD_CTL_A +
> > > _HSW_EDP_PSR_BASE)
> > > +#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev
> > > _priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> > > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(
> > > tran,
> > > _SRD_CTL_A))
> >
> > There are currently three instances of platform/gen checks in
> > i915_reg.h. They are the exception, and they're in individual
> > macros
> > that aren't even register offset definitions let alone helpers that
> > get
> > proliferated to several other macros.
> >
> > This change here is quite a big precedent in that regard, and not
> > to
> > be
> > done lightly. Usually the case is others will follow suit, so this
> > is
> > not just about this one instance. It's about deciding whether this
> > is
> > the direction we want to take. How far are we prepared to go and
> > how
> > do
> > we stop there?
> >
> > There's really no way to set the psr->transcoder such on HSW that
> > it
> > would work with MMIO_TRANS2()?
>
> I'm going to think about but right now the only other option that
> comes
> in my mind is to have the transcoder offset as macro parameter:
>
> #define _SRD_CTL 0x800
> #define EDP_PSR_CTL(trans) _MMIO(trans + _SRD_CTL)
>
> But we would lose the full offset address of PSR registers.
This is the only other good option that I can think about.
Any other idea DK?
>
> > BR,
> > Jani.
> >
> _______________________________________________
> 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] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-24 21:11 ` Souza, Jose
@ 2019-06-29 2:25 ` Dhinakaran Pandiyan
2019-07-01 22:26 ` Souza, Jose
0 siblings, 1 reply; 13+ messages in thread
From: Dhinakaran Pandiyan @ 2019-06-29 2:25 UTC (permalink / raw)
To: Souza, Jose, intel-gfx, jani.nikula
On Mon, 2019-06-24 at 14:11 -0700, Souza, Jose wrote:
> > > > +#define _HSW_EDP_PSR_BASE 0x64800
> > > > +#define _SRD_CTL_A 0x60800
> > > > +#define _SRD_CTL_EDP 0x6f800
> > > > +#define _HSW_PSR_ADJ(reg) ((reg) -
> > > > _SRD_CTL_A +
> > > > _HSW_EDP_PSR_BASE)
> > > > +#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev
> > > > _priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> > > > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(
> > > > tran,
> > > > _SRD_CTL_A))
> > >
> > > There are currently three instances of platform/gen checks in
> > > i915_reg.h. They are the exception, and they're in individual
> > > macros
> > > that aren't even register offset definitions let alone helpers that
> > > get
> > > proliferated to several other macros.
> > >
> > > This change here is quite a big precedent in that regard, and not
> > > to
> > > be
> > > done lightly. Usually the case is others will follow suit, so this
> > > is
> > > not just about this one instance. It's about deciding whether this
> > > is
> > > the direction we want to take. How far are we prepared to go and
> > > how
> > > do
> > > we stop there?
> > >
> > > There's really no way to set the psr->transcoder such on HSW that
> > > it
> > > would work with MMIO_TRANS2()?
> >
> > I'm going to think about but right now the only other option that
> > comes
> > in my mind is to have the transcoder offset as macro parameter:
> >
> > #define _SRD_CTL 0x800
> > #define EDP_PSR_CTL(trans) _MMIO(trans + _SRD_CTL)
> >
> > But we would lose the full offset address of PSR registers.
>
> This is the only other good option that I can think about.
>
> Any other idea DK?
No good ones unfortunately. This is the simplest one I could think of
intel_psr_init()
{
...
if(IS_HASWELL(dev_priv))
dev_priv->psr.hsw_adjust = _SRD_CTL_EDP - _HSW_EDP_PSR_BASE;
...
}
#define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg - dev_priv.psr.hsw_adjust)
#define EDP_PSR_CTL(tran) _MMIO_PSR(tran, _SRD_CTL_A)
should work because tran == TRANSCODER_EDP for HSW
-DK
BR,
Jani.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-06-29 2:25 ` Dhinakaran Pandiyan
@ 2019-07-01 22:26 ` Souza, Jose
2019-07-02 0:49 ` Souza, Jose
0 siblings, 1 reply; 13+ messages in thread
From: Souza, Jose @ 2019-07-01 22:26 UTC (permalink / raw)
To: ville.syrjala, intel-gfx, Pandiyan, Dhinakaran, jani.nikula
On Fri, 2019-06-28 at 19:25 -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2019-06-24 at 14:11 -0700, Souza, Jose wrote:
> > > > > +#define _HSW_EDP_PSR_BASE 0x64800
> > > > > +#define _SRD_CTL_A 0x60800
> > > > > +#define _SRD_CTL_EDP 0x6f800
> > > > > +#define _HSW_PSR_ADJ(reg) ((reg) -
> > > > > _SRD_CTL_A +
> > > > > _HSW_EDP_PSR_BASE)
> > > > > +#define _PSR_ADJ(tran,
> > > > > reg) (IS_HASWELL(dev
> > > > > _priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> > > > > +#define
> > > > > EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(
> > > > > tran,
> > > > > _SRD_CTL_A))
> > > >
> > > > There are currently three instances of platform/gen checks in
> > > > i915_reg.h. They are the exception, and they're in individual
> > > > macros
> > > > that aren't even register offset definitions let alone helpers
> > > > that
> > > > get
> > > > proliferated to several other macros.
> > > >
> > > > This change here is quite a big precedent in that regard, and
> > > > not
> > > > to
> > > > be
> > > > done lightly. Usually the case is others will follow suit, so
> > > > this
> > > > is
> > > > not just about this one instance. It's about deciding whether
> > > > this
> > > > is
> > > > the direction we want to take. How far are we prepared to go
> > > > and
> > > > how
> > > > do
> > > > we stop there?
> > > >
> > > > There's really no way to set the psr->transcoder such on HSW
> > > > that
> > > > it
> > > > would work with MMIO_TRANS2()?
> > >
> > > I'm going to think about but right now the only other option that
> > > comes
> > > in my mind is to have the transcoder offset as macro parameter:
> > >
> > > #define _SRD_CTL 0x800
> > > #define EDP_PSR_CTL(trans) _MMIO(trans + _SRD_CTL)
> > >
> > > But we would lose the full offset address of PSR registers.
> >
> > This is the only other good option that I can think about.
> >
> > Any other idea DK?
> No good ones unfortunately. This is the simplest one I could think of
>
> intel_psr_init()
> {
> ...
> if(IS_HASWELL(dev_priv))
> dev_priv->psr.hsw_adjust = _SRD_CTL_EDP - _HSW_EDP_PSR_BASE;
> ...
> }
>
>
> #define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg -
> dev_priv.psr.hsw_adjust)
> #define EDP_PSR_CTL(tran) _MMIO_PSR(tran, _SRD_CTL_A)
>
>
> should work because tran == TRANSCODER_EDP for HSW
The problem with this suggestion is that it would require more changes
to support multiple PSR instances in future, unless we make it required
to have struct intel_psr *psr defined like struct drm_i915_private
*dev_priv is required by I915_WRITE/READ().
#define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg - psr->hsw_adjust)
If chosen this approach we could already complete remove the tran
parameter:
#define _MMIO_PSR(reg) _MMIO_TRANS2(psr->transcoder, reg - psr-
>hsw_adjust)
#
So we have 4 options:
1 - The one implemented in this patch
2 - Offset as parameter
#define _SRD_CTL 0x800
#define EDP_PSR_CTL(trans_offset) _MMIO(trans_offset +
_SRD_CTL)
3 - DKs suggestion with one of the suggestions above to support
multiple PSR instances
4 - Have HSW PSR specific macros and have several if (IS_HASWELL())
spread to PSR code
My vote is option 1.
Please let me know your thoughts?
>
>
> -DK
>
> BR,
> Jani.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders
2019-07-01 22:26 ` Souza, Jose
@ 2019-07-02 0:49 ` Souza, Jose
0 siblings, 0 replies; 13+ messages in thread
From: Souza, Jose @ 2019-07-02 0:49 UTC (permalink / raw)
To: ville.syrjala, intel-gfx, Pandiyan, Dhinakaran, jani.nikula
On Mon, 2019-07-01 at 22:26 +0000, Souza, Jose wrote:
> On Fri, 2019-06-28 at 19:25 -0700, Dhinakaran Pandiyan wrote:
> > On Mon, 2019-06-24 at 14:11 -0700, Souza, Jose wrote:
> > > > > > +#define _HSW_EDP_PSR_BASE 0x64800
> > > > > > +#define _SRD_CTL_A 0x60800
> > > > > > +#define _SRD_CTL_EDP 0x6f800
> > > > > > +#define _HSW_PSR_ADJ(reg) ((reg) -
> > > > > > _SRD_CTL_A +
> > > > > > _HSW_EDP_PSR_BASE)
> > > > > > +#define _PSR_ADJ(tran,
> > > > > > reg) (IS_HASWELL(dev
> > > > > > _priv) ? _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> > > > > > +#define
> > > > > > EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(
> > > > > > tran,
> > > > > > _SRD_CTL_A))
> > > > >
> > > > > There are currently three instances of platform/gen checks in
> > > > > i915_reg.h. They are the exception, and they're in individual
> > > > > macros
> > > > > that aren't even register offset definitions let alone
> > > > > helpers
> > > > > that
> > > > > get
> > > > > proliferated to several other macros.
> > > > >
> > > > > This change here is quite a big precedent in that regard, and
> > > > > not
> > > > > to
> > > > > be
> > > > > done lightly. Usually the case is others will follow suit, so
> > > > > this
> > > > > is
> > > > > not just about this one instance. It's about deciding whether
> > > > > this
> > > > > is
> > > > > the direction we want to take. How far are we prepared to go
> > > > > and
> > > > > how
> > > > > do
> > > > > we stop there?
> > > > >
> > > > > There's really no way to set the psr->transcoder such on HSW
> > > > > that
> > > > > it
> > > > > would work with MMIO_TRANS2()?
> > > >
> > > > I'm going to think about but right now the only other option
> > > > that
> > > > comes
> > > > in my mind is to have the transcoder offset as macro parameter:
> > > >
> > > > #define _SRD_CTL 0x800
> > > > #define EDP_PSR_CTL(trans) _MMIO(trans + _SRD_CTL)
> > > >
> > > > But we would lose the full offset address of PSR registers.
> > >
> > > This is the only other good option that I can think about.
> > >
> > > Any other idea DK?
> > No good ones unfortunately. This is the simplest one I could think
> > of
> >
> > intel_psr_init()
> > {
> > ...
> > if(IS_HASWELL(dev_priv))
> > dev_priv->psr.hsw_adjust = _SRD_CTL_EDP - _HSW_EDP_PSR_BASE;
> > ...
> > }
> >
> >
> > #define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg -
> > dev_priv.psr.hsw_adjust)
> > #define EDP_PSR_CTL(tran) _MMIO_PSR(tran, _SRD_CTL_A)
> >
> >
> > should work because tran == TRANSCODER_EDP for HSW
>
> The problem with this suggestion is that it would require more
> changes
> to support multiple PSR instances in future, unless we make it
> required
> to have struct intel_psr *psr defined like struct drm_i915_private
> *dev_priv is required by I915_WRITE/READ().
>
>
> #define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg - psr-
> >hsw_adjust)
>
> If chosen this approach we could already complete remove the tran
> parameter:
>
> #define _MMIO_PSR(reg) _MMIO_TRANS2(psr->transcoder, reg - psr-
> > hsw_adjust)
Other option here:
#define _MMIO_PSR(tran, reg) _MMIO_TRANS2(tran, reg -
dev_priv.psr_hsw_adjust)
As HSW can only have one PSR instance.
>
> #
>
> So we have 4 options:
>
> 1 - The one implemented in this patch
>
> 2 - Offset as parameter
> #define _SRD_CTL 0x800
> #define EDP_PSR_CTL(trans_offset) _MMIO(trans_offset +
> _SRD_CTL)
>
> 3 - DKs suggestion with one of the suggestions above to support
> multiple PSR instances
>
> 4 - Have HSW PSR specific macros and have several if (IS_HASWELL())
> spread to PSR code
>
>
> My vote is option 1.
>
> Please let me know your thoughts?
>
>
> >
> > -DK
> >
> > BR,
> > Jani.
> >
> _______________________________________________
> 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] 13+ messages in thread
end of thread, other threads:[~2019-07-02 0:49 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 23:02 [PATCH v6 1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 2/4] drm/i915: Add _TRANS2() José Roberto de Souza
2019-06-19 23:02 ` [PATCH v6 3/4] drm/i915/psr: Make PSR registers relative to transcoders José Roberto de Souza
2019-06-20 8:13 ` Jani Nikula
2019-06-20 17:36 ` Souza, Jose
2019-06-24 21:11 ` Souza, Jose
2019-06-29 2:25 ` Dhinakaran Pandiyan
2019-07-01 22:26 ` Souza, Jose
2019-07-02 0:49 ` Souza, Jose
2019-06-19 23:02 ` [PATCH v6 4/4] drm/i915: Add transcoder restriction to PSR2 José Roberto de Souza
2019-06-19 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,1/4] drm/i915/bdw+: Move misc display IRQ handling to it own function Patchwork
2019-06-19 23:56 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-20 15:15 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.