| Message ID | 20190304144909.6267-4-helen.koike@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Fix fb changes for async updates | expand |
On 3/4/19 9:49 AM, Helen Koike wrote: > Async update callbacks are expected to set the old_fb in the new_state > so prepare/cleanup framebuffers are balanced. > > Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new > fb and put the old fb) is not required, as it's taken care by > drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). > > Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> > Signed-off-by: Helen Koike <helen.koike@collabora.com> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> I guess the swap itself should be enough here as per the commit description. It would have been nice if this patch dropped the old_plane_state->fb != new_plane_state->fb check too at the same time, but I suppose I can drop that later. It'll help us pass those failing IGT tests as well. Nicholas Kazlauskas > > --- > Hello, > > As mentioned in the cover letter, > I tested on the rockchip and on i915 using igt plane_cursor_legacy and > kms_cursor_legacy and I didn't see any regressions. > But I couldn't test on AMD because I don't have the hardware and I would > appreciate if anyone could test it. > > Also, I didn't CC to stable here as I saw the async_update function was only > added on v4.20, please let me know if I should CC to stable. > > Thanks! > Helen > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 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 3a6f595f295e..bc02800254bf 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3760,8 +3760,7 @@ static void dm_plane_atomic_async_update(struct drm_plane *plane, > struct drm_plane_state *old_state = > drm_atomic_get_old_plane_state(new_state->state, plane); > > - if (plane->state->fb != new_state->fb) > - drm_atomic_set_fb_for_plane(plane->state, new_state->fb); > + swap(plane->state->fb, new_state->fb); > > plane->state->src_x = new_state->src_x; > plane->state->src_y = new_state->src_y; >
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 3a6f595f295e..bc02800254bf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3760,8 +3760,7 @@ static void dm_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(new_state->state, plane); - if (plane->state->fb != new_state->fb) - drm_atomic_set_fb_for_plane(plane->state, new_state->fb); + swap(plane->state->fb, new_state->fb); plane->state->src_x = new_state->src_x; plane->state->src_y = new_state->src_y;
Async update callbacks are expected to set the old_fb in the new_state so prepare/cleanup framebuffers are balanced. Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new fb and put the old fb) is not required, as it's taken care by drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Helen Koike <helen.koike@collabora.com> --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. But I couldn't test on AMD because I don't have the hardware and I would appreciate if anyone could test it. Also, I didn't CC to stable here as I saw the async_update function was only added on v4.20, please let me know if I should CC to stable. Thanks! Helen drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)