[Intel-gfx] Watermark computation on i830 - potential bug?

[Daniel Vetter]

On Sun, Nov 17, 2013 at 12:48 PM, Thomas Richter <thor at math.tu-berlin.de> wrote:
>> First: Have you tried my little patch, since the current watermark
>> code for i830M is clearly completely busted?
>
>
> Yes. And it broke things completely. I send a report on Friday night. Now
> neither the display on the internal nor on the external screen are stable. I
> also mailed the register value of the watermark register, and the correct
> value.

Well that never arrived, and there's also nothing in the moderation
queue. Or I've missed it since it wasn't a direct reply to my patch.

>> Otherwise the spec is fairly clear that lower values means to fetch
>> earlier. One issue though is that atm we hardcode the burst-lenght to
>> 0x3, which means 8*32 bytes. If the watermark is lower than 8*32 then
>> the spec says that hilarity will ensue. I'll wip up a quick patch to
>> add some #defines to i915_reg.h and use them in the code for better
>> documentation.
>
>
> Daniel, don't mind about the specs for the moment. I can only tell you what
> I see. The modified patch sets the watermark values to 1, the old code set
> them to five and left the watermark value for pipe B unmodified, which was
> the only reason why the internal display kept working with the old code.
> According to your understanding, this should be "better" (earlier refetch)
> but it is actually worse, not to say unusable.
>
> With the new code, nothing works anymore. You need to set the watermark
> value to *at least* 6 on pipe A and pipe B to get a stable image, and the
> *lower* I set it, the less stable the display becomes. And not the *higher*.
> Thus, something is severely wrong here. Do you have the precise wording of
> the specs handy? This is probably a misinterpretation of what is stated
> there because it does not at all fit to the outcome of experiments.

Like I've said I guess there's a lower limit - we need to have a
watermark which is at least the burst rate, which atm is set to 8*16
bytes.

> If you like to, I can try to set the watermark to 0x3f (maximum) tomorrow
> and report back. According to the specs, this is the worst possible case,
> though my guess is that this will just work.

Spec says:

[DevALM, DevMGM]: Display Plane B FIFO Watermark:
Number in SW (32Bs) of space in the Display Plane B FIFO above which
the Display B Stream will
generate requests to Memory (Value should be as recommended in the
high priority bandwidth analysis
spreadsheet).
Programming Note:
•
 This value should always be set assuming a 32bpp display mode, even
though the display
mode may be 15 or 16 bpp.
‘00000’ = 1 SW
...

And for burst rate:

[DevALM, DevMGM]: Display Plane B Burst Length:
Size in SW (32Bs) of individual requests issued to Memory for Display Plane B.
‘000’ = 4
‘001’ = 8
...

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch