From 28a91920b2f50c7858c12601e3aff36886b34415 Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Tue, 23 Feb 2016 14:27:59 -0500 Subject: [PATCH 0840/1110] drm/amd/dal: fix ref count issue on validate failure When copying context, retain surfaces and targets and release when context is freed. This fixes crash on underscan test on CZ, which was caused by dangling pointer in the bandwidth validation failure code path. Signed-off-by: Eric Yang Acked-by: Harry Wentland --- drivers/gpu/drm/amd/dal/dc/core/dc.c | 24 +++++---------- drivers/gpu/drm/amd/dal/dc/core/dc_resource.c | 34 ++++++++++++++++++++-- drivers/gpu/drm/amd/dal/dc/core/dc_target.c | 9 ++++-- .../gpu/drm/amd/dal/dc/dce100/dce100_resource.c | 1 + .../gpu/drm/amd/dal/dc/dce110/dce110_resource.c | 1 + drivers/gpu/drm/amd/dal/dc/dce80/dce80_resource.c | 1 + drivers/gpu/drm/amd/dal/dc/inc/resource.h | 6 +++- 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/amd/dal/dc/core/dc.c b/drivers/gpu/drm/amd/dal/dc/core/dc.c index 9ecd60c..7eaf7ef 100644 --- a/drivers/gpu/drm/amd/dal/dc/core/dc.c +++ b/drivers/gpu/drm/amd/dal/dc/core/dc.c @@ -391,7 +391,7 @@ ctx_fail: static void destruct(struct dc *dc) { - destruct_val_ctx(&dc->current_context); + val_ctx_destruct(&dc->current_context); destroy_links(dc); dc->res_pool.funcs->destruct(&dc->res_pool); dal_logger_destroy(&dc->ctx->logger); @@ -452,7 +452,7 @@ bool dc_validate_resources( result = dc->res_pool.funcs->validate_with_context( dc, set, set_count, context); - destruct_val_ctx(context); + val_ctx_destruct(context); dm_free(dc->ctx, context); context_alloc_fail: @@ -563,6 +563,7 @@ bool dc_commit_targets( result = dc->res_pool.funcs->validate_with_context(dc, set, target_count, context); if (result != DC_OK){ BREAK_TO_DEBUGGER(); + val_ctx_destruct(context); goto fail; } @@ -592,25 +593,14 @@ bool dc_commit_targets( dc_target_enable_memory_requests(dc_target); } - /* Release old targets */ - for (i = 0; i < dc->current_context.target_count; i++) { - dc_target_release( - &dc->current_context.targets[i]->public); - dc->current_context.targets[i] = NULL; - } - /* Retain new targets*/ - for (i = 0; i < context->target_count; i++) { - dc_target_retain(&context->targets[i]->public); - } - - destruct_val_ctx(&dc->current_context); - - dc->current_context = *context; - program_timing_sync(dc->ctx, context); pplib_apply_display_requirements(dc, context, &context->pp_display_cfg); + val_ctx_destruct(&dc->current_context); + + dc->current_context = *context; + fail: dm_free(dc->ctx, context); diff --git a/drivers/gpu/drm/amd/dal/dc/core/dc_resource.c b/drivers/gpu/drm/amd/dal/dc/core/dc_resource.c index 42c456a..1bb4adb 100644 --- a/drivers/gpu/drm/amd/dal/dc/core/dc_resource.c +++ b/drivers/gpu/drm/amd/dal/dc/core/dc_resource.c @@ -407,11 +407,17 @@ bool attach_surfaces_to_context( } + /* retain new surfaces */ for (i = 0; i < surface_count; i++) dc_surface_retain(surfaces[i]); - /* Release after retain to account for surfaces remaining the same */ - for (i = 0; i < target_status->surface_count; i++) + + /* release existing surfaces*/ + for (i = 0; i < target_status->surface_count; i++) { dc_surface_release(target_status->surfaces[i]); + target_status->surfaces[i] = NULL; + } + + /* assign new surfaces*/ for (i = 0; i < surface_count; i++) target_status->surfaces[i] = surfaces[i]; @@ -1241,7 +1247,7 @@ static void set_vendor_info_packet(struct core_stream *stream, info_packet->valid = true; } -void destruct_val_ctx(struct validate_context *context) +void val_ctx_destruct(struct validate_context *context) { int i, j; @@ -1249,7 +1255,29 @@ void destruct_val_ctx(struct validate_context *context) for (j = 0; j < context->target_status[i].surface_count; j++) dc_surface_release( context->target_status[i].surfaces[j]); + context->target_status[i].surface_count = 0; + dc_target_release(&context->targets[i]->public); + } +} + +/* + * Copy src_ctx into dst_ctx and retain all surfaces and targets referenced + * by the src_ctx + */ +void val_ctx_copy_construct( + const struct validate_context *src_ctx, + struct validate_context *dst_ctx) +{ + int i, j; + + *dst_ctx = *src_ctx; + + for (i = 0; i < dst_ctx->target_count; i++) { + dc_target_retain(&dst_ctx->targets[i]->public); + for (j = 0; j < dst_ctx->target_status[i].surface_count; j++) + dc_surface_retain( + dst_ctx->target_status[i].surfaces[j]); } } diff --git a/drivers/gpu/drm/amd/dal/dc/core/dc_target.c b/drivers/gpu/drm/amd/dal/dc/core/dc_target.c index 87275b8..6a66ae9 100644 --- a/drivers/gpu/drm/amd/dal/dc/core/dc_target.c +++ b/drivers/gpu/drm/amd/dal/dc/core/dc_target.c @@ -219,13 +219,16 @@ bool dc_commit_surfaces_to_target( bool is_mpo_turning_on = false; context = dm_alloc(dc->ctx, sizeof(struct validate_context)); - *context = dc->current_context; + + val_ctx_copy_construct(&dc->current_context, context); /* Cannot commit surface to a target that is not commited */ for (i = 0; i < context->target_count; i++) if (target == context->targets[i]) break; + target_status = &context->target_status[i]; + if (!dal_adapter_service_is_in_accelerated_mode( dc->res_pool.adapter_srv) || i == context->target_count) { @@ -343,13 +346,15 @@ bool dc_commit_surfaces_to_target( &context->pp_display_cfg); } + val_ctx_destruct(&dc->current_context); dc->current_context = *context; dm_free(dc->ctx, context); return true; unexpected_fail: - destruct_val_ctx(context); + val_ctx_destruct(context); + dm_free(dc->ctx, context); return false; } diff --git a/drivers/gpu/drm/amd/dal/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/dal/dc/dce100/dce100_resource.c index c7145c5..a8c8f99 100644 --- a/drivers/gpu/drm/amd/dal/dc/dce100/dce100_resource.c +++ b/drivers/gpu/drm/amd/dal/dc/dce100/dce100_resource.c @@ -890,6 +890,7 @@ enum dc_status dce100_validate_with_context( bool unchanged = false; context->targets[i] = DC_TARGET_TO_CORE(set[i].target); + dc_target_retain(&context->targets[i]->public); context->target_count++; for (j = 0; j < dc->current_context.target_count; j++) diff --git a/drivers/gpu/drm/amd/dal/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/dal/dc/dce110/dce110_resource.c index 26e9df5..c079bb7 100644 --- a/drivers/gpu/drm/amd/dal/dc/dce110/dce110_resource.c +++ b/drivers/gpu/drm/amd/dal/dc/dce110/dce110_resource.c @@ -1002,6 +1002,7 @@ enum dc_status dce110_validate_with_context( bool unchanged = false; context->targets[i] = DC_TARGET_TO_CORE(set[i].target); + dc_target_retain(&context->targets[i]->public); context->target_count++; for (j = 0; j < dc->current_context.target_count; j++) diff --git a/drivers/gpu/drm/amd/dal/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/dal/dc/dce80/dce80_resource.c index c645d25..cf51a44 100644 --- a/drivers/gpu/drm/amd/dal/dc/dce80/dce80_resource.c +++ b/drivers/gpu/drm/amd/dal/dc/dce80/dce80_resource.c @@ -999,6 +999,7 @@ enum dc_status dce80_validate_with_context( bool unchanged = false; context->targets[i] = DC_TARGET_TO_CORE(set[i].target); + dc_target_retain(&context->targets[i]->public); context->target_count++; for (j = 0; j < dc->current_context.target_count; j++) diff --git a/drivers/gpu/drm/amd/dal/dc/inc/resource.h b/drivers/gpu/drm/amd/dal/dc/inc/resource.h index c2d6011..983d484 100644 --- a/drivers/gpu/drm/amd/dal/dc/inc/resource.h +++ b/drivers/gpu/drm/amd/dal/dc/inc/resource.h @@ -80,6 +80,10 @@ enum dc_status map_resources( const struct dc *dc, struct validate_context *context); -void destruct_val_ctx(struct validate_context *context); +void val_ctx_destruct(struct validate_context *context); + +void val_ctx_copy_construct( + const struct validate_context *src_ctx, + struct validate_context *dst_ctx); #endif /* DRIVERS_GPU_DRM_AMD_DAL_DEV_DC_INC_RESOURCE_H_ */ -- 2.7.4