From: Anshuman Gupta <anshuman.gupta@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>, jose.souza@intel.com
Cc: intel-gfx@lists.freedesktop.org,
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH v3 08/23] drm/i915/tgl: Access the right register when handling PSR interruptions
Date: Mon, 26 Aug 2019 15:23:55 +0530 [thread overview]
Message-ID: <20190826095354.GE30506@intel.com> (raw)
In-Reply-To: <20190823082055.5992-9-lucas.demarchi@intel.com>
On 2019-08-23 at 01:20:40 -0700, Lucas De Marchi wrote:
> From: José Roberto de Souza <jose.souza@intel.com>
>
> For older gens PSR IIR and IMR had a fixed address that was not
> relative to anything, but from TGL those registers moved to each
> transcoder offset.
>
> So here adding a new macro and a new PSR irq handler with the
> transcoder parameter.
>
There are few minor comments below, apart from below comments
patch is looks ok to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> 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>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 67 ++++++++++++++++++------
> drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 52 +++++++++++++++---
> drivers/gpu/drm/i915/i915_reg.h | 10 +++-
> 4 files changed, 107 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 2429328f963e..c33aa16ed038 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -91,20 +91,33 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
> static void psr_irq_control(struct drm_i915_private *dev_priv)
> {
> enum transcoder trans = dev_priv->psr.transcoder;
> - u32 val, mask;
> + u32 psr_error, psr_entry, psr_exit, mask, val;
> + i915_reg_t mask_reg;
> +
> + if (INTEL_GEN(dev_priv) >= 12) {
> + psr_error = TRANS_PSR_ERROR;
> + psr_entry = TRANS_PSR_PRE_ENTRY;
> + psr_exit = TRANS_PSR_POST_EXIT;
> + mask_reg = TRANS_PSR_IMR(trans);
> + } else {
> + psr_error = EDP_PSR_ERROR(trans);
> + psr_entry = EDP_PSR_PRE_ENTRY(trans);
> + psr_exit = EDP_PSR_POST_EXIT(trans);
> + mask_reg = EDP_PSR_IMR;
> + }
>
> - mask = EDP_PSR_ERROR(trans);
> + mask = psr_error;
> if (dev_priv->psr.debug & I915_PSR_DEBUG_IRQ)
> - mask |= EDP_PSR_POST_EXIT(trans) | EDP_PSR_PRE_ENTRY(trans);
> + mask |= psr_exit | psr_entry;
>
> /*
> * TODO: when handling multiple PSR instances a global spinlock will be
> * needed to synchronize the value of shared register
> */
> - val = I915_READ(EDP_PSR_IMR);
> - val &= ~EDP_PSR_TRANS_MASK(trans);
> + val = I915_READ(mask_reg);
> + val &= ~(psr_error | psr_entry | psr_exit);
> val |= ~mask;
> - I915_WRITE(EDP_PSR_IMR, val);
> + I915_WRITE(mask_reg, val);
> }
>
> static void psr_event_print(u32 val, bool psr2_enabled)
> @@ -147,9 +160,21 @@ static void psr_event_print(u32 val, bool psr2_enabled)
> void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> {
> enum transcoder cpu_transcoder = dev_priv->psr.transcoder;
> + u32 psr_error, psr_entry, psr_exit;
> ktime_t time_ns = ktime_get();
>
> - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> + if (INTEL_GEN(dev_priv) >= 12) {
> + psr_error = TRANS_PSR_ERROR;
> + psr_entry = TRANS_PSR_PRE_ENTRY;
> + psr_exit = TRANS_PSR_POST_EXIT;
> + } else {
> + psr_error = EDP_PSR_ERROR(cpu_transcoder);
> + psr_entry = EDP_PSR_PRE_ENTRY(cpu_transcoder);
> + psr_exit = EDP_PSR_POST_EXIT(cpu_transcoder);
> + }
> +
> + if (psr_iir & psr_error) {
> + i915_reg_t mask_reg;
> u32 val;
>
> DRM_WARN("[transcoder %s] PSR aux error\n",
> @@ -168,20 +193,25 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> * TODO: when handling multiple PSR instances a global spinlock
> * will be needed to synchronize the value of shared register
> */
> - val = I915_READ(EDP_PSR_IMR);
> - val |= EDP_PSR_ERROR(cpu_transcoder);
> - I915_WRITE(EDP_PSR_IMR, val);
> + if (INTEL_GEN(dev_priv) >= 12)
> + mask_reg = TRANS_PSR_IMR(cpu_transcoder);
> + else
> + mask_reg = EDP_PSR_IMR;
> +
> + val = I915_READ(mask_reg);
> + val |= psr_error;
> + I915_WRITE(mask_reg, val);
>
> schedule_work(&dev_priv->psr.work);
> }
>
> - if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> + if (psr_iir & psr_entry) {
> 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 & psr_exit) {
> dev_priv->psr.last_exit = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> transcoder_name(cpu_transcoder));
> @@ -632,7 +662,7 @@ 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 &&
> - psr2_supported(dev_priv, dev_priv->psr.transcoder))
> + transcoder_has_psr2(dev_priv, dev_priv->psr.transcoder))
> 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);
> @@ -730,8 +760,13 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> * 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(dev_priv->psr.transcoder);
> + if (INTEL_GEN(dev_priv) >= 12) {
> + val = I915_READ(TRANS_PSR_IIR(dev_priv->psr.transcoder));
> + val &= TRANS_PSR_ERROR;
> + } else {
> + val = I915_READ(EDP_PSR_IIR);
> + val &= EDP_PSR_ERROR(dev_priv->psr.transcoder);
> + }
> if (val) {
> dev_priv->psr.sink_not_reliable = true;
> DRM_DEBUG_KMS("PSR interruption error set, not enabling PSR\n");
> @@ -787,7 +822,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
>
> if (!dev_priv->psr.active) {
> if (INTEL_GEN(dev_priv) >= 9 &&
> - psr2_supported(dev_priv, dev_priv->psr.transcoder)) {
> + transcoder_has_psr2(dev_priv, dev_priv->psr.transcoder)) {
> val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> WARN_ON(val & EDP_PSR2_ENABLE);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 46e4de8b8cd5..6570a23a68b2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -6,6 +6,7 @@
> #ifndef __INTEL_PSR_H__
> #define __INTEL_PSR_H__
>
> +#include "intel_display.h"
IMO we don't need to include this header.
> #include "intel_frontbuffer.h"
>
> struct drm_i915_private;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 77391d8325bf..6024a6ef1c76 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2655,11 +2655,22 @@ gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
> }
>
> if (iir & GEN8_DE_EDP_PSR) {
> - u32 psr_iir = I915_READ(EDP_PSR_IIR);
> + u32 psr_iir;
> +
> + if (INTEL_GEN(dev_priv) >= 12) {
> + enum transcoder trans = dev_priv->psr.transcoder;
> +
> + psr_iir = I915_READ(TRANS_PSR_IIR(trans));
> + I915_WRITE(TRANS_PSR_IIR(trans), psr_iir);
> + } else {
> + psr_iir = I915_READ(EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IIR, psr_iir);
> + }
> +
> + if (psr_iir)
> + found = true;
>
> intel_psr_irq_handler(dev_priv, psr_iir);
> - I915_WRITE(EDP_PSR_IIR, psr_iir);
> - found = true;
> }
>
> if (!found)
> @@ -3279,8 +3290,23 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
>
> intel_uncore_write(uncore, GEN11_DISPLAY_INT_CTL, 0);
>
> - intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
> - intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
> + if (INTEL_GEN(dev_priv) >= 12) {
> + enum transcoder trans;
> +
> + for (trans = TRANSCODER_A; trans <= TRANSCODER_D; trans++) {
> + enum intel_display_power_domain domain;
> +
> + domain = POWER_DOMAIN_TRANSCODER(trans);
> + if (!intel_display_power_is_enabled(dev_priv, domain))
> + continue;
> +
> + intel_uncore_write(uncore, TRANS_PSR_IMR(trans), 0xffffffff);
> + intel_uncore_write(uncore, TRANS_PSR_IIR(trans), 0xffffffff);
Lines over 80 char.
> + }
> + } else {
> + intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
> + intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
> + }
>
> for_each_pipe(dev_priv, pipe)
> if (intel_display_power_is_enabled(dev_priv,
> @@ -3793,7 +3819,21 @@ 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(uncore, EDP_PSR_IIR);
> + if (INTEL_GEN(dev_priv) >= 12) {
> + enum transcoder trans;
> +
> + for (trans = TRANSCODER_A; trans <= TRANSCODER_D; trans++) {
> + enum intel_display_power_domain domain;
> +
> + domain = POWER_DOMAIN_TRANSCODER(trans);
> + if (!intel_display_power_is_enabled(dev_priv, domain))
> + continue;
> +
> + gen3_assert_iir_is_zero(uncore, TRANS_PSR_IIR(trans));
> + }
> + } else {
> + gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
> + }
>
> 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 1c6d99944630..3de02683d856 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4222,7 +4222,7 @@ 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 */
> +/* Bspec claims those aren't shifted but stay at 0x64800 until TGL */
> #define EDP_PSR_IMR _MMIO(0x64834)
> #define EDP_PSR_IIR _MMIO(0x64838)
> #define _EDP_PSR_TRANS_SHIFT(trans) ((trans) == TRANSCODER_EDP ? \
> @@ -4232,6 +4232,14 @@ enum {
> #define EDP_PSR_POST_EXIT(trans) (0x2 << _EDP_PSR_TRANS_SHIFT(trans))
> #define EDP_PSR_PRE_ENTRY(trans) (0x1 << _EDP_PSR_TRANS_SHIFT(trans))
>
> +#define _PSR_IMR_A 0x60814
> +#define _PSR_IIR_A 0x60818
> +#define TRANS_PSR_IMR(tran) _MMIO_TRANS2(tran, _PSR_IMR_A) /* TGL+ */
> +#define TRANS_PSR_IIR(tran) _MMIO_TRANS2(tran, _PSR_IIR_A) /* TGL+ */
IMO if /* TGL+ */ comment shifted above, it will satisfy the 80 char limit.
> +#define TRANS_PSR_ERROR (1 << 2)
> +#define TRANS_PSR_POST_EXIT (1 << 1)
> +#define TRANS_PSR_PRE_ENTRY (1 << 0)
> +
> #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))
> --
> 2.23.0
>
> _______________________________________________
> 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
next prev parent reply other threads:[~2019-08-26 9:57 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-23 8:20 [PATCH v3 00/23] Tiger Lake batch 3 Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 01/23] drm/i915/tgl: Move GTCR register to cope with GAM MMIO address remap Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 02/23] drm/i915/tgl: Enable VD HCP/MFX sub-pipe power gating Lucas De Marchi
2019-09-13 7:06 ` Chris Wilson
2019-08-23 8:20 ` [PATCH v3 03/23] drm/i915/psr: Only handle interruptions of the transcoder in use Lucas De Marchi
2019-08-26 17:28 ` Imre Deak
2019-08-27 16:50 ` Lucas De Marchi
2019-08-28 16:29 ` Imre Deak
2019-08-28 22:16 ` Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 04/23] drm/i915/bdw+: Enable PSR in any eDP port Lucas De Marchi
2019-08-26 13:41 ` Imre Deak
2019-08-26 17:43 ` Runyan, Arthur J
2019-08-27 16:36 ` Lucas De Marchi
2019-08-27 17:55 ` Souza, Jose
2019-08-23 8:20 ` [PATCH v3 05/23] drm/i915: Guard and warn if more than one eDP panel is present Lucas De Marchi
2019-08-26 6:41 ` Anshuman Gupta
2019-08-23 8:20 ` [PATCH v3 06/23] drm/i915: Do not read PSR2 register in transcoders without PSR2 Lucas De Marchi
2019-08-26 14:21 ` Imre Deak
2019-08-26 16:32 ` Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 07/23] drm/i915/tgl: PSR link standby is not supported anymore Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 08/23] drm/i915/tgl: Access the right register when handling PSR interruptions Lucas De Marchi
2019-08-26 9:53 ` Anshuman Gupta [this message]
2019-08-26 16:56 ` Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 09/23] drm/i915/tgl: Add maximum resolution supported by PSR2 HW Lucas De Marchi
2019-08-24 11:06 ` Anshuman Gupta
2019-08-26 17:10 ` Lucas De Marchi
2019-08-26 17:17 ` Souza, Jose
2019-08-26 17:29 ` Lucas De Marchi
2019-08-26 17:33 ` Gupta, Anshuman
2019-08-23 8:20 ` [PATCH v3 10/23] drm/i915: Add for_each_new_intel_connector_in_state() Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 11/23] drm: Add for_each_oldnew_intel_crtc_in_state_reverse() Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 12/23] drm/i915: Disable pipes in reverse order Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 13/23] FIXME: drm/i915/tgl: Select master transcoder in DP MST Lucas De Marchi
2019-08-23 13:02 ` Ville Syrjälä
2019-08-23 8:20 ` [PATCH v3 14/23] drm/i915/tgl: move DP_TP_* to transcoder Lucas De Marchi
2019-08-23 12:25 ` Ville Syrjälä
2019-08-23 12:39 ` Ville Syrjälä
2019-08-23 8:20 ` [PATCH v3 15/23] drm/i915/tgl: Implement TGL DisplayPort training sequence Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 16/23] drm/i915/tgl: Do not apply WaIncreaseDefaultTLBEntries from GEN12 onwards Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 17/23] FIXME: drm/i915/tgl: Register state context definition for Gen12 Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 18/23] drm/i915/tgl/perf: use the same oa ctx_id format as icl Lucas De Marchi
2019-08-23 8:30 ` Lionel Landwerlin
2019-08-23 18:16 ` Umesh Nerlige Ramappa
2019-08-23 8:20 ` [PATCH v3 19/23] drm/i915/tgl: Gen-12 display loses Yf tiling and legacy CCS support Lucas De Marchi
2019-08-28 23:04 ` Matt Roper
2019-08-28 23:59 ` Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 20/23] drm/framebuffer/tgl: Format modifier for Intel Gen-12 render compression Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 21/23] drm/i915/tgl: Gen-12 render decompression Lucas De Marchi
2019-08-29 0:33 ` Matt Roper
2019-09-13 0:31 ` Sripada, Radhakrishna
2019-08-23 8:20 ` [PATCH v3 22/23] drm/framebuffer/tgl: Format modifier for Intel Gen-12 media compression Lucas De Marchi
2019-08-23 8:20 ` [PATCH v3 23/23] drm/i915/tgl: " Lucas De Marchi
2019-08-23 13:24 ` ✗ Fi.CI.CHECKPATCH: warning for Tiger Lake batch 3 (rev5) Patchwork
2019-08-23 13:28 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-08-23 13:53 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-24 11:50 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190826095354.GE30506@intel.com \
--to=anshuman.gupta@intel.com \
--cc=dhinakaran.pandiyan@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.