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

[Paul Menzel]

Dear Simon,


Am 11.10.21 um 17:16 schrieb Simon Ser:
> Commit ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when
> using overlay") changed the atomic validation code to forbid the
> overlay plane from being used if it doesn't cover the whole CRTC. The
> motivation is that ChromeOS uses the atomic API for everything except

s/motivation/problem/

> 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?

> 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?

> 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?

> +

Could some log be added, if ChromeOS is detected?

>   	/* Check if primary plane is contained inside overlay */
>   	for_each_new_plane_in_state_reverse(state, plane, new_plane_state, i) {
>   		if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
> 


Kind regards,

Paul