Am 01.07.21 um 15:24 schrieb Pekka Paalanen: > On Thu, 1 Jul 2021 14:50:13 +0200 > Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx> wrote: > >> Am 01.07.21 um 10:07 schrieb Pekka Paalanen: >> >>> On Wed, 30 Jun 2021 11:20:18 +0200 >>> Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx> wrote: >>> >>>> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: >>>> >>>>> On Tue, 29 Jun 2021 13:39:18 +0200 >>>>> Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx> wrote: >>>>> >>>>>> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: >>>>>>> On Tue, 29 Jun 2021 08:12:54 +0000 >>>>>>> Simon Ser <contact@xxxxxxxxxxx> wrote: >>>>>>> >>>>>>>> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: >>>>>>>> >>>>>>>>> yes, I think this makes sense, even if it is a property that one can't >>>>>>>>> tell for sure what it does before hand. >>>>>>>>> >>>>>>>>> Using a pair of properties, preference and active, to ask for something >>>>>>>>> and then check what actually worked is good for reducing the >>>>>>>>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" >>>>>>>>> test different KMS configurations. Userspace has a better chance of >>>>>>>>> finding a configuration that is possible. >>>>>>>>> >>>>>>>>> OTOH, this has the problem than in UI one cannot tell the user in >>>>>>>>> advance which options are truly possible. Given that KMS properties are >>>>>>>>> rarely completely independent, and in this case known to depend on >>>>>>>>> several other KMS properties, I think it is good enough to know after >>>>>>>>> the fact. >>>>>>>>> >>>>>>>>> If a driver does not use what userspace prefers, there is no way to >>>>>>>>> understand why, or what else to change to make it happen. That problem >>>>>>>>> exists anyway, because TEST_ONLY commits do not give useful feedback >>>>>>>>> but only a yes/no. >>>>>>>> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one >>>>>>>> property at a time), user-space can discover which property makes the atomic >>>>>>>> commit fail. >>>>>>> That works if the properties are independent of each other. Color >>>>>>> range, color format, bpc and more may all be interconnected, >>>>>>> allowing only certain combinations to work. >>>>>>> >>>>>>> If all these properties have "auto" setting too, then it would be >>>>>>> possible to probe each property individually, but that still does not >>>>>>> tell which combinations are valid. >>>>>>> >>>>>>> If you probe towards a certain configuration by setting the properties >>>>>>> one by one, then depending on the order you pick the properties, you >>>>>>> may come to a different conclusion on which property breaks the >>>>>>> configuration. >>>>>> My mind crossed another point that must be considered: When plugin in >>>>>> a Monitor a list of possible Resolutions+Framerate combinations is >>>>>> created for xrandr and other userspace (I guess by atomic checks? but >>>>>> I don't know). >>>>> Hi, >>>>> >>>>> I would not think so, but I hope to be corrected if I'm wrong. >>>>> >>>>> My belief is that the driver collects a list of modes from EDID, some >>>>> standard modes, and maybe some other hardcoded modes, and then >>>>> validates each entry against all the known limitations like vertical >>>>> and horizontal frequency limits, discarding modes that do not fit. >>>>> >>>>> Not all limitations are known during that phase, which is why KMS >>>>> property "link-status" exists. When userspace actually programs a mode >>>>> (not a TEST_ONLY commit), the link training may fail. The kernel prunes >>>>> the mode from the list and sets the link status property to signal >>>>> failure, and sends a hotplug uevent. Userspace needs to re-check the >>>>> mode list and try again. >>>>> >>>>> That is a generic escape hatch for when TEST_ONLY commit succeeds, but >>>>> in reality the hardware cannot do it, you just cannot know until you >>>>> actually try for real. It causes end user visible flicker if it happens >>>>> on an already running connector, but since it usually happens when >>>>> turning a connector on to begin with, there is no flicker to be seen, >>>>> just a small delay in finding a mode that works. >>>>> >>>>>> During this drm >>>>>> properties are already considered, which is no problem atm because as >>>>>> far as i can tell there is currently no drm property that would make >>>>>> a certain Resolutions+Framerate combination unreachable that would be >>>>>> possible with everything on default. >>>>> I would not expect KMS properties to be considered at all. It would >>>>> reject modes that are actually possible if the some KMS properties were >>>>> changed. So at least going forward, current KMS property values cannot >>>>> factor in. >>>> At least the debugfs variable "force_yuv420_output" did change the >>>> available modes here: >>>> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 >>>> before my patch >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf >>> Hi, >>> >>> debugfs is not proper UAPI, so we can just ignore it. Display servers >>> cannot be expected to poke in debugfs. Debugfs is not even supposed to >>> exist in production systems, but I'm sure people use it for hacking >>> stuff. But that's all it is for: developer testing and hacking. >> e.g. Ubuntu has it active by default, but only read (and writable) by root. > Hi, > > that's normal, yes. Root can do damage anyway, and it's useful for > debugging. KMS clients OTOH often do not run as root. > >>> >>>> Forcing a color format via a DRM property in this function would >>>> reintroduce the problem. >>> The property would need to be defined differently because its presence >>> could otherwise break existing userspace. Well, I suppose it could >>> break existing userspace no matter what, so we would need the generic >>> "reset to sane defaults" mechanism first IMO. >>> >>> DRM has client caps for exposing video modes that old userspace might >>> not expect, to avoid breaking old userspace. Care needs to be taken >>> with all new UAPI, because if a kernel upgrade makes something wrong, >>> it's the kernel's fault no matter what userspace is doing, in principle. >> Can you give me a link describing how I define this caps? > I don't have any, but you can find all the existing ones by grepping > for DRM_CLIENT_CAP_. > > I'm not saying that we need it, but mentioning them as a possible > workaround if userspace breakage seems imminent or is proven. > >>> Debugfs has no problem breaking userspace AFAIU, since it's not proper >>> UAPI. >>> >>>> And I think i915 driver works similar in this regard. >>>> >>>>> >>>>>> However for example forcing YCbCr420 encoding would limit the >>>>>> available resolutions (my screen for example only supports YCbCr420 >>>>>> on 4k@60 and @50Hz and on no other resolution or frequency (native is >>>>>> 2560x1440@144Hz). >>>>>> >>>>>> So would a "force color format" that does not get resetted on >>>>>> repluging/reenabling a monitor break the output, for example, of an >>>>>> not updated xrandr, unaware of this new property? >>>>> Yes, not because the mode list would be missing the mode, but because >>>>> actually setting the mode would fail. >>>> Well, like described above, I think the mode would actually be missing, >>>> which is also an unexpected behavior from a user perspective. >>> I think that is not how the property should work. >>> >>> If KMS properties would affect the list of modes, then userspace would >>> need to set the properties for real (TEST_ONLY cannot change anything) >>> and re-fetch the mode lists (maybe there is a hotplug event, maybe >>> not). That workflow just doesn't work. >> The properties are set before the list is created in the first place. >> Because, in my example, the properties get set before the monitor is >> plugged in and the list can only be created as soon as the monitor is >> plugged in. > That's just an accident, it's not what I mean. > > What I mean is, we cannot have the KMS properties affect the list of > modes, because then userspace that want to use specific values on those > properties would have to program those properties first, and then get > the list of modes. KMS UAPI does not work that way AFAIK. > > If the initial mode list is created on hotplug like you say, then the > initial list could already be missing some modes that would be valid if > some KMS properties had different values. Depends if the mode list is created by TEST_ONLY: - The force properties should return false on TEST_ONLY - The force properties should not prevent the mode from showing up in the list If the list is created by TEST_ONLY both things can't be fulfilled at the same time obviously. I hope some can give more insights or has an idea how the properties could work best. > >>> A very interesting question is when should link-status failure not drop >>> the current mode from the mode list, if other KMS properties affect the >>> bandwidth etc. requirements of the mode. That mechanism is engineered >>> for old userspace that doesn't actually handle link-status but does >>> handle hotplug, so the hotplug triggers re-fetching the mode list and >>> userspace maybe trying again with a better luck since the offending >>> mode is gone. How to keep that working when introducing KMS properties >>> forcing the cable format, I don't know. >>> >>> As long as the other affecting KMS properties are all "auto", the >>> driver will probably exhaust all possibilities to make the mode work >>> before signalling link-status failure and pruning the mode. >>> Theoretically, as I have no idea what drivers actually do. >> Isn't that exactly how the "preferred color format" property works in >> my patchset now? > There was an argument that "preferred" with no guarantees is not > useful enough. So I'm considering the force property instead. > The problem is, "auto" is not the only possible value. > > When the value is not "auto", should link failure drop the mode or not? > Userspace might change the value back to "auto" next time. If you > dropped the mode, it would be gone. If you didn't drop the mode, > userspace might be stuck picking the same non-working mode again and > again if it doesn't know about the force mode property. > > You could argue that changing the value back to "auto" needs to reset > the mode list, but that only gets us back to the "need to set > properties before getting mode list". > > Maybe there needs to be an assumption that if "force color format" is > not "auto", then link failure does not drop modes and userspace knows > to handle this. Messy. > > I'm afraid I just don't know to give any clear answer. It's also > possible that, as I'm not a kernel dev, I have some false assumptions > here. > > > Thanks, > pq > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
