[PATCH weston v5 09/36] cms-colord: find a good head

[Derek Foreman]

On 2018-02-02 03:45 AM, Pekka Paalanen wrote:
> On Thu, 1 Feb 2018 22:10:33 +0000
> Daniel Stone <daniel at fooishbar.org> wrote:
> 
>> Hi,
>>
>> On 1 February 2018 at 22:00, Derek Foreman <derekf at osg.samsung.com> wrote:
>>> On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
>>>> The 'head' member of 'struct weston_output' is going to go unused and
>>>> then disappear, so stop using it and find a head from the proper list.
>>>>
>>>> However, this leaves a problem in cms-colord: if you have multiple
>>>> monitors driver with the same CRTC, what do you say to the color
>>>> management system? The monitors could be different, but all the color
>>>> LUTs etc. are in the CRTC and are shared, as is the framebuffer.
>>>>
>>>> Do the simple hack here and just use whatever head happens to be the
>>>> first in the list.
>>>
>>> I am a complete non-expert in this area, so if I'm making no sense feel free
>>> to tell me to shut up...
>>>
>>> I think if someone's going through the effort to properly setup color
>>> management, then we can't use cloned heads off a single CRTC?
>>>
>>> We should probably disallow a CRTCs to drive multiple heads if two or more
>>> of those heads have differing color profiles?
>>>
>>> If I went to the trouble of calibrating two displays, I would probably be
>>> extremely surprised to see them looking different with clone mode enabled.
>>
>> Yeah, what Derek said. I think non-homogeneous colour properties when
>> colour management is enabled, should be cause to unclone. That being
>> said, it's a niche enough usecase that I'd be comfortable landing it
>> with documentation that it was a known shortcoming. I'd be super
>> comfortable landing it if we also had an output configuration tweak we
>> could use to force non-clone mode, or really even just a simple
>> out-of-tree patch (or environment variable, a la atomic?) that allows
>> people to work around it easily enough by disabling clones.
> 
> Hi,
> 
> I agree with Derek's suggestion that we should check color properties
> and fail the shared-CRTC cloning, and with Daniel that we could just
> document this. I'm not sure what the "output configuration tweak"
> refers to. Do you mean an option that would just disable shared-CRTC
> cloning?

I can get behind just documenting it.

> We have Weston that handles all option parsing, and Weston also does
> the decision to use or not use shared-CRTC cloning by how it attaches
> heads to outputs. Libweston will not try shared-CRTC cloning unless
> explicitly told to, and it will not transparently fall back to
> something else. After the complete clone mode series, which will not
> include independent-CRTC cloning support, the option to disable
> shared-CRTC cloning would be equivalent to a weston.ini that does not
> configure cloning at all.

QED. :)

For myself, I've been struggling with the ramifications of shared-CRTC 
vs independent-CRTC cloning, and didn't realize until you pointed out to 
me out-of-band that independent-CRTC cloning isn't just fallback 
operation (and in fact won't work at all without additional work).

> However, the weston.ini syntax as is does not differentiate between
> shared-CRTC and independent-CRTC cloning. My intention is that Weston
> (not libweston) attemps shared-CRTC cloning and falls back to
> independent-CRTC cloning.
> 
> For the cms-colord plugin to be able to veto shared-CRTC cloning we
> would need libweston infrastructure work to either offer plugins a veto
> API or make libweston core aware of color profiles. An alternative
> would be to have Weston query the color management plugin directly if
> heads are compatible for shared-CRTC mode.

Yes, it had occurred to me that this could be particularly painful. :(

The plug-in veto idea, seems like it would be a fairly heavy amount of 
overkill if cms-colord would forever be the only plug-in to actually use it.

If I'm understanding all the pieces in play at this point...  We'd 
really just be giving cms-colord the ability to stop weston from 
launching at all, since independent-CRTC cloning doesn't actually work, 
and is (quite reasonably!) out of scope for this series and its follow up?

> One more idea coming to mind is that the "same-as" directive in
> weston.ini could refer to either exclusively shared-CRTC clone mode or
> shared-CRTC with fallback to independent-CRTC mode, and we could use a
> hypothetical future output layout configuration directives to force
> independent-CRTC mode by defining two outputs to show the same area of
> the desktop.
> 
> Given that, what would you recommend for now?

Can we generate a log message when cms-colord configuration results in 
suboptimal display for cloned heads (possibly including some strings to 
identify the display/displays that look wrong)?

Or even a warning when cms-colord and clones are both enabled at the 
same time?

At this point, I think whoever the first person to need clone+cms-colord 
simultaneously is going to have to step up and do some heavy lifting.

As long as we're not silently doing something surprising with color 
profiles, I think it should be ok?  Since clone mode is all new, this 
isn't going to cause a regression for anyone.

> 
> Thanks,
> pq
>