-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 2020-08-24 9:43 a.m., Pekka Paalanen wrote: > On Sat, 22 Aug 2020 11:59:26 +0200 Michel Dänzer > <michel@xxxxxxxxxxx> wrote: >> On 2020-08-21 8:07 p.m., Kazlauskas, Nicholas wrote: >>> On 2020-08-21 12:57 p.m., Michel Dänzer wrote: >>>> From: Michel Dänzer <mdaenzer@xxxxxxxxxx> >>>> >>>> Don't check drm_crtc_state::active for this either, per its >>>> documentation in include/drm/drm_crtc.h: >>>> >>>> * Hence drivers must not consult @active in their various * >>>> &drm_mode_config_funcs.atomic_check callback to reject an >>>> atomic * commit. >>>> >>>> The atomic helpers disable the CRTC as needed for disabling >>>> the primary plane. >>>> >>>> This prevents at least the following problems if the primary >>>> plane gets disabled (e.g. due to destroying the FB assigned >>>> to the primary plane, as happens e.g. with mutter in Wayland >>>> mode): >>>> >>>> * Toggling CRTC active to 1 failed if the cursor plane was >>>> enabled (e.g. via legacy DPMS property & cursor ioctl). * >>>> Enabling the cursor plane failed, e.g. via the legacy cursor >>>> ioctl. >>> >>> We previously had the requirement that the primary plane must >>> be enabled but some userspace expects that they can enable >>> just the overlay plane without anything else. >>> >>> I think the chromuiumos atomictest validates that this works >>> as well: >>> >>> So is DRM going forward then with the expectation that this is >>> wrong behavior from userspace? >>> >>> We require at least one plane to be enabled to display a >>> cursor, but it doesn't necessarily need to be the primary. >> >> It's a "pick your poison" situation: >> >> 1) Currently the checks are invalid (atomic_check must not >> decide based on drm_crtc_state::active), and it's easy for legacy >> KMS userspace to accidentally hit errors trying to enable/move >> the cursor or switch DPMS off → on. >> >> 2) Accurately rejecting only atomic states where the cursor >> plane is enabled but all other planes are off would break the >> KMS helper code, which can only deal with the "CRTC on & primary >> plane off is not allowed" case specifically. >> >> 3) This patch addresses 1) & 2) but may break existing atomic >> userspace which wants to enable an overlay plane while disabling >> the primary plane. >> >> >> I do think in principle atomic userspace is expected to handle >> case 3) and leave the primary plane enabled. > > Hi, > > my opinion as a userspace developer is that enabling a CRTC > without a primary plane has traditionally not worked, so userspace > cannot *rely* on it ever working. I think legacy KMS API does not > even allow to express that really, or has built-in assumptions that > it doesn't work - you can call the legacy cursor ioctls, they > don't fail, but also the CRTC remains off. Correct me if this is > not true. > > Atomic userspace OTOH is required to test the strange (and all!) > cases like this to see if they work or not, and carry on with a > fallback if they don't. Userspace that wants to use a CRTC without > a primary plane likely needs other CRTC properties as well, like > background color. > > I would guess that when two regressions conflict, the older > userspace would win. Hence legacy KMS using Mutter should keep > working, while atomic userspace is left with increased power > consumption. Not using a hardware cursor (Mutter) is much more > easily an end-user visible regression than slightly shorter > battery life. > > Atomic test commits are allowed to fail at any time for any reason > and something that previously worked can fail on the next frame or > on the next kernel, as far as I understand. All of this basically matches my understanding. > Sounds like the helpers you refer to are inadequate for your case. > Can't you fix the helpers in the long run and land this patch as an > immediate fix? I'd rather leave working on those helpers to KMS developers. :) - -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer -----BEGIN PGP SIGNATURE----- iF0EARECAB0WIQSwn681vpFFIZgJURRaga+OatuyAAUCX0UmXAAKCRBaga+Oatuy AIeNAJoC9UgOrF+qBq08uOyjaV7Vfp+PgACfSp3nXB3un3LUZQxrvaxMAON+eIs= =Pbd2 -----END PGP SIGNATURE----- _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
