* [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
@ 2018-03-16 23:04 ` José Roberto de Souza
2018-03-16 23:25 ` Rodrigo Vivi
2018-03-17 0:17 ` Pandiyan, Dhinakaran
2018-03-16 23:04 ` [PATCH v2 3/5] drm/i915/psr/cnl: Enable Y-coordinate support in source José Roberto de Souza
` (5 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: José Roberto de Souza @ 2018-03-16 23:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
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 <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
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, Table 3.10 */
@@ -478,15 +491,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
return false;
}
- /*
- * FIXME:enable psr2 only for y-cordinate psr2 panels
- * After gtc implementation , remove this restriction.
- */
- if (!dev_priv->psr.y_cord_support) {
- DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n");
- return false;
- }
-
return true;
}
@@ -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;
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
2018-03-16 23:04 ` [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd() José Roberto de Souza
@ 2018-03-16 23:25 ` Rodrigo Vivi
2018-03-17 0:25 ` Pandiyan, Dhinakaran
2018-03-17 0:17 ` Pandiyan, Dhinakaran
1 sibling, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2018-03-16 23:25 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
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 <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>
> 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, Table 3.10 */
> @@ -478,15 +491,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> - /*
> - * FIXME:enable psr2 only for y-cordinate psr2 panels
> - * After gtc implementation , remove this restriction.
> - */
> - if (!dev_priv->psr.y_cord_support) {
> - DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n");
> - return false;
> - }
> -
> return true;
> }
>
> @@ -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;
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
2018-03-16 23:25 ` Rodrigo Vivi
@ 2018-03-17 0:25 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-17 0:25 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx@lists.freedesktop.org
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 <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> > ---
> >
> > 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
2018-03-16 23:04 ` [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd() José Roberto de Souza
2018-03-16 23:25 ` Rodrigo Vivi
@ 2018-03-17 0:17 ` Pandiyan, Dhinakaran
1 sibling, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-17 0:17 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo
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 <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> 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_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, Table 3.10 */
> @@ -478,15 +491,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> - /*
> - * FIXME:enable psr2 only for y-cordinate psr2 panels
> - * After gtc implementation , remove this restriction.
> - */
> - if (!dev_priv->psr.y_cord_support) {
> - DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n");
> - return false;
> - }
> -
> return true;
> }
>
> @@ -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;
> I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
>
> I915_WRITE(EDP_PSR_DEBUG,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/5] drm/i915/psr/cnl: Enable Y-coordinate support in source
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
2018-03-16 23:04 ` [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd() José Roberto de Souza
@ 2018-03-16 23:04 ` José Roberto de Souza
2018-03-17 0:29 ` Pandiyan, Dhinakaran
2018-03-16 23:05 ` [PATCH v2 4/5] drm/i915/psr: Do not override PSR2 sink support José Roberto de Souza
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2018-03-16 23:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan
From: "Souza, Jose" <jose.souza@intel.com>
For Geminilake and Cannonlake+ the Y-coordinate support must be
enabled in PSR2_CTL too.
Spec: 7713
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
v2: This is specific to Geminilake and Cannonlake+
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 6 ++++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1e000f3004cb..bac54f744913 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3869,6 +3869,8 @@ enum {
#define EDP_PSR2_CTL _MMIO(0x6f900)
#define EDP_PSR2_ENABLE (1<<31)
#define EDP_SU_TRACK_ENABLE (1<<30)
+#define EDP_Y_COORDINATE_VALID (1<<26) /* GLK and CNL+ */
+#define EDP_Y_COORDINATE_ENABLE (1<<25) /* GLK and CNL+ */
#define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20)
#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f<<20)
#define EDP_PSR2_TP2_TIME_500 (0<<8)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5593d1f3049a..c5eeb13cbcfd 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -417,8 +417,10 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
/* FIXME: selective update is probably totally broken because it doesn't
* mesh at all with our frontbuffer tracking. And the hw alone isn't
* good enough. */
- val |= EDP_PSR2_ENABLE |
- EDP_SU_TRACK_ENABLE;
+ val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+ val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
+ }
if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_SYNCHRONIZATION_LATENCY_IN_SINK,
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/5] drm/i915/psr/cnl: Enable Y-coordinate support in source
2018-03-16 23:04 ` [PATCH v2 3/5] drm/i915/psr/cnl: Enable Y-coordinate support in source José Roberto de Souza
@ 2018-03-17 0:29 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-17 0:29 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Fri, 2018-03-16 at 16:04 -0700, José Roberto de Souza wrote:
> From: "Souza, Jose" <jose.souza@intel.com>
>
> For Geminilake and Cannonlake+ the Y-coordinate support must be
> enabled in PSR2_CTL too.
>
> Spec: 7713
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> v2: This is specific to Geminilake and Cannonlake+
>
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 6 ++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1e000f3004cb..bac54f744913 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3869,6 +3869,8 @@ enum {
> #define EDP_PSR2_CTL _MMIO(0x6f900)
> #define EDP_PSR2_ENABLE (1<<31)
> #define EDP_SU_TRACK_ENABLE (1<<30)
> +#define EDP_Y_COORDINATE_VALID (1<<26) /* GLK and CNL+ */
> +#define EDP_Y_COORDINATE_ENABLE (1<<25) /* GLK and CNL+ */
> #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20)
> #define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f<<20)
> #define EDP_PSR2_TP2_TIME_500 (0<<8)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 5593d1f3049a..c5eeb13cbcfd 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -417,8 +417,10 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> /* FIXME: selective update is probably totally broken because it doesn't
> * mesh at all with our frontbuffer tracking. And the hw alone isn't
> * good enough. */
> - val |= EDP_PSR2_ENABLE |
> - EDP_SU_TRACK_ENABLE;
> + val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> + val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
> + }
The corresponding bits for gen-9 are set in hsw_psr_enable_source.
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_SYNCHRONIZATION_LATENCY_IN_SINK,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 4/5] drm/i915/psr: Do not override PSR2 sink support
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
2018-03-16 23:04 ` [PATCH v2 2/5] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd() José Roberto de Souza
2018-03-16 23:04 ` [PATCH v2 3/5] drm/i915/psr/cnl: Enable Y-coordinate support in source José Roberto de Souza
@ 2018-03-16 23:05 ` José Roberto de Souza
2018-03-16 23:26 ` Rodrigo Vivi
2018-03-16 23:05 ` [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state José Roberto de Souza
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2018-03-16 23:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Sink can support our PSR2 requirements but userspace can request
a resolution that PSR2 hardware do not support, in this case it
was overwritten the PSR2 sink support.
Adding another flag here, this way if requested resolution changed
to a value that PSR2 hardware can handle, PSR2 can be enabled.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++------------------
3 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5378863e3238..4366adcad56d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2571,7 +2571,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
yesno(work_busy(&dev_priv->psr.work.work)));
if (HAS_DDI(dev_priv)) {
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
else
enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
@@ -2619,7 +2619,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
seq_printf(m, "Performance_Counter: %u\n", psrperf);
}
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
u32 psr2 = I915_READ(EDP_PSR2_STATUS);
seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d4bc8d18f56c..d4475196f78a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -601,11 +601,12 @@ struct i915_psr {
bool active;
struct delayed_work work;
unsigned busy_frontbuffer_bits;
- bool psr2_support;
+ bool sink_psr2_support;
bool link_standby;
bool colorimetry_support;
bool alpm;
bool has_hw_tracking;
+ bool psr2_enabled;
void (*enable_source)(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 c5eeb13cbcfd..aa4e03f65386 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -161,15 +161,15 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
*/
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)
+ dev_priv->psr.sink_psr2_support = frame_sync_cap && y_coord_req;
+ if (dev_priv->psr.sink_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) {
+ if (dev_priv->psr.sink_psr2_support) {
dev_priv->psr.colorimetry_support =
intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.alpm =
@@ -210,7 +210,7 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
struct edp_vsc_psr psr_vsc;
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
memset(&psr_vsc, 0, sizeof(psr_vsc));
psr_vsc.sdp_header.HB0 = 0;
@@ -282,12 +282,12 @@ 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.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
DP_AUX_FRAME_SYNC_ENABLE);
/* Enable ALPM at sink for psr2 */
- if (dev_priv->psr.psr2_support && dev_priv->psr.alpm)
+ if (dev_priv->psr.psr2_enabled && dev_priv->psr.alpm)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_RECEIVER_ALPM_CONFIG,
DP_ALPM_ENABLE);
@@ -455,7 +455,7 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
*/
/* psr1 and psr2 are mutually exclusive.*/
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
hsw_activate_psr2(intel_dp);
else
hsw_activate_psr1(intel_dp);
@@ -475,7 +475,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
* dynamically during PSR enable, and extracted from sink
* caps during eDP detection.
*/
- if (!dev_priv->psr.psr2_support)
+ if (!dev_priv->psr.sink_psr2_support)
return false;
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
@@ -574,7 +574,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
@@ -595,7 +595,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
psr_aux_io_power_get(intel_dp);
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER
| PSR2_ADD_VERTICAL_LINE_COUNT;
I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
@@ -648,7 +648,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
goto unlock;
}
- dev_priv->psr.psr2_support = crtc_state->has_psr2;
+ dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
dev_priv->psr.busy_frontbuffer_bits = 0;
dev_priv->psr.setup_vsc(intel_dp, crtc_state);
@@ -718,12 +718,12 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
i915_reg_t psr_status;
u32 psr_status_mask;
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
0);
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
psr_status = EDP_PSR2_STATUS;
psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
@@ -747,7 +747,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
dev_priv->psr.active = false;
} else {
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
@@ -807,7 +807,7 @@ static void intel_psr_work(struct work_struct *work)
* and be ready for re-enable.
*/
if (HAS_DDI(dev_priv)) {
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
if (intel_wait_for_register(dev_priv,
EDP_PSR2_STATUS,
EDP_PSR2_STATUS_STATE_MASK,
@@ -866,11 +866,11 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
return;
if (HAS_DDI(dev_priv)) {
- if (dev_priv->psr.psr2_support)
+ if (dev_priv->psr.psr2_enabled)
drm_dp_dpcd_writeb(&intel_dp->aux,
DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
0);
- if (dev_priv->psr.psr2_support) {
+ if (dev_priv->psr.psr2_enabled) {
val = I915_READ(EDP_PSR2_CTL);
WARN_ON(!(val & EDP_PSR2_ENABLE));
I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
@@ -1039,7 +1039,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
/* By definition flush = invalidate + flush */
if (frontbuffer_bits) {
- if (dev_priv->psr.psr2_support ||
+ if (dev_priv->psr.psr2_enabled ||
IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
intel_psr_exit(dev_priv);
} else {
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 4/5] drm/i915/psr: Do not override PSR2 sink support
2018-03-16 23:05 ` [PATCH v2 4/5] drm/i915/psr: Do not override PSR2 sink support José Roberto de Souza
@ 2018-03-16 23:26 ` Rodrigo Vivi
0 siblings, 0 replies; 16+ messages in thread
From: Rodrigo Vivi @ 2018-03-16 23:26 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 16, 2018 at 04:05:00PM -0700, José Roberto de Souza wrote:
> Sink can support our PSR2 requirements but userspace can request
> a resolution that PSR2 hardware do not support, in this case it
> was overwritten the PSR2 sink support.
> Adding another flag here, this way if requested resolution changed
> to a value that PSR2 hardware can handle, PSR2 can be enabled.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++------------------
> 3 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5378863e3238..4366adcad56d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2571,7 +2571,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> yesno(work_busy(&dev_priv->psr.work.work)));
>
> if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
> else
> enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
> @@ -2619,7 +2619,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>
> seq_printf(m, "Performance_Counter: %u\n", psrperf);
> }
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> u32 psr2 = I915_READ(EDP_PSR2_STATUS);
>
> seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d4bc8d18f56c..d4475196f78a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -601,11 +601,12 @@ struct i915_psr {
> bool active;
> struct delayed_work work;
> unsigned busy_frontbuffer_bits;
> - bool psr2_support;
> + bool sink_psr2_support;
> bool link_standby;
> bool colorimetry_support;
> bool alpm;
> bool has_hw_tracking;
> + bool psr2_enabled;
>
> void (*enable_source)(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 c5eeb13cbcfd..aa4e03f65386 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -161,15 +161,15 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> */
> 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)
> + dev_priv->psr.sink_psr2_support = frame_sync_cap && y_coord_req;
> + if (dev_priv->psr.sink_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) {
> + if (dev_priv->psr.sink_psr2_support) {
> dev_priv->psr.colorimetry_support =
> intel_dp_get_colorimetry_status(intel_dp);
> dev_priv->psr.alpm =
> @@ -210,7 +210,7 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
> struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> struct edp_vsc_psr psr_vsc;
>
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> /* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
> memset(&psr_vsc, 0, sizeof(psr_vsc));
> psr_vsc.sdp_header.HB0 = 0;
> @@ -282,12 +282,12 @@ 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.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> DP_AUX_FRAME_SYNC_ENABLE);
> /* Enable ALPM at sink for psr2 */
> - if (dev_priv->psr.psr2_support && dev_priv->psr.alpm)
> + if (dev_priv->psr.psr2_enabled && dev_priv->psr.alpm)
> drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
> DP_ALPM_ENABLE);
> @@ -455,7 +455,7 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
> */
>
> /* psr1 and psr2 are mutually exclusive.*/
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> hsw_activate_psr2(intel_dp);
> else
> hsw_activate_psr1(intel_dp);
> @@ -475,7 +475,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> * dynamically during PSR enable, and extracted from sink
> * caps during eDP detection.
> */
> - if (!dev_priv->psr.psr2_support)
> + if (!dev_priv->psr.sink_psr2_support)
> return false;
>
> if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> @@ -574,7 +574,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> @@ -595,7 +595,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
>
> psr_aux_io_power_get(intel_dp);
>
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER
> | PSR2_ADD_VERTICAL_LINE_COUNT;
> I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> @@ -648,7 +648,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> goto unlock;
> }
>
> - dev_priv->psr.psr2_support = crtc_state->has_psr2;
> + dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
> dev_priv->psr.busy_frontbuffer_bits = 0;
>
> dev_priv->psr.setup_vsc(intel_dp, crtc_state);
> @@ -718,12 +718,12 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> i915_reg_t psr_status;
> u32 psr_status_mask;
>
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> 0);
>
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> psr_status = EDP_PSR2_STATUS;
> psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
>
> @@ -747,7 +747,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
>
> dev_priv->psr.active = false;
> } else {
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> @@ -807,7 +807,7 @@ static void intel_psr_work(struct work_struct *work)
> * and be ready for re-enable.
> */
> if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> if (intel_wait_for_register(dev_priv,
> EDP_PSR2_STATUS,
> EDP_PSR2_STATUS_STATE_MASK,
> @@ -866,11 +866,11 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> return;
>
> if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.psr2_support)
> + if (dev_priv->psr.psr2_enabled)
> drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> 0);
> - if (dev_priv->psr.psr2_support) {
> + if (dev_priv->psr.psr2_enabled) {
> val = I915_READ(EDP_PSR2_CTL);
> WARN_ON(!(val & EDP_PSR2_ENABLE));
> I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> @@ -1039,7 +1039,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>
> /* By definition flush = invalidate + flush */
> if (frontbuffer_bits) {
> - if (dev_priv->psr.psr2_support ||
> + if (dev_priv->psr.psr2_enabled ||
> IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> intel_psr_exit(dev_priv);
> } else {
> --
> 2.16.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
` (2 preceding siblings ...)
2018-03-16 23:05 ` [PATCH v2 4/5] drm/i915/psr: Do not override PSR2 sink support José Roberto de Souza
@ 2018-03-16 23:05 ` José Roberto de Souza
2018-03-16 23:30 ` Rodrigo Vivi
2018-03-16 23:38 ` [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync Pandiyan, Dhinakaran
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2018-03-16 23:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Having has_psr and has_psr2 can be ambiguous and also uses one more
byte than needed(not taking in care struct alignment).
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
v2: Grouped the 2 bools into one u8 as suggested by Dhinakaran Pandiyan.
drivers/gpu/drm/i915/intel_drv.h | 3 +--
drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a215aa78b0be..a7383235f90a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -807,8 +807,7 @@ struct intel_crtc_state {
struct intel_link_m_n dp_m2_n2;
bool has_drrs;
- bool has_psr;
- bool has_psr2;
+ u8 psr;
/*
* Frequence the dpll for the port should run at. Differs from the
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index aa4e03f65386..78b5c0c88261 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -563,9 +563,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
return;
}
- crtc_state->has_psr = true;
- crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
- DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
+ if (intel_psr2_config_valid(intel_dp, crtc_state))
+ crtc_state->psr = DP_PSR2_IS_SUPPORTED;
+ else
+ crtc_state->psr = DP_PSR_IS_SUPPORTED;
+ DRM_DEBUG_KMS("Enabling PSR%d\n", crtc_state->psr);
}
static void intel_psr_activate(struct intel_dp *intel_dp)
@@ -635,7 +637,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- if (!crtc_state->has_psr)
+ if (!crtc_state->psr)
return;
if (WARN_ON(!CAN_PSR(dev_priv)))
@@ -648,7 +650,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
goto unlock;
}
- dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
+ dev_priv->psr.psr2_enabled = (crtc_state->psr == DP_PSR2_IS_SUPPORTED);
dev_priv->psr.busy_frontbuffer_bits = 0;
dev_priv->psr.setup_vsc(intel_dp, crtc_state);
@@ -770,7 +772,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- if (!old_crtc_state->has_psr)
+ if (!old_crtc_state->psr)
return;
if (WARN_ON(!CAN_PSR(dev_priv)))
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state
2018-03-16 23:05 ` [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state José Roberto de Souza
@ 2018-03-16 23:30 ` Rodrigo Vivi
2018-03-17 0:38 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2018-03-16 23:30 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Mar 16, 2018 at 04:05:01PM -0700, José Roberto de Souza wrote:
> Having has_psr and has_psr2 can be ambiguous and also uses one more
> byte than needed(not taking in care struct alignment).
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> v2: Grouped the 2 bools into one u8 as suggested by Dhinakaran Pandiyan.
>
> drivers/gpu/drm/i915/intel_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a215aa78b0be..a7383235f90a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -807,8 +807,7 @@ struct intel_crtc_state {
> struct intel_link_m_n dp_m2_n2;
> bool has_drrs;
>
> - bool has_psr;
> - bool has_psr2;
> + u8 psr;
>
> /*
> * Frequence the dpll for the port should run at. Differs from the
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index aa4e03f65386..78b5c0c88261 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -563,9 +563,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> return;
> }
>
> - crtc_state->has_psr = true;
> - crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> - DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> + if (intel_psr2_config_valid(intel_dp, crtc_state))
> + crtc_state->psr = DP_PSR2_IS_SUPPORTED;
> + else
> + crtc_state->psr = DP_PSR_IS_SUPPORTED;
> + DRM_DEBUG_KMS("Enabling PSR%d\n", crtc_state->psr);
Could we still continue writing "PSR" instead of "PSR1" ?
otherwise patch lgtm...
> }
>
> static void intel_psr_activate(struct intel_dp *intel_dp)
> @@ -635,7 +637,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - if (!crtc_state->has_psr)
> + if (!crtc_state->psr)
> return;
>
> if (WARN_ON(!CAN_PSR(dev_priv)))
> @@ -648,7 +650,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> goto unlock;
> }
>
> - dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
> + dev_priv->psr.psr2_enabled = (crtc_state->psr == DP_PSR2_IS_SUPPORTED);
> dev_priv->psr.busy_frontbuffer_bits = 0;
>
> dev_priv->psr.setup_vsc(intel_dp, crtc_state);
> @@ -770,7 +772,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> - if (!old_crtc_state->has_psr)
> + if (!old_crtc_state->psr)
> return;
>
> if (WARN_ON(!CAN_PSR(dev_priv)))
> --
> 2.16.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state
2018-03-16 23:30 ` Rodrigo Vivi
@ 2018-03-17 0:38 ` Pandiyan, Dhinakaran
2018-03-17 1:31 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-17 0:38 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx@lists.freedesktop.org
On Fri, 2018-03-16 at 16:30 -0700, Rodrigo Vivi wrote:
> On Fri, Mar 16, 2018 at 04:05:01PM -0700, José Roberto de Souza wrote:
> > Having has_psr and has_psr2 can be ambiguous and also uses one more
> > byte than needed(not taking in care struct alignment).
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >
> > v2: Grouped the 2 bools into one u8 as suggested by Dhinakaran Pandiyan.
> >
> > drivers/gpu/drm/i915/intel_drv.h | 3 +--
> > drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
> > 2 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a215aa78b0be..a7383235f90a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -807,8 +807,7 @@ struct intel_crtc_state {
> > struct intel_link_m_n dp_m2_n2;
> > bool has_drrs;
> >
> > - bool has_psr;
> > - bool has_psr2;
> > + u8 psr;
/* 0 = disabled, 1 = PSR1, 2 = PSR2 */
> >
> > /*
> > * Frequence the dpll for the port should run at. Differs from the
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index aa4e03f65386..78b5c0c88261 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -563,9 +563,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> > return;
> > }
> >
> > - crtc_state->has_psr = true;
> > - crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> > - DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> > + if (intel_psr2_config_valid(intel_dp, crtc_state))
> > + crtc_state->psr = DP_PSR2_IS_SUPPORTED;
We can avoid the dependency on an unrelated macro definition if you
explicitly set it to 1 or 2.
> > + else
> > + crtc_state->psr = DP_PSR_IS_SUPPORTED;
> > + DRM_DEBUG_KMS("Enabling PSR%d\n", crtc_state->psr);
Also I think you should initialize ->psr = 0
>
> Could we still continue writing "PSR" instead of "PSR1" ?
>
> otherwise patch lgtm...
>
> > }
> >
> > static void intel_psr_activate(struct intel_dp *intel_dp)
> > @@ -635,7 +637,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> >
> > - if (!crtc_state->has_psr)
> > + if (!crtc_state->psr)
> > return;
> >
> > if (WARN_ON(!CAN_PSR(dev_priv)))
> > @@ -648,7 +650,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> > goto unlock;
> > }
> >
> > - dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
> > + dev_priv->psr.psr2_enabled = (crtc_state->psr == DP_PSR2_IS_SUPPORTED);
> > dev_priv->psr.busy_frontbuffer_bits = 0;
> >
> > dev_priv->psr.setup_vsc(intel_dp, crtc_state);
> > @@ -770,7 +772,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> >
> > - if (!old_crtc_state->has_psr)
> > + if (!old_crtc_state->psr)
> > return;
> >
> > if (WARN_ON(!CAN_PSR(dev_priv)))
> > --
> > 2.16.2
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state
2018-03-17 0:38 ` Pandiyan, Dhinakaran
@ 2018-03-17 1:31 ` Souza, Jose
0 siblings, 0 replies; 16+ messages in thread
From: Souza, Jose @ 2018-03-17 1:31 UTC (permalink / raw)
To: Vivi, Rodrigo, Pandiyan, Dhinakaran; +Cc: intel-gfx@lists.freedesktop.org
On Fri, 2018-03-16 at 17:38 -0700, Pandiyan, Dhinakaran wrote:
>
>
> On Fri, 2018-03-16 at 16:30 -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 16, 2018 at 04:05:01PM -0700, José Roberto de Souza
> > wrote:
> > > Having has_psr and has_psr2 can be ambiguous and also uses one
> > > more
> > > byte than needed(not taking in care struct alignment).
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >
> > > v2: Grouped the 2 bools into one u8 as suggested by Dhinakaran
> > > Pandiyan.
> > >
> > > drivers/gpu/drm/i915/intel_drv.h | 3 +--
> > > drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++------
> > > 2 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index a215aa78b0be..a7383235f90a 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -807,8 +807,7 @@ struct intel_crtc_state {
> > > struct intel_link_m_n dp_m2_n2;
> > > bool has_drrs;
> > >
> > > - bool has_psr;
> > > - bool has_psr2;
> > > + u8 psr;
>
> /* 0 = disabled, 1 = PSR1, 2 = PSR2 */
>
> > >
> > > /*
> > > * Frequence the dpll for the port should run at.
> > > Differs from the
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index aa4e03f65386..78b5c0c88261 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -563,9 +563,11 @@ void intel_psr_compute_config(struct
> > > intel_dp *intel_dp,
> > > return;
> > > }
> > >
> > > - crtc_state->has_psr = true;
> > > - crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp,
> > > crtc_state);
> > > - DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ?
> > > "2" : "");
> > > + if (intel_psr2_config_valid(intel_dp, crtc_state))
> > > + crtc_state->psr = DP_PSR2_IS_SUPPORTED;
>
> We can avoid the dependency on an unrelated macro definition if you
> explicitly set it to 1 or 2.
I don't like the idea of using magic numbers, what do you think about
add enum?
>
> > > + else
> > > + crtc_state->psr = DP_PSR_IS_SUPPORTED;
> > > + DRM_DEBUG_KMS("Enabling PSR%d\n", crtc_state->psr);
>
> Also I think you should initialize ->psr = 0
> >
> > Could we still continue writing "PSR" instead of "PSR1" ?
> >
> > otherwise patch lgtm...
> >
> > > }
> > >
> > > static void intel_psr_activate(struct intel_dp *intel_dp)
> > > @@ -635,7 +637,7 @@ void intel_psr_enable(struct intel_dp
> > > *intel_dp,
> > > struct drm_device *dev = intel_dig_port->base.base.dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > >
> > > - if (!crtc_state->has_psr)
> > > + if (!crtc_state->psr)
> > > return;
> > >
> > > if (WARN_ON(!CAN_PSR(dev_priv)))
> > > @@ -648,7 +650,7 @@ void intel_psr_enable(struct intel_dp
> > > *intel_dp,
> > > goto unlock;
> > > }
> > >
> > > - dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
> > > + dev_priv->psr.psr2_enabled = (crtc_state->psr ==
> > > DP_PSR2_IS_SUPPORTED);
> > > dev_priv->psr.busy_frontbuffer_bits = 0;
> > >
> > > dev_priv->psr.setup_vsc(intel_dp, crtc_state);
> > > @@ -770,7 +772,7 @@ void intel_psr_disable(struct intel_dp
> > > *intel_dp,
> > > struct drm_device *dev = intel_dig_port->base.base.dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > >
> > > - if (!old_crtc_state->has_psr)
> > > + if (!old_crtc_state->psr)
> > > return;
> > >
> > > if (WARN_ON(!CAN_PSR(dev_priv)))
> > > --
> > > 2.16.2
> > >
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
` (3 preceding siblings ...)
2018-03-16 23:05 ` [PATCH v2 5/5] drm/i915/psr: Simply PSR computed state José Roberto de Souza
@ 2018-03-16 23:38 ` Pandiyan, Dhinakaran
2018-03-16 23:50 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/5] " Patchwork
2018-03-17 0:05 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-16 23:38 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo
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 <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/5] drm/i915/psr: Nuke aux_frame_sync
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
` (4 preceding siblings ...)
2018-03-16 23:38 ` [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync Pandiyan, Dhinakaran
@ 2018-03-16 23:50 ` Patchwork
2018-03-17 0:05 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-03-16 23:50 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/5] drm/i915/psr: Nuke aux_frame_sync
URL : https://patchwork.freedesktop.org/series/40138/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ed472e09578b drm/i915/psr: Nuke aux_frame_sync
7106739a3800 drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
ff9a3483c3d6 drm/i915/psr/cnl: Enable Y-coordinate support in source
-:26: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#26: FILE: drivers/gpu/drm/i915/i915_reg.h:3872:
+#define EDP_Y_COORDINATE_VALID (1<<26) /* GLK and CNL+ */
^
-:27: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#27: FILE: drivers/gpu/drm/i915/i915_reg.h:3873:
+#define EDP_Y_COORDINATE_ENABLE (1<<25) /* GLK and CNL+ */
^
total: 0 errors, 0 warnings, 2 checks, 20 lines checked
37e021ca152b drm/i915/psr: Do not override PSR2 sink support
a737ca97244b drm/i915/psr: Simply PSR computed state
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [v2,1/5] drm/i915/psr: Nuke aux_frame_sync
2018-03-16 23:04 [PATCH v2 1/5] drm/i915/psr: Nuke aux_frame_sync José Roberto de Souza
` (5 preceding siblings ...)
2018-03-16 23:50 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/5] " Patchwork
@ 2018-03-17 0:05 ` Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-03-17 0:05 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/5] drm/i915/psr: Nuke aux_frame_sync
URL : https://patchwork.freedesktop.org/series/40138/
State : failure
== Summary ==
Series 40138v1 series starting with [v2,1/5] drm/i915/psr: Nuke aux_frame_sync
https://patchwork.freedesktop.org/api/1.0/series/40138/revisions/1/mbox/
---- Possible new issues:
Test gem_sync:
Subgroup basic-many-each:
pass -> INCOMPLETE (fi-snb-2600)
---- Known issues:
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (fi-skl-6770hq) fdo#100368
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> PASS (fi-snb-2520m) fdo#103713
Subgroup suspend-read-crc-pipe-c:
pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:444s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:379s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:535s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s
fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:510s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:519s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:503s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:409s
fi-cfl-s2 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:582s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s
fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:539s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:584s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:425s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:319s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:535s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:402s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:417s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:473s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:430s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:467s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:516s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:649s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:445s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:531s
fi-skl-6700hq total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:539s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:508s
fi-skl-6770hq total:285 pass:264 dwarn:0 dfail:0 fail:1 skip:20 time:476s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:427s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:448s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:556s
fi-snb-2600 total:149 pass:132 dwarn:0 dfail:0 fail:0 skip:16
639c78d2805bec2022435a8184db27a3b9767fe7 drm-tip: 2018y-03m-16d-23h-05m-00s UTC integration manifest
a737ca97244b drm/i915/psr: Simply PSR computed state
37e021ca152b drm/i915/psr: Do not override PSR2 sink support
ff9a3483c3d6 drm/i915/psr/cnl: Enable Y-coordinate support in source
7106739a3800 drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
ed472e09578b drm/i915/psr: Nuke aux_frame_sync
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8385/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread