On Thu, 16 Jul 2015 13:30:11 +0100 Damien Lespiau <damien.lespiau@xxxxxxxxx> wrote: > On Wed, Jul 08, 2015 at 09:05:53AM -0700, Bob Paauwe wrote: > > Clearing the watermarks for all pipes/planes when updating the > > watermarks for a single CRTC change seems like the wrong thing to > > do here. As is, this code will update any pipe/plane watermarks > > that need updating and leave the remaining set to zero. Later, the > > watermark checks in check_wm_state() will flag these zero'd out pipe/plane > > watermarks and throw errors. > > > > By not clearing all pipe/plane watermark values, only those that > > require changes are changed and the remaining stay unchanged. > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@xxxxxxxxx> > > Hi Bob, > > The spirit of this patch looks good to me (thanks for looking into > this!), but we I think we still need to clear the data for the pipe > we're are updating as it will contain stale values from the previous > skl_update_wm(). > > For instance we don't reset ->dirty[pipe] to false after the register > write and were relying on that memset. 2 options I can see: > > - selectively zero all the fields for that pipe (not helped by the > fact we have arrays indexed by the pipe (array of structure Vs > structure of arrays)) > - make sure we don't rely on any field from the previous call (eg. > update ->dirty in skl_write_wm_values()) > > I have a slight preference for the first solution as it seems a bit more > fool proof, but don't mind either way. Thanks Damien, I'll look into implementing your first suggestion and see how it goes. I wasn't sure if anything needed to be cleared out or not. Based on tracing the code, it looked like the pipe (or pipes) that needed updating would get updated properly in the structure. And this did eliminate the DRM ERRORs so it seemed like it might be a valid solution. But I didn't look at the dirty flag to see if something might be wrong there. > > Thanks, > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
