[3/5] drm/amd: fix fb references in async update

Submitted by Helen Koike on March 4, 2019, 2:49 p.m.

Details

Message ID 20190304144909.6267-4-helen.koike@collabora.com
State Accepted
Commit 332af874db929f92931727bfe191b2c666438c81
Headers show
Series "drm: Fix fb changes for async updates" ( rev: 1 ) in DRI devel

Commit Message

Helen Koike March 4, 2019, 2:49 p.m.
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(-)

Patch hide | download patch | download mbox

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;

Comments

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;

>