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

From: vathsala nagaraju <vathsala.nagaraju@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>,
	rodrigo.vivi@intel.com, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
Cc: Puthikorn Voravootivat <puthik@chromium.org>
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2
Date: Mon, 25 Sep 2017 14:40:28 +0530	[thread overview]
Message-ID: <59C8C804.6040002@intel.com> (raw)
In-Reply-To: <87bmlz5gfn.fsf@nikula.org>


[-- 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