On 10/5/2023 09:27, Alex Deucher wrote:
On Wed, Oct 4, 2023 at 1:37 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
If there is memory pressure at suspend time then dynamically
allocating a large structure as part of DC suspend code will
fail.
Instead re-use the same structure and clear all members except
those that should be maintained.
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 25 -------------------
.../gpu/drm/amd/display/dc/core/dc_resource.c | 12 +++++++++
2 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 39e291a467e2..cb8c7c5a8807 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -4728,9 +4728,6 @@ bool dc_set_power_state(
struct dc *dc,
enum dc_acpi_cm_power_state power_state)
{
- struct kref refcount;
- struct display_mode_lib *dml;
-
if (!dc->current_state)
return true;
@@ -4750,30 +4747,8 @@ bool dc_set_power_state(
break;
default:
ASSERT(dc->current_state->stream_count == 0);
- /* Zero out the current context so that on resume we start with
- * clean state, and dc hw programming optimizations will not
- * cause any trouble.
- */
- dml = kzalloc(sizeof(struct display_mode_lib),
- GFP_KERNEL);
-
- ASSERT(dml);
- if (!dml)
- return false;
-
- /* Preserve refcount */
- refcount = dc->current_state->refcount;
- /* Preserve display mode lib */
- memcpy(dml, &dc->current_state->bw_ctx.dml, sizeof(struct display_mode_lib));
dc_resource_state_destruct(dc->current_state);
- memset(dc->current_state, 0,
- sizeof(*dc->current_state));
-
- dc->current_state->refcount = refcount;
- dc->current_state->bw_ctx.dml = *dml;
The dml dance seems a bit weird. I guess it's here because
dc_resource_state_destruct() might change it? Can we safely drop
this? If we do need it, we could pre-allocate a dml structure and use
that.
The dml structure is huge, so I think it's sub-optimal to have two
copies of it. That's why I aimed to just destroy everything else except
it instead.
The only reason it's "safe" to drop the whole above stuff is because of
"threading the needle" of what dc_resource_state_destruct() does.
In the earlier version I had a mistake to miss clearing the scratch
variable and it caused some IGT faliures.
This probably needs to be double checked with the DML2 series landing as
well to make sure it didn't get caught in the middle.
Alex
-
- kfree(dml);
break;
}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index aa7b5db83644..e487c966c118 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -4350,6 +4350,18 @@ void dc_resource_state_destruct(struct dc_state *context)
context->streams[i] = NULL;
}
context->stream_count = 0;
+ context->stream_mask = 0;
+ memset(&context->res_ctx, 0, sizeof(context->res_ctx));
+ memset(&context->pp_display_cfg, 0, sizeof(context->pp_display_cfg));
+ memset(&context->dcn_bw_vars, 0, sizeof(context->dcn_bw_vars));
+ context->clk_mgr = NULL;
+ memset(&context->bw_ctx.bw, 0, sizeof(context->bw_ctx.bw));
+ memset(context->block_sequence, 0, sizeof(context->block_sequence));
+ context->block_sequence_steps = 0;
+ memset(context->dc_dmub_cmd, 0, sizeof(context->dc_dmub_cmd));
+ context->dmub_cmd_count = 0;
+ memset(&context->perf_params, 0, sizeof(context->perf_params));
+ memset(&context->scratch, 0, sizeof(context->scratch));
}
void dc_resource_state_copy_construct(
--
2.34.1
