Re: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

2020-08-06 Thread Kazlauskas, Nicholas 

On 2020-08-05 5:11 p.m., Rodrigo Siqueira wrote:

On 07/30, Nicholas Kazlauskas wrote:

[Why]
Enabling or disable DCC or switching between tiled and linear formats
can require bandwidth updates.

They're currently skipping all DC validation by being treated as purely
surface updates.

[How]
Treat tiling_flag changes (which encode DCC state) as a condition for
resetting the plane.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Cc: Hersen Wu 
Signed-off-by: Nicholas Kazlauskas 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
  1 file changed, 16 insertions(+), 3 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 7cc5ab90ce13..bf1881bd492c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
 * TODO: Come up with a more elegant solution for this.
 */
for_each_oldnew_plane_in_state(state, other, old_other_state, 
new_other_state, i) {
+   struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
+
if (other->type == DRM_PLANE_TYPE_CURSOR)
continue;
  
@@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state *state,

if (old_other_state->crtc != new_other_state->crtc)
return true;
  
-		/* TODO: Remove this once we can handle fast format changes. */

-   if (old_other_state->fb && new_other_state->fb &&
-   old_other_state->fb->format != new_other_state->fb->format)
+   /* Framebuffer checks fall at the end. */
+   if (!old_other_state->fb || !new_other_state->fb)
+   continue;
+
+   /* Pixel format changes can require bandwidth updates. */
+   if (old_other_state->fb->format != new_other_state->fb->format)
+   return true;
+
+   old_dm_plane_state = to_dm_plane_state(old_other_state);
+   new_dm_plane_state = to_dm_plane_state(new_other_state);
+
+   /* Tiling and DCC changes also require bandwidth updates. */
+   if (old_dm_plane_state->tiling_flags !=
+   new_dm_plane_state->tiling_flags)


Why not add a case when we move to a TMZ area?

Reviewed-by: Rodrigo Siqueira 


TMZ doesn't affect DML calculations or validation in this case so we can 
safely skip it.


Regards,
Nicholas Kazlauskas




return true;
}
  
--

2.25.1





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

2020-08-05 Thread Rodrigo Siqueira 
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> Enabling or disable DCC or switching between tiled and linear formats
> can require bandwidth updates.
> 
> They're currently skipping all DC validation by being treated as purely
> surface updates.
> 
> [How]
> Treat tiling_flag changes (which encode DCC state) as a condition for
> resetting the plane.
> 
> Cc: Bhawanpreet Lakha 
> Cc: Rodrigo Siqueira 
> Cc: Hersen Wu 
> Signed-off-by: Nicholas Kazlauskas 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
>  1 file changed, 16 insertions(+), 3 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 7cc5ab90ce13..bf1881bd492c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state 
> *state,
>* TODO: Come up with a more elegant solution for this.
>*/
>   for_each_oldnew_plane_in_state(state, other, old_other_state, 
> new_other_state, i) {
> + struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
> +
>   if (other->type == DRM_PLANE_TYPE_CURSOR)
>   continue;
>  
> @@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state 
> *state,
>   if (old_other_state->crtc != new_other_state->crtc)
>   return true;
>  
> - /* TODO: Remove this once we can handle fast format changes. */
> - if (old_other_state->fb && new_other_state->fb &&
> - old_other_state->fb->format != new_other_state->fb->format)
> + /* Framebuffer checks fall at the end. */
> + if (!old_other_state->fb || !new_other_state->fb)
> + continue;
> +
> + /* Pixel format changes can require bandwidth updates. */
> + if (old_other_state->fb->format != new_other_state->fb->format)
> + return true;
> +
> + old_dm_plane_state = to_dm_plane_state(old_other_state);
> + new_dm_plane_state = to_dm_plane_state(new_other_state);
> +
> + /* Tiling and DCC changes also require bandwidth updates. */
> + if (old_dm_plane_state->tiling_flags !=
> + new_dm_plane_state->tiling_flags)

Why not add a case when we move to a TMZ area?

Reviewed-by: Rodrigo Siqueira 

>   return true;
>   }
>  
> -- 
> 2.25.1
> 

-- 
Rodrigo Siqueira
https://siqueira.tech


signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

2020-07-30 Thread Wu, Hersen 
[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Hersen Wu 

-Original Message-
From: Nicholas Kazlauskas  
Sent: Thursday, July 30, 2020 4:37 PM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Cc: Kazlauskas, Nicholas ; Lakha, Bhawanpreet 
; Siqueira, Rodrigo ; Wu, 
Hersen 
Subject: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

[Why]
Enabling or disable DCC or switching between tiled and linear formats can 
require bandwidth updates.

They're currently skipping all DC validation by being treated as purely surface 
updates.

[How]
Treat tiling_flag changes (which encode DCC state) as a condition for resetting 
the plane.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Cc: Hersen Wu 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
 1 file changed, 16 insertions(+), 3 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 7cc5ab90ce13..bf1881bd492c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
 * TODO: Come up with a more elegant solution for this.
 */
for_each_oldnew_plane_in_state(state, other, old_other_state, 
new_other_state, i) {
+   struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
+
if (other->type == DRM_PLANE_TYPE_CURSOR)
continue;
 
@@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
if (old_other_state->crtc != new_other_state->crtc)
return true;
 
-   /* TODO: Remove this once we can handle fast format changes. */
-   if (old_other_state->fb && new_other_state->fb &&
-   old_other_state->fb->format != new_other_state->fb->format)
+   /* Framebuffer checks fall at the end. */
+   if (!old_other_state->fb || !new_other_state->fb)
+   continue;
+
+   /* Pixel format changes can require bandwidth updates. */
+   if (old_other_state->fb->format != new_other_state->fb->format)
+   return true;
+
+   old_dm_plane_state = to_dm_plane_state(old_other_state);
+   new_dm_plane_state = to_dm_plane_state(new_other_state);
+
+   /* Tiling and DCC changes also require bandwidth updates. */
+   if (old_dm_plane_state->tiling_flags !=
+   new_dm_plane_state->tiling_flags)
return true;
}
 
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
Enabling or disable DCC or switching between tiled and linear formats
can require bandwidth updates.

They're currently skipping all DC validation by being treated as purely
surface updates.

[How]
Treat tiling_flag changes (which encode DCC state) as a condition for
resetting the plane.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Cc: Hersen Wu 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
 1 file changed, 16 insertions(+), 3 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 7cc5ab90ce13..bf1881bd492c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
 * TODO: Come up with a more elegant solution for this.
 */
for_each_oldnew_plane_in_state(state, other, old_other_state, 
new_other_state, i) {
+   struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
+
if (other->type == DRM_PLANE_TYPE_CURSOR)
continue;
 
@@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
if (old_other_state->crtc != new_other_state->crtc)
return true;
 
-   /* TODO: Remove this once we can handle fast format changes. */
-   if (old_other_state->fb && new_other_state->fb &&
-   old_other_state->fb->format != new_other_state->fb->format)
+   /* Framebuffer checks fall at the end. */
+   if (!old_other_state->fb || !new_other_state->fb)
+   continue;
+
+   /* Pixel format changes can require bandwidth updates. */
+   if (old_other_state->fb->format != new_other_state->fb->format)
+   return true;
+
+   old_dm_plane_state = to_dm_plane_state(old_other_state);
+   new_dm_plane_state = to_dm_plane_state(new_other_state);
+
+   /* Tiling and DCC changes also require bandwidth updates. */
+   if (old_dm_plane_state->tiling_flags !=
+   new_dm_plane_state->tiling_flags)
return true;
}
 
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx