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

[Alex Deucher <alexdeucher@xxxxxxxxx>]

On Tue, Oct 12, 2021 at 4:57 PM Harry Wentland <harry.wentland@xxxxxxx> wrote:
>
>
>
> On 10/12/21 3:57 PM, Alex Deucher wrote:
> > On Tue, Oct 12, 2021 at 10:39 AM Harry Wentland <harry.wentland@xxxxxxx> wrote:
> >>
> >> On 2021-10-11 11:16, Simon Ser wrote:
> >>> 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
> >>> 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).
> >>>
> >>> 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.
> >>>
> >>> v4: fix ChromeOS detection (Harry)
> >>>
> >>> v5: fix conflict with linux-next
> >>>
> >>> [1]: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Famd-gfx%2FJIQ_93_cHcshiIDsrMU1huBzx9P9LVQxucx8hQArpQu7Wk5DrCl_vTXj_Q20m_L-8C8A5dSpNcSJ8ehfcCrsQpfB5QG_Spn14EYkH9chtg0%3D%40emersion.fr%2F&amp;data=04%7C01%7Charry.wentland%40amd.com%7Cf5038651be2d44b2d11208d98dba8a8e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637696654602344329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=83wfZCmSw3IpY%2BRxgnVB4YqABUf8W%2BgYCynDzLvFU7g%3D&amp;reserved=0>>
> >>> Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
> >>> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> >>> Cc: Harry Wentland <hwentlan@xxxxxxx>
> >>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> >>> Cc: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
> >>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
> >>> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> >>> Fixes: ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when using overlay")
> >>
> >> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
> >
> > @Harry Wentland, @Simon Ser Do you have a preference on whether we
> > apply this patch or revert ddab8bd788f5?  I'm fine with either.
> >
>
> Is get_mm_exe_file missing on linux-next? I'm okay either
> way but haven't looked closely at linux-next.

Yes, it was removed in 5.15.

Alex

>
> Another option, as discussed by Simon on IRC, might be
> to take this patch only on the Chrome kernels, though
> it would be nice to avoid custom patches on Chrome kernels.
>
> Harry
>
> > Alex
> >
> >>
> >> Harry
> >>
> >>> ---
> >>>   .../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;
> >>> +
> >>>        /* 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) {
> >>>
> >>