diff mbox

[v3,2/4] drm/i915: Enable edp psr error interrupts on bdw+

Message ID 20180403212420.25007-2-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan April 3, 2018, 9:24 p.m. UTC 
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(-)

Comments

Souza, Jose April 5, 2018, 8:38 p.m. UTC | #1 
On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 

> Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr

> on

> any transcoder in theory, though the we don't currenty enable PSR

> except

> on the EDP transcoder.

> 

> v2: From DK

>  * Rebased on drm-tip

> v3: Switched author to Ville based on IRC discussion.

> 

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>


Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>


> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> ---

>  drivers/gpu/drm/i915/i915_irq.c      | 57

> ++++++++++++++++++++++++++++--------

>  drivers/gpu/drm/i915/i915_reg.h      |  7 +++--

>  drivers/gpu/drm/i915/intel_display.h |  4 +++

>  3 files changed, 52 insertions(+), 16 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_irq.c

> b/drivers/gpu/drm/i915/i915_irq.c

> index c2d3f30778ee..8a894adf2ca1 100644

> --- a/drivers/gpu/drm/i915/i915_irq.c

> +++ b/drivers/gpu/drm/i915/i915_irq.c

> @@ -2394,20 +2394,34 @@ static void ilk_display_irq_handler(struct

> drm_i915_private *dev_priv,

>  static void hsw_edp_psr_irq_handler(struct drm_i915_private

> *dev_priv)


nitpick: Like I said in the other patch I would like to have this
function renamed here.

>  {

>  	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);

> +	u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);

> +	u32 mask = BIT(TRANSCODER_EDP);

> +	enum transcoder cpu_transcoder;

>  

> -	if (edp_psr_iir & EDP_PSR_ERROR)

> -		DRM_DEBUG_KMS("PSR error\n");

> -

> -	if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {

> -		DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");

> -		I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);

> -	}

> +	if (INTEL_GEN(dev_priv) >= 8)

> +		mask |= BIT(TRANSCODER_A) |

> +			BIT(TRANSCODER_B) |

> +			BIT(TRANSCODER_C);

> +

> +	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,

> mask) {

> +		if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))

> +			DRM_DEBUG_KMS("Transcoder %s PSR error\n",

> +				      transcoder_name(cpu_transcoder

> ));

> +

> +		if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))

> {

> +			DRM_DEBUG_KMS("Transcoder %s PSR prepare

> entry in 2 vblanks\n",

> +				      transcoder_name(cpu_transcoder

> ));

> +			edp_psr_imr |=

> EDP_PSR_PRE_ENTRY(cpu_transcoder);

> +		}

>  

> -	if (edp_psr_iir & EDP_PSR_POST_EXIT) {

> -		DRM_DEBUG_KMS("PSR exit completed\n");

> -		I915_WRITE(EDP_PSR_IMR, 0);

> +		if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))

> {

> +			DRM_DEBUG_KMS("Transcoder %s PSR exit

> completed\n",

> +				      transcoder_name(cpu_transcoder

> ));

> +			edp_psr_imr &=

> ~EDP_PSR_PRE_ENTRY(cpu_transcoder);

> +		}

>  	}

>  

> +	I915_WRITE(EDP_PSR_IMR, edp_psr_imr);

>  	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);

>  }

>  

> @@ -2555,11 +2569,22 @@ gen8_de_irq_handler(struct drm_i915_private

> *dev_priv, u32 master_ctl)

>  	if (master_ctl & GEN8_DE_MISC_IRQ) {

>  		iir = I915_READ(GEN8_DE_MISC_IIR);

>  		if (iir) {

> +			bool found = false;

> +

>  			I915_WRITE(GEN8_DE_MISC_IIR, iir);

>  			ret = IRQ_HANDLED;

> -			if (iir & GEN8_DE_MISC_GSE)

> +

> +			if (iir & GEN8_DE_MISC_GSE) {

>  				intel_opregion_asle_intr(dev_priv);

> -			else

> +				found = true;

> +			}

> +

> +			if (iir & GEN8_DE_EDP_PSR) {

> +				hsw_edp_psr_irq_handler(dev_priv);

> +				found = true;

> +			}

> +

> +			if (!found)

>  				DRM_ERROR("Unexpected DE Misc

> interrupt\n");

>  		}

>  		else

> @@ -3326,6 +3351,9 @@ static void gen8_irq_reset(struct drm_device

> *dev)

>  

>  	gen8_gt_irq_reset(dev_priv);

>  

> +	I915_WRITE(EDP_PSR_IMR, 0xffffffff);

> +	I915_WRITE(EDP_PSR_IIR, 0xffffffff);

> +

>  	for_each_pipe(dev_priv, pipe)

>  		if (intel_display_power_is_enabled(dev_priv,

>  						   POWER_DOMAIN_PIPE

> (pipe)))

> @@ -3815,7 +3843,7 @@ static void gen8_de_irq_postinstall(struct

> drm_i915_private *dev_priv)

>  	uint32_t de_pipe_enables;

>  	u32 de_port_masked = GEN8_AUX_CHANNEL_A;

>  	u32 de_port_enables;

> -	u32 de_misc_masked = GEN8_DE_MISC_GSE;

> +	u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;

>  	enum pipe pipe;

>  

>  	if (INTEL_GEN(dev_priv) >= 9) {

> @@ -3840,6 +3868,9 @@ static void gen8_de_irq_postinstall(struct

> drm_i915_private *dev_priv)

>  	else if (IS_BROADWELL(dev_priv))

>  		de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;

>  

> +	gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);

> +	I915_WRITE(EDP_PSR_IMR, 0);

> +

>  	for_each_pipe(dev_priv, pipe) {

>  		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;

>  

> diff --git a/drivers/gpu/drm/i915/i915_reg.h

> b/drivers/gpu/drm/i915/i915_reg.h

> index f5783d6db614..6eb351177a68 100644

> --- a/drivers/gpu/drm/i915/i915_reg.h

> +++ b/drivers/gpu/drm/i915/i915_reg.h

> @@ -4014,9 +4014,9 @@ enum {

>  /* Bspec claims those aren't shifted but stay at 0x64800 */

>  #define EDP_PSR_IMR				_MMIO(0x64834)

>  #define EDP_PSR_IIR				_MMIO(0x64838)

> -#define   EDP_PSR_ERROR				(1<<2)

> -#define   EDP_PSR_POST_EXIT			(1<<1)

> -#define   EDP_PSR_PRE_ENTRY			(1<<0)

> +#define   EDP_PSR_ERROR(trans)			(1 <<

> (((trans) * 8 + 10) & 31))

> +#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) *

> 8 + 9) & 31))

> +#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) *

> 8 + 8) & 31))

>  


Clever macros

>  #define EDP_PSR_AUX_CTL				_MMIO(dev_pri

> v->psr_mmio_base + 0x10)

>  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)

> @@ -6952,6 +6952,7 @@ enum {

>  #define GEN8_DE_MISC_IIR _MMIO(0x44468)

>  #define GEN8_DE_MISC_IER _MMIO(0x4446c)

>  #define  GEN8_DE_MISC_GSE		(1 << 27)

> +#define  GEN8_DE_EDP_PSR		(1 << 19)

>  

>  #define GEN8_PCU_ISR _MMIO(0x444e0)

>  #define GEN8_PCU_IMR _MMIO(0x444e4)

> diff --git a/drivers/gpu/drm/i915/intel_display.h

> b/drivers/gpu/drm/i915/intel_display.h

> index 4e7418b345bc..2ef31617614a 100644

> --- a/drivers/gpu/drm/i915/intel_display.h

> +++ b/drivers/gpu/drm/i915/intel_display.h

> @@ -218,6 +218,10 @@ struct intel_link_m_n {

>  	for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;

> (__p)++) \

>  		for_each_if((__mask) & BIT(__p))

>  

> +#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \

> +	for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++)	

> \

> +		for_each_if ((__mask) & (1 << (__t)))

> +

>  #define for_each_universal_plane(__dev_priv, __pipe, __p)		

> \

>  	for ((__p) = 0;						

> 	\

>  	     (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +

> 1;	\
Dhinakaran Pandiyan April 5, 2018, 9:25 p.m. UTC | #2 
On Thu, 2018-04-05 at 20:38 +0000, Souza, Jose wrote:
> On Tue, 2018-04-03 at 14:24 -0700, Dhinakaran Pandiyan wrote:

> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>

> > 

> > Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr

> > on

> > any transcoder in theory, though the we don't currenty enable PSR

> > except

> > on the EDP transcoder.

> > 

> > v2: From DK

> >  * Rebased on drm-tip

> > v3: Switched author to Ville based on IRC discussion.

> > 

> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> 

> Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com>

> 

> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  drivers/gpu/drm/i915/i915_irq.c      | 57

> > ++++++++++++++++++++++++++++--------

> >  drivers/gpu/drm/i915/i915_reg.h      |  7 +++--

> >  drivers/gpu/drm/i915/intel_display.h |  4 +++

> >  3 files changed, 52 insertions(+), 16 deletions(-)

> > 

> > diff --git a/drivers/gpu/drm/i915/i915_irq.c

> > b/drivers/gpu/drm/i915/i915_irq.c

> > index c2d3f30778ee..8a894adf2ca1 100644

> > --- a/drivers/gpu/drm/i915/i915_irq.c

> > +++ b/drivers/gpu/drm/i915/i915_irq.c

> > @@ -2394,20 +2394,34 @@ static void ilk_display_irq_handler(struct

> > drm_i915_private *dev_priv,

> >  static void hsw_edp_psr_irq_handler(struct drm_i915_private

> > *dev_priv)

> 

> nitpick: Like I said in the other patch I would like to have this

> function renamed here.



To intel_psr_irq_handler? I renamed it in patch 3 because the function
was moved out of the file. I would like to leave this patch as it is, as
the original author intended.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c2d3f30778ee..8a894adf2ca1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2394,20 +2394,34 @@  static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
 static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
 {
 	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+	u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
+	u32 mask = BIT(TRANSCODER_EDP);
+	enum transcoder cpu_transcoder;
 
-	if (edp_psr_iir & EDP_PSR_ERROR)
-		DRM_DEBUG_KMS("PSR error\n");
-
-	if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
-		DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
-		I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
-	}
+	if (INTEL_GEN(dev_priv) >= 8)
+		mask |= BIT(TRANSCODER_A) |
+			BIT(TRANSCODER_B) |
+			BIT(TRANSCODER_C);
+
+	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
+		if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+			DRM_DEBUG_KMS("Transcoder %s PSR error\n",
+				      transcoder_name(cpu_transcoder));
+
+		if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+			DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 vblanks\n",
+				      transcoder_name(cpu_transcoder));
+			edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
+		}
 
-	if (edp_psr_iir & EDP_PSR_POST_EXIT) {
-		DRM_DEBUG_KMS("PSR exit completed\n");
-		I915_WRITE(EDP_PSR_IMR, 0);
+		if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+			DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
+				      transcoder_name(cpu_transcoder));
+			edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
+		}
 	}
 
+	I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
 	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
 }
 
@@ -2555,11 +2569,22 @@  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 	if (master_ctl & GEN8_DE_MISC_IRQ) {
 		iir = I915_READ(GEN8_DE_MISC_IIR);
 		if (iir) {
+			bool found = false;
+
 			I915_WRITE(GEN8_DE_MISC_IIR, iir);
 			ret = IRQ_HANDLED;
-			if (iir & GEN8_DE_MISC_GSE)
+
+			if (iir & GEN8_DE_MISC_GSE) {
 				intel_opregion_asle_intr(dev_priv);
-			else
+				found = true;
+			}
+
+			if (iir & GEN8_DE_EDP_PSR) {
+				hsw_edp_psr_irq_handler(dev_priv);
+				found = true;
+			}
+
+			if (!found)
 				DRM_ERROR("Unexpected DE Misc interrupt\n");
 		}
 		else
@@ -3326,6 +3351,9 @@  static void gen8_irq_reset(struct drm_device *dev)
 
 	gen8_gt_irq_reset(dev_priv);
 
+	I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+	I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+
 	for_each_pipe(dev_priv, pipe)
 		if (intel_display_power_is_enabled(dev_priv,
 						   POWER_DOMAIN_PIPE(pipe)))
@@ -3815,7 +3843,7 @@  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	uint32_t de_pipe_enables;
 	u32 de_port_masked = GEN8_AUX_CHANNEL_A;
 	u32 de_port_enables;
-	u32 de_misc_masked = GEN8_DE_MISC_GSE;
+	u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
 	enum pipe pipe;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
@@ -3840,6 +3868,9 @@  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	else if (IS_BROADWELL(dev_priv))
 		de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
 
+	gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+	I915_WRITE(EDP_PSR_IMR, 0);
+
 	for_each_pipe(dev_priv, pipe) {
 		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f5783d6db614..6eb351177a68 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4014,9 +4014,9 @@  enum {
 /* Bspec claims those aren't shifted but stay at 0x64800 */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
-#define   EDP_PSR_ERROR				(1<<2)
-#define   EDP_PSR_POST_EXIT			(1<<1)
-#define   EDP_PSR_PRE_ENTRY			(1<<0)
+#define   EDP_PSR_ERROR(trans)			(1 << (((trans) * 8 + 10) & 31))
+#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) * 8 + 9) & 31))
+#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) * 8 + 8) & 31))
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
@@ -6952,6 +6952,7 @@  enum {
 #define GEN8_DE_MISC_IIR _MMIO(0x44468)
 #define GEN8_DE_MISC_IER _MMIO(0x4446c)
 #define  GEN8_DE_MISC_GSE		(1 << 27)
+#define  GEN8_DE_EDP_PSR		(1 << 19)
 
 #define GEN8_PCU_ISR _MMIO(0x444e0)
 #define GEN8_PCU_IMR _MMIO(0x444e4)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 4e7418b345bc..2ef31617614a 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -218,6 +218,10 @@  struct intel_link_m_n {
 	for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++) \
 		for_each_if((__mask) & BIT(__p))
 
+#define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
+	for ((__t) = 0; (__t) < I915_MAX_TRANSCODERS; (__t)++)	\
+		for_each_if ((__mask) & (1 << (__t)))
+
 #define for_each_universal_plane(__dev_priv, __pipe, __p)		\
 	for ((__p) = 0;							\
 	     (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1;	\