Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/psr: Remove intel_crtc_state parameter from disable()

2018-04-19 Thread Ville Syrjälä 
On Wed, Apr 18, 2018 at 03:43:05PM -0700, José Roberto de Souza wrote:
> It is only used by VLV/CHV and we can get this information from
> intel_dp for those platforms.

But why? Abusing active_pipe (which is there just for the pps
tracking) is not a good idea IMO.

> 
> Signed-off-by: José Roberto de Souza 
> Cc: Dhinakaran Pandiyan 
> Cc: Rodrigo Vivi 
> Cc: Lucas De Marchi 
> Cc: Ville Syrjälä 
> ---
> 
> Changes from v1:
> - not using legacy drm_crtc pointer(struct drm_crtc *crtc = 
> intel_dig_port->base.base.crtc)
> 
>  drivers/gpu/drm/i915/i915_drv.h  |  3 +--
>  drivers/gpu/drm/i915/intel_psr.c | 17 +++--
>  2 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 476bca872d48..f5ffb3d72cef 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -612,8 +612,7 @@ struct i915_psr {
>  
>   void (*enable_source)(struct intel_dp *,
> const struct intel_crtc_state *);
> - void (*disable_source)(struct intel_dp *,
> -const struct intel_crtc_state *);
> + void (*disable_source)(struct intel_dp *intel_dp);
>   void (*enable_sink)(struct intel_dp *);
>   void (*activate)(struct intel_dp *);
>   void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index ebc47e2b08ca..934498505356 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -665,38 +665,35 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>   mutex_unlock(_priv->psr.lock);
>  }
>  
> -static void vlv_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void vlv_psr_disable(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>   uint32_t val;
>  
>   if (dev_priv->psr.active) {
>   /* Put VLV PSR back to PSR_state 0 (disabled). */
>   if (intel_wait_for_register(dev_priv,
> - VLV_PSRSTAT(crtc->pipe),
> + VLV_PSRSTAT(intel_dp->active_pipe),
>   VLV_EDP_PSR_IN_TRANS,
>   0,
>   1))
>   WARN(1, "PSR transition took longer than expected\n");
>  
> - val = I915_READ(VLV_PSRCTL(crtc->pipe));
> + val = I915_READ(VLV_PSRCTL(intel_dp->active_pipe));
>   val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
>   val &= ~VLV_EDP_PSR_ENABLE;
>   val &= ~VLV_EDP_PSR_MODE_MASK;
> - I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
> + I915_WRITE(VLV_PSRCTL(intel_dp->active_pipe), val);
>  
>   dev_priv->psr.active = false;
>   } else {
> - WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
> + WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_dp->active_pipe));
>   }
>  }
>  
> -static void hsw_psr_disable(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *old_crtc_state)
> +static void hsw_psr_disable(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -765,7 +762,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>   return;
>   }
>  
> - dev_priv->psr.disable_source(intel_dp, old_crtc_state);
> + dev_priv->psr.disable_source(intel_dp);
>  
>   /* Disable PSR on Sink */
>   drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, 0);
> -- 
> 2.17.0

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 3/9] drm/i915/psr: Remove intel_crtc_state parameter from disable()

2018-04-18 Thread José Roberto de Souza 
It is only used by VLV/CHV and we can get this information from
intel_dp for those platforms.

Signed-off-by: José Roberto de Souza 
Cc: Dhinakaran Pandiyan 
Cc: Rodrigo Vivi 
Cc: Lucas De Marchi 
Cc: Ville Syrjälä 
---

Changes from v1:
- not using legacy drm_crtc pointer(struct drm_crtc *crtc = 
intel_dig_port->base.base.crtc)

 drivers/gpu/drm/i915/i915_drv.h  |  3 +--
 drivers/gpu/drm/i915/intel_psr.c | 17 +++--
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 476bca872d48..f5ffb3d72cef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -612,8 +612,7 @@ struct i915_psr {
 
void (*enable_source)(struct intel_dp *,
  const struct intel_crtc_state *);
-   void (*disable_source)(struct intel_dp *,
-  const struct intel_crtc_state *);
+   void (*disable_source)(struct intel_dp *intel_dp);
void (*enable_sink)(struct intel_dp *);
void (*activate)(struct intel_dp *);
void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index ebc47e2b08ca..934498505356 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -665,38 +665,35 @@ void intel_psr_enable(struct intel_dp *intel_dp,
mutex_unlock(_priv->psr.lock);
 }
 
-static void vlv_psr_disable(struct intel_dp *intel_dp,
-   const struct intel_crtc_state *old_crtc_state)
+static void vlv_psr_disable(struct intel_dp *intel_dp)
 {
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
uint32_t val;
 
if (dev_priv->psr.active) {
/* Put VLV PSR back to PSR_state 0 (disabled). */
if (intel_wait_for_register(dev_priv,
-   VLV_PSRSTAT(crtc->pipe),
+   VLV_PSRSTAT(intel_dp->active_pipe),
VLV_EDP_PSR_IN_TRANS,
0,
1))
WARN(1, "PSR transition took longer than expected\n");
 
-   val = I915_READ(VLV_PSRCTL(crtc->pipe));
+   val = I915_READ(VLV_PSRCTL(intel_dp->active_pipe));
val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
val &= ~VLV_EDP_PSR_ENABLE;
val &= ~VLV_EDP_PSR_MODE_MASK;
-   I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
+   I915_WRITE(VLV_PSRCTL(intel_dp->active_pipe), val);
 
dev_priv->psr.active = false;
} else {
-   WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
+   WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_dp->active_pipe));
}
 }
 
-static void hsw_psr_disable(struct intel_dp *intel_dp,
-   const struct intel_crtc_state *old_crtc_state)
+static void hsw_psr_disable(struct intel_dp *intel_dp)
 {
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -765,7 +762,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
return;
}
 
-   dev_priv->psr.disable_source(intel_dp, old_crtc_state);
+   dev_priv->psr.disable_source(intel_dp);
 
/* Disable PSR on Sink */
drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, 0);
-- 
2.17.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx