On Tue, 11 Jul 2017, Rodrigo Vivi <[email protected]> wrote:
> This old PSR implementation died on gen8LP and had many
> limitations such:
> - no functional HW tracking
> - required link standby with single frame updates
>
> So, let's group them in a easier way to filter it now on.
>
> Cc: Dhinakaran Pandiyan <[email protected]>
> Cc: Jim Bride <[email protected]>
> Signed-off-by: Rodrigo Vivi <[email protected]>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 5 ++++-
>  drivers/gpu/drm/i915/i915_pci.c  | 4 ++--
>  drivers/gpu/drm/i915/intel_dp.c  | 2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 7 +++----
>  4 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 81cd21ecfa7d..d0a49307c6fb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -769,6 +769,7 @@ struct intel_csr {
>       func(has_pipe_cxsr); \
>       func(has_pooled_eu); \
>       func(has_psr); \
> +     func(has_vlv_psr); \
>       func(has_rc6); \
>       func(has_rc6p); \
>       func(has_resource_streamer); \
> @@ -2983,7 +2984,9 @@ intel_info(const struct drm_i915_private *dev_priv)
>  
>  #define HAS_DDI(dev_priv)             ((dev_priv)->info.has_ddi)
>  #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
> -#define HAS_PSR(dev_priv)             ((dev_priv)->info.has_psr)
> +#define HAS_PSR(dev_priv)             ((dev_priv)->info.has_psr || \
> +                                       (dev_priv)->info.has_vlv_psr)
> +#define HAS_VLV_PSR(dev_priv)                 ((dev_priv)->info.has_vlv_psr)

I'm not convinced adding a intel info field for this is warranted. How
about just:

#define HAS_VLV_PSR(dev_priv) (HAS_PSR(dev_priv) && \
        (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))

Though I'm not convinced about the HS_VLV_PSR name either!

BR,
Jani.


>  #define HAS_RC6(dev_priv)             ((dev_priv)->info.has_rc6)
>  #define HAS_RC6p(dev_priv)            ((dev_priv)->info.has_rc6p)
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index a1e6b696bcfa..e0e972210090 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -274,7 +274,7 @@ static const struct intel_device_info 
> intel_valleyview_info = {
>       .gen = 7,
>       .is_lp = 1,
>       .num_pipes = 2,
> -     .has_psr = 1,
> +     .has_vlv_psr = 1,
>       .has_runtime_pm = 1,
>       .has_rc6 = 1,
>       .has_gmbus_irq = 1,
> @@ -334,7 +334,7 @@ static const struct intel_device_info 
> intel_cherryview_info = {
>       .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>       .platform = INTEL_CHERRYVIEW,
>       .has_64bit_reloc = 1,
> -     .has_psr = 1,
> +     .has_vlv_psr = 1,
>       .has_runtime_pm = 1,
>       .has_resource_streamer = 1,
>       .has_rc6 = 1,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2d42d09428c9..3f2191edc2c9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2653,7 +2653,7 @@ static void intel_disable_dp(struct intel_encoder 
> *encoder,
>       if (old_crtc_state->has_audio)
>               intel_audio_codec_disable(encoder);
>  
> -     if (HAS_PSR(dev_priv) && !HAS_DDI(dev_priv))
> +     if (HAS_VLV_PSR(dev_priv))
>               intel_psr_disable(intel_dp);
>  
>       /* Make sure the panel is off before trying to change the mode. But also
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 559f1ab42bfc..461c7e4f6ecc 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -401,8 +401,7 @@ static bool intel_psr_match_conditions(struct intel_dp 
> *intel_dp)
>               return false;
>       }
>  
> -     if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> -         !dev_priv->psr.link_standby) {
> +     if (HAS_VLV_PSR(dev_priv) && !dev_priv->psr.link_standby) {
>               DRM_ERROR("PSR condition failed: Link off requested but not 
> supported on this platform\n");
>               return false;
>       }
> @@ -827,7 +826,7 @@ void intel_psr_single_frame_update(struct 
> drm_i915_private *dev_priv,
>        * Single frame update is already supported on BDW+ but it requires
>        * many W/A and it isn't really needed.
>        */
> -     if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +     if (!HAS_VLV_PSR(dev_priv))
>               return;
>  
>       mutex_lock(&dev_priv->psr.lock);
> @@ -949,7 +948,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>       if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>               /* HSW and BDW require workarounds that we don't implement. */
>               dev_priv->psr.link_standby = false;
> -     else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +     else if (HAS_VLV_PSR(dev_priv))
>               /* On VLV and CHV only standby mode is supported. */
>               dev_priv->psr.link_standby = true;
>       else

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to