[Intel-gfx] [PATCH v3 16/16] drm: Replace mode->export_head with a boolean

[Emil Velikov]

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 at collabora.com>

On Tue, 28 Apr 2020 at 18:20, Ville Syrjala
<ville.syrjala at 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).

> 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