[Intel-gfx] [PATCH] drm/i915: Refresh VLV/CHV PSR comments on HW PSR_state machine.
DK had pointed out a comment there was hard to understand, so I
tried to read back again and I couldn't understand that as well.
So let me re-phrase that in a way that anyone can understand
later, even myself.
Also fixed the comment block style.
v2: Accept DK's suggestion on PSR_state 2 and PSR_state 3 named
as spec.
Cc: Dhinakaran Pandiyan
Signed-off-by: Rodrigo Vivi
Reviewed-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_psr.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index fdd9e3d95efb..55b4002bbe53 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -234,7 +234,7 @@ static void vlv_psr_enable_source(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
- /* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
+ /* Transition from PSR_state 0 (disabled) to PSR_state 1 (inactive) */
I915_WRITE(VLV_PSRCTL(crtc->pipe),
VLV_EDP_PSR_MODE_SW_TIMER |
VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
@@ -249,10 +249,11 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
struct drm_crtc *crtc = dig_port->base.base.crtc;
enum pipe pipe = to_intel_crtc(crtc)->pipe;
- /* Let's do the transition from PSR_state 1 to PSR_state 2
-* that is PSR transition to active - static frame transmission.
-* Then Hardware is responsible for the transition to PSR_state 3
-* that is PSR active - no Remote Frame Buffer (RFB) update.
+ /*
+* Let's do the transition from PSR_state 1 (inactive) to
+* PSR_state 2 (transition to active - static frame transmission).
+* Then Hardware is responsible for the transition to
+* PSR_state 3 (active - no Remote Frame Buffer (RFB) update).
*/
I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
VLV_EDP_PSR_ACTIVE_ENTRY);
@@ -576,7 +577,7 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
uint32_t val;
if (dev_priv->psr.active) {
- /* Put VLV PSR back to PSR_state 0 that is PSR Disabled. */
+ /* Put VLV PSR back to PSR_state 0 (disabled). */
if (intel_wait_for_register(dev_priv,
VLV_PSRSTAT(crtc->pipe),
VLV_EDP_PSR_IN_TRANS,
@@ -766,16 +767,20 @@ static void intel_psr_exit(struct drm_i915_private
*dev_priv)
} else {
val = I915_READ(VLV_PSRCTL(pipe));
- /* Here we do the transition from PSR_state 3 to PSR_state 5
-* directly once PSR State 4 that is active with single frame
-* update can be skipped. PSR_state 5 that is PSR exit then
-* Hardware is responsible to transition back to PSR_state 1
-* that is PSR inactive. Same state after vlv_psr_enable_source.
+ /*
+* Here we do the transition drirectly from
+* PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
+* PSR_state 5 (exit).
+* PSR State 4 (active with single frame update) can be skipped.
+* On PSR_state 5 (exit) Hardware is responsible to transition
+* back to PSR_state 1 (inactive).
+* Now we are at Same state after vlv_psr_enable_source.
*/
val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
I915_WRITE(VLV_PSRCTL(pipe), val);
- /* Send AUX wake up - Spec says after transitioning to PSR
+ /*
+* Send AUX wake up - Spec says after transitioning to PSR
* active we have to send AUX wake up by writing 01h in DPCD
* 600h of sink device.
* XXX: This might slow down the transition, but without this
--
2.13.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Refresh VLV/CHV PSR comments on HW PSR_state machine.
On Mon, 2017-09-11 at 15:42 -0700, Rodrigo Vivi wrote:
> DK had pointed out a comment there was hard to understand, so I
> tried to read back again and I couldn't understand that as well.
> So let me re-phrase that in a way that anyone can understand
> later, even myself.
>
This reads much better, thanks for the patch. I've got just one nit
below.
Reviewed-by: Dhinakaran Pandiyan
> Also fixed the comment block style.
>
> Cc: Dhinakaran Pandiyan
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_psr.c | 29 +
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index fdd9e3d95efb..1c2875b74bc9 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -234,7 +234,7 @@ static void vlv_psr_enable_source(struct intel_dp
> *intel_dp,
> struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>
> - /* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
> + /* Transition from PSR_state 0 (disabled) to PSR_state 1 (inactive) */
> I915_WRITE(VLV_PSRCTL(crtc->pipe),
> VLV_EDP_PSR_MODE_SW_TIMER |
> VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
> @@ -249,10 +249,11 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
> struct drm_crtc *crtc = dig_port->base.base.crtc;
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
>
> - /* Let's do the transition from PSR_state 1 to PSR_state 2
> - * that is PSR transition to active - static frame transmission.
> - * Then Hardware is responsible for the transition to PSR_state 3
> - * that is PSR active - no Remote Frame Buffer (RFB) update.
> + /*
> + * Let's do the transition from PSR_state 1 (inactive) to
> + * PSR_state 2 (active - static frame transmission).
nit: The spec calls the state 2 itself as "transition to active - static
frame transmission" and state 3 as active.
> + * Then Hardware is responsible for the transition to
> + * PSR_state 3 (no Remote Frame Buffer (RFB) update).
>*/
> I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
> VLV_EDP_PSR_ACTIVE_ENTRY);
> @@ -576,7 +577,7 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
> uint32_t val;
>
> if (dev_priv->psr.active) {
> - /* Put VLV PSR back to PSR_state 0 that is PSR Disabled. */
> + /* Put VLV PSR back to PSR_state 0 (disabled). */
> if (intel_wait_for_register(dev_priv,
> VLV_PSRSTAT(crtc->pipe),
> VLV_EDP_PSR_IN_TRANS,
> @@ -766,16 +767,20 @@ static void intel_psr_exit(struct drm_i915_private
> *dev_priv)
> } else {
> val = I915_READ(VLV_PSRCTL(pipe));
>
> - /* Here we do the transition from PSR_state 3 to PSR_state 5
> - * directly once PSR State 4 that is active with single frame
> - * update can be skipped. PSR_state 5 that is PSR exit then
> - * Hardware is responsible to transition back to PSR_state 1
> - * that is PSR inactive. Same state after vlv_psr_enable_source.
> + /*
> + * Here we do the transition drirectly from
> + * PSR_state 3 (no Remote Frame Buffer (RFB) update) to
> + * PSR_state 5 (exit).
> + * PSR State 4 (active with single frame update) can be skipped.
> + * On PSR_state 5 (exit) Hardware is responsible to transition
> + * back to PSR_state 1 (inactive).
> + * Now we are at Same state after vlv_psr_enable_source.
>*/
> val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> I915_WRITE(VLV_PSRCTL(pipe), val);
>
> - /* Send AUX wake up - Spec says after transitioning to PSR
> + /*
> + * Send AUX wake up - Spec says after transitioning to PSR
>* active we have to send AUX wake up by writing 01h in DPCD
>* 600h of sink device.
>* XXX: This might slow down the transition, but without this
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Refresh VLV/CHV PSR comments on HW PSR_state machine.
DK had pointed out a comment there was hard to understand, so I
tried to read back again and I couldn't understand that as well.
So let me re-phrase that in a way that anyone can understand
later, even myself.
Also fixed the comment block style.
Cc: Dhinakaran Pandiyan
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_psr.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index fdd9e3d95efb..1c2875b74bc9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -234,7 +234,7 @@ static void vlv_psr_enable_source(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
- /* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
+ /* Transition from PSR_state 0 (disabled) to PSR_state 1 (inactive) */
I915_WRITE(VLV_PSRCTL(crtc->pipe),
VLV_EDP_PSR_MODE_SW_TIMER |
VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
@@ -249,10 +249,11 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
struct drm_crtc *crtc = dig_port->base.base.crtc;
enum pipe pipe = to_intel_crtc(crtc)->pipe;
- /* Let's do the transition from PSR_state 1 to PSR_state 2
-* that is PSR transition to active - static frame transmission.
-* Then Hardware is responsible for the transition to PSR_state 3
-* that is PSR active - no Remote Frame Buffer (RFB) update.
+ /*
+* Let's do the transition from PSR_state 1 (inactive) to
+* PSR_state 2 (active - static frame transmission).
+* Then Hardware is responsible for the transition to
+* PSR_state 3 (no Remote Frame Buffer (RFB) update).
*/
I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
VLV_EDP_PSR_ACTIVE_ENTRY);
@@ -576,7 +577,7 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
uint32_t val;
if (dev_priv->psr.active) {
- /* Put VLV PSR back to PSR_state 0 that is PSR Disabled. */
+ /* Put VLV PSR back to PSR_state 0 (disabled). */
if (intel_wait_for_register(dev_priv,
VLV_PSRSTAT(crtc->pipe),
VLV_EDP_PSR_IN_TRANS,
@@ -766,16 +767,20 @@ static void intel_psr_exit(struct drm_i915_private
*dev_priv)
} else {
val = I915_READ(VLV_PSRCTL(pipe));
- /* Here we do the transition from PSR_state 3 to PSR_state 5
-* directly once PSR State 4 that is active with single frame
-* update can be skipped. PSR_state 5 that is PSR exit then
-* Hardware is responsible to transition back to PSR_state 1
-* that is PSR inactive. Same state after vlv_psr_enable_source.
+ /*
+* Here we do the transition drirectly from
+* PSR_state 3 (no Remote Frame Buffer (RFB) update) to
+* PSR_state 5 (exit).
+* PSR State 4 (active with single frame update) can be skipped.
+* On PSR_state 5 (exit) Hardware is responsible to transition
+* back to PSR_state 1 (inactive).
+* Now we are at Same state after vlv_psr_enable_source.
*/
val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
I915_WRITE(VLV_PSRCTL(pipe), val);
- /* Send AUX wake up - Spec says after transitioning to PSR
+ /*
+* Send AUX wake up - Spec says after transitioning to PSR
* active we have to send AUX wake up by writing 01h in DPCD
* 600h of sink device.
* XXX: This might slow down the transition, but without this
--
2.13.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
