[Intel-gfx] [PATCH] drm/i915: Simplify PSR debugfs

Rodrigo Vivi rodrigo.vivi at gmail.com
Tue Oct 1 22:26:44 CEST 2013


On Tue, Oct 1, 2013 at 4:16 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Oct 01, 2013 at 03:26:29PM -0300, Rodrigo Vivi wrote:
>> for igt test case.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
>
> This changes the layout of the debugfs file, so if QA runs this on older
> kernels (e.g. for release testing) I think we'll get failures. On a quick
> look this file should have all the information we need to support the
> testcase ...

This is why I put this skip on test case:

 igt_skip("i915_edp_psr_status format not supported by this test case\n");

> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 126 +++---------------------------------
>>  drivers/gpu/drm/i915/i915_drv.h     |  16 ++---
>>  drivers/gpu/drm/i915/intel_dp.c     |  35 +++++-----
>>  3 files changed, 28 insertions(+), 149 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index a6f4cb5..ada12f7 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1655,126 +1655,18 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>>       struct drm_info_node *node = m->private;
>>       struct drm_device *dev = node->minor->dev;
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> -     u32 psrstat, psrperf;
>> +     u32 psrperf = 0;
>> +     bool enabled = false;
>>
>> -     if (!IS_HASWELL(dev)) {
>> -             seq_puts(m, "PSR not supported on this platform\n");
>> -     } else if (IS_HASWELL(dev) && I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE) {
>> -             seq_puts(m, "PSR enabled\n");
>> -     } else {
>> -             seq_puts(m, "PSR disabled: ");
>> -             switch (dev_priv->no_psr_reason) {
>> -             case PSR_NO_SOURCE:
>> -                     seq_puts(m, "not supported on this platform");
>> -                     break;
>> -             case PSR_NO_SINK:
>> -                     seq_puts(m, "not supported by panel");
>> -                     break;
>> -             case PSR_MODULE_PARAM:
>> -                     seq_puts(m, "disabled by flag");
>> -                     break;
>> -             case PSR_CRTC_NOT_ACTIVE:
>> -                     seq_puts(m, "crtc not active");
>> -                     break;
>> -             case PSR_PWR_WELL_ENABLED:
>> -                     seq_puts(m, "power well enabled");
>> -                     break;
>> -             case PSR_NOT_TILED:
>> -                     seq_puts(m, "not tiled");
>> -                     break;
>> -             case PSR_SPRITE_ENABLED:
>> -                     seq_puts(m, "sprite enabled");
>> -                     break;
>> -             case PSR_S3D_ENABLED:
>> -                     seq_puts(m, "stereo 3d enabled");
>> -                     break;
>> -             case PSR_INTERLACED_ENABLED:
>> -                     seq_puts(m, "interlaced enabled");
>> -                     break;
>> -             case PSR_HSW_NOT_DDIA:
>> -                     seq_puts(m, "HSW ties PSR to DDI A (eDP)");
>> -                     break;
>> -             default:
>> -                     seq_puts(m, "unknown reason");
>> -             }
>> -             seq_puts(m, "\n");
>> -             return 0;
>> -     }
>> -
>> -     psrstat = I915_READ(EDP_PSR_STATUS_CTL);
>> -
>> -     seq_puts(m, "PSR Current State: ");
>> -     switch (psrstat & EDP_PSR_STATUS_STATE_MASK) {
>> -     case EDP_PSR_STATUS_STATE_IDLE:
>> -             seq_puts(m, "Reset state\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_SRDONACK:
>> -             seq_puts(m, "Wait for TG/Stream to send on frame of data after SRD conditions are met\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_SRDENT:
>> -             seq_puts(m, "SRD entry\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_BUFOFF:
>> -             seq_puts(m, "Wait for buffer turn off\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_BUFON:
>> -             seq_puts(m, "Wait for buffer turn on\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_AUXACK:
>> -             seq_puts(m, "Wait for AUX to acknowledge on SRD exit\n");
>> -             break;
>> -     case EDP_PSR_STATUS_STATE_SRDOFFACK:
>> -             seq_puts(m, "Wait for TG/Stream to acknowledge the SRD VDM exit\n");
>> -             break;
>> -     default:
>> -             seq_puts(m, "Unknown\n");
>> -             break;
>> -     }
>> -
>> -     seq_puts(m, "Link Status: ");
>> -     switch (psrstat & EDP_PSR_STATUS_LINK_MASK) {
>> -     case EDP_PSR_STATUS_LINK_FULL_OFF:
>> -             seq_puts(m, "Link is fully off\n");
>> -             break;
>> -     case EDP_PSR_STATUS_LINK_FULL_ON:
>> -             seq_puts(m, "Link is fully on\n");
>> -             break;
>> -     case EDP_PSR_STATUS_LINK_STANDBY:
>> -             seq_puts(m, "Link is in standby\n");
>> -             break;
>> -     default:
>> -             seq_puts(m, "Unknown\n");
>> -             break;
>> -     }
>> -
>> -     seq_printf(m, "PSR Entry Count: %u\n",
>> -                psrstat >> EDP_PSR_STATUS_COUNT_SHIFT &
>> -                EDP_PSR_STATUS_COUNT_MASK);
>> -
>> -     seq_printf(m, "Max Sleep Timer Counter: %u\n",
>> -                psrstat >> EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT &
>> -                EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK);
>> -
>> -     seq_printf(m, "Had AUX error: %s\n",
>> -                yesno(psrstat & EDP_PSR_STATUS_AUX_ERROR));
>> -
>> -     seq_printf(m, "Sending AUX: %s\n",
>> -                yesno(psrstat & EDP_PSR_STATUS_AUX_SENDING));
>> -
>> -     seq_printf(m, "Sending Idle: %s\n",
>> -                yesno(psrstat & EDP_PSR_STATUS_SENDING_IDLE));
>> -
>> -     seq_printf(m, "Sending TP2 TP3: %s\n",
>> -                yesno(psrstat & EDP_PSR_STATUS_SENDING_TP2_TP3));
>> -
>> -     seq_printf(m, "Sending TP1: %s\n",
>> -                yesno(psrstat & EDP_PSR_STATUS_SENDING_TP1));
>> +     seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support));
>> +     seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok));
>>
>> -     seq_printf(m, "Idle Count: %u\n",
>> -                psrstat & EDP_PSR_STATUS_IDLE_MASK);
>> +     enabled = IS_HASWELL(dev) && I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
>> +     seq_printf(m, "Enabled: %s\n", yesno(enabled));
>>
>> -     psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & EDP_PSR_PERF_CNT_MASK;
>> -     seq_printf(m, "Performance Counter: %u\n", psrperf);
>> +     if (IS_HASWELL(dev))
>> +             psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & EDP_PSR_PERF_CNT_MASK;
>> +     seq_printf(m, "Performance_Counter: %u\n", psrperf);
>>
>>       return 0;
>>  }
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 35874b3..cad2207 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -621,17 +621,9 @@ struct i915_fbc {
>>       } no_fbc_reason;
>>  };
>>
>> -enum no_psr_reason {
>> -     PSR_NO_SOURCE, /* Not supported on platform */
>> -     PSR_NO_SINK, /* Not supported by panel */
>> -     PSR_MODULE_PARAM,
>> -     PSR_CRTC_NOT_ACTIVE,
>> -     PSR_PWR_WELL_ENABLED,
>> -     PSR_NOT_TILED,
>> -     PSR_SPRITE_ENABLED,
>> -     PSR_S3D_ENABLED,
>> -     PSR_INTERLACED_ENABLED,
>> -     PSR_HSW_NOT_DDIA,
>> +struct i915_psr {
>> +     bool sink_support;
>> +     bool source_ok;
>>  };
>>
>>  enum intel_pch {
>> @@ -1298,7 +1290,7 @@ typedef struct drm_i915_private {
>>       /* Haswell power well */
>>       struct i915_power_well power_well;
>>
>> -     enum no_psr_reason no_psr_reason;
>> +     struct i915_psr psr;
>>
>>       struct i915_gpu_error gpu_error;
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index ff3e8b9..524d3d5 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1392,10 +1392,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>       }
>>  }
>>
>> -static bool is_edp_psr(struct intel_dp *intel_dp)
>> +static bool is_edp_psr(struct drm_device *dev)
>>  {
>> -     return is_edp(intel_dp) &&
>> -             intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +     return dev_priv->psr.sink_support;
>>  }
>>
>>  static bool intel_edp_is_psr_enabled(struct drm_device *dev)
>> @@ -1522,42 +1523,33 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>>       struct drm_i915_gem_object *obj = to_intel_framebuffer(crtc->fb)->obj;
>>       struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>>
>> +     dev_priv->psr.source_ok = false;
>> +
>>       if (!IS_HASWELL(dev)) {
>>               DRM_DEBUG_KMS("PSR not supported on this platform\n");
>> -             dev_priv->no_psr_reason = PSR_NO_SOURCE;
>>               return false;
>>       }
>>
>>       if ((intel_encoder->type != INTEL_OUTPUT_EDP) ||
>>           (dig_port->port != PORT_A)) {
>>               DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
>> -             dev_priv->no_psr_reason = PSR_HSW_NOT_DDIA;
>> -             return false;
>> -     }
>> -
>> -     if (!is_edp_psr(intel_dp)) {
>> -             DRM_DEBUG_KMS("PSR not supported by this panel\n");
>> -             dev_priv->no_psr_reason = PSR_NO_SINK;
>>               return false;
>>       }
>>
>>       if (!i915_enable_psr) {
>>               DRM_DEBUG_KMS("PSR disable by flag\n");
>> -             dev_priv->no_psr_reason = PSR_MODULE_PARAM;
>>               return false;
>>       }
>>
>>       crtc = dig_port->base.base.crtc;
>>       if (crtc == NULL) {
>>               DRM_DEBUG_KMS("crtc not active for PSR\n");
>> -             dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
>>               return false;
>>       }
>>
>>       intel_crtc = to_intel_crtc(crtc);
>>       if (!intel_crtc->active || !crtc->fb || !crtc->mode.clock) {
>>               DRM_DEBUG_KMS("crtc not active for PSR\n");
>> -             dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
>>               return false;
>>       }
>>
>> @@ -1565,29 +1557,26 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>>       if (obj->tiling_mode != I915_TILING_X ||
>>           obj->fence_reg == I915_FENCE_REG_NONE) {
>>               DRM_DEBUG_KMS("PSR condition failed: fb not tiled or fenced\n");
>> -             dev_priv->no_psr_reason = PSR_NOT_TILED;
>>               return false;
>>       }
>>
>>       if (I915_READ(SPRCTL(intel_crtc->pipe)) & SPRITE_ENABLE) {
>>               DRM_DEBUG_KMS("PSR condition failed: Sprite is Enabled\n");
>> -             dev_priv->no_psr_reason = PSR_SPRITE_ENABLED;
>>               return false;
>>       }
>>
>>       if (I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config.cpu_transcoder)) &
>>           S3D_ENABLE) {
>>               DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
>> -             dev_priv->no_psr_reason = PSR_S3D_ENABLED;
>>               return false;
>>       }
>>
>>       if (crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) {
>>               DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
>> -             dev_priv->no_psr_reason = PSR_INTERLACED_ENABLED;
>>               return false;
>>       }
>>
>> +     dev_priv->psr.source_ok = true;
>>       return true;
>>  }
>>
>> @@ -1643,7 +1632,7 @@ void intel_edp_psr_update(struct drm_device *dev)
>>               if (encoder->type == INTEL_OUTPUT_EDP) {
>>                       intel_dp = enc_to_intel_dp(&encoder->base);
>>
>> -                     if (!is_edp_psr(intel_dp))
>> +                     if (!is_edp_psr(dev))
>>                               return;
>>
>>                       if (!intel_edp_psr_match_conditions(intel_dp))
>> @@ -2558,6 +2547,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>>  static bool
>>  intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>  {
>> +     struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> +     struct drm_device *dev = dig_port->base.base.dev;
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>>       char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
>>
>>       if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
>> @@ -2576,8 +2569,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>       intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
>>                                      intel_dp->psr_dpcd,
>>                                      sizeof(intel_dp->psr_dpcd));
>> -     if (is_edp_psr(intel_dp))
>> +     if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
>> +             dev_priv->psr.sink_support = true;
>>               DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
>> +     }
>>       if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>>             DP_DWN_STRM_PORT_PRESENT))
>>               return true; /* native DP sink */
>> --
>> 1.7.11.7
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br



More information about the Intel-gfx mailing list