[PATCH 1/2] drm/dp: Add defines for latency in sink

[PATCH 1/2] drm/dp: Add defines for latency in sink

[vathsala nagaraju]

Add defines for dpcd register 2009 (synchronization latency
in sink).

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 include/drm/drm_dp_helper.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 11c39f1..846004e6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -735,6 +735,9 @@
 # define DP_PSR_SINK_INTERNAL_ERROR         7
 # define DP_PSR_SINK_STATE_MASK             0x07
 
+#define DP_SINK_SYNCHRONIZATION_LATENCY	    0x2009
+# define DP_MAX_RESYNC_FRAME_CNT_MASK	    0xf
+
 #define DP_RECEIVER_ALPM_STATUS		    0x200b  /* eDP 1.4 */
 # define DP_ALPM_LOCK_TIMEOUT_ERROR	    (1 << 0)
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

v3 : (Rodrigo)
 - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
 - replace with &=

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..b880c84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,7 +4047,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK		0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
+#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
 
 #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..adf7abc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val;
+	uint8_t sink_latency;
 
 	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE |
-		EDP_SU_TRACK_ENABLE |
-		EDP_FRAMES_BEFORE_SU_ENTRY;
+		EDP_SU_TRACK_ENABLE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+				&sink_latency) == 1) {
+		sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
+	} else {
+		sink_latency = 0;
+	}
+	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for series starting with [1/2] drm/dp: Add defines for latency in sink

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/dp: Add defines for latency in sink
URL   : https://patchwork.freedesktop.org/series/30797/
State : success

== Summary ==

Series 30797v1 series starting with [1/2] drm/dp: Add defines for latency in sink
https://patchwork.freedesktop.org/api/1.0/series/30797/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-kbl-7500u) fdo#102850

fdo#102850 https://bugs.freedesktop.org/show_bug.cgi?id=102850

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:468s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:419s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:520s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:276s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:504s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:490s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:496s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:544s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:414s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:558s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:420s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:403s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:431s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:486s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:457s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:466s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:593s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:536s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:747s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:470s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:567s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:412s

6aa0df37d3fc238146f0445f71bb0738490cb6dc drm-tip: 2017y-09m-22d-21h-24m-10s UTC integration manifest
11fe95fd5633 drm/i915/psr: Set frames before SU entry for psr2
7bd1f311bec4 drm/dp: Add defines for latency in sink

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5798/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/dp: Add defines for latency in sink

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/dp: Add defines for latency in sink
URL   : https://patchwork.freedesktop.org/series/30797/
State : success

== Summary ==

shard-hsw        total:2429 pass:1329 dwarn:5   dfail:0   fail:12  skip:1083 time:9777s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5798/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/dp: Add defines for latency in sink

[Jani Nikula]

On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> Add defines for dpcd register 2009 (synchronization latency
> in sink).
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  include/drm/drm_dp_helper.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 11c39f1..846004e6 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -735,6 +735,9 @@
>  # define DP_PSR_SINK_INTERNAL_ERROR         7
>  # define DP_PSR_SINK_STATE_MASK             0x07
>  
> +#define DP_SINK_SYNCHRONIZATION_LATENCY	    0x2009
> +# define DP_MAX_RESYNC_FRAME_CNT_MASK	    0xf

For the DP spec, please don't invent the names, use the ones from the
spec. At most drop excess stuff from the end.

#define DP_SYNCHRONIZATION_LATENCY_IN_SINK
# define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
# define DP_MAX_RESYNC_FRAME_COUNT_MASK

And while at it, please add the full register contents.

BR,
Jani.

> +
>  #define DP_RECEIVER_ALPM_STATUS		    0x200b  /* eDP 1.4 */
>  # define DP_ALPM_LOCK_TIMEOUT_ERROR	    (1 << 0)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Jani Nikula]

On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
>
> v2 :
>  - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>  - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>  - add check ==1 for dpcd_read call (ville)
>
> v3 : (Rodrigo)
>  - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
>  - replace with &=
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 82f36dd..b880c84 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4047,7 +4047,7 @@ enum {
>  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>  #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
>  #define   EDP_PSR2_IDLE_MASK		0xf
> -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)

In the register definitions we use the shift values directly, not the
macro. That's the style we've adopted. Please stick to it.

Ditto for the indent, why do you remove it?


BR,
Jani.

>  
>  #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
>  #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0a17d1f..adf7abc 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 */
>  	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>  	uint32_t val;
> +	uint8_t sink_latency;
>  
>  	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
>  	 * good enough. */
>  	val |= EDP_PSR2_ENABLE |
> -		EDP_SU_TRACK_ENABLE |
> -		EDP_FRAMES_BEFORE_SU_ENTRY;
> +		EDP_SU_TRACK_ENABLE;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +				&sink_latency) == 1) {
> +		sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
> +	} else {
> +		sink_latency = 0;
> +	}
> +	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
>  
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>  		val |= EDP_PSR2_TP2_TIME_2500;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/dp: Add defines for latency in sink

[vathsala nagaraju]

[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]

On Monday 25 September 2017 01:53 PM, Jani Nikula wrote:
> On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
>> Add defines for dpcd register 2009 (synchronization latency
>> in sink).
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> CC: Puthikorn Voravootivat <puthik@chromium.org>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>> ---
>>   include/drm/drm_dp_helper.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index 11c39f1..846004e6 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -735,6 +735,9 @@
>>   # define DP_PSR_SINK_INTERNAL_ERROR         7
>>   # define DP_PSR_SINK_STATE_MASK             0x07
>>   
>> +#define DP_SINK_SYNCHRONIZATION_LATENCY	    0x2009
>> +# define DP_MAX_RESYNC_FRAME_CNT_MASK	    0xf
> For the DP spec, please don't invent the names, use the ones from the
> spec. At most drop excess stuff from the end.
In edp 1.4b spec , the register name 2009 is "DEBUG 0 SYNCHRONIZATION 
LATENCY SINK "  and bit 0:3 "MAX RE-SYNC FRAME COUNT"
>
> #define DP_SYNCHRONIZATION_LATENCY_IN_SINK
> # define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
> # define DP_MAX_RESYNC_FRAME_COUNT_MASK
>
> And while at it, please add the full register contents.
>
> BR,
> Jani.
>
>> +
>>   #define DP_RECEIVER_ALPM_STATUS		    0x200b  /* eDP 1.4 */
>>   # define DP_ALPM_LOCK_TIMEOUT_ERROR	    (1 << 0)


[-- Attachment #1.2: Type: text/html, Size: 2532 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

[-- Attachment #1.1: Type: text/plain, Size: 2884 bytes --]

On Monday 25 September 2017 02:00 PM, Jani Nikula wrote:
> On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
>> Set frames before SU entry value for max resync frame count of
>> dpcd register 2009, bit field 0:3.
>>
>> v2 :
>>   - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>>   - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>>   - add check ==1 for dpcd_read call (ville)
>>
>> v3 : (Rodrigo)
>>   - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
>>   - replace with &=
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> CC: Puthikorn Voravootivat <puthik@chromium.org>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>>   drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>>   2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 82f36dd..b880c84 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4047,7 +4047,7 @@ enum {
>>   #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>>   #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
>>   #define   EDP_PSR2_IDLE_MASK		0xf
>> -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
>> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
> In the register definitions we use the shift values directly, not the
> macro. That's the style we've adopted. Please stick to it.
Macro was suggested by Rodrigo.
>
> Ditto for the indent, why do you remove it?
BR, Jani.
>>   
>>   #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
>>   #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 0a17d1f..adf7abc 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>>   	 */
>>   	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>>   	uint32_t val;
>> +	uint8_t sink_latency;
>>   
>>   	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>>   
>> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>>   	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
>>   	 * good enough. */
>>   	val |= EDP_PSR2_ENABLE |
>> -		EDP_SU_TRACK_ENABLE |
>> -		EDP_FRAMES_BEFORE_SU_ENTRY;
>> +		EDP_SU_TRACK_ENABLE;
>> +
>> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
>> +				&sink_latency) == 1) {
>> +		sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
>> +	} else {
>> +		sink_latency = 0;
>> +	}
>> +	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
>>   
>>   	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>>   		val |= EDP_PSR2_TP2_TIME_2500;


[-- Attachment #1.2: Type: text/html, Size: 3906 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Rodrigo Vivi]

On Mon, Sep 25, 2017 at 09:10:28AM +0000, vathsala nagaraju wrote:
> On Monday 25 September 2017 02:00 PM, Jani Nikula wrote:
> 
>     On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> 
>         Set frames before SU entry value for max resync frame count of
>         dpcd register 2009, bit field 0:3.
> 
>         v2 :
>          - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>          - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>          - add check ==1 for dpcd_read call (ville)
> 
>         v3 : (Rodrigo)
>          - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
>          - replace with &=
> 
>         Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>         CC: Puthikorn Voravootivat <puthik@chromium.org>
>         Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>         Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>         ---
>          drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>          drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>          2 files changed, 11 insertions(+), 3 deletions(-)
> 
>         diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>         index 82f36dd..b880c84 100644
>         --- a/drivers/gpu/drm/i915/i915_reg.h
>         +++ b/drivers/gpu/drm/i915/i915_reg.h
>         @@ -4047,7 +4047,7 @@ enum {
>          #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>          #define   EDP_PSR2_FRAME_BEFORE_SU_MASK        (0xf<<4)
>          #define   EDP_PSR2_IDLE_MASK           0xf
>         -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
>         +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
> 
>     In the register definitions we use the shift values directly, not the
>     macro. That's the style we've adopted. Please stick to it.
> 
> Macro was suggested by Rodrigo.

Well, one bad review is not an excuse to ignore a good review ;)

But what Jani mentioned and he is absolutelly right is to use the shift value
directly instead the SHIFT macro, not to avoid the function-like macro that I
suggested.

In other words:

#define   EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4)

Please also note his request on the spaces required for proper identation.

And also please address his comments on the first patch to make the defines
in total sync with DP Spec.

Thanks,
Rodrigo.

> 
> 
>     Ditto for the indent, why do you remove it?
> 
> BR, Jani.
> 
> 
>          #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
>          #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
>         diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>         index 0a17d1f..adf7abc 100644
>         --- a/drivers/gpu/drm/i915/intel_psr.c
>         +++ b/drivers/gpu/drm/i915/intel_psr.c
>         @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>                  */
>                 uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>                 uint32_t val;
>         +       uint8_t sink_latency;
> 
>                 val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> 
>         @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>                  * mesh at all with our frontbuffer tracking. And the hw alone isn't
>                  * good enough. */
>                 val |= EDP_PSR2_ENABLE |
>         -               EDP_SU_TRACK_ENABLE |
>         -               EDP_FRAMES_BEFORE_SU_ENTRY;
>         +               EDP_SU_TRACK_ENABLE;
>         +
>         +       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
>         +                               &sink_latency) == 1) {
>         +               sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
>         +       } else {
>         +               sink_latency = 0;
>         +       }
>         +       val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
> 
>                 if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>                         val |= EDP_PSR2_TP2_TIME_2500;
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/dp: Add defines for latency in sink

[Rodrigo Vivi]

On Mon, Sep 25, 2017 at 09:01:38AM +0000, vathsala nagaraju wrote:
> On Monday 25 September 2017 01:53 PM, Jani Nikula wrote:
> 
>     On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> 
>         Add defines for dpcd register 2009 (synchronization latency
>         in sink).
> 
>         Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>         CC: Puthikorn Voravootivat <puthik@chromium.org>
>         Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>         Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>         ---
>          include/drm/drm_dp_helper.h | 3 +++
>          1 file changed, 3 insertions(+)
> 
>         diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>         index 11c39f1..846004e6 100644
>         --- a/include/drm/drm_dp_helper.h
>         +++ b/include/drm/drm_dp_helper.h
>         @@ -735,6 +735,9 @@
>          # define DP_PSR_SINK_INTERNAL_ERROR         7
>          # define DP_PSR_SINK_STATE_MASK             0x07
> 
>         +#define DP_SINK_SYNCHRONIZATION_LATENCY            0x2009
>         +# define DP_MAX_RESYNC_FRAME_CNT_MASK      0xf
> 
>     For the DP spec, please don't invent the names, use the ones from the
>     spec. At most drop excess stuff from the end.
> 
> In edp 1.4b spec , the register name 2009 is "DEBUG 0 SYNCHRONIZATION LATENCY SINK "  and
> bit 0:3 "MAX RE-SYNC FRAME COUNT"

probably he meant something like:

# define DP_SYNCHRONIZATION_LATENCY_IN_SINK            0x2009
# define DP_MAX_RESYNC_FRAME_COUNT_MASK      0xf

but he is the best one to confirm that...

> 
> 
>     #define DP_SYNCHRONIZATION_LATENCY_IN_SINK
>     # define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
>     # define DP_MAX_RESYNC_FRAME_COUNT_MASK
> 
>     And while at it, please add the full register contents.
> 
>     BR,
>     Jani.
> 
> 
>         +
>          #define DP_RECEIVER_ALPM_STATUS                    0x200b  /* eDP 1.4 */
>          # define DP_ALPM_LOCK_TIMEOUT_ERROR        (1 << 0)
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/dp: Add defines for latency in sink

[Jani Nikula]

On Mon, 25 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Sep 25, 2017 at 09:01:38AM +0000, vathsala nagaraju wrote:
>> On Monday 25 September 2017 01:53 PM, Jani Nikula wrote:
>> 
>>     On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
>> 
>>         Add defines for dpcd register 2009 (synchronization latency
>>         in sink).
>> 
>>         Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>         CC: Puthikorn Voravootivat <puthik@chromium.org>
>>         Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>         Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>>         ---
>>          include/drm/drm_dp_helper.h | 3 +++
>>          1 file changed, 3 insertions(+)
>> 
>>         diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>>         index 11c39f1..846004e6 100644
>>         --- a/include/drm/drm_dp_helper.h
>>         +++ b/include/drm/drm_dp_helper.h
>>         @@ -735,6 +735,9 @@
>>          # define DP_PSR_SINK_INTERNAL_ERROR         7
>>          # define DP_PSR_SINK_STATE_MASK             0x07
>> 
>>         +#define DP_SINK_SYNCHRONIZATION_LATENCY            0x2009
>>         +# define DP_MAX_RESYNC_FRAME_CNT_MASK      0xf
>> 
>>     For the DP spec, please don't invent the names, use the ones from the
>>     spec. At most drop excess stuff from the end.
>> 
>> In edp 1.4b spec , the register name 2009 is "DEBUG 0 SYNCHRONIZATION LATENCY SINK "  and
>> bit 0:3 "MAX RE-SYNC FRAME COUNT"
>
> probably he meant something like:
>
> # define DP_SYNCHRONIZATION_LATENCY_IN_SINK            0x2009
> # define DP_MAX_RESYNC_FRAME_COUNT_MASK      0xf
>
> but he is the best one to confirm that...

I wrote exactly that in my review, see below.

BR,
Jani.

>
>> 
>> 
>>     #define DP_SYNCHRONIZATION_LATENCY_IN_SINK
>>     # define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
>>     # define DP_MAX_RESYNC_FRAME_COUNT_MASK
>> 
>>     And while at it, please add the full register contents.
>> 
>>     BR,
>>     Jani.
>> 
>> 
>>         +
>>          #define DP_RECEIVER_ALPM_STATUS                    0x200b  /* eDP 1.4 */
>>          # define DP_ALPM_LOCK_TIMEOUT_ERROR        (1 << 0)
>> 
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/2] drm/dp: Add defines for latency in sink

[Daniel Vetter]

On Mon, Sep 25, 2017 at 11:23:26AM +0300, Jani Nikula wrote:
> On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> > Add defines for dpcd register 2009 (synchronization latency
> > in sink).
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > CC: Puthikorn Voravootivat <puthik@chromium.org>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> > ---
> >  include/drm/drm_dp_helper.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 11c39f1..846004e6 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -735,6 +735,9 @@
> >  # define DP_PSR_SINK_INTERNAL_ERROR         7
> >  # define DP_PSR_SINK_STATE_MASK             0x07
> >  
> > +#define DP_SINK_SYNCHRONIZATION_LATENCY	    0x2009
> > +# define DP_MAX_RESYNC_FRAME_CNT_MASK	    0xf
> 
> For the DP spec, please don't invent the names, use the ones from the
> spec. At most drop excess stuff from the end.
> 
> #define DP_SYNCHRONIZATION_LATENCY_IN_SINK
> # define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
> # define DP_MAX_RESYNC_FRAME_COUNT_MASK
> 
> And while at it, please add the full register contents.

Please also annotate in which version of the spec we can find this, e.g.

#define DP_SYNCHRONIZATION_LATENCY_IN_SINK	/* eDP 1.4b */
# define DP_MAX_RESYNC_FRAME_COUNT_SHIFT
# define DP_MAX_RESYNC_FRAME_COUNT_MASK

Bringing this up since previous versions of this confused Rodrigo
-Daniel

> 
> BR,
> Jani.
> 
> > +
> >  #define DP_RECEIVER_ALPM_STATUS		    0x200b  /* eDP 1.4 */
> >  # define DP_ALPM_LOCK_TIMEOUT_ERROR	    (1 << 0)
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Jani Nikula]

On Thu, 28 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Merged both patches to dinq. Thanks for the patches.

While patch 1 was a simple addition of a few DP macros, we need to get
ack from Dave or (preferrably non-Intel) drm-misc maintainers before
queuing non-i915 patches through drm-intel.

Dave, Sean, ack after the fact...?

The patch is [1].


BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/1506419953-32605-1-git-send-email-vathsala.nagaraju@intel.com

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Rodrigo Vivi]

On Fri, Sep 22, 2017 at 03:58:36PM +0000, vathsala nagaraju wrote:
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> v2 :
>  - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>  - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>  - add check ==1 for dpcd_read call (ville)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>

Merged both patches to dinq. Thanks for the patches.

I'm anxiously waiting the PSR2 related workaroud(s)! ;)

Thanks,
Rodrigo.

> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 82f36dd..89c5249 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -170,6 +170,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  	(mask) << 16 | (value); })
>  #define _MASKED_BIT_ENABLE(a)	({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
>  #define _MASKED_BIT_DISABLE(a)	(_MASKED_FIELD((a), 0))
> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
>  
>  /* Engine ID */
>  
> @@ -4047,7 +4048,6 @@ enum {
>  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>  #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
>  #define   EDP_PSR2_IDLE_MASK		0xf
> -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
>  
>  #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
>  #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0a17d1f..e505fa6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 */
>  	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>  	uint32_t val;
> +	uint8_t sink_latency;
>  
>  	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
>  	 * good enough. */
>  	val |= EDP_PSR2_ENABLE |
> -		EDP_SU_TRACK_ENABLE |
> -		EDP_FRAMES_BEFORE_SU_ENTRY;
> +		EDP_SU_TRACK_ENABLE;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +				&sink_latency) == 1) {
> +		sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
> +	} else {
> +		sink_latency = 0;
> +	}
> +	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
>  
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>  		val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

v3 : (Rodrigo)
 - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
 - replace with &=

v4 :
 - change the macro to shift value (jani)
 - updated register names

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..7e7aa60 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,7 +4047,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK		0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
+#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a)<<4)
 
 #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..5419cda 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val;
+	uint8_t sink_latency;
 
 	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,16 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE |
-		EDP_SU_TRACK_ENABLE |
-		EDP_FRAMES_BEFORE_SU_ENTRY;
+		EDP_SU_TRACK_ENABLE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux,
+				DP_SYNCHRONIZATION_LATENCY_IN_SINK,
+				&sink_latency) == 1) {
+		sink_latency &= DP_MAX_RESYNC_FRAME_COUNT_MASK;
+	} else {
+		sink_latency = 0;
+	}
+	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Rodrigo Vivi]

On Fri, Sep 22, 2017 at 03:58:36PM +0000, vathsala nagaraju wrote:
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> v2 :
>  - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>  - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>  - add check ==1 for dpcd_read call (ville)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 82f36dd..89c5249 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -170,6 +170,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  	(mask) << 16 | (value); })
>  #define _MASKED_BIT_ENABLE(a)	({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
>  #define _MASKED_BIT_DISABLE(a)	(_MASKED_FIELD((a), 0))
> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)

not here

>  
>  /* Engine ID */
>  
> @@ -4047,7 +4048,6 @@ enum {
>  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>  #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
>  #define   EDP_PSR2_IDLE_MASK		0xf
> -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)

move here

>  
>  #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
>  #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0a17d1f..e505fa6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 */
>  	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>  	uint32_t val;
> +	uint8_t sink_latency;
>  
>  	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
>  	 * good enough. */
>  	val |= EDP_PSR2_ENABLE |
> -		EDP_SU_TRACK_ENABLE |
> -		EDP_FRAMES_BEFORE_SU_ENTRY;
> +		EDP_SU_TRACK_ENABLE;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +				&sink_latency) == 1) {
> +		sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;

		sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;

with those changes

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> +	} else {
> +		sink_latency = 0;
> +	}
> +	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
>  
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>  		val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..89c5249 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -170,6 +170,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 	(mask) << 16 | (value); })
 #define _MASKED_BIT_ENABLE(a)	({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
 #define _MASKED_BIT_DISABLE(a)	(_MASKED_FIELD((a), 0))
+#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
 
 /* Engine ID */
 
@@ -4047,7 +4048,6 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK		0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
 
 #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..e505fa6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val;
+	uint8_t sink_latency;
 
 	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE |
-		EDP_SU_TRACK_ENABLE |
-		EDP_FRAMES_BEFORE_SU_ENTRY;
+		EDP_SU_TRACK_ENABLE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+				&sink_latency) == 1) {
+		sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
+	} else {
+		sink_latency = 0;
+	}
+	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

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

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Vivi, Rodrigo]

> On Sep 20, 2017, at 7:33 AM, Nagaraju, Vathsala <vathsala.nagaraju@intel.com> wrote:
> 
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index acb5094..04b253f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>     */
>    uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>    uint32_t val;
> +    uint8_t sink_latency;
> 
>    val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> 
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>     * mesh at all with our frontbuffer tracking. And the hw alone isn't
>     * good enough. */
>    val |= EDP_PSR2_ENABLE |
> -        EDP_SU_TRACK_ENABLE |
> -        EDP_FRAMES_BEFORE_SU_ENTRY;

Please also remove the definition of this su_entry since it was not following the new standards anyway...
Probably good to replace with function macro style for better use below...

> +        EDP_SU_TRACK_ENABLE;
> +
> +    if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +                &sink_latency)) {
> +        sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
> +        val |= (sink_latency + 1) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT;

... so you could use
val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency +1);


> +    } else {
> +        val |= EDP_FRAMES_BEFORE_SU_ENTRY;
> +    }
> 
>    if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>        val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Ville Syrjälä]

On Wed, Sep 20, 2017 at 08:02:35PM +0530, vathsala nagaraju wrote:
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index acb5094..04b253f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 */
>  	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>  	uint32_t val;
> +	uint8_t sink_latency;
>  
>  	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
>  	 * good enough. */
>  	val |= EDP_PSR2_ENABLE |
> -		EDP_SU_TRACK_ENABLE |
> -		EDP_FRAMES_BEFORE_SU_ENTRY;
> +		EDP_SU_TRACK_ENABLE;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +				&sink_latency)) {

== 1

> +		sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
> +		val |= (sink_latency + 1) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT;
> +	} else {
> +		val |= EDP_FRAMES_BEFORE_SU_ENTRY;
> +	}
>  
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>  		val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index acb5094..04b253f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val;
+	uint8_t sink_latency;
 
 	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE |
-		EDP_SU_TRACK_ENABLE |
-		EDP_FRAMES_BEFORE_SU_ENTRY;
+		EDP_SU_TRACK_ENABLE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+				&sink_latency)) {
+		sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
+		val |= (sink_latency + 1) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT;
+	} else {
+		val |= EDP_FRAMES_BEFORE_SU_ENTRY;
+	}
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel