On Thu, Apr 30, 2020 at 02:50:52PM +0100, Emil Velikov wrote: > 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
Sorry, forgot to reply to this in a timely manner. > > Patches 01-11 and 14-16 are: > Reviewed-by: Emil Velikov <emil.veli...@collabora.com> > > On Tue, 28 Apr 2020 at 18:20, Ville Syrjala > <ville.syrj...@linux.intel.com> 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). Dropped. > > > 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? No. export_list starts out empty so only the modes we explicitly add to the list can be reached. Thus any dangling pointers in some other mode's export_head are of no concern. Pushed the last few patches to drm-misc-next. Thanks for the reviews everyone. > > 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 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
