[Intel-gfx] [PATCH] drm/i915/skl: Increase ddb blocks to support large cursor sizes

[Kumar, Shobhit]

On 02/16/2016 03:44 PM, Kumar, Shobhit wrote:
> On 01/05/2016 03:44 PM, Daniel Vetter wrote:
>> On Mon, Jan 04, 2016 at 07:18:26PM +0530, Kumar, Shobhit wrote:
>>> On 12/23/2015 08:22 AM, Kumar, Shobhit wrote:
>>>> On 12/21/2015 05:39 PM, Daniel Vetter wrote:
>>>>> On Fri, Dec 18, 2015 at 07:24:19AM -0800, Matt Roper wrote:
>>>>>> On Fri, Dec 18, 2015 at 07:14:17AM -0800, Matt Roper wrote:
>>>>>>> On Fri, Dec 18, 2015 at 05:10:12PM +0200, Ville Syrjälä wrote:
>>>>>>>> On Fri, Dec 18, 2015 at 06:58:58AM -0800, Matt Roper wrote:
>>>>>>>>> On Fri, Dec 18, 2015 at 12:35:47PM +0200, Ville Syrjälä wrote:
>>>>>>>>>> On Wed, Dec 16, 2015 at 07:06:20PM -0800, Radhakrishna Sripada
>>>>>>>>>> wrote:
>>>>>>>>>>> Original value of 32 blocks is not sufficient when using cursor
>>>>>>>>>>> size of
>>>>>>>>>>> 256x256 causing FIFO underruns when the reworked wm
>>>>>>>>>>> caluclations in
>>>>>>>>>>>
>>>>>>>>>>> commit 024c9045221fe45482863c47c4b4c47d37f97cbf
>>>>>>>>>>> Author: Matt Roper <matthew.d.roper at intel.com>
>>>>>>>>>>> Date:   Thu Sep 24 15:53:11 2015 -0700
>>>>>>>>>>>
>>>>>>>>>>>      drm/i915/skl: Eliminate usage of pipe_wm_parameters from
>>>>>>>>>>> SKL-style WM (v4)
>>>>>>>>>>
>>>>>>>>>> Well that commit is obviously incorrect. It's now using the
>>>>>>>>>> pipe src
>>>>>>>>>> width as the plane width for all planes.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yeah, we already noted that bug in another email thread, but
>>>>>>>>> decided
>>>>>>>>> that it was unrelated to the problems Radhakrishna is facing.
>>>>>>>>> Radhakrishna is only using a cursor (which doesn't use that buggy
>>>>>>>>> function)
>>>>>>>>
>>>>>>>> Pop quiz: what does it use then?
>>>>>>>
>>>>>>> All non-cursor planes (i.e., primary+sprite).  Cursors use a
>>>>>>> fixed DDB
>>>>>>> allocation (currently 32 blocks as suggested by bspec, but
>>>>>>> Radhakrishna's testing has found this to be too small, so his patch
>>>>>>> here
>>>>>>> is bumping that number up.
>>>>>>>
>>>>>>
>>>>>> To elaborate on this some more, we have some suspicions about why the
>>>>>> bspec-suggested 32 blocks might not be enough.  There's some new
>>>>>> workaround information in the bspec about sometimes needing to
>>>>>> disable
>>>>>> the system agent (SAGV) on non-BXT gen9, depending on the
>>>>>> configuration
>>>>>> and that's not something that's been implemented in our driver
>>>>>> code yet.
>>>>>> There's also another new workaround that checks raw system memory
>>>>>> bandwidth and adjusts watermark programming that hasn't been
>>>>>> implemented
>>>>>> yet either.  It could be that one of those two missing workarounds is
>>>>>> the true culprit, but Radhakrishna's patch here looks like a
>>>>>> reasonable
>>>>>> short-term workaround; my main worry is that if he needs to bump the
>>>>>> hardcoded 32 up to to 52 for the single pipe case, he might also
>>>>>> need to
>>>>>> bump the hardcoded 8 up as well for the multi-pipe case; I don't
>>>>>> think
>>>>>> anyone has tested that yet (and this seems to be a SKL-specific
>>>>>> problem,
>>>>>> so I can't reproduce it on my BXT).
>>>>>
>>>>> This needs at least a very big comment that we're just hacking around
>>>>> with
>>>>> duct-tape. Also allocating DDB shouldn't matter for correctness, as
>>>>> long
>>>>> as we compute the WM values correctly. I wonder how this fixes
>>>>> anything
>>>>> really (except making it a bit harder to hit perhaps due to the
>>>>> enlarged
>>>>> buffering it affords for the cursor).
>>>>
>>>> Agree Daniel, but in our detailed testing this is found to have fixed
>>>> reliably all known flickers. I recommend to have a detailed comment and
>>>> *put* the duct-tape. I can have a rerun of detailed tests again to
>>>> ensure that its indeed fixing everything.
>>>>
>>>>>
>>>>> Imo better to just implement the other workarounds first, then see
>>>>> what's
>>>>> left.
>>>>
>>>> I know there are few known workarounds pending. Lets continue
>>>> implementing them and more. But with Matt's patch not fixing the issue,
>>>> lets take this as an immediate short term fix and come back to remove
>>>> the duct-tape when we have correct fix.
>>>
>>> I experimented and found that reverting this patch but having arbitrated
>>> display bandwidth based workaround to increase latency values for all
>>> levels
>>> is fixing the flicker. Will be working on this along with Mahesh, unless
>>> Matt, you have a patch for this already.
>>
>> Excellent, really happy that we can go ahead with a reasonable looking
>> fix
>> right away here.
>
> Daniel, we implemented the memory latency increase for quick test and it
> worked, but in reality that situation was not hitting even with a QHD
> eDP and 4k at 60 DP with a two channel 1867 MHz memory. So this was not a
> solution.
>
> There have been another patch series to try to fix this that I posted -
> https://patchwork.freedesktop.org/patch/71758/
>
> and we have been trying with lot of Matt's patches for watermark, and
> along with what I posted but all of this has not yielded perfect under
> run error free results though improved a little.
>
> But cursor DDB bump for both internal and external pipe is resulting in
> complete fix of the problem. As of now that is the only solution I can
> think off and has been extensively tested with QHD eDP/ 19x10 eDP + 4k
> DP/HDMI and working reliably. I think we should be dynamically
> allocating cursor DDB as well so that it will increase the DDB if cursor
> size is big.
>
> But for now proposal is to statically increase these cursor DDB to 52
> for single pipe as in this patch and additionally have it 32 in case of
> multi pipe scenario. Once we have all water mark thingy sorted out, we
> can come back and re-look at this.

Daniel, any comments or suggestions ?

Regards
Shobhit

>
> Regards
> Shobhit
>
>
>>
>> Thanks, Daniel
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx