[Intel-gfx] [PATCH 1/6] drm: Create Color Management DRM properties

[Daniel Stone]

Hi,

On 5 January 2016 at 10:23, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Dec 23, 2015 at 09:47:00AM +0000, Daniel Stone wrote:
>> It's not even a legacy vs. atomic thing, this can happen in
>> pure-atomic as well. Same as the render-compression plane property
>> that I just replied to here as well.
>>
>> - Weston starts and sets up palette/CTM properties
>> - VT switch to Mutter, which isn't aware of new properties
>> - Mutter atomically sets new plane state, containing framebuffer which
>> is already colour-corrected for the target
>> - result is now double-corrected as Mutter didn't know to unset the
>> old properties
>>
>> IOW, in the current model, any user of CM has to explicitly unset it
>> before handover (not always actually possible), or any generic
>> userspace must unset every property it sees and doesn't know about,
>> hoping that setting to 0 will do the right thing.
>>
>> Or we could add another client cap, which would prevent the CM
>> properties from ever being exposed to userspace which doesn't know how
>> to work with it. Passing the client caps through to
>> plane_duplicate_state also means that we can unset the CM properties
>> at this early point for unaware clients. This would mean that we
>> wouldn't have to have code to explicitly unset it in, e.g., every
>> legacy hook.
>
> We don't have any support for unsetting anything really. Same argument you
> make for CM here applies to rotation too. The only generic solution (if
> you really care about this) would be for logind to once sample all atomic
> state on boot-up, and force-restore that state when switching. Restoring
> atomic state doesn't require userspace to understanding the semantics
> really.

Sure, but on the other hand, rotation has been around ~forever, and is
implemented in multiple places. Colour management is a lot less
obvious.

> This kind of force-restoring is probably something we should implement in
> the fbdev emulation, which would cover about 99% of all cases where
> developers/users want to run different compositors. Or fbdev should simply
> reset CM state, like it does for rotation already (that part is easy to
> add, but indeed seems to be missing).
>
> Trying to create an ad-hoc solution (using opt-in flags) to this problem
> for every single feature seems pointless imo.

Fair enough, I guess it could be more difficult, so how about a new
flag to the atomic ioctl which requests state be _reset_ to scratch
rather than duplicated? That way, clients could be really sure they
weren't going to get screwed by rotation / colour management / render
compression / whatever.

>> Yeah, there is a single 'length' property per table, but given the
>> dizzying array of options available, I'm not really sure if that's
>> sufficient.
>
> The idea is to just give a hint to generic userspace. Fancy userspace that
> wants to exploit all the other options needs to have some baked-in
> knowledge of the hardware it runs on. This is kinda similar to how you can
> do dumb buffers, but for fancy ones userspace (at least currently) needs
> to just know how to allocate stuff. Imo there's just plain limits to how
> generic KMS can be. And it's not worth pushing beyond those.

I get the point, but am really struggling to make sense of the options
presented, and how a halfway-sensible generic userspace would work.
Exposing length/depth tuples might not be the worst idea; it also
sounds like there's more to dig out in terms of index interpolation as
well. But these are much smaller objections than having unwitting
clients inherit the CM state ...

Cheers,
Daniel