Received-SPF: pass (google.com: domain of linux-kernel+bounces-28669-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 17 Jan 2024 10:55:09 +0200
From: Pekka Paalanen <ppaalanen@gmail.com>
To: Xaver Hugl <xaver.hugl@gmail.com>
Cc: =?UTF-8?B?QW5kcsOp?= Almeida <andrealmeid@igalia.com>,
 dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
 linux-kernel@vger.kernel.org, kernel-dev@igalia.com,
 alexander.deucher@amd.com, christian.koenig@amd.com, Simon Ser
 <contact@emersion.fr>, daniel@ffwll.ch, Daniel Stone
 <daniel@fooishbar.org>, Marek =?UTF-8?B?T2zFocOhaw==?= <maraeo@gmail.com>,
 Dave Airlie <airlied@gmail.com>, ville.syrjala@linux.intel.com, Joshua
 Ashton <joshua@froggi.es>
Subject: Re: [PATCH 0/2] drm/atomic: Allow drivers to write their own plane
 check for async
Message-ID: <20240117105509.1984837f@eldfell>
In-Reply-To: <CAFZQkGyOQ5Tfu++-cHqgZ9NOJxqxm8cAF5XT18LmisuPAUbXAg@mail.gmail.com>
References: <20240116045159.1015510-1-andrealmeid@igalia.com>
	<20240116114522.5b83d8b6@eldfell>
	<a6099681-1ae9-48ef-99bc-d3c919007413@igalia.com>
	<20240116151414.10b831e6@eldfell>
	<47c6866a-34d6-48b1-a977-d21c48d991dc@igalia.com>
	<CAFZQkGyOQ5Tfu++-cHqgZ9NOJxqxm8cAF5XT18LmisuPAUbXAg@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="Sig_/uaSCk2iCuPC2HhY2IGqSpLb";
 protocol="application/pgp-signature"; micalg=pgp-sha256

--Sig_/uaSCk2iCuPC2HhY2IGqSpLb
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Tue, 16 Jan 2024 17:10:18 +0100
Xaver Hugl <xaver.hugl@gmail.com> wrote:

> My plan is to require support for IN_FENCE_FD at least. If the driver
> doesn't
> allow tearing with that, then tearing just doesn't happen.

That's an excellent point. I think this is important enough in its own
right, that it should be called out in the patch series.

Is it important enough to be special-cased, e.g. to be always allowed
with async commits?

Now that I think of it, if userspace needs to wait for the in-fence
itself before kicking KMS async, that would defeat much of the async's
point, right? And cases where in-fence is not necessary are so rare
they might not even exist?

So if driver/hardware cannot do IN_FENCE_FD with async, is there any
use of supporting async to begin with?

> For overlay planes though, it depends on how the compositor prioritizes
> things.
> If the compositor prioritizes overlay planes and would like to do tearing
> if possible,
> then this patch works.

Ok, I can see that.

> If the compositor prioritizes tearing and would like to do overlay planes
> if possible,
> it would have to know that switching to synchronous commits for a single
> frame,
> setting up the overlay planes and then switching back to async commits
> works, and
> that can't be figured out with TEST_ONLY commits.

I had to ponder a bit why. So I guess the synchronous commit in between
is because driver/hardware may not be able to enable/disable extra
planes in async, so you need a synchronous commit to set them up, but
afterwards updates can tear.

The comment about Intel needing one more synchronous commit when
switching from sync to async updates comes to mind as well, would that
be a problem?

> So I think having a CAP or immutable plane property to signal that async
> commits
> with overlay and/or cursor planes is supported would be useful.

Async cursor planes a good point, particularly moving them around. I'm
not too informed about the prior/on-going efforts to allow cursor
movement more often than refresh rate, I recall something about
amending atomic commits? How would these interact?

I suppose the kernel still prevents any new async commit while a
previous commit is not finished, so amending commits would still be
necessary for cursor plane motion? Or would it, if you time "big
commits" to finish quickly and then spam async "cursor commits" in the
mean time?


Thanks,
pq

> Am Di., 16. Jan. 2024 um 14:35 Uhr schrieb Andr=C3=A9 Almeida <
> andrealmeid@igalia.com>: =20
>=20
> > + Joshua
> >
> > Em 16/01/2024 10:14, Pekka Paalanen escreveu: =20
> > > On Tue, 16 Jan 2024 08:50:59 -0300
> > > Andr=C3=A9 Almeida <andrealmeid@igalia.com> wrote:
> > > =20
> > >> Hi Pekka,
> > >>
> > >> Em 16/01/2024 06:45, Pekka Paalanen escreveu: =20
> > >>> On Tue, 16 Jan 2024 01:51:57 -0300
> > >>> Andr=C3=A9 Almeida <andrealmeid@igalia.com> wrote:
> > >>> =20
> > >>>> Hi,
> > >>>>
> > >>>> AMD hardware can do more on the async flip path than just the prim=
ary =20
> > plane, so =20
> > >>>> to lift up the current restrictions, this patchset allows drivers =
to =20
> > write their =20
> > >>>> own check for planes for async flips. =20
> > >>>
> > >>> Hi,
> > >>>
> > >>> what's the userspace story for this, how could userspace know it co=
uld =20
> > do more? =20
> > >>> What kind of userspace would take advantage of this and in what =20
> > situations? =20
> > >>>
> > >>> Or is this not meant for generic userspace? =20
> > >>
> > >> Sorry, I forgot to document this. So the idea is that userspace will
> > >> query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls,
> > >> instead of having capabilities for each prop. =20
> > >
> > > That's the theory, but do you have a practical example?
> > >
> > > What other planes and props would one want change in some specific use
> > > case?
> > >
> > > Is it just "all or nothing", or would there be room to choose and pick
> > > which props you change and which you don't based on what the driver
> > > supports? If the latter, then relying on TEST_ONLY might be yet anoth=
er
> > > combinatorial explosion to iterate through.
> > > =20
> >
> > That's a good question, maybe Simon, Xaver or Joshua can share how they
> > were planning to use this on Gamescope or Kwin.
> > =20
> > >
> > > Thanks,
> > > pq
> > > =20
> > >>>> I'm not sure if adding something new to drm_plane_funcs is the rig=
ht =20
> > way to do, =20
> > >>>> because if we want to expand the other object types (crtc, connect=
or) =20
> > we would =20
> > >>>> need to add their own drm_XXX_funcs, so feedbacks are welcome!
> > >>>>
> > >>>>    Andr=C3=A9
> > >>>>
> > >>>> Andr=C3=A9 Almeida (2):
> > >>>>     drm/atomic: Allow drivers to write their own plane check for a=
sync
> > >>>>       flips
> > >>>>     drm/amdgpu: Implement check_async_props for planes
> > >>>>
> > >>>>    .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 30 +++++++++
> > >>>>    drivers/gpu/drm/drm_atomic_uapi.c             | 62 =20
> > ++++++++++++++----- =20
> > >>>>    include/drm/drm_atomic_uapi.h                 | 12 ++++
> > >>>>    include/drm/drm_plane.h                       |  5 ++
> > >>>>    4 files changed, 92 insertions(+), 17 deletions(-)
> > >>>> =20
> > >>> =20
> > > =20
> > =20


--Sig_/uaSCk2iCuPC2HhY2IGqSpLb
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEJQjwWQChkWOYOIONI1/ltBGqqqcFAmWnle0ACgkQI1/ltBGq
qqc0zRAAmsYLm9QbFAy7oGFE5ESCY1Ws0GyM7zBZ9ZbDTOGpyvJT5ShZzao3l/j2
sDO1wxeC7xW9KhHQGEmJlWMlG+dWv1pXimYEKV0DY5+6e+6WfGA1sGODtn9xKJKd
hFBnZqMnfvKLeowHBIt/00aJoyqwBXovoWyH59kAqSsh8TIKsyp7G5uwFtSHrJl7
wiOysCWzlY8db9onusa9LMBgLG+2K/wDm1PDpjUqKDGidY23CMvz+L2afZf92Hl9
zglYB4UrR9JAVBH88Bi6rhGVMKsidnLpNsPK07ZIvTU8dxlqU5Bxp6zKZmWIOSMF
weCqzq1CatqKo1WvMKU92rS8wQSSEa+TN0/9JIN8TbOb0k6UJwv3yxJ8VjjRPiV3
5L3TjjJOqLxzM4j5K4wS6UQvzY/lpecydBiTNesdniMdCpuNcX3TmAlF8zxJin/G
olK37BM0+4qeEuqRTbYrhDVzmfaASSEzY4i0AgT7Zkk+wYCYReQ0Z6o6wSN570V9
/H1dWPoQlD60lV/2ZPZTGRmLzo3KIgtrNMFqquuE2eIn02PkWSp78B/MORKAslol
EYqBIHM3+DBy5LZiA3kZGO0V1Gj1j4/eShzSxx7WoF0X4FIP5OhMd01AQir0n1B1
h1tk+5WRYgxVja8ZD3ZbySDboTrJw61iD5EFlLA537G9vBSwhRI=
=JhIu
-----END PGP SIGNATURE-----

--Sig_/uaSCk2iCuPC2HhY2IGqSpLb--