On Thu, Jan 11, 2024 at 05:17:46PM +0000, Andri Yngvason wrote: > mið., 10. jan. 2024 kl. 13:26 skrifaði Daniel Stone <daniel@xxxxxxxxxxxxx>: > > > > > > This thing here works entirely differently, and I think we need somewhat > > > new semantics for this: > > > > > > - I agree it should be read-only for userspace, so immutable sounds right. > > > > > > - But I also agree with Daniel Stone that this should be tied more > > > directly to the modeset state. > > > > > > So I think the better approach would be to put the output type into > > > drm_connector_state, require that drivers compute it in their > > > ->atomic_check code (which in the future would allow us to report it out > > > for TEST_ONLY commits too), and so guarantee that the value is updated > > > right after the kms ioctl returns (and not somewhen later for non-blocking > > > commits). > > > > That's exactly the point. Whether userspace gets an explicit > > notification or it has to 'know' when to read isn't much of an issue - > > just as long as it's well defined. I think the suggestion of 'do it in > > atomic_check and then it's guaranteed to be readable when the commit > > completes' is a good one. > > > > I do still have some reservations - for instance, why do we have the > > fallback to auto when userspace has explicitly requested a certain > > type? - but they may have been covered previously. > > > > We discussed this further on IRC and this is summary of that discussion: > > Sima proposed a new type of property that can be used to git feedback to > userspace after atomic ioctl. The user supplies a list of output properties > that they want to query and the kernel fills it in before returning from the > ioctl. This would help to get some information about why things failed > during TEST_ONLY. > > Emersion raised the point that you might not know how much memory is needed > beforehand for the returned properties, to which sima replied: blob > property. There was some discussion about how that makes it possible to leak > kernel memory, especially if userspace does not know about a new property > blob. Emersion pointed out that userspace should only request properties > that it understands and pq agreed. > > Emersion asked how the user should inform the kernel that it's done with the > blob, to which sima replied: DRM_IOCTL_MODE_DESTROYPROPBLOB. Sima also > mentioned using some sort of weak reference garbage collection scheme for > properties and there was some further discussion, but I'm not sure there was > any conclusion. > > I asked if it made sense to add color format capabilities to the mode info > struct, but the conclusion was that it wouldn't really be useful because we > need TEST_ONLY anyway to see if the color format setting is compatible with > other settings. > > I asked again if we should drop the "active color format" property as it > seems to be more trouble than it's worth for now. pq mentioned that there > are 2 separate use cases (in his words): > - People playing with setting UI would like to know what "auto" would result > in, but that's just nice to have > - The other use case is the flicker-free boot into known configuration He > went on to point out that the main problem with "auto" is that any modeset > could make the driver decide differently. This means that we cannot fully > rely on the previously set property. > > However, leaving out "active color property" did not put us in a worse > situation than before, so the conclusion was that we should leave it out for > now. For GUI selectors, the current TEST_ONLY should be good enough, and all > the fancy stuff discussed previously isn't needed for now. > > To summarise the summary: this means that we will drop the "active > color format" property and rename the "preferred color format" > property to "force color format" or just "color format" and failing to > satisfy that constraint will fail the modeset rather than falling back > to the "auto" behaviour. That's a good idea. Anything else won't work with the new color pipeline API anyways because user space will be responsible for setting up the color pipeline API in a way so that the monitor will receive the correctly encoded data. > Cheers, > Andri >
