[Intel-gfx] [PATCH v4 1/8] drm/i915/skl+: use linetime latency instead of ddb size

[Mahesh Kumar]

Hi,


On Monday 31 October 2016 11:33 PM, Paulo Zanoni wrote:
> Em Qui, 2016-10-13 às 16:28 +0530, Kumar, Mahesh escreveu:
>> This patch make changes to use linetime latency instead of allocated
>> DDB size during plane watermark calculation in switch case, This is
>> required to implement new DDB allocation algorithm.
>>
>> In New Algorithm DDB is allocated based on WM values, because of
>> which
>> number of DDB blocks will not be available during WM calculation,
>> So this "linetime latency" is suggested by SV/HW team to use during
>> switch-case for WM blocks selection.
>>
>> Changes since v1:
>>   - Rebase on top of Paulo's patch series
>> Changes since v2:
>>   - Fix if-else condition (pointed by Maarten)
>>
>> Signed-off-by: "Kumar, Mahesh" <mahesh1.kumar at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 7f1748a..098336d 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3616,10 +3616,14 @@ static int skl_compute_plane_wm(const struct
>> drm_i915_private *dev_priv,
>>   	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
>>   		selected_result = max(method2, y_tile_minimum);
>>   	} else {
>> +		uint32_t linetime_us = 0;
>> +
>> +		linetime_us = DIV_ROUND_UP(width * 1000,
>> +				skl_pipe_pixel_rate(cstate));
> Can't we just call skl_compute_linetime_wm() here? I don't like having
> two pieces of the code computing the same thing. My last round of bug
> fixes included a fix for duplicated code that got out of sync after
> spec changes.
These two have different values in skl_compute_linetime_wm we multiply 
by 8, not here.
>
>>   		if ((cpp * cstate->base.adjusted_mode.crtc_htotal /
>> 512 < 1) &&
>>   		    (plane_bytes_per_line / 512 < 1))
>>   			selected_result = method2;
>> -		else if ((ddb_allocation / plane_blocks_per_line) >=
>> 1)
>> +		else if (latency >= linetime_us)
> Still doesn't match the spec. The "ddb_allocation /
> planes_block_per_line" is still necessary.
After New algorithm patch, we will not have access to ddb_allocation, as 
allocation will happen later.
So we can't use ddb_allocation in our calculation, This check in Bspec 
is kept for the environment/OS where we don't allocate DDB dynamically.
hence those OS will have access to ddb_allocation during WM calculation 
phase.
thanks,

Regards,
-Mahesh
>
>>   			selected_result = min(method1, method2);
>>   		else
>>   			selected_result = method1;