On Wed, 2018-03-21 at 21:45 +0200, Ville Syrjälä wrote:
> On Tue, Mar 20, 2018 at 03:41:50PM -0700, Dhinakaran Pandiyan wrote:
> > Interrupts other than the one for AUX errors are required only for debug,
> > so unmask them via debugfs when the user requests debug.
> > 
> > User can make such a request with
> > echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug
> > 
> > Cc: Rodrigo Vivi <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Signed-off-by: Dhinakaran Pandiyan <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 36 +++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> >  drivers/gpu/drm/i915/i915_irq.c     | 68 
> > ++++++++++++++++---------------------
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_psr.c    | 56 ++++++++++++++++++++++++++++++
> >  5 files changed, 123 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 964ea1a12357..669f3d56054a 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2690,6 +2690,39 @@ static int i915_edp_psr_status(struct seq_file *m, 
> > void *data)
> >     return 0;
> >  }
> >  
> > +static int
> > +i915_edp_psr_debug_set(void *data, u64 val)
> > +{
> > +   struct drm_i915_private *dev_priv = data;
> > +
> > +   if (!CAN_PSR(dev_priv))
> > +           return -ENODEV;
> > +
> > +   if (val < 0  || val > 1)
> > +           return -EINVAL;
> > +
> > +   DRM_DEBUG_KMS("%s PSR debug\n", val == 1 ? "Enabling" : "Disabling");
> > +   intel_psr_debug_control(dev_priv, val);
> > +
> > +   return 0;
> > +}
> > +
> > +static int
> > +i915_edp_psr_debug_get(void *data, u64 *val)
> > +{
> > +   struct drm_i915_private *dev_priv = data;
> > +
> > +   if (!CAN_PSR(dev_priv))
> > +           return -ENODEV;
> > +
> > +   *val = READ_ONCE(dev_priv->psr.debug);
> > +   return 0;
> > +}
> > +
> > +DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
> > +                   i915_edp_psr_debug_get, i915_edp_psr_debug_set,
> > +                   "%llu\n");
> > +
> >  static int i915_sink_crc(struct seq_file *m, void *data)
> >  {
> >     struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > @@ -4811,7 +4844,8 @@ static const struct i915_debugfs_files {
> >     {"i915_guc_log_relay", &i915_guc_log_relay_fops},
> >     {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> >     {"i915_ipc_status", &i915_ipc_status_fops},
> > -   {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> > +   {"i915_drrs_ctl", &i915_drrs_ctl_fops},
> > +   {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
> >  };
> >  
> >  int i915_debugfs_register(struct drm_i915_private *dev_priv)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index e27ba8fb64e6..136fa2267a66 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -608,6 +608,7 @@ struct i915_psr {
> >     bool colorimetry_support;
> >     bool alpm;
> >     bool has_hw_tracking;
> > +   bool debug;
> >  
> >     void (*enable_source)(struct intel_dp *,
> >                           const struct intel_crtc_state *);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 56228e8ed980..94941b52f1cf 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2392,40 +2392,6 @@ static void ilk_display_irq_handler(struct 
> > drm_i915_private *dev_priv,
> >             ironlake_rps_change_irq_handler(dev_priv);
> >  }
> >  
> > -static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
> > -{
> > -   u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > -   u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
> > -   u32 mask = BIT(TRANSCODER_EDP);
> > -   enum transcoder cpu_transcoder;
> > -
> > -   if (INTEL_GEN(dev_priv) >= 8)
> > -           mask |= BIT(TRANSCODER_A) |
> > -                   BIT(TRANSCODER_B) |
> > -                   BIT(TRANSCODER_C);
> > -
> > -   for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
> > -           if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > -                   DRM_DEBUG_KMS("Transcoder %s PSR error\n",
> > -                                 transcoder_name(cpu_transcoder));
> > -
> > -           if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > -                   DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 
> > vblanks\n",
> > -                                 transcoder_name(cpu_transcoder));
> > -                   edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
> > -           }
> > -
> > -           if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> > -                   DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
> > -                                 transcoder_name(cpu_transcoder));
> > -                   edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
> > -           }
> > -   }
> > -
> > -   I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
> > -   I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > -}
> > -
> >  static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
> >                                 u32 de_iir)
> >  {
> > @@ -2438,8 +2404,12 @@ static void ivb_display_irq_handler(struct 
> > drm_i915_private *dev_priv,
> >     if (de_iir & DE_ERR_INT_IVB)
> >             ivb_err_int_handler(dev_priv);
> >  
> > -   if (de_iir & DE_EDP_PSR_INT_HSW)
> > -           hsw_edp_psr_irq_handler(dev_priv);
> > +   if (de_iir & DE_EDP_PSR_INT_HSW) {
> > +           u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > +           intel_psr_irq_handler(dev_priv, psr_iir);
> > +           I915_WRITE(EDP_PSR_IIR, psr_iir);
> > +   }
> >  
> >     if (de_iir & DE_AUX_CHANNEL_A_IVB)
> >             dp_aux_irq_handler(dev_priv);
> > @@ -2581,7 +2551,10 @@ gen8_de_irq_handler(struct drm_i915_private 
> > *dev_priv, u32 master_ctl)
> >                     }
> >  
> >                     if (iir & GEN8_DE_EDP_PSR) {
> > -                           hsw_edp_psr_irq_handler(dev_priv);
> > +                           u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > +                           intel_psr_irq_handler(dev_priv, psr_iir);
> > +                           I915_WRITE(EDP_PSR_IIR, psr_iir);
> >                             found = true;
> >                     }
> >  
> > @@ -3699,6 +3672,23 @@ static void gen5_gt_irq_postinstall(struct 
> > drm_device *dev)
> >     }
> >  }
> >  
> > +static inline u32 psr_mask(struct drm_i915_private *dev_priv)
> > +{
> > +   u32 transcoders = BIT(TRANSCODER_EDP);
> > +   u32 mask = 0;
> > +   enum transcoder cpu_transcoder;
> > +
> > +   if (INTEL_GEN(dev_priv) >= 8)
> > +           transcoders |= BIT(TRANSCODER_A) |
> > +                          BIT(TRANSCODER_B) |
> > +                          BIT(TRANSCODER_C);
> > +
> > +   for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders)
> > +           mask |=  EDP_PSR_ERROR(cpu_transcoder);
> > +
> > +   return ~mask;
> > +}
> > +
> >  static int ironlake_irq_postinstall(struct drm_device *dev)
> >  {
> >     struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -3721,7 +3711,7 @@ static int ironlake_irq_postinstall(struct drm_device 
> > *dev)
> >  
> >     if (IS_HASWELL(dev_priv)) {
> >             gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > -           I915_WRITE(EDP_PSR_IMR, 0);
> > +           I915_WRITE(EDP_PSR_IMR, psr_mask(dev_priv));
> >             display_mask |= DE_EDP_PSR_INT_HSW;
> >     }
> >  
> > @@ -3861,7 +3851,7 @@ static void gen8_de_irq_postinstall(struct 
> > drm_i915_private *dev_priv)
> >             de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
> >  
> >     gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> > -   I915_WRITE(EDP_PSR_IMR, 0);
> > +   I915_WRITE(EDP_PSR_IMR, psr_mask(dev_priv));
> 
> psr_mask() seems somewhat pointless to me. Could perhaps just
> write the full mask by hand in these cases.
> 

Okay. I'll replace these with

EDP_PSR_ERROR(TRANSCODER_EDP)

and 

psr_mask = PSR_ERROR(TRANSCODER_EDP) |
           PSR_ERROR(TRANSCODER_A) |
           PSR_ERROR(TRANSCODER_B) |
           PSR_ERROR(TRANSCODER_C) |

> >  
> >     for_each_pipe(dev_priv, pipe) {
> >             dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index a215aa78b0be..8be75cc5bf24 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1887,6 +1887,8 @@ void intel_psr_single_frame_update(struct 
> > drm_i915_private *dev_priv,
> >                                unsigned frontbuffer_bits);
> >  void intel_psr_compute_config(struct intel_dp *intel_dp,
> >                           struct intel_crtc_state *crtc_state);
> > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool 
> > enable);
> > +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
> >  
> >  /* intel_runtime_pm.c */
> >  int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 317cb4a12693..64ecea80438d 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -93,6 +93,62 @@ static void psr_aux_io_power_put(struct intel_dp 
> > *intel_dp)
> >     intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> >  }
> >  
> > +void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool 
> > enable)
> > +{
> > +   u32 transcoders = BIT(TRANSCODER_EDP);
> > +   u32 mask = 0;
> > +   enum transcoder cpu_transcoder;
> > +
> > +   /* No PSR interrupts on VLV/CHV */
> > +   if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > +           return;
> > +
> > +   if (INTEL_GEN(dev_priv) >= 8)
> > +           transcoders |= BIT(TRANSCODER_A) |
> > +                          BIT(TRANSCODER_B) |
> > +                          BIT(TRANSCODER_C);
> > +
> > +   for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders)
> > +           mask = EDP_PSR_POST_EXIT(cpu_transcoder) |
> > +                  EDP_PSR_PRE_ENTRY(cpu_transcoder);
> 
> This too could be done w/o the loop I think.
> 

Okay, will write it as.

mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
DP_PSR_PRE_ENTRY(TRANSCODER_EDP);

if (INTEL_GEN(dev_priv) >= 8)
        mask |=  EDP_PSR_POST_EXIT(TRANSCODER_A) |
EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
        EDP_PSR_POST_EXIT(TRANSCODER_B) | EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
        EDP_PSR_POST_EXIT(TRANSCODER_C) | EDP_PSR_PRE_ENTRY(TRANSCODER_C);


> > +
> > +   if (enable) {
> > +           WRITE_ONCE(dev_priv->psr.debug, true);
> > +           I915_WRITE(EDP_PSR_IMR, ~mask);
> > +   } else {
> > +           I915_WRITE(EDP_PSR_IMR, mask);
> > +           WRITE_ONCE(dev_priv->psr.debug, false);
> > +   }
> > +}
> > +
> > +void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> > +{
> > +   u32 transcoders = BIT(TRANSCODER_EDP);
> > +   enum transcoder cpu_transcoder;
> > +
> > +   if (INTEL_GEN(dev_priv) >= 8)
> > +           transcoders |= BIT(TRANSCODER_A) |
> > +                          BIT(TRANSCODER_B) |
> > +                          BIT(TRANSCODER_C);
> > +
> > +   for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> > +           /* FIXME: Exit PSR when this happens. */
> > +           if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > +                   DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
> > +                                 transcoder_name(cpu_transcoder));
> > +
> > +           if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > +                   DRM_DEBUG_KMS("[transcoder %s] PSR entry in 2 
> > vblanks\n",
> > +                                 transcoder_name(cpu_transcoder));
> > +           }
> > +
> > +           if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> > +                   DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > +                                 transcoder_name(cpu_transcoder));
> > +           }
> > +   }
> > +}
> > +
> >  static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> >  {
> >     uint8_t psr_caps = 0;
> > -- 
> > 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to