Re: [PATCH] drm/amd/display: Trivial swizzle-related code clean-ups
Applied. Thanks!
Alex
On Tue, Jan 31, 2023 at 9:40 AM Harry Wentland wrote:
>
> On 1/30/23 14:56, Guilherme G. Piccoli wrote:
> > This is a very trivial code clean-up related to commit 5468c36d6285
> > ("drm/amd/display: Filter Invalid 420 Modes for HDMI TMDS"). This commit
> > added a validation on driver probe to prevent invalid TMDS modes, but one
> > of the fake properties (swizzle) ended-up causing a warning on driver
> > probe; was reported here:
> > https://gitlab.freedesktop.org/drm/amd/-/issues/2264.
> >
> > It was fixed by commit 105a8b8698e2 ("drm/amd/display: patch cases with
> > unknown plane state to prevent warning"), but the validation code had
> > a double variable assignment, which we hereby remove. Also, the fix relies
> > in the dcn2{0,1}patch_unknown_plane_state() callbacks, so while at it we
> > took the opportunity to perform a small code clean-up in such routines.
> >
> > Cc: Aurabindo Pillai
> > Cc: Daniel Wheeler
> > Cc: Fangzhi Zuo
> > Cc: Harry Wentland
> > Cc: Leo Li
> > Cc: Mark Broadworth
> > Cc: Melissa Wen
> > Cc: Rodrigo Siqueira
> > Cc: Sung Joon Kim
> > Cc: Swapnil Patel
> > Signed-off-by: Guilherme G. Piccoli
>
> Reviewed-by: Harry Wentland
>
> Harry
>
> > ---
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
> > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 8 ++--
> > drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 6 ++
> > 3 files changed, 4 insertions(+), 11 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 86a2f7f58550..e71e94663d14 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6336,7 +6336,6 @@ static enum dc_status
> > dm_validate_stream_and_context(struct dc *dc,
> > dc_plane_state->plane_size.surface_size.width = stream->src.width;
> > dc_plane_state->plane_size.chroma_size.height = stream->src.height;
> > dc_plane_state->plane_size.chroma_size.width = stream->src.width;
> > - dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
> > dc_plane_state->format = SURFACE_PIXEL_FORMAT_GRPH_ARGB;
> > dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
> > dc_plane_state->rotation = ROTATION_ANGLE_0;
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > index 531f405d2554..3af24ef9cb2d 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > @@ -2225,14 +2225,10 @@ enum dc_status
> > dcn20_patch_unknown_plane_state(struct dc_plane_state *plane_stat
> > enum surface_pixel_format surf_pix_format = plane_state->format;
> > unsigned int bpp = resource_pixel_format_to_bpp(surf_pix_format);
> >
> > - enum swizzle_mode_values swizzle = DC_SW_LINEAR;
> > -
> > + plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_S;
> > if (bpp == 64)
> > - swizzle = DC_SW_64KB_D;
> > - else
> > - swizzle = DC_SW_64KB_S;
> > + plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_D;
> >
> > - plane_state->tiling_info.gfx9.swizzle = swizzle;
> > return DC_OK;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> > b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> > index fbcf0afeae0d..8f9244fe5c86 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> > @@ -1393,15 +1393,13 @@ static uint32_t read_pipe_fuses(struct dc_context
> > *ctx)
> >
> > static enum dc_status dcn21_patch_unknown_plane_state(struct
> > dc_plane_state *plane_state)
> > {
> > - enum dc_status result = DC_OK;
> > -
> > if (plane_state->ctx->dc->debug.disable_dcc == DCC_ENABLE) {
> > plane_state->dcc.enable = 1;
> > /* align to our worst case block width */
> > plane_state->dcc.meta_pitch = ((plane_state->src_rect.width +
> > 1023) / 1024) * 1024;
> > }
> > - result = dcn20_patch_unknown_plane_state(plane_state);
> > - return result;
> > +
> > + return dcn20_patch_unknown_plane_state(plane_state);
> > }
> >
> > static const struct resource_funcs dcn21_res_pool_funcs = {
>
Re: [PATCH] drm/amd/display: Trivial swizzle-related code clean-ups
On 1/30/23 14:56, Guilherme G. Piccoli wrote:
> This is a very trivial code clean-up related to commit 5468c36d6285
> ("drm/amd/display: Filter Invalid 420 Modes for HDMI TMDS"). This commit
> added a validation on driver probe to prevent invalid TMDS modes, but one
> of the fake properties (swizzle) ended-up causing a warning on driver
> probe; was reported here:
> https://gitlab.freedesktop.org/drm/amd/-/issues/2264.
>
> It was fixed by commit 105a8b8698e2 ("drm/amd/display: patch cases with
> unknown plane state to prevent warning"), but the validation code had
> a double variable assignment, which we hereby remove. Also, the fix relies
> in the dcn2{0,1}patch_unknown_plane_state() callbacks, so while at it we
> took the opportunity to perform a small code clean-up in such routines.
>
> Cc: Aurabindo Pillai
> Cc: Daniel Wheeler
> Cc: Fangzhi Zuo
> Cc: Harry Wentland
> Cc: Leo Li
> Cc: Mark Broadworth
> Cc: Melissa Wen
> Cc: Rodrigo Siqueira
> Cc: Sung Joon Kim
> Cc: Swapnil Patel
> Signed-off-by: Guilherme G. Piccoli
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 8 ++--
> drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 6 ++
> 3 files changed, 4 insertions(+), 11 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 86a2f7f58550..e71e94663d14 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6336,7 +6336,6 @@ static enum dc_status
> dm_validate_stream_and_context(struct dc *dc,
> dc_plane_state->plane_size.surface_size.width = stream->src.width;
> dc_plane_state->plane_size.chroma_size.height = stream->src.height;
> dc_plane_state->plane_size.chroma_size.width = stream->src.width;
> - dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
> dc_plane_state->format = SURFACE_PIXEL_FORMAT_GRPH_ARGB;
> dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
> dc_plane_state->rotation = ROTATION_ANGLE_0;
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 531f405d2554..3af24ef9cb2d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -2225,14 +2225,10 @@ enum dc_status dcn20_patch_unknown_plane_state(struct
> dc_plane_state *plane_stat
> enum surface_pixel_format surf_pix_format = plane_state->format;
> unsigned int bpp = resource_pixel_format_to_bpp(surf_pix_format);
>
> - enum swizzle_mode_values swizzle = DC_SW_LINEAR;
> -
> + plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_S;
> if (bpp == 64)
> - swizzle = DC_SW_64KB_D;
> - else
> - swizzle = DC_SW_64KB_S;
> + plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_D;
>
> - plane_state->tiling_info.gfx9.swizzle = swizzle;
> return DC_OK;
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> index fbcf0afeae0d..8f9244fe5c86 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
> @@ -1393,15 +1393,13 @@ static uint32_t read_pipe_fuses(struct dc_context
> *ctx)
>
> static enum dc_status dcn21_patch_unknown_plane_state(struct dc_plane_state
> *plane_state)
> {
> - enum dc_status result = DC_OK;
> -
> if (plane_state->ctx->dc->debug.disable_dcc == DCC_ENABLE) {
> plane_state->dcc.enable = 1;
> /* align to our worst case block width */
> plane_state->dcc.meta_pitch = ((plane_state->src_rect.width +
> 1023) / 1024) * 1024;
> }
> - result = dcn20_patch_unknown_plane_state(plane_state);
> - return result;
> +
> + return dcn20_patch_unknown_plane_state(plane_state);
> }
>
> static const struct resource_funcs dcn21_res_pool_funcs = {
[PATCH] drm/amd/display: Trivial swizzle-related code clean-ups
This is a very trivial code clean-up related to commit 5468c36d6285
("drm/amd/display: Filter Invalid 420 Modes for HDMI TMDS"). This commit
added a validation on driver probe to prevent invalid TMDS modes, but one
of the fake properties (swizzle) ended-up causing a warning on driver
probe; was reported here: https://gitlab.freedesktop.org/drm/amd/-/issues/2264.
It was fixed by commit 105a8b8698e2 ("drm/amd/display: patch cases with
unknown plane state to prevent warning"), but the validation code had
a double variable assignment, which we hereby remove. Also, the fix relies
in the dcn2{0,1}patch_unknown_plane_state() callbacks, so while at it we
took the opportunity to perform a small code clean-up in such routines.
Cc: Aurabindo Pillai
Cc: Daniel Wheeler
Cc: Fangzhi Zuo
Cc: Harry Wentland
Cc: Leo Li
Cc: Mark Broadworth
Cc: Melissa Wen
Cc: Rodrigo Siqueira
Cc: Sung Joon Kim
Cc: Swapnil Patel
Signed-off-by: Guilherme G. Piccoli
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 8 ++--
drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 6 ++
3 files changed, 4 insertions(+), 11 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 86a2f7f58550..e71e94663d14 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6336,7 +6336,6 @@ static enum dc_status
dm_validate_stream_and_context(struct dc *dc,
dc_plane_state->plane_size.surface_size.width = stream->src.width;
dc_plane_state->plane_size.chroma_size.height = stream->src.height;
dc_plane_state->plane_size.chroma_size.width = stream->src.width;
- dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
dc_plane_state->format = SURFACE_PIXEL_FORMAT_GRPH_ARGB;
dc_plane_state->tiling_info.gfx9.swizzle = DC_SW_UNKNOWN;
dc_plane_state->rotation = ROTATION_ANGLE_0;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 531f405d2554..3af24ef9cb2d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -2225,14 +2225,10 @@ enum dc_status dcn20_patch_unknown_plane_state(struct
dc_plane_state *plane_stat
enum surface_pixel_format surf_pix_format = plane_state->format;
unsigned int bpp = resource_pixel_format_to_bpp(surf_pix_format);
- enum swizzle_mode_values swizzle = DC_SW_LINEAR;
-
+ plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_S;
if (bpp == 64)
- swizzle = DC_SW_64KB_D;
- else
- swizzle = DC_SW_64KB_S;
+ plane_state->tiling_info.gfx9.swizzle = DC_SW_64KB_D;
- plane_state->tiling_info.gfx9.swizzle = swizzle;
return DC_OK;
}
diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
index fbcf0afeae0d..8f9244fe5c86 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
@@ -1393,15 +1393,13 @@ static uint32_t read_pipe_fuses(struct dc_context *ctx)
static enum dc_status dcn21_patch_unknown_plane_state(struct dc_plane_state
*plane_state)
{
- enum dc_status result = DC_OK;
-
if (plane_state->ctx->dc->debug.disable_dcc == DCC_ENABLE) {
plane_state->dcc.enable = 1;
/* align to our worst case block width */
plane_state->dcc.meta_pitch = ((plane_state->src_rect.width +
1023) / 1024) * 1024;
}
- result = dcn20_patch_unknown_plane_state(plane_state);
- return result;
+
+ return dcn20_patch_unknown_plane_state(plane_state);
}
static const struct resource_funcs dcn21_res_pool_funcs = {
--
2.39.0
