Message-ID: <d3674d49-8bca-7ecf-1735-7bff2d9d526e@tuxedocomputers.com>
Date:   Wed, 30 Jun 2021 11:20:18 +0200
From:   Werner Sembach <wse@...edocomputers.com>
To:     Pekka Paalanen <ppaalanen@...il.com>
Cc:     sunpeng.li@....com, Simon Ser <contact@...rsion.fr>,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, airlied@...ux.ie,
        amd-gfx@...ts.freedesktop.org, tzimmermann@...e.de,
        rodrigo.vivi@...el.com, alexander.deucher@....com,
        christian.koenig@....com
Subject: Re: [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm
 property as setting for userspace

Am 30.06.21 um 10:41 schrieb Pekka Paalanen:

> On Tue, 29 Jun 2021 13:39:18 +0200
> Werner Sembach <wse@...edocomputers.com> wrote:
>
>> Am 29.06.21 um 13:17 schrieb Pekka Paalanen:
>>> On Tue, 29 Jun 2021 08:12:54 +0000
>>> Simon Ser <contact@...rsion.fr> wrote:
>>>   
>>>> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen <ppaalanen@...il.com> 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

Forcing a color format via a DRM property in this function would 
reintroduce the problem.

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@...Hz).
>>
>> 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.
>
> RandR in particular is problematic, because it does not actually
> understand any KMS properties, it is merely a relay. So anything
> that *uses* RandR protocol or xrandr command would also need to be
> patched to understand the new properties.
>
> The kernel automatically resetting *some* properties in *some*
> occasions seems really fragile and complicated to me, which is why I'm
> a lot more keen to see a "reset everything to sensible defaults"
> generic mechanism added to KMS.
Would you see that mechanism not (yet) existing a blocker for this 
patchset/the "force-" properties?
>
>
> Thanks,
> pq
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Powered by blists - more mailing lists