[PATCH] drm/amd/display: log amdgpu_dm_atomic_check() failure cause
S, Shirish
sshankar at amd.com
Mon Nov 8 11:13:53 UTC 2021
Hi Paul,
On 11/8/2021 2:27 PM, Paul Menzel wrote:
> Dear Shrish,
>
>
> Am 08.11.21 um 09:40 schrieb Shirish S:
>> update user with next level of info about which condition led to
>> atomic check failure.
>>
>> Signed-off-by: Shirish S <shirish.s at amd.com>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 70 ++++++++++++++-----
>> 1 file changed, 52 insertions(+), 18 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 1e26d9be8993..37ea8a76fa09 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -10746,8 +10746,10 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> trace_amdgpu_dm_atomic_check_begin(state);
>> ret = drm_atomic_helper_check_modeset(dev, state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "drm_atomic_helper_check_modeset()
>> failed\n");
>
> Does the Linux kernel provide means (for example ftrace) to trace such
> things, so the (generic) debug lines don’t have to be added? Or is it
> to better debug user bug reports?
>
ftrace requires additional tooling, am trying to avoid it and make the
error reporting more trivial to the developers, in case there is a
failure in atomic_check.
Regards,
Shirish S
>
> Kind regards,
>
> Paul
>
>
>> goto fail;
>> + }
>> /* Check connector changes */
>> for_each_oldnew_connector_in_state(state, connector,
>> old_con_state, new_con_state, i) {
>> @@ -10763,6 +10765,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_crtc_state = drm_atomic_get_crtc_state(state,
>> new_con_state->crtc);
>> if (IS_ERR(new_crtc_state)) {
>> + DRM_DEV_ERROR(adev->dev, "drm_atomic_get_crtc_state()
>> failed\n");
>> ret = PTR_ERR(new_crtc_state);
>> goto fail;
>> }
>> @@ -10777,8 +10780,10 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
>> new_crtc_state, i) {
>> if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
>> ret = add_affected_mst_dsc_crtcs(state, crtc);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev,
>> "add_affected_mst_dsc_crtcs() failed\n");
>> goto fail;
>> + }
>> }
>> }
>> }
>> @@ -10793,19 +10798,25 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> continue;
>> ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "amdgpu_dm_verify_lut_sizes()
>> failed\n");
>> goto fail;
>> + }
>> if (!new_crtc_state->enable)
>> continue;
>> ret = drm_atomic_add_affected_connectors(state, crtc);
>> - if (ret)
>> - return ret;
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev,
>> "drm_atomic_add_affected_connectors() failed\n");
>> + goto fail;
>> + }
>> ret = drm_atomic_add_affected_planes(state, crtc);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev,
>> "drm_atomic_add_affected_planes() failed\n");
>> goto fail;
>> + }
>> if (dm_old_crtc_state->dsc_force_changed)
>> new_crtc_state->mode_changed = true;
>> @@ -10842,6 +10853,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> if (IS_ERR(new_plane_state)) {
>> ret = PTR_ERR(new_plane_state);
>> + DRM_DEV_ERROR(adev->dev, "new_plane_state is BAD\n");
>> goto fail;
>> }
>> }
>> @@ -10854,8 +10866,10 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_plane_state,
>> false,
>> &lock_and_validation_needed);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "dm_update_plane_state()
>> failed\n");
>> goto fail;
>> + }
>> }
>> /* Disable all crtcs which require disable */
>> @@ -10865,8 +10879,10 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_crtc_state,
>> false,
>> &lock_and_validation_needed);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "DISABLE:
>> dm_update_crtc_state() failed\n");
>> goto fail;
>> + }
>> }
>> /* Enable all crtcs which require enable */
>> @@ -10876,8 +10892,10 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_crtc_state,
>> true,
>> &lock_and_validation_needed);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "ENABLE: dm_update_crtc_state()
>> failed\n");
>> goto fail;
>> + }
>> }
>> /* Add new/modified planes */
>> @@ -10887,20 +10905,26 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_plane_state,
>> true,
>> &lock_and_validation_needed);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "dm_update_plane_state()
>> failed\n");
>> goto fail;
>> + }
>> }
>> /* Run this here since we want to validate the streams we
>> created */
>> ret = drm_atomic_helper_check_planes(dev, state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "drm_atomic_helper_check_planes()
>> failed\n");
>> goto fail;
>> + }
>> /* Check cursor planes scaling */
>> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> ret = dm_check_crtc_cursor(state, crtc, new_crtc_state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "dm_check_crtc_cursor()
>> failed\n");
>> goto fail;
>> + }
>> }
>> if (state->legacy_cursor_update) {
>> @@ -10987,20 +11011,28 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> */
>> if (lock_and_validation_needed) {
>> ret = dm_atomic_get_state(state, &dm_state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "dm_atomic_get_state() failed\n");
>> goto fail;
>> + }
>> ret = do_aquire_global_lock(dev, state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "do_aquire_global_lock()
>> failed\n");
>> goto fail;
>> + }
>> #if defined(CONFIG_DRM_AMD_DC_DCN)
>> - if (!compute_mst_dsc_configs_for_state(state,
>> dm_state->context, vars))
>> + if (!compute_mst_dsc_configs_for_state(state,
>> dm_state->context, vars)) {
>> + DRM_DEV_ERROR(adev->dev,
>> "compute_mst_dsc_configs_for_state() failed\n");
>> goto fail;
>> + }
>> ret = dm_update_mst_vcpi_slots_for_dsc(state,
>> dm_state->context, vars);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev,
>> "dm_update_mst_vcpi_slots_for_dsc() failed\n");
>> goto fail;
>> + }
>> #endif
>> /*
>> @@ -11010,11 +11042,13 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> * to get stuck in an infinite loop and hang eventually.
>> */
>> ret = drm_dp_mst_atomic_check(state);
>> - if (ret)
>> + if (ret) {
>> + DRM_DEV_ERROR(adev->dev, "drm_dp_mst_atomic_check()
>> failed\n");
>> goto fail;
>> + }
>> status = dc_validate_global_state(dc, dm_state->context,
>> false);
>> if (status != DC_OK) {
>> - drm_dbg_atomic(dev,
>> + DRM_DEV_ERROR(adev->dev,
>> "DC global validation failure: %s (%d)",
>> dc_status_to_str(status), status);
>> ret = -EINVAL;
>>
More information about the amd-gfx
mailing list