Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Fri, 2018-03-16 at 16:25 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 16, 2018 at 04:04:58PM -0700, José Roberto de Souza wrote:
> > Move to only one place the sink requirements that the actual driver
> > needs to enable PSR2.
> >
> > Also intel_psr2_config_valid() is called every time the crtc config
> > is computed, wasting some time every time it was checking for
> > Y coordinate requirement.
> >
> > This allow us to nuke y_cord_support and some of VSC setup code that
> > was handling a scenario that would never happen(PSR2 without Y
> > coordinate).
> >
> > Cc: Dhinakaran Pandiyan
> > Cc: Rodrigo Vivi
> > Signed-off-by: José Roberto de Souza
>
> Reviewed-by: Rodrigo Vivi
>
> > ---
> >
> > v2: Changes in comment requested by Rodrigo Vivi
> >
> > drivers/gpu/drm/i915/i915_drv.h | 1 -
> > drivers/gpu/drm/i915/intel_psr.c | 56
> > +---
> > 2 files changed, 29 insertions(+), 28 deletions(-)
> >
> >
> > @@ -586,14 +590,12 @@ static void hsw_psr_enable_source(struct intel_dp
> > *intel_dp,
> > struct drm_device *dev = dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > - u32 chicken;
> >
> > psr_aux_io_power_get(intel_dp);
> >
> > if (dev_priv->psr.psr2_support) {
> > - chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> > - if (dev_priv->psr.y_cord_support)
> > - chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
> > + u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER
> > + | PSR2_ADD_VERTICAL_LINE_COUNT;
I think I answered my own question.
Please check the bits on these register. Seems like these bits were
replaced by
PSR2_CTL[EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE] bits on gen10
If I am correct, this register should be set only for gen-9. It'd be
nice to fix PSR2 while making these changes :)
> > I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> >
> > I915_WRITE(EDP_PSR_DEBUG,
> > --
> > 2.16.2
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Fri, 2018-03-16 at 16:04 -0700, José Roberto de Souza wrote:
> Move to only one place the sink requirements that the actual driver
> needs to enable PSR2.
>
> Also intel_psr2_config_valid() is called every time the crtc config
> is computed, wasting some time every time it was checking for
> Y coordinate requirement.
>
> This allow us to nuke y_cord_support and some of VSC setup code that
> was handling a scenario that would never happen(PSR2 without Y
> coordinate).
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
>
> v2: Changes in comment requested by Rodrigo Vivi
>
> drivers/gpu/drm/i915/i915_drv.h | 1 -
> drivers/gpu/drm/i915/intel_psr.c | 56
> +---
> 2 files changed, 29 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8a584273f897..d4bc8d18f56c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -603,7 +603,6 @@ struct i915_psr {
> unsigned busy_frontbuffer_bits;
> bool psr2_support;
> bool link_standby;
> - bool y_cord_support;
> bool colorimetry_support;
> bool alpm;
> bool has_hw_tracking;
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 9811f5f0bc75..5593d1f3049a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
> intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> }
>
> -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> +static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
> {
> uint8_t psr_caps = 0;
>
> @@ -137,22 +137,39 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>
> if (INTEL_GEN(dev_priv) >= 9 &&
> (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> - uint8_t frame_sync_cap;
> + uint8_t frame_sync_cap, y_coord_req;
>
> dev_priv->psr.sink_support = true;
> +
> + /* PSR2 needs frame sync to do selective updates */
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> &frame_sync_cap) != 1)
> frame_sync_cap = 0;
> frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP);
> - /* PSR2 needs frame sync as well */
> - dev_priv->psr.psr2_support = frame_sync_cap;
> - DRM_DEBUG_KMS("PSR2 %s on sink",
> - dev_priv->psr.psr2_support ? "supported" : "not
> supported");
> +
> + /*
> + * All panels that supports PSR version 03h (PSR2 +
> + * Y-coordinate)
nit: Perhaps check for PSR version 03h as a minimum requirement for
PSR2, a bit redundant but is useful documentation in code.
> support can handle Y-coordinates in VSC
> + * but we are only sure that it is going to be used when
> + * required by the panel. This way panel is capable to do
> + * selective update even without a valid aux frame sync.
> + *
> + * To support PSR version 02h and PSR version 03h without
> + * Y-coordinate requirement panels we would need to enable
> + * GTC first.
> + */
> + y_coord_req = intel_dp_get_y_coord_required(intel_dp);
> +
> + dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req;
> + if (dev_priv->psr.psr2_support)
> + DRM_DEBUG_KMS("PSR2 supported on sink\n");
> + else
> + DRM_DEBUG_KMS("PSR2 not supported on sink"
> + "(frame sync: %d Y-coord required: %d)\n",
> + frame_sync_cap, y_coord_req);
>
> if (dev_priv->psr.psr2_support) {
> - dev_priv->psr.y_cord_support =
> - intel_dp_get_y_cord_status(intel_dp);
> dev_priv->psr.colorimetry_support =
> intel_dp_get_colorimetry_status(intel_dp);
> dev_priv->psr.alpm =
> @@ -198,16 +215,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
> memset(&psr_vsc, 0, sizeof(psr_vsc));
> psr_vsc.sdp_header.HB0 = 0;
> psr_vsc.sdp_header.HB1 = 0x7;
> - if (dev_priv->psr.colorimetry_support &&
> - dev_priv->psr.y_cord_support) {
> + if (dev_priv->psr.colorimetry_support) {
> psr_vsc.sdp_header.HB2 = 0x5;
> psr_vsc.sdp_header.HB3 = 0x13;
> - } else if (dev_priv->psr.y_cord_support) {
> + } else {
> psr_vsc.sdp_h
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Fri, Mar 16, 2018 at 04:04:58PM -0700, José Roberto de Souza wrote:
> Move to only one place the sink requirements that the actual driver
> needs to enable PSR2.
>
> Also intel_psr2_config_valid() is called every time the crtc config
> is computed, wasting some time every time it was checking for
> Y coordinate requirement.
>
> This allow us to nuke y_cord_support and some of VSC setup code that
> was handling a scenario that would never happen(PSR2 without Y
> coordinate).
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
Reviewed-by: Rodrigo Vivi
> ---
>
> v2: Changes in comment requested by Rodrigo Vivi
>
> drivers/gpu/drm/i915/i915_drv.h | 1 -
> drivers/gpu/drm/i915/intel_psr.c | 56
> +---
> 2 files changed, 29 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8a584273f897..d4bc8d18f56c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -603,7 +603,6 @@ struct i915_psr {
> unsigned busy_frontbuffer_bits;
> bool psr2_support;
> bool link_standby;
> - bool y_cord_support;
> bool colorimetry_support;
> bool alpm;
> bool has_hw_tracking;
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 9811f5f0bc75..5593d1f3049a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
> intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
> }
>
> -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> +static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
> {
> uint8_t psr_caps = 0;
>
> @@ -137,22 +137,39 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>
> if (INTEL_GEN(dev_priv) >= 9 &&
> (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> - uint8_t frame_sync_cap;
> + uint8_t frame_sync_cap, y_coord_req;
>
> dev_priv->psr.sink_support = true;
> +
> + /* PSR2 needs frame sync to do selective updates */
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> &frame_sync_cap) != 1)
> frame_sync_cap = 0;
> frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP);
> - /* PSR2 needs frame sync as well */
> - dev_priv->psr.psr2_support = frame_sync_cap;
> - DRM_DEBUG_KMS("PSR2 %s on sink",
> - dev_priv->psr.psr2_support ? "supported" : "not
> supported");
> +
> + /*
> + * All panels that supports PSR version 03h (PSR2 +
> + * Y-coordinate) support can handle Y-coordinates in VSC
> + * but we are only sure that it is going to be used when
> + * required by the panel. This way panel is capable to do
> + * selective update even without a valid aux frame sync.
> + *
> + * To support PSR version 02h and PSR version 03h without
> + * Y-coordinate requirement panels we would need to enable
> + * GTC first.
> + */
> + y_coord_req = intel_dp_get_y_coord_required(intel_dp);
> +
> + dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req;
> + if (dev_priv->psr.psr2_support)
> + DRM_DEBUG_KMS("PSR2 supported on sink\n");
> + else
> + DRM_DEBUG_KMS("PSR2 not supported on sink"
> + "(frame sync: %d Y-coord required: %d)\n",
> + frame_sync_cap, y_coord_req);
>
> if (dev_priv->psr.psr2_support) {
> - dev_priv->psr.y_cord_support =
> - intel_dp_get_y_cord_status(intel_dp);
> dev_priv->psr.colorimetry_support =
> intel_dp_get_colorimetry_status(intel_dp);
> dev_priv->psr.alpm =
> @@ -198,16 +215,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
> memset(&psr_vsc, 0, sizeof(psr_vsc));
> psr_vsc.sdp_header.HB0 = 0;
> psr_vsc.sdp_header.HB1 = 0x7;
> - if (dev_priv->psr.colorimetry_support &&
> - dev_priv->psr.y_cord_support) {
> + if (dev_priv->psr.colorimetry_support) {
> psr_vsc.sdp_header.HB2 = 0x5;
> psr_vsc.sdp_header.HB3 = 0x13;
> - } else if (dev_priv->psr.y_cord_support) {
> + } else {
> psr_vsc.sdp_header.HB2 = 0x4;
> psr_vsc.sdp_header.HB3 = 0xe;
> - } else {
> -
[Intel-gfx] [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
Move to only one place the sink requirements that the actual driver
needs to enable PSR2.
Also intel_psr2_config_valid() is called every time the crtc config
is computed, wasting some time every time it was checking for
Y coordinate requirement.
This allow us to nuke y_cord_support and some of VSC setup code that
was handling a scenario that would never happen(PSR2 without Y
coordinate).
Cc: Dhinakaran Pandiyan
Cc: Rodrigo Vivi
Signed-off-by: José Roberto de Souza
---
v2: Changes in comment requested by Rodrigo Vivi
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_psr.c | 56 +---
2 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8a584273f897..d4bc8d18f56c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -603,7 +603,6 @@ struct i915_psr {
unsigned busy_frontbuffer_bits;
bool psr2_support;
bool link_standby;
- bool y_cord_support;
bool colorimetry_support;
bool alpm;
bool has_hw_tracking;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 9811f5f0bc75..5593d1f3049a 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
}
-static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
+static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
{
uint8_t psr_caps = 0;
@@ -137,22 +137,39 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
if (INTEL_GEN(dev_priv) >= 9 &&
(intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
- uint8_t frame_sync_cap;
+ uint8_t frame_sync_cap, y_coord_req;
dev_priv->psr.sink_support = true;
+
+ /* PSR2 needs frame sync to do selective updates */
if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
&frame_sync_cap) != 1)
frame_sync_cap = 0;
frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP);
- /* PSR2 needs frame sync as well */
- dev_priv->psr.psr2_support = frame_sync_cap;
- DRM_DEBUG_KMS("PSR2 %s on sink",
- dev_priv->psr.psr2_support ? "supported" : "not
supported");
+
+ /*
+* All panels that supports PSR version 03h (PSR2 +
+* Y-coordinate) support can handle Y-coordinates in VSC
+* but we are only sure that it is going to be used when
+* required by the panel. This way panel is capable to do
+* selective update even without a valid aux frame sync.
+*
+* To support PSR version 02h and PSR version 03h without
+* Y-coordinate requirement panels we would need to enable
+* GTC first.
+*/
+ y_coord_req = intel_dp_get_y_coord_required(intel_dp);
+
+ dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req;
+ if (dev_priv->psr.psr2_support)
+ DRM_DEBUG_KMS("PSR2 supported on sink\n");
+ else
+ DRM_DEBUG_KMS("PSR2 not supported on sink"
+ "(frame sync: %d Y-coord required: %d)\n",
+ frame_sync_cap, y_coord_req);
if (dev_priv->psr.psr2_support) {
- dev_priv->psr.y_cord_support =
- intel_dp_get_y_cord_status(intel_dp);
dev_priv->psr.colorimetry_support =
intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.alpm =
@@ -198,16 +215,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
memset(&psr_vsc, 0, sizeof(psr_vsc));
psr_vsc.sdp_header.HB0 = 0;
psr_vsc.sdp_header.HB1 = 0x7;
- if (dev_priv->psr.colorimetry_support &&
- dev_priv->psr.y_cord_support) {
+ if (dev_priv->psr.colorimetry_support) {
psr_vsc.sdp_header.HB2 = 0x5;
psr_vsc.sdp_header.HB3 = 0x13;
- } else if (dev_priv->psr.y_cord_support) {
+ } else {
psr_vsc.sdp_header.HB2 = 0x4;
psr_vsc.sdp_header.HB3 = 0xe;
- } else {
- psr_vsc.sdp_header.HB2 = 0x3;
- psr_vsc.sdp_header.HB3 = 0xc;
}
} else {
/* Prepare VSC packet as per EDP 1.3 spec, Tab
