Re: [PATCH 3/7] drm/amd/display: Avoid using unvalidated tiling_flags and tmz_surface in prepare_planes

[Kazlauskas, Nicholas]

On 2020-08-07 4:30 a.m., dan...@ffwll.ch wrote:
On Thu, Jul 30, 2020 at 04:36:38PM -0400, Nicholas Kazlauskas wrote:
[Why]
We're racing with userspace as the flags could potentially change
from when we acquired and validated them in commit_check.

Uh ... I didn't know these could change. I think my comments on Bas'
series are even more relevant now. I think long term would be best to bake
these flags in at addfb time when modifiers aren't set. And otherwise
always use the modifiers flag, and completely ignore the legacy flags
here.
-Daniel


There's a set tiling/mod flags IOCTL that can be called after addfb 
happens, so unless there's some sort of driver magic preventing this 
from working when it's already been pinned for scanout then I don't see 
anything stopping this from happening.

I still need to review the modifiers series in a little more detail but 
that looks like a good approach to fixing these kind of issues.

Regards,
Nicholas Kazlauskas


[How]
We unfortunately can't drop this function in its entirety from
prepare_planes since we don't know the afb->address at commit_check
time yet.

So instead of querying new tiling_flags and tmz_surface use the ones
from the plane_state directly.

While we're at it, also update the force_disable_dcc option based
on the state from atomic check.

Cc: Bhawanpreet Lakha <bhawanpreet.la...@amd.com>
Cc: Rodrigo Siqueira <rodrigo.sique...@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlaus...@amd.com>
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ++++++++++---------
  1 file changed, 19 insertions(+), 17 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 bf1881bd492c..f78c09c9585e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5794,14 +5794,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane 
*plane,
        struct list_head list;
        struct ttm_validate_buffer tv;
        struct ww_acquire_ctx ticket;
-       uint64_t tiling_flags;
        uint32_t domain;
        int r;
-       bool tmz_surface = false;
-       bool force_disable_dcc = false;
-
-       dm_plane_state_old = to_dm_plane_state(plane->state);
-       dm_plane_state_new = to_dm_plane_state(new_state);
  
  	if (!new_state->fb) {
                DRM_DEBUG_DRIVER("No FB bound\n");
@@ -5845,27 +5839,35 @@ static int dm_plane_helper_prepare_fb(struct drm_plane 
*plane,
                return r;
        }
  
-	amdgpu_bo_get_tiling_flags(rbo, &tiling_flags);
-
-       tmz_surface = amdgpu_bo_encrypted(rbo);
-
        ttm_eu_backoff_reservation(&ticket, &list);
  
  	afb->address = amdgpu_bo_gpu_offset(rbo);
  
  	amdgpu_bo_ref(rbo);
  
+	/**
+        * We don't do surface updates on planes that have been newly created,
+        * but we also don't have the afb->address during atomic check.
+        *
+        * Fill in buffer attributes depending on the address here, but only on
+        * newly created planes since they're not being used by DC yet and this
+        * won't modify global state.
+        */
+       dm_plane_state_old = to_dm_plane_state(plane->state);
+       dm_plane_state_new = to_dm_plane_state(new_state);
+
        if (dm_plane_state_new->dc_state &&
-                       dm_plane_state_old->dc_state != 
dm_plane_state_new->dc_state) {
-               struct dc_plane_state *plane_state = 
dm_plane_state_new->dc_state;
+           dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
+               struct dc_plane_state *plane_state =
+                       dm_plane_state_new->dc_state;
+               bool force_disable_dcc = !plane_state->dcc.enable;
  
-		force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend;
                fill_plane_buffer_attributes(
                        adev, afb, plane_state->format, plane_state->rotation,
-                       tiling_flags, &plane_state->tiling_info,
-                       &plane_state->plane_size, &plane_state->dcc,
-                       &plane_state->address, tmz_surface,
-                       force_disable_dcc);
+                       dm_plane_state_new->tiling_flags,
+                       &plane_state->tiling_info, &plane_state->plane_size,
+                       &plane_state->dcc, &plane_state->address,
+                       dm_plane_state_new->tmz_surface, force_disable_dcc);
        }
  
  	return 0;
--
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel