diff options
Diffstat (limited to 'common/recipes-kernel/linux/linux-yocto-4.19.8/0853-drm-amd-display-Use-private-obj-helpers-for-dm_atomi.patch')
-rw-r--r-- | common/recipes-kernel/linux/linux-yocto-4.19.8/0853-drm-amd-display-Use-private-obj-helpers-for-dm_atomi.patch | 612 |
1 files changed, 612 insertions, 0 deletions
diff --git a/common/recipes-kernel/linux/linux-yocto-4.19.8/0853-drm-amd-display-Use-private-obj-helpers-for-dm_atomi.patch b/common/recipes-kernel/linux/linux-yocto-4.19.8/0853-drm-amd-display-Use-private-obj-helpers-for-dm_atomi.patch new file mode 100644 index 00000000..ece63f1c --- /dev/null +++ b/common/recipes-kernel/linux/linux-yocto-4.19.8/0853-drm-amd-display-Use-private-obj-helpers-for-dm_atomi.patch @@ -0,0 +1,612 @@ +From cdd1dedfdd13826715f87a1b45af25acf2e63c3c Mon Sep 17 00:00:00 2001 +From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> +Date: Thu, 22 Nov 2018 12:34:36 -0500 +Subject: [PATCH 0853/2940] drm/amd/display: Use private obj helpers for + dm_atomic_state + +[Why] +Two non-blocking commits in succession can result in a sequence where +the same dc->current_state is queried for both commits. + +1. 1st commit -> check -> commit -> swaps atomic state -> queues work +2. 2nd commit -> check -> commit -> swaps atomic state -> queues work +3. 1st commit work finishes + +The issue with this sequence is that the same dc->current_state is +read in both atomic checks. If the first commit modifies streams or +planes those will be missing from the dc->current_state for the +second atomic check. This result in many stream and plane errors in +atomic commit tail. + +[How] +The driver still needs to track old to new state to determine if the +commit in its current implementation. Updating the dc_state in +atomic tail is wrong since the dc_state swap should be happening as +part of drm_atomic_helper_swap_state *before* the worker queue kicks +its work off. + +The simplest replacement for the subclassing (which doesn't properly +manage the old to new atomic state swap) is to use the drm private +object helpers. While some of the dc_state members could be merged +into dm_crtc_state or dm_plane_state and copied over that way it is +easier for now to just treat the whole dc_state structure as a single +private object. + +This allows amdgpu_dm to drop the dc->current_state copy from within +atomic check. It's replaced by a copy from the current atomic state +which is propagated correctly for the sequence described above. + +Since access to the dm_state private object is now locked this should +also fix issues that could arise if submitting non-blocking commits +from different threads. + +Cc: Harry Wentland <harry.wentland@amd.com> +Cc: Leo Li <sunpeng.li@amd.com> +Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> +Reviewed-by: Leo Li <sunpeng.li@amd.com> +Signed-off-by: Alex Deucher <alexander.deucher@amd.com> +--- + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 290 ++++++++++++++---- + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 13 +- + 2 files changed, 234 insertions(+), 69 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 a6e3e4d7e883..23f19a964dd1 100644 +--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c ++++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +@@ -989,45 +989,6 @@ const struct amdgpu_ip_block_version dm_ip_block = + }; + + +-static struct drm_atomic_state * +-dm_atomic_state_alloc(struct drm_device *dev) +-{ +- struct dm_atomic_state *state = kzalloc(sizeof(*state), GFP_KERNEL); +- +- if (!state) +- return NULL; +- +- if (drm_atomic_state_init(dev, &state->base) < 0) +- goto fail; +- +- return &state->base; +- +-fail: +- kfree(state); +- return NULL; +-} +- +-static void +-dm_atomic_state_clear(struct drm_atomic_state *state) +-{ +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); +- +- if (dm_state->context) { +- dc_release_state(dm_state->context); +- dm_state->context = NULL; +- } +- +- drm_atomic_state_default_clear(state); +-} +- +-static void +-dm_atomic_state_alloc_free(struct drm_atomic_state *state) +-{ +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); +- drm_atomic_state_default_release(state); +- kfree(dm_state); +-} +- + /** + * DOC: atomic + * +@@ -1039,9 +1000,6 @@ static const struct drm_mode_config_funcs amdgpu_dm_mode_funcs = { + .output_poll_changed = drm_fb_helper_output_poll_changed, + .atomic_check = amdgpu_dm_atomic_check, + .atomic_commit = amdgpu_dm_atomic_commit, +- .atomic_state_alloc = dm_atomic_state_alloc, +- .atomic_state_clear = dm_atomic_state_clear, +- .atomic_state_free = dm_atomic_state_alloc_free + }; + + static struct drm_mode_config_helper_funcs amdgpu_dm_mode_config_helperfuncs = { +@@ -1562,8 +1520,117 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) + } + #endif + ++/* ++ * Acquires the lock for the atomic state object and returns ++ * the new atomic state. ++ * ++ * This should only be called during atomic check. ++ */ ++static int dm_atomic_get_state(struct drm_atomic_state *state, ++ struct dm_atomic_state **dm_state) ++{ ++ struct drm_device *dev = state->dev; ++ struct amdgpu_device *adev = dev->dev_private; ++ struct amdgpu_display_manager *dm = &adev->dm; ++ struct drm_private_state *priv_state; ++ int ret; ++ ++ if (*dm_state) ++ return 0; ++ ++ ret = drm_modeset_lock(&dm->atomic_obj_lock, state->acquire_ctx); ++ if (ret) ++ return ret; ++ ++ priv_state = drm_atomic_get_private_obj_state(state, &dm->atomic_obj); ++ if (IS_ERR(priv_state)) ++ return PTR_ERR(priv_state); ++ ++ *dm_state = to_dm_atomic_state(priv_state); ++ ++ return 0; ++} ++ ++struct dm_atomic_state * ++dm_atomic_get_new_state(struct drm_atomic_state *state) ++{ ++ struct drm_device *dev = state->dev; ++ struct amdgpu_device *adev = dev->dev_private; ++ struct amdgpu_display_manager *dm = &adev->dm; ++ struct drm_private_obj *obj; ++ struct drm_private_state *new_obj_state; ++ int i; ++ ++ for_each_new_private_obj_in_state(state, obj, new_obj_state, i) { ++ if (obj->funcs == dm->atomic_obj.funcs) ++ return to_dm_atomic_state(new_obj_state); ++ } ++ ++ return NULL; ++} ++ ++struct dm_atomic_state * ++dm_atomic_get_old_state(struct drm_atomic_state *state) ++{ ++ struct drm_device *dev = state->dev; ++ struct amdgpu_device *adev = dev->dev_private; ++ struct amdgpu_display_manager *dm = &adev->dm; ++ struct drm_private_obj *obj; ++ struct drm_private_state *old_obj_state; ++ int i; ++ ++ for_each_old_private_obj_in_state(state, obj, old_obj_state, i) { ++ if (obj->funcs == dm->atomic_obj.funcs) ++ return to_dm_atomic_state(old_obj_state); ++ } ++ ++ return NULL; ++} ++ ++static struct drm_private_state * ++dm_atomic_duplicate_state(struct drm_private_obj *obj) ++{ ++ struct dm_atomic_state *old_state, *new_state; ++ ++ new_state = kzalloc(sizeof(*new_state), GFP_KERNEL); ++ if (!new_state) ++ return NULL; ++ ++ __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base); ++ ++ new_state->context = dc_create_state(); ++ if (!new_state->context) { ++ kfree(new_state); ++ return NULL; ++ } ++ ++ old_state = to_dm_atomic_state(obj->state); ++ if (old_state && old_state->context) ++ dc_resource_state_copy_construct(old_state->context, ++ new_state->context); ++ ++ return &new_state->base; ++} ++ ++static void dm_atomic_destroy_state(struct drm_private_obj *obj, ++ struct drm_private_state *state) ++{ ++ struct dm_atomic_state *dm_state = to_dm_atomic_state(state); ++ ++ if (dm_state && dm_state->context) ++ dc_release_state(dm_state->context); ++ ++ kfree(dm_state); ++} ++ ++static struct drm_private_state_funcs dm_atomic_state_funcs = { ++ .atomic_duplicate_state = dm_atomic_duplicate_state, ++ .atomic_destroy_state = dm_atomic_destroy_state, ++}; ++ + static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) + { ++ struct dm_atomic_state *state; + int r; + + adev->mode_info.mode_config_initialized = true; +@@ -1581,6 +1648,24 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) + + adev->ddev->mode_config.fb_base = adev->gmc.aper_base; + ++ drm_modeset_lock_init(&adev->dm.atomic_obj_lock); ++ ++ state = kzalloc(sizeof(*state), GFP_KERNEL); ++ if (!state) ++ return -ENOMEM; ++ ++ state->context = dc_create_state(); ++ if (!state->context) { ++ kfree(state); ++ return -ENOMEM; ++ } ++ ++ dc_resource_state_copy_construct_current(adev->dm.dc, state->context); ++ ++ drm_atomic_private_obj_init(&adev->dm.atomic_obj, ++ &state->base, ++ &dm_atomic_state_funcs); ++ + r = amdgpu_display_modeset_create_props(adev); + if (r) + return r; +@@ -1907,6 +1992,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) + static void amdgpu_dm_destroy_drm_device(struct amdgpu_display_manager *dm) + { + drm_mode_config_cleanup(dm->ddev); ++ drm_atomic_private_obj_fini(&dm->atomic_obj); + return; + } + +@@ -4390,6 +4476,20 @@ static void prepare_flip_isr(struct amdgpu_crtc *acrtc) + acrtc->crtc_id); + } + ++struct dc_stream_status *dc_state_get_stream_status( ++ struct dc_state *state, ++ struct dc_stream_state *stream) ++{ ++ uint8_t i; ++ ++ for (i = 0; i < state->stream_count; i++) { ++ if (stream == state->streams[i]) ++ return &state->stream_status[i]; ++ } ++ ++ return NULL; ++} ++ + /* + * Executes flip + * +@@ -4599,6 +4699,7 @@ static bool commit_planes_to_stream( + } + + static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, ++ struct dc_state *dc_state, + struct drm_device *dev, + struct amdgpu_display_manager *dm, + struct drm_crtc *pcrtc, +@@ -4615,7 +4716,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, + struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state); + struct dm_crtc_state *dm_old_crtc_state = + to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); + int planes_count = 0; + unsigned long flags; + +@@ -4676,7 +4776,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, + crtc, + fb, + (uint32_t)drm_crtc_vblank_count(crtc) + *wait_for_vblank, +- dm_state->context); ++ dc_state); + } + + } +@@ -4702,7 +4802,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, + planes_count, + acrtc_state, + dm_old_crtc_state, +- dm_state->context)) ++ dc_state)) + dm_error("%s: Failed to attach plane!\n", __func__); + } else { + /*TODO BUG Here should go disable planes on CRTC. */ +@@ -4768,6 +4868,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + struct amdgpu_device *adev = dev->dev_private; + struct amdgpu_display_manager *dm = &adev->dm; + struct dm_atomic_state *dm_state; ++ struct dc_state *dc_state = NULL, *dc_state_temp = NULL; + uint32_t i, j; + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; +@@ -4780,7 +4881,16 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + + drm_atomic_helper_update_legacy_modeset_state(dev, state); + +- dm_state = to_dm_atomic_state(state); ++ dm_state = dm_atomic_get_new_state(state); ++ if (dm_state && dm_state->context) { ++ dc_state = dm_state->context; ++ } else { ++ /* No state changes, retain current state. */ ++ dc_state_temp = dc_create_state(); ++ ASSERT(dc_state_temp); ++ dc_state = dc_state_temp; ++ dc_resource_state_copy_construct_current(dm->dc, dc_state); ++ } + + /* update changed items */ + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { +@@ -4853,9 +4963,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + } + } /* for_each_crtc_in_state() */ + +- if (dm_state->context) { +- dm_enable_per_frame_crtc_master_sync(dm_state->context); +- WARN_ON(!dc_commit_state(dm->dc, dm_state->context)); ++ if (dc_state) { ++ dm_enable_per_frame_crtc_master_sync(dc_state); ++ WARN_ON(!dc_commit_state(dm->dc, dc_state)); + } + + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { +@@ -4867,6 +4977,10 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + const struct dc_stream_status *status = + dc_stream_get_status(dm_new_crtc_state->stream); + ++ if (!status) ++ status = dc_state_get_stream_status(dc_state, ++ dm_new_crtc_state->stream); ++ + if (!status) + DC_ERR("got no status for stream %p on acrtc%p\n", dm_new_crtc_state->stream, acrtc); + else +@@ -4957,7 +5071,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); + + if (dm_new_crtc_state->stream) +- amdgpu_dm_commit_planes(state, dev, dm, crtc, &wait_for_vblank); ++ amdgpu_dm_commit_planes(state, dc_state, dev, ++ dm, crtc, &wait_for_vblank); + } + + +@@ -4996,6 +5111,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) + for (i = 0; i < crtc_disable_count; i++) + pm_runtime_put_autosuspend(dev->dev); + pm_runtime_mark_last_busy(dev->dev); ++ ++ if (dc_state_temp) ++ dc_release_state(dc_state_temp); + } + + +@@ -5182,11 +5300,11 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm, + bool enable, + bool *lock_and_validation_needed) + { ++ struct dm_atomic_state *dm_state = NULL; + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + int i; + struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state; +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); + struct dc_stream_state *new_stream; + int ret = 0; + +@@ -5285,6 +5403,10 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm, + if (!dm_old_crtc_state->stream) + goto next_crtc; + ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) ++ goto fail; ++ + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n", + crtc->base.id); + +@@ -5319,6 +5441,10 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm, + + WARN_ON(dm_new_crtc_state->stream); + ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) ++ goto fail; ++ + dm_new_crtc_state->stream = new_stream; + + dc_stream_retain(new_stream); +@@ -5393,12 +5519,13 @@ static int dm_update_planes_state(struct dc *dc, + bool enable, + bool *lock_and_validation_needed) + { ++ ++ struct dm_atomic_state *dm_state = NULL; + struct drm_crtc *new_plane_crtc, *old_plane_crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_plane *plane; + struct drm_plane_state *old_plane_state, *new_plane_state; + struct dm_crtc_state *dm_new_crtc_state, *dm_old_crtc_state; +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); + struct dm_plane_state *dm_new_plane_state, *dm_old_plane_state; + int i ; + /* TODO return page_flip_needed() function */ +@@ -5436,6 +5563,10 @@ static int dm_update_planes_state(struct dc *dc, + DRM_DEBUG_ATOMIC("Disabling DRM plane: %d on DRM crtc %d\n", + plane->base.id, old_plane_crtc->base.id); + ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) ++ return ret; ++ + if (!dc_remove_plane_from_context( + dc, + dm_old_crtc_state->stream, +@@ -5490,6 +5621,12 @@ static int dm_update_planes_state(struct dc *dc, + return ret; + } + ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) { ++ dc_plane_state_release(dc_new_plane_state); ++ return ret; ++ } ++ + /* + * Any atomic check errors that occur after this will + * not need a release. The plane state will be attached +@@ -5521,11 +5658,14 @@ static int dm_update_planes_state(struct dc *dc, + + return ret; + } +-enum surface_update_type dm_determine_update_type_for_commit(struct dc *dc, struct drm_atomic_state *state) +-{ +- + +- int i, j, num_plane; ++static int ++dm_determine_update_type_for_commit(struct dc *dc, ++ struct drm_atomic_state *state, ++ enum surface_update_type *out_type) ++{ ++ struct dm_atomic_state *dm_state = NULL, *old_dm_state = NULL; ++ int i, j, num_plane, ret = 0; + struct drm_plane_state *old_plane_state, *new_plane_state; + struct dm_plane_state *new_dm_plane_state, *old_dm_plane_state; + struct drm_crtc *new_plane_crtc, *old_plane_crtc; +@@ -5545,7 +5685,7 @@ enum surface_update_type dm_determine_update_type_for_commit(struct dc *dc, stru + DRM_ERROR("Plane or surface update failed to allocate"); + /* Set type to FULL to avoid crashing in DC*/ + update_type = UPDATE_TYPE_FULL; +- goto ret; ++ goto cleanup; + } + + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { +@@ -5599,27 +5739,40 @@ enum surface_update_type dm_determine_update_type_for_commit(struct dc *dc, stru + } + + if (num_plane > 0) { +- status = dc_stream_get_status(new_dm_crtc_state->stream); ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) ++ goto cleanup; ++ ++ old_dm_state = dm_atomic_get_old_state(state); ++ if (!old_dm_state) { ++ ret = -EINVAL; ++ goto cleanup; ++ } ++ ++ status = dc_state_get_stream_status(old_dm_state->context, ++ new_dm_crtc_state->stream); ++ + update_type = dc_check_update_surfaces_for_stream(dc, updates, num_plane, + &stream_update, status); + + if (update_type > UPDATE_TYPE_MED) { + update_type = UPDATE_TYPE_FULL; +- goto ret; ++ goto cleanup; + } + } + + } else if (!new_dm_crtc_state->stream && old_dm_crtc_state->stream) { + update_type = UPDATE_TYPE_FULL; +- goto ret; ++ goto cleanup; + } + } + +-ret: ++cleanup: + kfree(updates); + kfree(surface); + +- return update_type; ++ *out_type = update_type; ++ return ret; + } + + /** +@@ -5651,8 +5804,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) + { + struct amdgpu_device *adev = dev->dev_private; ++ struct dm_atomic_state *dm_state = NULL; + struct dc *dc = adev->dm.dc; +- struct dm_atomic_state *dm_state = to_dm_atomic_state(state); + struct drm_connector *connector; + struct drm_connector_state *old_con_state, *new_con_state; + struct drm_crtc *crtc; +@@ -5693,10 +5846,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, + goto fail; + } + +- dm_state->context = dc_create_state(); +- ASSERT(dm_state->context); +- dc_resource_state_copy_construct_current(dc, dm_state->context); +- + /* Remove exiting planes if they are modified */ + ret = dm_update_planes_state(dc, state, false, &lock_and_validation_needed); + if (ret) { +@@ -5749,7 +5898,9 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, + lock_and_validation_needed = true; + } + +- update_type = dm_determine_update_type_for_commit(dc, state); ++ ret = dm_determine_update_type_for_commit(dc, state, &update_type); ++ if (ret) ++ goto fail; + + if (overall_update_type < update_type) + overall_update_type = update_type; +@@ -5767,6 +5918,9 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, + + + if (overall_update_type > UPDATE_TYPE_FAST) { ++ ret = dm_atomic_get_state(state, &dm_state); ++ if (ret) ++ goto fail; + + ret = do_aquire_global_lock(dev, state); + if (ret) +diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +index a84d72120275..f55cdc94902f 100644 +--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h ++++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +@@ -121,6 +121,17 @@ struct amdgpu_display_manager { + struct drm_device *ddev; + u16 display_indexes_num; + ++ /** ++ * @atomic_obj ++ * ++ * In combination with &dm_atomic_state it helps manage ++ * global atomic state that doesn't map cleanly into existing ++ * drm resources, like &dc_context. ++ */ ++ struct drm_private_obj atomic_obj; ++ ++ struct drm_modeset_lock atomic_obj_lock; ++ + /** + *@irq_handler_list_low_tab: + * +@@ -251,7 +262,7 @@ struct dm_crtc_state { + #define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base) + + struct dm_atomic_state { +- struct drm_atomic_state base; ++ struct drm_private_state base; + + struct dc_state *context; + }; +-- +2.17.1 + |