[Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config

[Sripada, Radhakrishna]

Thanks for the input Rodrigo, Arthur. I will update commit message and repost the patch.

Thanks,
Radhakrishna Sripada
-----Original Message-----
From: Runyan, Arthur J 
Sent: Thursday, June 23, 2016 1:04 PM
To: Rodrigo Vivi <rodrigo.vivi at gmail.com>
Cc: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; intel-gfx <intel-gfx at lists.freedesktop.org>; drm-intel-fixes at lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config

That part is trying to just allocate 8 to each cursor.  The buffer used up will be 8*numpipes, but that's because its assuming you can end up enabling a cursor on each pipe.

I think its good to go up to 16.  The kind of latencies we get on skl mean that a 64x64 32bpp cursor with 8 blocks will be restricting package C states even at 1920x1080 60hz.  The 8 number was based on what hardware did for allocation on past projects.

-----Original Message-----
From: Rodrigo Vivi [mailto:rodrigo.vivi at gmail.com]
Sent: Thursday, June 23, 2016 12:50 PM
To: Runyan, Arthur J
Cc: Sripada, Radhakrishna; intel-gfx; drm-intel-fixes at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config

Thanks Art. I believe the commit message should be updated to reflect this is flexible. Probably coping and pasting this part of spec: "More allocation might be required to support deeper low power states."

So I went now to the spec to review the code and besides the line above I also notice for this specific case BSpec actually recommend 8
* num_active.
"For each enabled cursor CursorBufAlloc = 8"  "BlocksAvailable = TotalBlocksAvailable - (8 * NumPipes)."

What I believe in this code it should be return 8 * num_active instead of a fixed number of 8 or 16. Right?

Thanks,
Rodrigo.

On Thu, Jun 23, 2016 at 12:20 PM, Runyan, Arthur J <arthur.j.runyan at intel.com> wrote:
> The bspec says "These are basic methods that can be used for single and multi-pipe modes. For optimal power usage, the display driver can choose to use more advanced allocation techniques as desired."
> So we leave it up to the driver to optimize as it sees fit.
>
> -----Original Message-----
> From: Rodrigo Vivi [mailto:rodrigo.vivi at gmail.com]
> Sent: Thursday, June 16, 2016 4:20 PM
> To: Sripada, Radhakrishna
> Cc: intel-gfx; drm-intel-fixes at lists.freedesktop.org; Runyan, Arthur J
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb 
> blocks in multi-pipe config
>
> I believe we should use whatever BSpec recommends.
>
> If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there.
>
> Art? thoughts?
>
> On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada at intel.com> wrote:
>> The bspec suggests giving cursor planes a fixed allocation of 8 
>> blocks when running in a multi-CRTC configuration.  However we have 
>> found that this small allocation can only accommodate level
>> 0 watermarks on many platforms, which in turn prevents the system 
>> from entering deeper sleep states.  Let's use a slightly higher 
>> allocation of 16 blocks for the cursor to increase our chances of 
>> enabling lower power states.
>>
>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
>> b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active)
>>         if (num_active == 1)
>>                 return 32;
>>
>> -       return 8;
>> +       /* higher than bspec recommendation (8) */
>> +       return 16;
>>  }
>>
>>  static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry,
>> u32 reg)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br