On 2021-10-21 13:55, Rodrigo Siqueira Jordao wrote:
> Hi Simon,
>
> I tested this patch and it lgtm. I also agree to revert it.
>
> Btw, did you send the revert patch for "amd/display: only require overlay
> plane to cover whole CRTC on ChromeOS"? I think we need to revert it as well.
>
Agreed that this patch is good but we'll need to also revert the is_chromeos
w/a.
This patch is
Reviewed-by: Harry Wentland <harry.wentl...@amd.com>
Harry
> Sean, Mark
>
> For ChromeOS, we should ignore this patch. Do we need to take any action to
> avoid landing this patch on ChromeOS tree?
>
> Thanks
> Siqueira
>
> On 2021-10-14 11:35 a.m., Simon Ser wrote:
>> This reverts commits ddab8bd788f5 ("drm/amd/display: Fix two cursor
>> duplication
>> when using overlay") and e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay
>> validation by considering cursors"").
>>
>> tl;dr ChromeOS uses the atomic interface for everything except the cursor.
>> This
>> is incorrect and forces amdgpu to disable some hardware features. Let's
>> revert
>> the ChromeOS-specific workaround in mainline and allow the Chrome team to
>> keep
>> it internally in their own tree.
>>
>> See [1] for more details. This patch is an alternative to [2], which added
>> ChromeOS detection.
>>
>> [1]:
>> https://lore.kernel.org/amd-gfx/JIQ_93_cHcshiIDsrMU1huBzx9P9LVQxucx8hQArpQu7Wk5DrCl_vTXj_Q20m_L-8C8A5dSpNcSJ8ehfcCrsQpfB5QG_Spn14EYkH9chtg0=@emersion.fr/>>
>> [2]:
>> https://lore.kernel.org/amd-gfx/20211011151609.452132-1-cont...@emersion.fr/>>
>> Signed-off-by: Simon Ser <cont...@emersion.fr>
>> Cc: Alex Deucher <alexander.deuc...@amd.com>
>> Cc: Harry Wentland <hwent...@amd.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlaus...@amd.com>
>> Cc: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>
>> Cc: Rodrigo Siqueira <rodrigo.sique...@amd.com>
>> Cc: Sean Paul <seanp...@chromium.org>
>> Fixes: ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when using
>> overlay")
>> Fixes: e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay validation by
>> considering cursors"")
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 -------------------
>> 1 file changed, 51 deletions(-)
>>
>> 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 20065a145851..014c5a9fe461 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -10628,53 +10628,6 @@ static int add_affected_mst_dsc_crtcs(struct
>> drm_atomic_state *state, struct drm
>> }
>> #endif
>> -static int validate_overlay(struct drm_atomic_state *state)
>> -{
>> - int i;
>> - struct drm_plane *plane;
>> - struct drm_plane_state *new_plane_state;
>> - struct drm_plane_state *primary_state, *overlay_state = NULL;
>> -
>> - /* 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) {
>> - if (drm_atomic_plane_disabling(plane->state, new_plane_state))
>> - return 0;
>> -
>> - overlay_state = new_plane_state;
>> - continue;
>> - }
>> - }
>> -
>> - /* check if we're making changes to the overlay plane */
>> - if (!overlay_state)
>> - return 0;
>> -
>> - /* check if overlay plane is enabled */
>> - if (!overlay_state->crtc)
>> - return 0;
>> -
>> - /* find the primary plane for the CRTC that the overlay is enabled on */
>> - primary_state = drm_atomic_get_plane_state(state,
>> overlay_state->crtc->primary);
>> - if (IS_ERR(primary_state))
>> - return PTR_ERR(primary_state);
>> -
>> - /* check if primary plane is enabled */
>> - if (!primary_state->crtc)
>> - return 0;
>> -
>> - /* Perform the bounds check to ensure the overlay plane covers the
>> primary */
>> - if (primary_state->crtc_x < overlay_state->crtc_x ||
>> - primary_state->crtc_y < overlay_state->crtc_y ||
>> - primary_state->crtc_x + primary_state->crtc_w >
>> overlay_state->crtc_x + overlay_state->crtc_w ||
>> - primary_state->crtc_y + primary_state->crtc_h >
>> overlay_state->crtc_y + overlay_state->crtc_h) {
>> - DRM_DEBUG_ATOMIC("Overlay plane is enabled with hardware cursor but
>> does not fully cover primary plane\n");
>> - return -EINVAL;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> /**
>> * amdgpu_dm_atomic_check() - Atomic check implementation for AMDgpu DM.
>> * @dev: The DRM device
>> @@ -10856,10 +10809,6 @@ static int amdgpu_dm_atomic_check(struct drm_device
>> *dev,
>> goto fail;
>> }
>> - ret = validate_overlay(state);
>> - if (ret)
>> - goto fail;
>> -
>> /* Add new/modified planes */
>> for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
>> new_plane_state, i) {
>> ret = dm_update_plane_state(dc, state, plane,
>>
>
