[PATCH v5] amd/display: only require overlay plane to cover whole CRTC on ChromeOS

[Simon Ser]

On Tuesday, October 12th, 2021 at 11:08, Paul Menzel <pmenzel at molgen.mpg.de> wrote:

> > the cursor plane (which uses the legacy API). Thus amdgpu must always
> > be prepared to enable/disable/move the cursor plane at any time without
> > failing (or else ChromeOS will trip over).
>
> What ChromeOS version did you test with? Are there plans to improve
> ChromeOS?

No idea, I haven't received feedback from the ChromeOS folks.

> > As discussed in [1], there's no reason why the ChromeOS limitation
> > should prevent other fully atomic users from taking advantage of the
> > overlay plane. Let's limit the check to ChromeOS.
>
> How do we know, no other userspace programs are affected, breaking
> Linux’ no-regression in userspace policy?

Actually this is the other way around: the ChromeOS fix which landed
has broken my user-space. This patch tries to fix the situation for
both ChromeOS and gamescope.

That said, it seems like amdgpu maintainers are open to just revert the
ChromeOS fix, thus fixing gamescope. ChromeOS can carry the fix in their
kernel tree. More on that soon.

> > v4: fix ChromeOS detection (Harry)
> >
> > v5: fix conflict with linux-next
> >
> > [1]: https://lore.kernel.org/amd-gfx/JIQ_93_cHcshiIDsrMU1huBzx9P9LVQxucx8hQArpQu7Wk5DrCl_vTXj_Q20m_L-8C8A5dSpNcSJ8ehfcCrsQpfB5QG_Spn14EYkH9chtg0=@emersion.fr/
> >
> > Signed-off-by: Simon Ser <contact at emersion.fr>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Cc: Harry Wentland <hwentlan at amd.com>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> > Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> > Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> > Cc: Sean Paul <seanpaul at chromium.org>
> > Fixes: ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when using overlay")
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 29 +++++++++++++++++++
> >   1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index f35561b5a465..2eeda1fec506 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -10594,6 +10594,31 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm
> >   }
> >   #endif
> >
> > +static bool is_chromeos(void)
> > +{
> > +	struct mm_struct *mm = current->mm;
> > +	struct file *exe_file;
> > +	bool ret;
> > +
> > +	/* ChromeOS renames its thread to DrmThread. Also check the executable
> > +	 * name. */
> > +	if (strcmp(current->comm, "DrmThread") != 0 || !mm)
> > +		return false;
> > +
> > +	rcu_read_lock();
> > +	exe_file = rcu_dereference(mm->exe_file);
> > +	if (exe_file && !get_file_rcu(exe_file))
> > +		exe_file = NULL;
> > +	rcu_read_unlock();
> > +
> > +	if (!exe_file)
> > +		return false;
> > +	ret = strcmp(exe_file->f_path.dentry->d_name.name, "chrome") == 0;
> > +	fput(exe_file);
> > +
> > +	return ret;
> > +}
> > +
> >   static int validate_overlay(struct drm_atomic_state *state)
> >   {
> >   	int i;
> > @@ -10601,6 +10626,10 @@ static int validate_overlay(struct drm_atomic_state *state)
> >   	struct drm_plane_state *new_plane_state;
> >   	struct drm_plane_state *primary_state, *overlay_state = NULL;
> >
> > +	/* This is a workaround for ChromeOS only */
> > +	if (!is_chromeos())
> > +		return 0;
>
> I would have expected the check to be the other way around, as no the
> behavior on non-Chrome OS is changed?

This function performs a check which is only necessary on ChromeOS. On
non-ChromeOS, this function prevents user-space from using some hardware
features. The early return ensures non-ChromeOS user-space can use these
features.