Op 27-10-17 om 03:09 schreef Matt Roper: > On Wed, Oct 25, 2017 at 08:03:47AM +0200, Maarten Lankhorst wrote: >> Op 25-10-17 om 01:01 schreef Matt Roper: >>> On Thu, Oct 19, 2017 at 05:13:40PM +0200, Maarten Lankhorst wrote: >>>> The original intent was to preserve watermarks as much as possible >>>> in intel_pipe_wm.raw_wm, and put the validated ones in intel_pipe_wm.wm. >>>> >>>> It seems this approach is insufficient and we don't always preserve >>>> the raw watermarks, so just use the atomic iterator we're already using >>>> to get a const pointer to all bound planes on the crtc. >>>> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102373 >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >>>> Cc: stable@xxxxxxxxxxxxxxx #v4.8+ >>> It's been a while since I looked at this code, so I'm not sure I'm >>> following all the context/history here. Before this patch, we had >>> calculated watermarks for all levels (even those above the maximum >>> usable watermark level as determined by sprite usage) into raw_wm. But >>> as far as I can tell, we never actually used those values for anything >>> so that was no different than just throwing the values away? >> No, the trick introduced there was that we preserved the raw watermarks for future >> calculations to not include all planes in a commit. This is now solved by using the >> read-only drm_atomic_crtc_state_for_each_plane_state iterator, to get all >> const plane states without having to include them. >> >>> Is my understanding correct that this is mostly a revert of 71f0a62614 >>> ("drm/i915: Only use sanitized values for ILK watermarks) except that it >>> also modifies the level validation loop so that we're only calling >>> ilk_compute_wm_level() on the levels up to usable_level whereas before >>> that patch we were calling it on all levels, but then >>> setting wm->enable = false for the unusable levels? >>> >>> By my understanding, this looks like a safe simplification of the >>> current logic, but I don't see where the functional change is here. >>> Was the Buzilla: reference supposed to be tied to patch #2 of this >>> series or am I missing something important? >> We now throw away all watermarks, and use drm_atomic_crtc_state_for_each_plane_state >> to get the plane states. As a result we can throw away the raw watermarks, which have >> proven to be insufficiently preserved. :) > Okay, makes more sense now. Thanks for clarifying. > > Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Thanks, pushed both patches. Hopefully issues are gone now. :) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
