[Intel-gfx] [RFC 6/6] drm/i915/vlv: Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark

[Ding, ChiX]

The watermark means that how much is consumed from a full fifo for VLV/CHV.
>From VLV spec, "Display Plane A FIFO Watermark: Number in 64Bs of space in FIFO above which the Display A Stream
will generate requests to Memory"
It seems that smaller value should be picked to avoid FIFO underrun when computing intermediate watermark.

There is a page about DSPARB Display Arbitration Control register for VLV/CHV
https://gfxspecs.intel.com/Predator/Home/Index/16287




-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com] 
Sent: Wednesday, June 15, 2016 11:50 AM
To: Roper, Matthew D
Cc: Ding, ChiX; Maarten Lankhorst; Adebisi, YetundeX; isg-gms at eclists.intel.com; intel-gfx at lists.freedesktop.org
Subject: Re: [RFC 6/6] drm/i915/vlv: Add intermediate field in intel_crtc_wm_state and handlers for two-level watermark

On Tue, Jun 14, 2016 at 03:57:28PM -0700, Matt Roper wrote:
> On Tue, Jun 14, 2016 at 07:40:54AM -0700, Ding, ChiX wrote:
> > Hi Maarten
> > Thanks for the reply. Please correct me if I'm wrong : 
> > If the intermediate watermark is bigger than any of the old and the new, then it's not really safe.
> > It could be bigger than the FIFO size. 
> > 
> > I did some tests using average value and it ran into FIFO underrun, e.g.
> > [100424.405624] [drm:vlv_invert_wms] Pipe A fifo for primary, 
> > sprite0, sprite 1 : 511 / 0 / 0 [100424.413231] 
> > [drm:vlv_compute_fifo] Pipe A fifo for primary, sprite0, sprite 1 : 
> > 256 / 255 / 0 [100424.413242] [drm:vlv_compute_pipe_wm] Pipe A 
> > [100424.413245] [drm:vlv_compute_pipe_wm] level : 0 [100424.413249] 
> > [drm:vlv_compute_pipe_wm] Watermark values for primary, sprite0, 
> > sprite1 : 136, 135, 0 [100424.413253] 
> > [drm:vlv_compute_intermediate_wm] level : 0 [100424.413257] 
> > [drm:vlv_compute_intermediate_wm] A watermark values for primary, 
> > sprite0, sprite1 : 136, 135, 0 [100424.413261] 
> > [drm:vlv_compute_intermediate_wm] B watermark values for primary, 
> > sprite0, sprite1 : 391, 0, 0 [100424.429823] 
> > [drm:vlv_initial_watermarks] vlv_initial_watermarks
> > 
> > At first, the current active
> > fifo sizes 511 / 0 / 0 for primary, sprite0, sprite 1 watermark 391, 
> > 0, 0
> > 
> > Then the optimal values are calculated : 
> > fifo sizes 256 / 255 / 0
> > watermarks 136, 135, 0
> > 
> > The average watermark for primary plane is 391+136/2 = 263, bigger 
> > than the primary plane fifo size for optimal : 256. It causes 
> > underrun later on when writing intermediate watermark and the fifo 
> > size is changed to 256 for primary plane by vlv_pipe_set_fifo_size() function.
> > 
> > If I use min() for intermediate watermark, I don't run into underrun 
> > issue in the tests.  My understanding is that with min(), in some 
> > cases we get 0,0 for watermark , but it's still a legitimate value.
> > Please let me know if I'm mistaken.
> > 
> > 
> > Another question: the sum of fifo sizes for primary plane, sprite 0, 
> > sprite 1 should always be 512-1 = 511.  But the watermarks of the 
> > planes don't need to add up to 511. They need to stay under or at 
> > most equal to the respective fifo size, right?
> 
> I may be overlooking something since I haven't studied VLV/CHV in as 
> much detail as the other platforms, but there are basically two parts 
> to the per-crtc watermark calculations we do during the atomic 'check'
> phase.
> 
> The first part is slicing up the fifo/DDB between the various planes 
> that are in use.  The fifo buffer is a fixed size resource made up of 
> blocks, but it's programmable how much of that buffer is devoted to 
> driving a specific plane.  The formula we use for slicing up the fifo 
> is to just give each plane a number of blocks proportional to the 
> fraction of the total data rate that the plane contributes (with some 
> extra logic at the end to deal with leftover blocks due to integer division).
> 
> The second part is calculating the watermark values for each plane.  
> The fifo holds data that's been fetched from RAM for the display 
> controller to consume; the watermark values (measured in fifo blocks) 
> are basically telling the hardware how/when it needs to wake up and 
> fetch more data from RAM into the fifo to ensure the fifo doesn't 
> reach empty.  If you set the watermark value for a plane too low, then 
> the hardware won't fetch more data from RAM early enough and the fifo 
> will run dry before new data from RAM shows up; when this happens, the 
> display controller doesn't have any data to tell it how to keep 
> driving the display so you can get various kinds of display corruption 
> (and dmesg warnings about a FIFO underrun).  Programming watermarks 
> too high is always "fine" from a correctness point of view --- you're 
> just causing the hardware to wake up and fetch more data from RAM 
> earlier than it really had to.  You may be impacting power usage, but you're safe from underruns/corruption.
> 
> For intermediate watermarks, we need "worst case" values that satisfy 
> both the old and new configuration (i.e., won't cause underruns for 
> either config).  So I think you want to be using max() instead of 
> min() to calculate your intermediate values.  Once you've calculated 
> the worst-case (max) values for each plane, I figure you'd want to 
> work backwards from there and slice up your intermediate per-place 
> fifo allocation second...I guess you'd probably just set each plane's 
> fifo size equal to its watermark value to start with, then distribute 
> the remaining blocks equally between all of the planes that are active 
> in either new or old configuration (and if you don't have any 
> remaining blocks, then there's no valid intermediate configuration so 
> we should probably fail the request).
> 
> Again, this is just completely off the top of my head and I haven't 
> actually looked at the VLV/CHV bspecs lately, so I might be completely 
> overlooking some important detail about these platforms.

There won't be any intermediate FIFO sizes. The FIFO repartitioning happens atomically with plane updates, or rather should, but the exact nature of the register double buffering is still a bit of a mystery.
How exactly the double buffering works may dictate how we have to do things, so until someone figures it out, I don't think there's any point in trying to fix the watermark updates.

step 1. figure out the DSBARB double buffering step 2. ?
step 3. profit

--
Ville Syrjälä
Intel OTC