Hi Ville I don't fully grok the i915 changes to provide meaningful review. There are couple of small comments below, but regardless of those Patches 01-11 and 14-16 are: Reviewed-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> On Tue, 28 Apr 2020 at 18:20, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > The downside is that drm_mode_expose_to_userspace() gets to > iterate a few more modes. It already was O(n^2), now it's > a slightly worse O(n^2). > Personally I'd drop the O() sentence, or change it to It already was O(n^2), now it's slightly worse O((n+y)^2). > Another alternative would be a temp bitmask so we wouldn't have > to have anything in the mode struct itself. The main issue is > how large of a bitmask do we need? I guess we could allocate > it dynamically but that means an extra kcalloc() and an extra > loop through the modes to count them first (or grow the bitmask > with krealloc() as needed). > If the walk is even remotely close to being an issue, we could consider the bitmask. I don't think that's the case yet. Hmm the original code never discards any entries from export_head. I wonder if there's some corner case where we could end with an "old" mode showing in the list? For example: - creates a user mode via drmModeCreatePropertyBlob() - calls drmModeGetConnector() and sees their mode - optional (?) does a modeset to and away from said mode - removes their blob drmModeDestroyPropertyBlob() - calls drmModeGetConnector() and still sees their removed mode. If this is a bug (?) that we care about, we might want to add an igt test for it. Conversely, if this is a behaviour we want to keep this patch needs some work. HTH Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
