| Message ID | 20200730203642.17553-3-nicholas.kazlauskas@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: Drop DRM private objects from amdgpu_dm | expand |
[AMD Official Use Only - Internal Distribution Only] Reviewed-by: Hersen Wu <hersenxs.wu@amd.com> -----Original Message----- From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> Sent: Thursday, July 30, 2020 4:37 PM To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Lakha, Bhawanpreet <Bhawanpreet.Lakha@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Wu, Hersen <hersenxs.wu@amd.com> 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 <Bhawanpreet.Lakha@amd.com> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Cc: Hersen Wu <hersenxs.wu@amd.com> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> --- .../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
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 <Bhawanpreet.Lakha@amd.com> > Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> > Cc: Hersen Wu <hersenxs.wu@amd.com> > Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > --- > .../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 <Rodrigo.Siqueira@amd.com> > return true; > } > > -- > 2.25.1 >
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 <Bhawanpreet.Lakha@amd.com> >> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> >> Cc: Hersen Wu <hersenxs.wu@amd.com> >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> >> --- >> .../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 <Rodrigo.Siqueira@amd.com> 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 >> >
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; }
[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 <Bhawanpreet.Lakha@amd.com> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Cc: Hersen Wu <hersenxs.wu@amd.com> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)