Re: [Intel-gfx] [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync

2018-03-16 Thread Pandiyan, Dhinakaran

On Fri, 2018-03-16 at 16:04 -0700, José Roberto de Souza wrote:
> PSR2 selective update requires aux frame sync(even though we don't
> support it in i915)

Any chance I could get you to fix this line? 

i915 does not support aux frame sync ^
PSR2 panel needs to support aux frame sync (see below)

I find this contradictory, please explain what you believe is going on.


>  and do not makes sense active PSR2 to only do
> full screen updates aka PSR1.
> Having aux_frame_sync flag could cause it be set to true even when
> the PSR1 is being used, see intel_psr2_config_valid().
> 
> Cc: Dhinakaran Pandiyan 
> Reviewed-by: Rodrigo Vivi 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 -
>  drivers/gpu/drm/i915/intel_psr.c | 10 +-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e27ba8fb64e6..8a584273f897 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -602,7 +602,6 @@ struct i915_psr {
>   struct delayed_work work;
>   unsigned busy_frontbuffer_bits;
>   bool psr2_support;
> - bool aux_frame_sync;
>   bool link_standby;
>   bool y_cord_support;
>   bool colorimetry_support;
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 317cb4a12693..9811f5f0bc75 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -144,9 +144,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> &frame_sync_cap) != 1)
>   frame_sync_cap = 0;
> - dev_priv->psr.aux_frame_sync = frame_sync_cap & 
> DP_AUX_FRAME_SYNC_CAP;
> + frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP);
>   /* PSR2 needs frame sync as well */
> - dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> + dev_priv->psr.psr2_support = frame_sync_cap;
>   DRM_DEBUG_KMS("PSR2 %s on sink",
> dev_priv->psr.psr2_support ? "supported" : "not 
> supported");
>  
> @@ -269,7 +269,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>   aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>  
>   /* Enable AUX frame sync at sink */
> - if (dev_priv->psr.aux_frame_sync)
> + if (dev_priv->psr.psr2_support)
>   drm_dp_dpcd_writeb(&intel_dp->aux,
>   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
>   DP_AUX_FRAME_SYNC_ENABLE);
> @@ -714,7 +714,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
>   i915_reg_t psr_status;
>   u32 psr_status_mask;
>  
> - if (dev_priv->psr.aux_frame_sync)
> + if (dev_priv->psr.psr2_support)
>   drm_dp_dpcd_writeb(&intel_dp->aux,
>   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
>   0);
> @@ -862,7 +862,7 @@ static void intel_psr_exit(struct drm_i915_private 
> *dev_priv)
>   return;
>  
>   if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.aux_frame_sync)
> + if (dev_priv->psr.psr2_support)
>   drm_dp_dpcd_writeb(&intel_dp->aux,
>   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
>   0);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync

2018-03-16 Thread José Roberto de Souza
PSR2 selective update requires aux frame sync(even though we don't
support it in i915) and do not makes sense active PSR2 to only do
full screen updates aka PSR1.
Having aux_frame_sync flag could cause it be set to true even when
the PSR1 is being used, see intel_psr2_config_valid().

Cc: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_psr.c | 10 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e27ba8fb64e6..8a584273f897 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -602,7 +602,6 @@ struct i915_psr {
struct delayed_work work;
unsigned busy_frontbuffer_bits;
bool psr2_support;
-   bool aux_frame_sync;
bool link_standby;
bool y_cord_support;
bool colorimetry_support;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 317cb4a12693..9811f5f0bc75 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -144,9 +144,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
  DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
  &frame_sync_cap) != 1)
frame_sync_cap = 0;
-   dev_priv->psr.aux_frame_sync = frame_sync_cap & 
DP_AUX_FRAME_SYNC_CAP;
+   frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP);
/* PSR2 needs frame sync as well */
-   dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
+   dev_priv->psr.psr2_support = frame_sync_cap;
DRM_DEBUG_KMS("PSR2 %s on sink",
  dev_priv->psr.psr2_support ? "supported" : "not 
supported");
 
@@ -269,7 +269,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
 
/* Enable AUX frame sync at sink */
-   if (dev_priv->psr.aux_frame_sync)
+   if (dev_priv->psr.psr2_support)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
DP_AUX_FRAME_SYNC_ENABLE);
@@ -714,7 +714,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
i915_reg_t psr_status;
u32 psr_status_mask;
 
-   if (dev_priv->psr.aux_frame_sync)
+   if (dev_priv->psr.psr2_support)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
0);
@@ -862,7 +862,7 @@ static void intel_psr_exit(struct drm_i915_private 
*dev_priv)
return;
 
if (HAS_DDI(dev_priv)) {
-   if (dev_priv->psr.aux_frame_sync)
+   if (dev_priv->psr.psr2_support)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
0);
-- 
2.16.2

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