* [PATCH v3 0/3] Add devcoredump support for DPU @ 2021-04-09 2:28 ` Abhinav Kumar 0 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: Abhinav Kumar, linux-arm-msm, freedreno, robdclark, seanpaul, swboyd, nganji, aravindh, khsieh, daniel This series adds support to use devcoredump for DPU driver. It introduces the dpu_dbg module which assists in the capturing of register dumps during error scenarios. When a display related error happens, the dpu_dbg module captures all the relevant register dumps along with the snapshot of the drm atomic state and triggers a devcoredump. changes in v3: - Get rid of registration mechanism for sub-modules and instead get this information from the dpu catalog itself - Get rid of global dpu_dbg struct and instead store it in dpu_kms - delegate the power management of the sub-modules to the resp drivers - refactor and remove the linked list logic and simplify it to have just an array Abhinav Kumar (3): drm: allow drm_atomic_print_state() to accept any drm_printer drm/msm/dpu: add support to dump dpu registers drm/msm/dpu: add dpu_dbg points across dpu driver drivers/gpu/drm/drm_atomic.c | 28 ++- drivers/gpu/drm/drm_atomic_uapi.c | 4 +- drivers/gpu/drm/drm_crtc_internal.h | 4 +- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 ++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 ++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 14 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + drivers/gpu/drm/msm/dp/dp_display.c | 37 +++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 4 + drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ++ drivers/gpu/drm/msm/msm_drv.c | 29 ++- drivers/gpu/drm/msm/msm_drv.h | 2 + drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 + 23 files changed, 950 insertions(+), 18 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 0/3] Add devcoredump support for DPU @ 2021-04-09 2:28 ` Abhinav Kumar 0 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno This series adds support to use devcoredump for DPU driver. It introduces the dpu_dbg module which assists in the capturing of register dumps during error scenarios. When a display related error happens, the dpu_dbg module captures all the relevant register dumps along with the snapshot of the drm atomic state and triggers a devcoredump. changes in v3: - Get rid of registration mechanism for sub-modules and instead get this information from the dpu catalog itself - Get rid of global dpu_dbg struct and instead store it in dpu_kms - delegate the power management of the sub-modules to the resp drivers - refactor and remove the linked list logic and simplify it to have just an array Abhinav Kumar (3): drm: allow drm_atomic_print_state() to accept any drm_printer drm/msm/dpu: add support to dump dpu registers drm/msm/dpu: add dpu_dbg points across dpu driver drivers/gpu/drm/drm_atomic.c | 28 ++- drivers/gpu/drm/drm_atomic_uapi.c | 4 +- drivers/gpu/drm/drm_crtc_internal.h | 4 +- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 ++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 ++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 14 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + drivers/gpu/drm/msm/dp/dp_display.c | 37 +++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 4 + drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ++ drivers/gpu/drm/msm/msm_drv.c | 29 ++- drivers/gpu/drm/msm/msm_drv.h | 2 + drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 + 23 files changed, 950 insertions(+), 18 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] drm: allow drm_atomic_print_state() to accept any drm_printer 2021-04-09 2:28 ` Abhinav Kumar @ 2021-04-09 2:28 ` Abhinav Kumar -1 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: Abhinav Kumar, linux-arm-msm, freedreno, robdclark, seanpaul, swboyd, nganji, aravindh, khsieh, daniel Currently drm_atomic_print_state() internally allocates and uses a drm_info printer. Allow it to accept any drm_printer type so that the API can be leveraged even for taking drm snapshot. Rename the drm_atomic_print_state() to drm_atomic_print_new_state() so that it reflects its functionality better. changes in v3: - Remove empty line in the kernel doc Change-Id: Ie425b15b9d5e84f7bad2514f9990181d05019cbf Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_atomic.c | 28 +++++++++++++++++++----- drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- drivers/gpu/drm/drm_crtc_internal.h | 4 +++- drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 + 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dda6005..7041a26 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1573,9 +1574,20 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config); -void drm_atomic_print_state(const struct drm_atomic_state *state) +/** + * drm_atomic_print_new_state - prints drm atomic state + * @state: atomic configuration to check + * @p: drm printer + * + * This functions prints the drm atomic state snapshot using the drm printer + * which is passed to it. This snapshot can be used for debugging purposes. + * + * Note that this function looks into the new state objects and hence its not + * safe to be used after the call to drm_atomic_helper_commit_hw_done(). + */ +void drm_atomic_print_new_state(const struct drm_atomic_state *state, + struct drm_printer *p) { - struct drm_printer p = drm_info_printer(state->dev->dev); struct drm_plane *plane; struct drm_plane_state *plane_state; struct drm_crtc *crtc; @@ -1584,17 +1596,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) struct drm_connector_state *connector_state; int i; + if (!p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + DRM_DEBUG_ATOMIC("checking %p\n", state); for_each_new_plane_in_state(state, plane, plane_state, i) - drm_atomic_plane_print_state(&p, plane_state); + drm_atomic_plane_print_state(p, plane_state); for_each_new_crtc_in_state(state, crtc, crtc_state, i) - drm_atomic_crtc_print_state(&p, crtc_state); + drm_atomic_crtc_print_state(p, crtc_state); for_each_new_connector_in_state(state, connector, connector_state, i) - drm_atomic_connector_print_state(&p, connector_state); + drm_atomic_connector_print_state(p, connector_state); } +EXPORT_SYMBOL(drm_atomic_print_new_state); static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, bool take_locks) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 268bb69..c340a67 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -2,6 +2,7 @@ * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. * Copyright (C) 2018 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1321,6 +1322,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; + struct drm_printer p = drm_info_printer(dev->dev); /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1453,7 +1455,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ret = drm_atomic_nonblocking_commit(state); } else { if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_state(state); + drm_atomic_print_new_state(state, &p); ret = drm_atomic_commit(state); } diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 54d4cf1..1ca51ad 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -5,6 +5,7 @@ * Jesse Barnes <jesse.barnes@intel.com> * Copyright © 2014 Intel Corporation * Daniel Vetter <daniel.vetter@ffwll.ch> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -236,7 +237,8 @@ int __drm_atomic_helper_disable_plane(struct drm_plane *plane, int __drm_atomic_helper_set_config(struct drm_mode_set *set, struct drm_atomic_state *state); -void drm_atomic_print_state(const struct drm_atomic_state *state); +void drm_atomic_print_new_state(const struct drm_atomic_state *state, + struct drm_printer *p); /* drm_atomic_uapi.c */ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c b/drivers/gpu/drm/selftests/test-drm_framebuffer.c index 789f227..61b44d3 100644 --- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c +++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c @@ -8,6 +8,7 @@ #include <drm/drm_device.h> #include <drm/drm_mode.h> #include <drm/drm_fourcc.h> +#include <drm/drm_print.h> #include "../drm_crtc_internal.h" -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] drm: allow drm_atomic_print_state() to accept any drm_printer @ 2021-04-09 2:28 ` Abhinav Kumar 0 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno Currently drm_atomic_print_state() internally allocates and uses a drm_info printer. Allow it to accept any drm_printer type so that the API can be leveraged even for taking drm snapshot. Rename the drm_atomic_print_state() to drm_atomic_print_new_state() so that it reflects its functionality better. changes in v3: - Remove empty line in the kernel doc Change-Id: Ie425b15b9d5e84f7bad2514f9990181d05019cbf Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_atomic.c | 28 +++++++++++++++++++----- drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- drivers/gpu/drm/drm_crtc_internal.h | 4 +++- drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 + 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dda6005..7041a26 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1573,9 +1574,20 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config); -void drm_atomic_print_state(const struct drm_atomic_state *state) +/** + * drm_atomic_print_new_state - prints drm atomic state + * @state: atomic configuration to check + * @p: drm printer + * + * This functions prints the drm atomic state snapshot using the drm printer + * which is passed to it. This snapshot can be used for debugging purposes. + * + * Note that this function looks into the new state objects and hence its not + * safe to be used after the call to drm_atomic_helper_commit_hw_done(). + */ +void drm_atomic_print_new_state(const struct drm_atomic_state *state, + struct drm_printer *p) { - struct drm_printer p = drm_info_printer(state->dev->dev); struct drm_plane *plane; struct drm_plane_state *plane_state; struct drm_crtc *crtc; @@ -1584,17 +1596,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) struct drm_connector_state *connector_state; int i; + if (!p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + DRM_DEBUG_ATOMIC("checking %p\n", state); for_each_new_plane_in_state(state, plane, plane_state, i) - drm_atomic_plane_print_state(&p, plane_state); + drm_atomic_plane_print_state(p, plane_state); for_each_new_crtc_in_state(state, crtc, crtc_state, i) - drm_atomic_crtc_print_state(&p, crtc_state); + drm_atomic_crtc_print_state(p, crtc_state); for_each_new_connector_in_state(state, connector, connector_state, i) - drm_atomic_connector_print_state(&p, connector_state); + drm_atomic_connector_print_state(p, connector_state); } +EXPORT_SYMBOL(drm_atomic_print_new_state); static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, bool take_locks) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 268bb69..c340a67 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -2,6 +2,7 @@ * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. * Copyright (C) 2018 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1321,6 +1322,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; + struct drm_printer p = drm_info_printer(dev->dev); /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1453,7 +1455,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ret = drm_atomic_nonblocking_commit(state); } else { if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_state(state); + drm_atomic_print_new_state(state, &p); ret = drm_atomic_commit(state); } diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 54d4cf1..1ca51ad 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -5,6 +5,7 @@ * Jesse Barnes <jesse.barnes@intel.com> * Copyright © 2014 Intel Corporation * Daniel Vetter <daniel.vetter@ffwll.ch> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -236,7 +237,8 @@ int __drm_atomic_helper_disable_plane(struct drm_plane *plane, int __drm_atomic_helper_set_config(struct drm_mode_set *set, struct drm_atomic_state *state); -void drm_atomic_print_state(const struct drm_atomic_state *state); +void drm_atomic_print_new_state(const struct drm_atomic_state *state, + struct drm_printer *p); /* drm_atomic_uapi.c */ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c b/drivers/gpu/drm/selftests/test-drm_framebuffer.c index 789f227..61b44d3 100644 --- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c +++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c @@ -8,6 +8,7 @@ #include <drm/drm_device.h> #include <drm/drm_mode.h> #include <drm/drm_fourcc.h> +#include <drm/drm_print.h> #include "../drm_crtc_internal.h" -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers 2021-04-09 2:28 ` Abhinav Kumar @ 2021-04-09 2:28 ` Abhinav Kumar -1 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: Abhinav Kumar, linux-arm-msm, freedreno, robdclark, seanpaul, swboyd, nganji, aravindh, khsieh, daniel Add the dpu_dbg module which adds supports to dump dpu registers which can be used in case of error conditions. changes in v3: - Get rid of registration mechanism for sub-modules and instead get this information from the dpu catalog itself - Get rid of global dpu_dbg struct and instead store it in dpu_kms - delegate the power management of the sub-modules to the resp drivers - refactor and remove the linked list logic and simplify it to have just an array Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 +++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 4 + drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ drivers/gpu/drm/msm/msm_drv.c | 29 ++- drivers/gpu/drm/msm/msm_drv.h | 2 + 16 files changed, 889 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 3cc9061..9166417 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -54,6 +54,8 @@ msm-y := \ disp/dpu1/dpu_core_irq.o \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ + disp/dpu1/dpu_dbg.o \ + disp/dpu1/dpu_dbg_util.o \ disp/dpu1/dpu_encoder.o \ disp/dpu1/dpu_encoder_phys_cmd.o \ disp/dpu1/dpu_encoder_phys_vid.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c new file mode 100644 index 0000000..9ec642e --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ + +#include "dpu_dbg.h" +#include "dpu_kms.h" +#include "dpu_hw_catalog.h" + +#ifdef CONFIG_DEV_COREDUMP +static ssize_t dpu_devcoredump_read(char *buffer, loff_t offset, + size_t count, void *data, size_t datalen) +{ + struct drm_print_iterator iter; + struct drm_printer p; + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = data; + + iter.data = buffer; + iter.offset = 0; + iter.start = offset; + iter.remain = count; + + p = drm_coredump_printer(&iter); + + drm_printf(&p, "---\n"); + + drm_printf(&p, "module: " KBUILD_MODNAME "\n"); + drm_printf(&p, "dpu devcoredump\n"); + drm_printf(&p, "timestamp %lld\n", ktime_to_ns(dpu_dbg->timestamp)); + + dpu_dbg->dpu_dbg_printer = &p; + + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_COREDUMP); + + drm_printf(&p, "===================dpu drm state================\n"); + + if (dpu_dbg->atomic_state) + drm_atomic_print_new_state(dpu_dbg->atomic_state, + &p); + + return count - iter.remain; +} + +static void dpu_devcoredump_free(void *data) +{ + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = data; + + if (dpu_dbg->atomic_state) { + drm_atomic_state_put(dpu_dbg->atomic_state); + dpu_dbg->atomic_state = NULL; + } + + dpu_dbg_free_blk_mem(dpu_dbg->drm_dev); + + dpu_dbg->coredump_pending = false; +} + +static void dpu_devcoredump_capture_state(struct dpu_dbg_base *dpu_dbg) +{ + struct drm_device *ddev; + struct drm_modeset_acquire_ctx ctx; + + dpu_dbg->timestamp = ktime_get(); + + ddev = dpu_dbg->drm_dev; + + drm_modeset_acquire_init(&ctx, 0); + + while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0) + drm_modeset_backoff(&ctx); + + dpu_dbg->atomic_state = drm_atomic_helper_duplicate_state(ddev, + &ctx); + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); +} +#else +static void dpu_devcoredump_capture_state(struct dpu_dbg_base *dpu_dbg) +{ +} +#endif /* CONFIG_DEV_COREDUMP */ + +static void _dpu_dump_work(struct kthread_work *work) +{ + struct dpu_dbg_base *dpu_dbg = container_of(work, struct dpu_dbg_base, dump_work); + struct drm_printer p; + + /* reset the reg_dump_method to IN_MEM before every dump */ + dpu_dbg->reg_dump_method = DPU_DBG_DUMP_IN_MEM; + + dpu_dbg_dump_blks(dpu_dbg); + + dpu_devcoredump_capture_state(dpu_dbg); + + if (DPU_DBG_DUMP_IN_CONSOLE) { + p = drm_info_printer(dpu_dbg->drm_dev->dev); + dpu_dbg->dpu_dbg_printer = &p; + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_LOG); + } + +#ifdef CONFIG_DEV_COREDUMP + dev_coredumpm(dpu_dbg->dev, THIS_MODULE, dpu_dbg, 0, GFP_KERNEL, + dpu_devcoredump_read, dpu_devcoredump_free); + dpu_dbg->coredump_pending = true; +#endif +} + +void dpu_dbg_dump(struct drm_device *drm_dev, const char *name, ...) +{ + va_list args; + char *blk_name = NULL; + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_dbg_base *dpu_dbg; + int index = 0; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + if (!dpu_dbg) { + DRM_ERROR("invalid params\n"); + return; + } + + /* + * if there is a coredump pending return immediately till dump + * if read by userspace or timeout happens + */ + if (((dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) || + (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_COREDUMP)) && + dpu_dbg->coredump_pending) { + DRM_DEBUG("coredump is pending read\n"); + return; + } + + va_start(args, name); + + while ((blk_name = va_arg(args, char*))) { + + if (IS_ERR_OR_NULL(blk_name)) + break; + + if (index < DPU_DBG_BASE_MAX) + dpu_dbg->blk_names[index++] = blk_name; + else + DRM_ERROR("too many blk names\n"); + } + va_end(args); + + kthread_queue_work(dpu_dbg->dump_worker, + &dpu_dbg->dump_work); +} + +int dpu_dbg_init(struct drm_device *drm_dev) +{ + struct dpu_kms *dpu_kms; + struct msm_drm_private *priv; + struct dpu_dbg_base *dpu_dbg; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return -EINVAL; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + dpu_dbg = devm_kzalloc(drm_dev->dev, sizeof(struct dpu_dbg_base), GFP_KERNEL); + + mutex_init(&dpu_dbg->mutex); + + dpu_dbg->dev = drm_dev->dev; + dpu_dbg->drm_dev = drm_dev; + + dpu_dbg->reg_dump_method = DEFAULT_REGDUMP; + + dpu_dbg->dump_worker = kthread_create_worker(0, "%s", "dpu_dbg"); + if (IS_ERR(dpu_dbg->dump_worker)) + DRM_ERROR("failed to create dpu dbg task\n"); + + kthread_init_work(&dpu_dbg->dump_work, _dpu_dump_work); + + dpu_kms->dpu_dbg = dpu_dbg; + + dpu_dbg_init_blk_info(drm_dev); + + return 0; +} + +void dpu_dbg_destroy(struct drm_device *drm_dev) +{ + struct dpu_kms *dpu_kms; + struct msm_drm_private *priv; + struct dpu_dbg_base *dpu_dbg; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + if (dpu_dbg->dump_worker) + kthread_destroy_worker(dpu_dbg->dump_worker); + + mutex_destroy(&dpu_dbg->mutex); +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h new file mode 100644 index 0000000..302205a --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h @@ -0,0 +1,200 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#ifndef DPU_DBG_H_ +#define DPU_DBG_H_ + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_device.h> +#include "../../../drm_crtc_internal.h" +#include <drm/drm_print.h> +#include <drm/drm_atomic.h> +#include <linux/debugfs.h> +#include <linux/list.h> +#include <linux/delay.h> +#include <linux/spinlock.h> +#include <linux/ktime.h> +#include <linux/debugfs.h> +#include <linux/uaccess.h> +#include <linux/dma-buf.h> +#include <linux/slab.h> +#include <linux/list_sort.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/kthread.h> +#include <linux/devcoredump.h> +#include <stdarg.h> +#include "dpu_hw_catalog.h" +#include "dpu_kms.h" +#include "dsi.h" + +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) + +enum dpu_dbg_dump_flag { + DPU_DBG_DUMP_IN_LOG = BIT(0), + DPU_DBG_DUMP_IN_MEM = BIT(1), + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), +}; + +#define DPU_DBG_BASE_MAX 10 + +#define DEFAULT_PANIC 0 +#define DEFAULT_REGDUMP DPU_DBG_DUMP_IN_MEM +#define ROW_BYTES 16 +#define RANGE_NAME_LEN 40 +#define REG_BASE_NAME_LEN 80 + +/* debug option to print the registers in logs */ +#define DPU_DBG_DUMP_IN_CONSOLE 0 + +/* print debug ranges in groups of 4 u32s */ +#define REG_DUMP_ALIGN 16 + +struct dpu_mdp_regs { + u32 **ctl; + u32 **sspp; + u32 *top; + u32 **pp; + u32 **intf; + u32 **dspp; +}; + +/** + * struct dpu_dbg_base - sde debug base structure + * @evtlog: event log instance + * @reg_base_list: list of register dumping regions + * @dev: device pointer + * @drm_dev: drm device pointer + * @mutex: mutex to serialize access to serialze dumps, debugfs access + * @dsi_ctrl_regs: array storing dsi controller registers + * @dp_ctrl_regs: array storing dp controller registers + * @mdp_regs: pointer to struct containing mdp register dump + * @reg_dump_method: whether to dump registers into memory, kernel log, or both + * @coredump_pending: coredump is pending read from userspace + * @atomic_state: atomic state duplicated at the time of the error + * @dump_worker: kworker thread which runs the dump work + * @dump_work: kwork which dumps the registers and drm state + * @timestamp: timestamp at which the coredump was captured + * @dpu_dbg_printer: drm printer handle used to take drm snapshot + */ +struct dpu_dbg_base { + struct device *dev; + struct drm_device *drm_dev; + struct mutex mutex; + + u32 **dsi_ctrl_regs; + + u32 *dp_ctrl_regs; + + struct dpu_mdp_regs *mdp_regs; + + char *blk_names[DPU_DBG_BASE_MAX]; + + u32 reg_dump_method; + + bool coredump_pending; + + struct drm_atomic_state *atomic_state; + + struct kthread_worker *dump_worker; + struct kthread_work dump_work; + ktime_t timestamp; + + struct drm_printer *dpu_dbg_printer; +}; + +/** + * DPU_DBG_DUMP - trigger dumping of all dpu_dbg facilities + * @va_args: list of named register dump ranges and regions to dump + * currently "mdp", "dsi" and "dp" are supported to dump + * mdp, dsi and dp register space respectively + */ +#define DPU_DBG_DUMP(drm_dev, ...) dpu_dbg_dump(drm_dev, __func__, \ + ##__VA_ARGS__, DPU_DBG_DUMP_DATA_LIMITER) + +/** + * dpu_dbg_init - initialize global sde debug facilities: evtlog, regdump + * @dev: device handle + * Returns: 0 or -ERROR + */ +int dpu_dbg_init(struct drm_device *drm_dev); + +/** + * dpu_dbg_destroy - destroy the global sde debug facilities + * Returns: none + */ +void dpu_dbg_destroy(struct drm_device *drm_dev); + +/** + * dpu_dbg_dump - trigger dumping of all dpu_dbg facilities + * @name: string indicating origin of dump + * @va_args: list of named register dump ranges and regions to dump + * currently "mdp", "dsi" and "dp" are supported to dump + * mdp, dsi and dp register space respectively + * + * Returns: none + */ +void dpu_dbg_dump(struct drm_device *drm_dev, const char *name, ...); + +/** + * dpu_dbg_dump_regs - utility to store the register dumps in the specified memory + * @reg: memory where the registers need to be dumped + * @len: size of the register space which needs to be dumped + * @base_addr: base address of the module which needs to be dumped + * @dump_op: op specifying if the dump needs to be in memory, in log or in coredump + * @p: handle to drm_printer + * Returns: none + */ +void dpu_dbg_dump_regs(u32 **reg, u32 len, void __iomem *base_addr, + u32 dump_op, struct drm_printer *p); + +/** + * dpu_dbg_get - get the handle to dpu_dbg struct from the drm device + * @drm: handle to drm device + + * Returns: handle to the dpu_dbg_base struct + */ +struct dpu_dbg_base *dpu_dbg_get(struct drm_device *drm); + +/** + * dpu_dbg_print_regs - print out the module registers to either log or drm printer + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_print_regs(struct drm_device *dev, u8 reg_dump_method); + +/** + * dpu_dbg_dump_blks - utility to dump out the registers as per their names + * @dpu_dbg: handle to dpu_dbg_base struct + + * Returns: none + */ +void dpu_dbg_dump_blks(struct dpu_dbg_base *dpu_dbg); + +/** + * dpu_dbg_init_blk_info - allocate memory for hw blocks based on hw catalog + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_init_blk_info(struct drm_device *dev); + +/** + * dpu_dbg_free_blk_mem - free the memory after the coredump has been read + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_free_blk_mem(struct drm_device *drm_dev); + +/** + * dpu_dbg_is_drm_printer_needed - checks if a valid drm printer is needed for this dump type + * @dpu_dbg: handle to the dpu_dbg_base struct + * Returns: none + */ +bool dpu_dbg_is_drm_printer_needed(struct dpu_dbg_base *dpu_dbg); + +#endif /* DPU_DBG_H_ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c new file mode 100644 index 0000000..36647bf --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c @@ -0,0 +1,257 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ + +#include "dpu_dbg.h" + +void dpu_dbg_dump_regs(u32 **reg, u32 len, void __iomem *base_addr, + u32 dump_op, struct drm_printer *p) +{ + u32 len_aligned, len_padded; + u32 x0, x4, x8, xc; + void __iomem *addr; + u32 *dump_addr = NULL; + void __iomem *end_addr; + int i; + + len_aligned = (len + REG_DUMP_ALIGN - 1) / REG_DUMP_ALIGN; + len_padded = len_aligned * REG_DUMP_ALIGN; + + addr = base_addr; + end_addr = base_addr + len; + + if (dump_op == DPU_DBG_DUMP_IN_COREDUMP && !p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + + if (dump_op == DPU_DBG_DUMP_IN_MEM && !(*reg)) + *reg = kzalloc(len_padded, GFP_KERNEL); + + if (*reg) + dump_addr = *reg; + + for (i = 0; i < len_aligned; i++) { + if (dump_op == DPU_DBG_DUMP_IN_MEM) { + x0 = (addr < end_addr) ? readl_relaxed(addr + 0x0) : 0; + x4 = (addr + 0x4 < end_addr) ? readl_relaxed(addr + 0x4) : 0; + x8 = (addr + 0x8 < end_addr) ? readl_relaxed(addr + 0x8) : 0; + xc = (addr + 0xc < end_addr) ? readl_relaxed(addr + 0xc) : 0; + } + + if (dump_addr) { + if (dump_op == DPU_DBG_DUMP_IN_MEM) { + dump_addr[i * 4] = x0; + dump_addr[i * 4 + 1] = x4; + dump_addr[i * 4 + 2] = x8; + dump_addr[i * 4 + 3] = xc; + } else if (dump_op == DPU_DBG_DUMP_IN_COREDUMP) { + drm_printf(p, "0x%lx : %08x %08x %08x %08x\n", + (unsigned long)(addr - base_addr), + dump_addr[i * 4], dump_addr[i * 4 + 1], + dump_addr[i * 4 + 2], dump_addr[i * 4 + 3]); + } + pr_debug("0x%lx : %08x %08x %08x %08x\n", + (unsigned long)(addr - base_addr), + dump_addr[i * 4], dump_addr[i * 4 + 1], + dump_addr[i * 4 + 2], dump_addr[i * 4 + 3]); + } + + addr += REG_DUMP_ALIGN; + } +} + +struct dpu_dbg_base *dpu_dbg_get(struct drm_device *drm) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + + priv = drm->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + return dpu_kms->dpu_dbg; +} + +bool dpu_dbg_is_drm_printer_needed(struct dpu_dbg_base *dpu_dbg) +{ + if ((dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_COREDUMP) || + (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_LOG)) + return true; + else + return false; +} + +static void dpu_dbg_dump_dsi_regs(struct drm_device *dev) +{ + struct msm_drm_private *priv; + int i; + + priv = dev->dev_private; + + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { + if (!priv->dsi[i]) + continue; + + msm_dsi_dump_regs(priv->dsi[i]); + } +} + +static void dpu_dbg_dump_dp_regs(struct drm_device *dev) +{ + struct msm_drm_private *priv; + + priv = dev->dev_private; + + if (priv->dp) + msm_dp_dump_regs(priv->dp); + +} + +static void dpu_dbg_dump_mdp_regs(struct drm_device *dev) +{ + dpu_kms_dump_mdp_regs(dev); +} + +static void dpu_dbg_dump_all_regs(struct drm_device *dev) +{ + dpu_dbg_dump_mdp_regs(dev); + dpu_dbg_dump_dsi_regs(dev); + dpu_dbg_dump_dp_regs(dev); +} + +void dpu_dbg_print_regs(struct drm_device *dev, u8 reg_dump_method) +{ + struct msm_drm_private *priv; + int i; + struct dpu_dbg_base *dpu_dbg; + struct drm_printer *p; + + priv = dev->dev_private; + dpu_dbg = dpu_dbg_get(dev); + dpu_dbg->reg_dump_method = DPU_DBG_DUMP_IN_COREDUMP; + + p = dpu_dbg->dpu_dbg_printer; + + drm_printf(p, "===================mdp regs================\n"); + dpu_kms_dump_mdp_regs(dev); + + drm_printf(p, "===================dsi regs================\n"); + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { + if (!priv->dsi[i]) + continue; + + msm_dsi_dump_regs(priv->dsi[i]); + } + + drm_printf(p, "===================dp regs================\n"); + + if (priv->dp) + msm_dp_dump_regs(priv->dp); +} + +void dpu_dbg_dump_blks(struct dpu_dbg_base *dpu_dbg) +{ + int i; + + for (i = 0; i < DPU_DBG_BASE_MAX; i++) { + if (dpu_dbg->blk_names[i] != NULL) { + DRM_DEBUG("blk name is %s\n", dpu_dbg->blk_names[i]); + if (!strcmp(dpu_dbg->blk_names[i], "all")) { + dpu_dbg_dump_all_regs(dpu_dbg->drm_dev); + break; + } else if (!strcmp(dpu_dbg->blk_names[i], "mdp")) { + dpu_dbg_dump_mdp_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dsi")) { + dpu_dbg_dump_dsi_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dp")) { + dpu_dbg_dump_dp_regs(dpu_dbg->drm_dev); + } else { + DRM_ERROR("blk name not found %s\n", dpu_dbg->blk_names[i]); + } + } + } +} + +void dpu_dbg_free_blk_mem(struct drm_device *drm_dev) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + int i; + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + + /* free CTL sub-blocks mem */ + for (i = 0; i < cat->ctl_count; i++) + kfree(dpu_dbg->mdp_regs->ctl[i]); + + /* free DSPP sub-blocks mem */ + for (i = 0; i < cat->dspp_count; i++) + kfree(dpu_dbg->mdp_regs->dspp[i]); + + /* free INTF sub-blocks mem */ + for (i = 0; i < cat->intf_count; i++) + kfree(dpu_dbg->mdp_regs->intf[i]); + + /* free INTF sub-blocks mem */ + for (i = 0; i < cat->pingpong_count; i++) + kfree(dpu_dbg->mdp_regs->pp[i]); + + /* free SSPP sub-blocks mem */ + for (i = 0; i < cat->sspp_count; i++) + kfree(dpu_dbg->mdp_regs->sspp[i]); + + /* free TOP sub-blocks mem */ + kfree(dpu_dbg->mdp_regs->top); + + /* free DSI regs mem */ + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) + kfree(dpu_dbg->dsi_ctrl_regs[i]); + + /* free DP regs mem */ + kfree(dpu_dbg->dp_ctrl_regs); +} + +void dpu_dbg_init_blk_info(struct drm_device *drm_dev) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + dpu_dbg = dpu_kms->dpu_dbg; + + dpu_dbg->mdp_regs = devm_kzalloc(drm_dev->dev, sizeof(struct dpu_mdp_regs), GFP_KERNEL); + + if (dpu_dbg->mdp_regs) { + dpu_dbg->mdp_regs->ctl = devm_kzalloc(drm_dev->dev, + cat->ctl_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->dspp = devm_kzalloc(drm_dev->dev, + cat->dspp_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->intf = devm_kzalloc(drm_dev->dev, + cat->intf_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->sspp = devm_kzalloc(drm_dev->dev, + cat->sspp_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->pp = devm_kzalloc(drm_dev->dev, + cat->pingpong_count * sizeof(u32 **), GFP_KERNEL); + } + + dpu_dbg->dsi_ctrl_regs = devm_kzalloc(drm_dev->dev, ARRAY_SIZE(priv->dsi) * + sizeof(dpu_dbg->dsi_ctrl_regs), GFP_KERNEL); +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index ea4647d..d4893c2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #ifndef _DPU_HW_CATALOG_H diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 17f7102..46eb9ec 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -797,6 +797,92 @@ static void dpu_irq_uninstall(struct msm_kms *kms) dpu_core_irq_uninstall(dpu_kms); } +void dpu_kms_dump_mdp_regs(struct drm_device *dev) +{ + int i; + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + struct drm_printer *p; + bool in_drm_printer = false; + + priv = dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + p = dpu_dbg->dpu_dbg_printer; + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + + in_drm_printer = dpu_dbg_is_drm_printer_needed(dpu_dbg); + + if (in_drm_printer && !p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_get_sync(&dpu_kms->pdev->dev); + + /* dump CTL sub-blocks HW regs info */ + for (i = 0; i < cat->ctl_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================ctl %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->ctl[i], + cat->ctl[i].len, dpu_kms->mmio + cat->ctl[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump DSPP sub-blocks HW regs info */ + for (i = 0; i < cat->dspp_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================dspp %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->dspp[i], + cat->dspp[i].len, dpu_kms->mmio + cat->dspp[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump INTF sub-blocks HW regs info */ + for (i = 0; i < cat->intf_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================intf %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->intf[i], + cat->intf[i].len, dpu_kms->mmio + cat->intf[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump PP sub-blocks HW regs info */ + for (i = 0; i < cat->pingpong_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================ping-pong %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->pp[i], + cat->pingpong[i].len, dpu_kms->mmio + cat->pingpong[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump SSPP sub-blocks HW regs info */ + for (i = 0; i < cat->sspp_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================sspp %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->sspp[i], + cat->sspp[i].len, dpu_kms->mmio + cat->sspp[i].base, + dpu_dbg->reg_dump_method, p); + } + + if (in_drm_printer) + drm_printf(p, "===================mdp top regs================\n"); + + /* dump TOP sub-blocks HW regs info */ + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->top, + top->hw.length, dpu_kms->mmio + top->hw.blk_off, + dpu_dbg->reg_dump_method, p); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_put_sync(&dpu_kms->pdev->dev); +} + static const struct msm_kms_funcs kms_funcs = { .hw_init = dpu_kms_hw_init, .irq_preinstall = dpu_irq_preinstall, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index d6717d6..e582bb7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -24,6 +24,7 @@ #include "dpu_io_util.h" #include "dpu_rm.h" #include "dpu_core_perf.h" +#include "dpu_dbg.h" #define DRMID(x) ((x) ? (x)->base.id : -1) @@ -132,6 +133,8 @@ struct dpu_kms { struct opp_table *opp_table; + struct dpu_dbg_base *dpu_dbg; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can @@ -265,4 +268,6 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder); */ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name); +void dpu_kms_dump_mdp_regs(struct drm_device *dev); + #endif /* __dpu_kms_H__ */ diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index b1a9b1b..1f0f6f2 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -62,6 +62,16 @@ struct dp_catalog_private { u8 aux_lut_cfg_index[PHY_AUX_CFG_MAX]; }; +void dp_catalog_dump_dbg_regs(struct dp_catalog *dp_catalog, u32 **regs, u8 reg_dump_method, + struct drm_printer *p) +{ + struct dp_catalog_private *catalog = container_of(dp_catalog, + struct dp_catalog_private, dp_catalog); + + dpu_dbg_dump_regs(regs, catalog->io->dp_controller.len, + catalog->io->dp_controller.base, reg_dump_method, p); +} + static inline u32 dp_read_aux(struct dp_catalog_private *catalog, u32 offset) { offset += MSM_DP_CONTROLLER_AUX_OFFSET; diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h index 176a902..4b0f77f 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.h +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h @@ -9,6 +9,7 @@ #include <drm/drm_modes.h> #include "dp_parser.h" +#include "dpu_dbg.h" /* interrupts */ #define DP_INTR_HPD BIT(0) @@ -71,6 +72,10 @@ struct dp_catalog { u32 audio_data; }; +/* Debug module */ +void dp_catalog_dump_dbg_regs(struct dp_catalog *dp_catalog, u32 **regs, u8 reg_dump_method, + struct drm_printer *p); + /* AUX APIs */ u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog); int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 5a39da6..b0edfbf 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1009,6 +1009,43 @@ int dp_display_get_test_bpp(struct msm_dp *dp) dp_display->link->test_video.test_bit_depth); } +void msm_dp_dump_regs(struct msm_dp *dp) +{ + struct dp_display_private *dp_display; + struct drm_device *drm; + struct dpu_dbg_base *dpu_dbg; + + dp_display = container_of(dp, struct dp_display_private, dp_display); + drm = dp->drm_dev; + dpu_dbg = dpu_dbg_get(drm); + + /* + * if we are reading registers we need the link clocks to be on + * however till DP cable is connected this will not happen as we + * do not know the resolution to power up with. Hence check the + * power_on status before dumping DP registers to avoid crash due + * to unclocked access + */ + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) { + mutex_lock(&dp_display->event_mutex); + if (!dp->power_on) { + mutex_unlock(&dp_display->event_mutex); + return; + } + } + + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && !dpu_dbg->dpu_dbg_printer) { + pr_err("invalid drm printer\n"); + return; + } + + dp_catalog_dump_dbg_regs(dp_display->catalog, &dpu_dbg->dp_ctrl_regs, + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + mutex_unlock(&dp_display->event_mutex); +} + static void dp_display_config_hpd(struct dp_display_private *dp) { diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index 6092ba1..aa66c3d 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -8,6 +8,7 @@ #include "dp_panel.h" #include <sound/hdmi-codec.h> +#include "dpu_dbg.h" struct msm_dp { struct drm_device *drm_dev; diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 62704885..9a86f5e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -266,3 +266,8 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, return ret; } +void msm_dsi_dump_regs(struct msm_dsi *msm_dsi) +{ + msm_dsi_host_dump_regs(msm_dsi->host); +} + diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 78ef5d4..c9fccc4 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -15,6 +15,7 @@ #include <drm/drm_panel.h> #include "msm_drv.h" +#include "dpu_dbg.h" #define DSI_0 0 #define DSI_1 1 @@ -102,6 +103,8 @@ static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi) return msm_dsi->panel || msm_dsi->external_bridge; } +void msm_dsi_dump_regs(struct msm_dsi *msm_dsi); + struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi); /* dsi pll */ @@ -197,6 +200,7 @@ int dsi_clk_init_v2(struct msm_dsi_host *msm_host); int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host); int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_dual_dsi); int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_dual_dsi); +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host); /* dsi phy */ struct msm_dsi_phy; diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index ab281cb..d1675ee 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -2489,3 +2489,28 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) return of_drm_find_bridge(msm_host->device_node); } + +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) +{ + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + struct drm_device *dev = msm_host->dev; + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = dpu_dbg_get(dev); + + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && + !dpu_dbg->dpu_dbg_printer) { + pr_err("invalid drm printer\n"); + return; + } + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_get_sync(&msm_host->pdev->dev); + + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), msm_host->ctrl_base, + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_put_sync(&msm_host->pdev->dev); +} diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 898e6d0..c4405b6 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2018, 2020-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark <robdclark@gmail.com> */ @@ -19,6 +19,7 @@ #include <drm/drm_of.h> #include <drm/drm_vblank.h> +#include "dpu_dbg.h" #include "msm_drv.h" #include "msm_debugfs.h" #include "msm_fence.h" @@ -166,6 +167,24 @@ void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name, return _msm_ioremap(pdev, name, dbgname, true); } +unsigned long msm_iomap_size(struct platform_device *pdev, const char *name) +{ + struct resource *res; + + if (name) + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name); + else + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + if (!res) { + dev_dbg(&pdev->dev, "failed to get memory resource: %s\n", + name); + return 0; + } + + return resource_size(res); +} + void msm_writel(u32 data, void __iomem *addr) { if (reglog) @@ -277,6 +296,8 @@ static int msm_drm_uninit(struct device *dev) msm_fbdev_free(ddev); #endif + dpu_dbg_destroy(ddev); + drm_mode_config_cleanup(ddev); pm_runtime_get_sync(dev); @@ -549,6 +570,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) if (ret) goto err_msm_uninit; + if (get_mdp_ver(pdev) == KMS_DPU) { + ret = dpu_dbg_init(ddev); + if (ret) + DRM_DEV_ERROR(dev, "dpu_dbg_init failed ret = %d\n", ret); + } + drm_mode_config_reset(ddev); #ifdef CONFIG_DRM_FBDEV_EMULATION diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 6a42cdf..b8de6f3 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -362,6 +362,7 @@ void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); void msm_dp_irq_postinstall(struct msm_dp *dp_display); +void msm_dp_dump_regs(struct msm_dp *dp_display); void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor); @@ -445,6 +446,7 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name, const char *dbgname); void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name, const char *dbgname); +unsigned long msm_iomap_size(struct platform_device *pdev, const char *name); void msm_writel(u32 data, void __iomem *addr); u32 msm_readl(const void __iomem *addr); void msm_rmw(void __iomem *addr, u32 mask, u32 or); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers @ 2021-04-09 2:28 ` Abhinav Kumar 0 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno Add the dpu_dbg module which adds supports to dump dpu registers which can be used in case of error conditions. changes in v3: - Get rid of registration mechanism for sub-modules and instead get this information from the dpu catalog itself - Get rid of global dpu_dbg struct and instead store it in dpu_kms - delegate the power management of the sub-modules to the resp drivers - refactor and remove the linked list logic and simplify it to have just an array Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 +++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 4 + drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ drivers/gpu/drm/msm/msm_drv.c | 29 ++- drivers/gpu/drm/msm/msm_drv.h | 2 + 16 files changed, 889 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 3cc9061..9166417 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -54,6 +54,8 @@ msm-y := \ disp/dpu1/dpu_core_irq.o \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ + disp/dpu1/dpu_dbg.o \ + disp/dpu1/dpu_dbg_util.o \ disp/dpu1/dpu_encoder.o \ disp/dpu1/dpu_encoder_phys_cmd.o \ disp/dpu1/dpu_encoder_phys_vid.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c new file mode 100644 index 0000000..9ec642e --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ + +#include "dpu_dbg.h" +#include "dpu_kms.h" +#include "dpu_hw_catalog.h" + +#ifdef CONFIG_DEV_COREDUMP +static ssize_t dpu_devcoredump_read(char *buffer, loff_t offset, + size_t count, void *data, size_t datalen) +{ + struct drm_print_iterator iter; + struct drm_printer p; + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = data; + + iter.data = buffer; + iter.offset = 0; + iter.start = offset; + iter.remain = count; + + p = drm_coredump_printer(&iter); + + drm_printf(&p, "---\n"); + + drm_printf(&p, "module: " KBUILD_MODNAME "\n"); + drm_printf(&p, "dpu devcoredump\n"); + drm_printf(&p, "timestamp %lld\n", ktime_to_ns(dpu_dbg->timestamp)); + + dpu_dbg->dpu_dbg_printer = &p; + + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_COREDUMP); + + drm_printf(&p, "===================dpu drm state================\n"); + + if (dpu_dbg->atomic_state) + drm_atomic_print_new_state(dpu_dbg->atomic_state, + &p); + + return count - iter.remain; +} + +static void dpu_devcoredump_free(void *data) +{ + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = data; + + if (dpu_dbg->atomic_state) { + drm_atomic_state_put(dpu_dbg->atomic_state); + dpu_dbg->atomic_state = NULL; + } + + dpu_dbg_free_blk_mem(dpu_dbg->drm_dev); + + dpu_dbg->coredump_pending = false; +} + +static void dpu_devcoredump_capture_state(struct dpu_dbg_base *dpu_dbg) +{ + struct drm_device *ddev; + struct drm_modeset_acquire_ctx ctx; + + dpu_dbg->timestamp = ktime_get(); + + ddev = dpu_dbg->drm_dev; + + drm_modeset_acquire_init(&ctx, 0); + + while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0) + drm_modeset_backoff(&ctx); + + dpu_dbg->atomic_state = drm_atomic_helper_duplicate_state(ddev, + &ctx); + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); +} +#else +static void dpu_devcoredump_capture_state(struct dpu_dbg_base *dpu_dbg) +{ +} +#endif /* CONFIG_DEV_COREDUMP */ + +static void _dpu_dump_work(struct kthread_work *work) +{ + struct dpu_dbg_base *dpu_dbg = container_of(work, struct dpu_dbg_base, dump_work); + struct drm_printer p; + + /* reset the reg_dump_method to IN_MEM before every dump */ + dpu_dbg->reg_dump_method = DPU_DBG_DUMP_IN_MEM; + + dpu_dbg_dump_blks(dpu_dbg); + + dpu_devcoredump_capture_state(dpu_dbg); + + if (DPU_DBG_DUMP_IN_CONSOLE) { + p = drm_info_printer(dpu_dbg->drm_dev->dev); + dpu_dbg->dpu_dbg_printer = &p; + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_LOG); + } + +#ifdef CONFIG_DEV_COREDUMP + dev_coredumpm(dpu_dbg->dev, THIS_MODULE, dpu_dbg, 0, GFP_KERNEL, + dpu_devcoredump_read, dpu_devcoredump_free); + dpu_dbg->coredump_pending = true; +#endif +} + +void dpu_dbg_dump(struct drm_device *drm_dev, const char *name, ...) +{ + va_list args; + char *blk_name = NULL; + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_dbg_base *dpu_dbg; + int index = 0; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + if (!dpu_dbg) { + DRM_ERROR("invalid params\n"); + return; + } + + /* + * if there is a coredump pending return immediately till dump + * if read by userspace or timeout happens + */ + if (((dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) || + (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_COREDUMP)) && + dpu_dbg->coredump_pending) { + DRM_DEBUG("coredump is pending read\n"); + return; + } + + va_start(args, name); + + while ((blk_name = va_arg(args, char*))) { + + if (IS_ERR_OR_NULL(blk_name)) + break; + + if (index < DPU_DBG_BASE_MAX) + dpu_dbg->blk_names[index++] = blk_name; + else + DRM_ERROR("too many blk names\n"); + } + va_end(args); + + kthread_queue_work(dpu_dbg->dump_worker, + &dpu_dbg->dump_work); +} + +int dpu_dbg_init(struct drm_device *drm_dev) +{ + struct dpu_kms *dpu_kms; + struct msm_drm_private *priv; + struct dpu_dbg_base *dpu_dbg; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return -EINVAL; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + dpu_dbg = devm_kzalloc(drm_dev->dev, sizeof(struct dpu_dbg_base), GFP_KERNEL); + + mutex_init(&dpu_dbg->mutex); + + dpu_dbg->dev = drm_dev->dev; + dpu_dbg->drm_dev = drm_dev; + + dpu_dbg->reg_dump_method = DEFAULT_REGDUMP; + + dpu_dbg->dump_worker = kthread_create_worker(0, "%s", "dpu_dbg"); + if (IS_ERR(dpu_dbg->dump_worker)) + DRM_ERROR("failed to create dpu dbg task\n"); + + kthread_init_work(&dpu_dbg->dump_work, _dpu_dump_work); + + dpu_kms->dpu_dbg = dpu_dbg; + + dpu_dbg_init_blk_info(drm_dev); + + return 0; +} + +void dpu_dbg_destroy(struct drm_device *drm_dev) +{ + struct dpu_kms *dpu_kms; + struct msm_drm_private *priv; + struct dpu_dbg_base *dpu_dbg; + + if (!drm_dev) { + DRM_ERROR("invalid params\n"); + return; + } + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + if (dpu_dbg->dump_worker) + kthread_destroy_worker(dpu_dbg->dump_worker); + + mutex_destroy(&dpu_dbg->mutex); +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h new file mode 100644 index 0000000..302205a --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h @@ -0,0 +1,200 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#ifndef DPU_DBG_H_ +#define DPU_DBG_H_ + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_device.h> +#include "../../../drm_crtc_internal.h" +#include <drm/drm_print.h> +#include <drm/drm_atomic.h> +#include <linux/debugfs.h> +#include <linux/list.h> +#include <linux/delay.h> +#include <linux/spinlock.h> +#include <linux/ktime.h> +#include <linux/debugfs.h> +#include <linux/uaccess.h> +#include <linux/dma-buf.h> +#include <linux/slab.h> +#include <linux/list_sort.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/kthread.h> +#include <linux/devcoredump.h> +#include <stdarg.h> +#include "dpu_hw_catalog.h" +#include "dpu_kms.h" +#include "dsi.h" + +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) + +enum dpu_dbg_dump_flag { + DPU_DBG_DUMP_IN_LOG = BIT(0), + DPU_DBG_DUMP_IN_MEM = BIT(1), + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), +}; + +#define DPU_DBG_BASE_MAX 10 + +#define DEFAULT_PANIC 0 +#define DEFAULT_REGDUMP DPU_DBG_DUMP_IN_MEM +#define ROW_BYTES 16 +#define RANGE_NAME_LEN 40 +#define REG_BASE_NAME_LEN 80 + +/* debug option to print the registers in logs */ +#define DPU_DBG_DUMP_IN_CONSOLE 0 + +/* print debug ranges in groups of 4 u32s */ +#define REG_DUMP_ALIGN 16 + +struct dpu_mdp_regs { + u32 **ctl; + u32 **sspp; + u32 *top; + u32 **pp; + u32 **intf; + u32 **dspp; +}; + +/** + * struct dpu_dbg_base - sde debug base structure + * @evtlog: event log instance + * @reg_base_list: list of register dumping regions + * @dev: device pointer + * @drm_dev: drm device pointer + * @mutex: mutex to serialize access to serialze dumps, debugfs access + * @dsi_ctrl_regs: array storing dsi controller registers + * @dp_ctrl_regs: array storing dp controller registers + * @mdp_regs: pointer to struct containing mdp register dump + * @reg_dump_method: whether to dump registers into memory, kernel log, or both + * @coredump_pending: coredump is pending read from userspace + * @atomic_state: atomic state duplicated at the time of the error + * @dump_worker: kworker thread which runs the dump work + * @dump_work: kwork which dumps the registers and drm state + * @timestamp: timestamp at which the coredump was captured + * @dpu_dbg_printer: drm printer handle used to take drm snapshot + */ +struct dpu_dbg_base { + struct device *dev; + struct drm_device *drm_dev; + struct mutex mutex; + + u32 **dsi_ctrl_regs; + + u32 *dp_ctrl_regs; + + struct dpu_mdp_regs *mdp_regs; + + char *blk_names[DPU_DBG_BASE_MAX]; + + u32 reg_dump_method; + + bool coredump_pending; + + struct drm_atomic_state *atomic_state; + + struct kthread_worker *dump_worker; + struct kthread_work dump_work; + ktime_t timestamp; + + struct drm_printer *dpu_dbg_printer; +}; + +/** + * DPU_DBG_DUMP - trigger dumping of all dpu_dbg facilities + * @va_args: list of named register dump ranges and regions to dump + * currently "mdp", "dsi" and "dp" are supported to dump + * mdp, dsi and dp register space respectively + */ +#define DPU_DBG_DUMP(drm_dev, ...) dpu_dbg_dump(drm_dev, __func__, \ + ##__VA_ARGS__, DPU_DBG_DUMP_DATA_LIMITER) + +/** + * dpu_dbg_init - initialize global sde debug facilities: evtlog, regdump + * @dev: device handle + * Returns: 0 or -ERROR + */ +int dpu_dbg_init(struct drm_device *drm_dev); + +/** + * dpu_dbg_destroy - destroy the global sde debug facilities + * Returns: none + */ +void dpu_dbg_destroy(struct drm_device *drm_dev); + +/** + * dpu_dbg_dump - trigger dumping of all dpu_dbg facilities + * @name: string indicating origin of dump + * @va_args: list of named register dump ranges and regions to dump + * currently "mdp", "dsi" and "dp" are supported to dump + * mdp, dsi and dp register space respectively + * + * Returns: none + */ +void dpu_dbg_dump(struct drm_device *drm_dev, const char *name, ...); + +/** + * dpu_dbg_dump_regs - utility to store the register dumps in the specified memory + * @reg: memory where the registers need to be dumped + * @len: size of the register space which needs to be dumped + * @base_addr: base address of the module which needs to be dumped + * @dump_op: op specifying if the dump needs to be in memory, in log or in coredump + * @p: handle to drm_printer + * Returns: none + */ +void dpu_dbg_dump_regs(u32 **reg, u32 len, void __iomem *base_addr, + u32 dump_op, struct drm_printer *p); + +/** + * dpu_dbg_get - get the handle to dpu_dbg struct from the drm device + * @drm: handle to drm device + + * Returns: handle to the dpu_dbg_base struct + */ +struct dpu_dbg_base *dpu_dbg_get(struct drm_device *drm); + +/** + * dpu_dbg_print_regs - print out the module registers to either log or drm printer + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_print_regs(struct drm_device *dev, u8 reg_dump_method); + +/** + * dpu_dbg_dump_blks - utility to dump out the registers as per their names + * @dpu_dbg: handle to dpu_dbg_base struct + + * Returns: none + */ +void dpu_dbg_dump_blks(struct dpu_dbg_base *dpu_dbg); + +/** + * dpu_dbg_init_blk_info - allocate memory for hw blocks based on hw catalog + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_init_blk_info(struct drm_device *dev); + +/** + * dpu_dbg_free_blk_mem - free the memory after the coredump has been read + * @drm: handle to drm device + + * Returns: none + */ +void dpu_dbg_free_blk_mem(struct drm_device *drm_dev); + +/** + * dpu_dbg_is_drm_printer_needed - checks if a valid drm printer is needed for this dump type + * @dpu_dbg: handle to the dpu_dbg_base struct + * Returns: none + */ +bool dpu_dbg_is_drm_printer_needed(struct dpu_dbg_base *dpu_dbg); + +#endif /* DPU_DBG_H_ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c new file mode 100644 index 0000000..36647bf --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c @@ -0,0 +1,257 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. + */ + +#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ + +#include "dpu_dbg.h" + +void dpu_dbg_dump_regs(u32 **reg, u32 len, void __iomem *base_addr, + u32 dump_op, struct drm_printer *p) +{ + u32 len_aligned, len_padded; + u32 x0, x4, x8, xc; + void __iomem *addr; + u32 *dump_addr = NULL; + void __iomem *end_addr; + int i; + + len_aligned = (len + REG_DUMP_ALIGN - 1) / REG_DUMP_ALIGN; + len_padded = len_aligned * REG_DUMP_ALIGN; + + addr = base_addr; + end_addr = base_addr + len; + + if (dump_op == DPU_DBG_DUMP_IN_COREDUMP && !p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + + if (dump_op == DPU_DBG_DUMP_IN_MEM && !(*reg)) + *reg = kzalloc(len_padded, GFP_KERNEL); + + if (*reg) + dump_addr = *reg; + + for (i = 0; i < len_aligned; i++) { + if (dump_op == DPU_DBG_DUMP_IN_MEM) { + x0 = (addr < end_addr) ? readl_relaxed(addr + 0x0) : 0; + x4 = (addr + 0x4 < end_addr) ? readl_relaxed(addr + 0x4) : 0; + x8 = (addr + 0x8 < end_addr) ? readl_relaxed(addr + 0x8) : 0; + xc = (addr + 0xc < end_addr) ? readl_relaxed(addr + 0xc) : 0; + } + + if (dump_addr) { + if (dump_op == DPU_DBG_DUMP_IN_MEM) { + dump_addr[i * 4] = x0; + dump_addr[i * 4 + 1] = x4; + dump_addr[i * 4 + 2] = x8; + dump_addr[i * 4 + 3] = xc; + } else if (dump_op == DPU_DBG_DUMP_IN_COREDUMP) { + drm_printf(p, "0x%lx : %08x %08x %08x %08x\n", + (unsigned long)(addr - base_addr), + dump_addr[i * 4], dump_addr[i * 4 + 1], + dump_addr[i * 4 + 2], dump_addr[i * 4 + 3]); + } + pr_debug("0x%lx : %08x %08x %08x %08x\n", + (unsigned long)(addr - base_addr), + dump_addr[i * 4], dump_addr[i * 4 + 1], + dump_addr[i * 4 + 2], dump_addr[i * 4 + 3]); + } + + addr += REG_DUMP_ALIGN; + } +} + +struct dpu_dbg_base *dpu_dbg_get(struct drm_device *drm) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + + priv = drm->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + return dpu_kms->dpu_dbg; +} + +bool dpu_dbg_is_drm_printer_needed(struct dpu_dbg_base *dpu_dbg) +{ + if ((dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_COREDUMP) || + (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_LOG)) + return true; + else + return false; +} + +static void dpu_dbg_dump_dsi_regs(struct drm_device *dev) +{ + struct msm_drm_private *priv; + int i; + + priv = dev->dev_private; + + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { + if (!priv->dsi[i]) + continue; + + msm_dsi_dump_regs(priv->dsi[i]); + } +} + +static void dpu_dbg_dump_dp_regs(struct drm_device *dev) +{ + struct msm_drm_private *priv; + + priv = dev->dev_private; + + if (priv->dp) + msm_dp_dump_regs(priv->dp); + +} + +static void dpu_dbg_dump_mdp_regs(struct drm_device *dev) +{ + dpu_kms_dump_mdp_regs(dev); +} + +static void dpu_dbg_dump_all_regs(struct drm_device *dev) +{ + dpu_dbg_dump_mdp_regs(dev); + dpu_dbg_dump_dsi_regs(dev); + dpu_dbg_dump_dp_regs(dev); +} + +void dpu_dbg_print_regs(struct drm_device *dev, u8 reg_dump_method) +{ + struct msm_drm_private *priv; + int i; + struct dpu_dbg_base *dpu_dbg; + struct drm_printer *p; + + priv = dev->dev_private; + dpu_dbg = dpu_dbg_get(dev); + dpu_dbg->reg_dump_method = DPU_DBG_DUMP_IN_COREDUMP; + + p = dpu_dbg->dpu_dbg_printer; + + drm_printf(p, "===================mdp regs================\n"); + dpu_kms_dump_mdp_regs(dev); + + drm_printf(p, "===================dsi regs================\n"); + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { + if (!priv->dsi[i]) + continue; + + msm_dsi_dump_regs(priv->dsi[i]); + } + + drm_printf(p, "===================dp regs================\n"); + + if (priv->dp) + msm_dp_dump_regs(priv->dp); +} + +void dpu_dbg_dump_blks(struct dpu_dbg_base *dpu_dbg) +{ + int i; + + for (i = 0; i < DPU_DBG_BASE_MAX; i++) { + if (dpu_dbg->blk_names[i] != NULL) { + DRM_DEBUG("blk name is %s\n", dpu_dbg->blk_names[i]); + if (!strcmp(dpu_dbg->blk_names[i], "all")) { + dpu_dbg_dump_all_regs(dpu_dbg->drm_dev); + break; + } else if (!strcmp(dpu_dbg->blk_names[i], "mdp")) { + dpu_dbg_dump_mdp_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dsi")) { + dpu_dbg_dump_dsi_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dp")) { + dpu_dbg_dump_dp_regs(dpu_dbg->drm_dev); + } else { + DRM_ERROR("blk name not found %s\n", dpu_dbg->blk_names[i]); + } + } + } +} + +void dpu_dbg_free_blk_mem(struct drm_device *drm_dev) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + int i; + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + + /* free CTL sub-blocks mem */ + for (i = 0; i < cat->ctl_count; i++) + kfree(dpu_dbg->mdp_regs->ctl[i]); + + /* free DSPP sub-blocks mem */ + for (i = 0; i < cat->dspp_count; i++) + kfree(dpu_dbg->mdp_regs->dspp[i]); + + /* free INTF sub-blocks mem */ + for (i = 0; i < cat->intf_count; i++) + kfree(dpu_dbg->mdp_regs->intf[i]); + + /* free INTF sub-blocks mem */ + for (i = 0; i < cat->pingpong_count; i++) + kfree(dpu_dbg->mdp_regs->pp[i]); + + /* free SSPP sub-blocks mem */ + for (i = 0; i < cat->sspp_count; i++) + kfree(dpu_dbg->mdp_regs->sspp[i]); + + /* free TOP sub-blocks mem */ + kfree(dpu_dbg->mdp_regs->top); + + /* free DSI regs mem */ + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) + kfree(dpu_dbg->dsi_ctrl_regs[i]); + + /* free DP regs mem */ + kfree(dpu_dbg->dp_ctrl_regs); +} + +void dpu_dbg_init_blk_info(struct drm_device *drm_dev) +{ + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + + priv = drm_dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + dpu_dbg = dpu_kms->dpu_dbg; + + dpu_dbg->mdp_regs = devm_kzalloc(drm_dev->dev, sizeof(struct dpu_mdp_regs), GFP_KERNEL); + + if (dpu_dbg->mdp_regs) { + dpu_dbg->mdp_regs->ctl = devm_kzalloc(drm_dev->dev, + cat->ctl_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->dspp = devm_kzalloc(drm_dev->dev, + cat->dspp_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->intf = devm_kzalloc(drm_dev->dev, + cat->intf_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->sspp = devm_kzalloc(drm_dev->dev, + cat->sspp_count * sizeof(u32 **), GFP_KERNEL); + dpu_dbg->mdp_regs->pp = devm_kzalloc(drm_dev->dev, + cat->pingpong_count * sizeof(u32 **), GFP_KERNEL); + } + + dpu_dbg->dsi_ctrl_regs = devm_kzalloc(drm_dev->dev, ARRAY_SIZE(priv->dsi) * + sizeof(dpu_dbg->dsi_ctrl_regs), GFP_KERNEL); +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index ea4647d..d4893c2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #ifndef _DPU_HW_CATALOG_H diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 17f7102..46eb9ec 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -797,6 +797,92 @@ static void dpu_irq_uninstall(struct msm_kms *kms) dpu_core_irq_uninstall(dpu_kms); } +void dpu_kms_dump_mdp_regs(struct drm_device *dev) +{ + int i; + struct msm_drm_private *priv; + struct dpu_kms *dpu_kms; + struct dpu_mdss_cfg *cat; + struct dpu_hw_mdp *top; + struct dpu_dbg_base *dpu_dbg; + struct drm_printer *p; + bool in_drm_printer = false; + + priv = dev->dev_private; + dpu_kms = to_dpu_kms(priv->kms); + dpu_dbg = dpu_kms->dpu_dbg; + p = dpu_dbg->dpu_dbg_printer; + + cat = dpu_kms->catalog; + top = dpu_kms->hw_mdp; + + in_drm_printer = dpu_dbg_is_drm_printer_needed(dpu_dbg); + + if (in_drm_printer && !p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_get_sync(&dpu_kms->pdev->dev); + + /* dump CTL sub-blocks HW regs info */ + for (i = 0; i < cat->ctl_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================ctl %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->ctl[i], + cat->ctl[i].len, dpu_kms->mmio + cat->ctl[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump DSPP sub-blocks HW regs info */ + for (i = 0; i < cat->dspp_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================dspp %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->dspp[i], + cat->dspp[i].len, dpu_kms->mmio + cat->dspp[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump INTF sub-blocks HW regs info */ + for (i = 0; i < cat->intf_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================intf %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->intf[i], + cat->intf[i].len, dpu_kms->mmio + cat->intf[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump PP sub-blocks HW regs info */ + for (i = 0; i < cat->pingpong_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================ping-pong %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->pp[i], + cat->pingpong[i].len, dpu_kms->mmio + cat->pingpong[i].base, + dpu_dbg->reg_dump_method, p); + } + + /* dump SSPP sub-blocks HW regs info */ + for (i = 0; i < cat->sspp_count; i++) { + if (in_drm_printer) + drm_printf(p, "===================sspp %d regs================\n", i); + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->sspp[i], + cat->sspp[i].len, dpu_kms->mmio + cat->sspp[i].base, + dpu_dbg->reg_dump_method, p); + } + + if (in_drm_printer) + drm_printf(p, "===================mdp top regs================\n"); + + /* dump TOP sub-blocks HW regs info */ + dpu_dbg_dump_regs(&dpu_dbg->mdp_regs->top, + top->hw.length, dpu_kms->mmio + top->hw.blk_off, + dpu_dbg->reg_dump_method, p); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_put_sync(&dpu_kms->pdev->dev); +} + static const struct msm_kms_funcs kms_funcs = { .hw_init = dpu_kms_hw_init, .irq_preinstall = dpu_irq_preinstall, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index d6717d6..e582bb7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -24,6 +24,7 @@ #include "dpu_io_util.h" #include "dpu_rm.h" #include "dpu_core_perf.h" +#include "dpu_dbg.h" #define DRMID(x) ((x) ? (x)->base.id : -1) @@ -132,6 +133,8 @@ struct dpu_kms { struct opp_table *opp_table; + struct dpu_dbg_base *dpu_dbg; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can @@ -265,4 +268,6 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder); */ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name); +void dpu_kms_dump_mdp_regs(struct drm_device *dev); + #endif /* __dpu_kms_H__ */ diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index b1a9b1b..1f0f6f2 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -62,6 +62,16 @@ struct dp_catalog_private { u8 aux_lut_cfg_index[PHY_AUX_CFG_MAX]; }; +void dp_catalog_dump_dbg_regs(struct dp_catalog *dp_catalog, u32 **regs, u8 reg_dump_method, + struct drm_printer *p) +{ + struct dp_catalog_private *catalog = container_of(dp_catalog, + struct dp_catalog_private, dp_catalog); + + dpu_dbg_dump_regs(regs, catalog->io->dp_controller.len, + catalog->io->dp_controller.base, reg_dump_method, p); +} + static inline u32 dp_read_aux(struct dp_catalog_private *catalog, u32 offset) { offset += MSM_DP_CONTROLLER_AUX_OFFSET; diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h index 176a902..4b0f77f 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.h +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h @@ -9,6 +9,7 @@ #include <drm/drm_modes.h> #include "dp_parser.h" +#include "dpu_dbg.h" /* interrupts */ #define DP_INTR_HPD BIT(0) @@ -71,6 +72,10 @@ struct dp_catalog { u32 audio_data; }; +/* Debug module */ +void dp_catalog_dump_dbg_regs(struct dp_catalog *dp_catalog, u32 **regs, u8 reg_dump_method, + struct drm_printer *p); + /* AUX APIs */ u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog); int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 5a39da6..b0edfbf 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1009,6 +1009,43 @@ int dp_display_get_test_bpp(struct msm_dp *dp) dp_display->link->test_video.test_bit_depth); } +void msm_dp_dump_regs(struct msm_dp *dp) +{ + struct dp_display_private *dp_display; + struct drm_device *drm; + struct dpu_dbg_base *dpu_dbg; + + dp_display = container_of(dp, struct dp_display_private, dp_display); + drm = dp->drm_dev; + dpu_dbg = dpu_dbg_get(drm); + + /* + * if we are reading registers we need the link clocks to be on + * however till DP cable is connected this will not happen as we + * do not know the resolution to power up with. Hence check the + * power_on status before dumping DP registers to avoid crash due + * to unclocked access + */ + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) { + mutex_lock(&dp_display->event_mutex); + if (!dp->power_on) { + mutex_unlock(&dp_display->event_mutex); + return; + } + } + + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && !dpu_dbg->dpu_dbg_printer) { + pr_err("invalid drm printer\n"); + return; + } + + dp_catalog_dump_dbg_regs(dp_display->catalog, &dpu_dbg->dp_ctrl_regs, + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + mutex_unlock(&dp_display->event_mutex); +} + static void dp_display_config_hpd(struct dp_display_private *dp) { diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index 6092ba1..aa66c3d 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -8,6 +8,7 @@ #include "dp_panel.h" #include <sound/hdmi-codec.h> +#include "dpu_dbg.h" struct msm_dp { struct drm_device *drm_dev; diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 62704885..9a86f5e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -266,3 +266,8 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, return ret; } +void msm_dsi_dump_regs(struct msm_dsi *msm_dsi) +{ + msm_dsi_host_dump_regs(msm_dsi->host); +} + diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 78ef5d4..c9fccc4 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -15,6 +15,7 @@ #include <drm/drm_panel.h> #include "msm_drv.h" +#include "dpu_dbg.h" #define DSI_0 0 #define DSI_1 1 @@ -102,6 +103,8 @@ static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi) return msm_dsi->panel || msm_dsi->external_bridge; } +void msm_dsi_dump_regs(struct msm_dsi *msm_dsi); + struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi); /* dsi pll */ @@ -197,6 +200,7 @@ int dsi_clk_init_v2(struct msm_dsi_host *msm_host); int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host); int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_dual_dsi); int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_dual_dsi); +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host); /* dsi phy */ struct msm_dsi_phy; diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index ab281cb..d1675ee 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -2489,3 +2489,28 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) return of_drm_find_bridge(msm_host->device_node); } + +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) +{ + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + struct drm_device *dev = msm_host->dev; + struct dpu_dbg_base *dpu_dbg; + + dpu_dbg = dpu_dbg_get(dev); + + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && + !dpu_dbg->dpu_dbg_printer) { + pr_err("invalid drm printer\n"); + return; + } + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_get_sync(&msm_host->pdev->dev); + + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), msm_host->ctrl_base, + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); + + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) + pm_runtime_put_sync(&msm_host->pdev->dev); +} diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 898e6d0..c4405b6 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2018, 2020-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark <robdclark@gmail.com> */ @@ -19,6 +19,7 @@ #include <drm/drm_of.h> #include <drm/drm_vblank.h> +#include "dpu_dbg.h" #include "msm_drv.h" #include "msm_debugfs.h" #include "msm_fence.h" @@ -166,6 +167,24 @@ void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name, return _msm_ioremap(pdev, name, dbgname, true); } +unsigned long msm_iomap_size(struct platform_device *pdev, const char *name) +{ + struct resource *res; + + if (name) + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name); + else + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + if (!res) { + dev_dbg(&pdev->dev, "failed to get memory resource: %s\n", + name); + return 0; + } + + return resource_size(res); +} + void msm_writel(u32 data, void __iomem *addr) { if (reglog) @@ -277,6 +296,8 @@ static int msm_drm_uninit(struct device *dev) msm_fbdev_free(ddev); #endif + dpu_dbg_destroy(ddev); + drm_mode_config_cleanup(ddev); pm_runtime_get_sync(dev); @@ -549,6 +570,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) if (ret) goto err_msm_uninit; + if (get_mdp_ver(pdev) == KMS_DPU) { + ret = dpu_dbg_init(ddev); + if (ret) + DRM_DEV_ERROR(dev, "dpu_dbg_init failed ret = %d\n", ret); + } + drm_mode_config_reset(ddev); #ifdef CONFIG_DRM_FBDEV_EMULATION diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 6a42cdf..b8de6f3 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -362,6 +362,7 @@ void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); void msm_dp_irq_postinstall(struct msm_dp *dp_display); +void msm_dp_dump_regs(struct msm_dp *dp_display); void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor); @@ -445,6 +446,7 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name, const char *dbgname); void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name, const char *dbgname); +unsigned long msm_iomap_size(struct platform_device *pdev, const char *name); void msm_writel(u32 data, void __iomem *addr); u32 msm_readl(const void __iomem *addr); void msm_rmw(void __iomem *addr, u32 mask, u32 or); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers 2021-04-09 2:28 ` Abhinav Kumar (?) @ 2021-04-09 6:41 ` kernel test robot -1 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-04-09 6:41 UTC (permalink / raw) To: Abhinav Kumar, dri-devel Cc: kbuild-all, linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno [-- Attachment #1: Type: text/plain, Size: 3727 bytes --] Hi Abhinav, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.12-rc6 next-20210408] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 git checkout 8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_free_blk_mem': >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:183:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 183 | struct dpu_hw_mdp *top; | ^~~ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_init_blk_info': drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:230:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 230 | struct dpu_hw_mdp *top; | ^~~ vim +/top +183 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c 177 178 void dpu_dbg_free_blk_mem(struct drm_device *drm_dev) 179 { 180 struct msm_drm_private *priv; 181 struct dpu_kms *dpu_kms; 182 struct dpu_mdss_cfg *cat; > 183 struct dpu_hw_mdp *top; 184 struct dpu_dbg_base *dpu_dbg; 185 int i; 186 187 priv = drm_dev->dev_private; 188 dpu_kms = to_dpu_kms(priv->kms); 189 dpu_dbg = dpu_kms->dpu_dbg; 190 191 cat = dpu_kms->catalog; 192 top = dpu_kms->hw_mdp; 193 194 /* free CTL sub-blocks mem */ 195 for (i = 0; i < cat->ctl_count; i++) 196 kfree(dpu_dbg->mdp_regs->ctl[i]); 197 198 /* free DSPP sub-blocks mem */ 199 for (i = 0; i < cat->dspp_count; i++) 200 kfree(dpu_dbg->mdp_regs->dspp[i]); 201 202 /* free INTF sub-blocks mem */ 203 for (i = 0; i < cat->intf_count; i++) 204 kfree(dpu_dbg->mdp_regs->intf[i]); 205 206 /* free INTF sub-blocks mem */ 207 for (i = 0; i < cat->pingpong_count; i++) 208 kfree(dpu_dbg->mdp_regs->pp[i]); 209 210 /* free SSPP sub-blocks mem */ 211 for (i = 0; i < cat->sspp_count; i++) 212 kfree(dpu_dbg->mdp_regs->sspp[i]); 213 214 /* free TOP sub-blocks mem */ 215 kfree(dpu_dbg->mdp_regs->top); 216 217 /* free DSI regs mem */ 218 for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) 219 kfree(dpu_dbg->dsi_ctrl_regs[i]); 220 221 /* free DP regs mem */ 222 kfree(dpu_dbg->dp_ctrl_regs); 223 } 224 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 54383 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers @ 2021-04-09 6:41 ` kernel test robot 0 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-04-09 6:41 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3823 bytes --] Hi Abhinav, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.12-rc6 next-20210408] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 git checkout 8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_free_blk_mem': >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:183:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 183 | struct dpu_hw_mdp *top; | ^~~ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_init_blk_info': drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:230:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 230 | struct dpu_hw_mdp *top; | ^~~ vim +/top +183 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c 177 178 void dpu_dbg_free_blk_mem(struct drm_device *drm_dev) 179 { 180 struct msm_drm_private *priv; 181 struct dpu_kms *dpu_kms; 182 struct dpu_mdss_cfg *cat; > 183 struct dpu_hw_mdp *top; 184 struct dpu_dbg_base *dpu_dbg; 185 int i; 186 187 priv = drm_dev->dev_private; 188 dpu_kms = to_dpu_kms(priv->kms); 189 dpu_dbg = dpu_kms->dpu_dbg; 190 191 cat = dpu_kms->catalog; 192 top = dpu_kms->hw_mdp; 193 194 /* free CTL sub-blocks mem */ 195 for (i = 0; i < cat->ctl_count; i++) 196 kfree(dpu_dbg->mdp_regs->ctl[i]); 197 198 /* free DSPP sub-blocks mem */ 199 for (i = 0; i < cat->dspp_count; i++) 200 kfree(dpu_dbg->mdp_regs->dspp[i]); 201 202 /* free INTF sub-blocks mem */ 203 for (i = 0; i < cat->intf_count; i++) 204 kfree(dpu_dbg->mdp_regs->intf[i]); 205 206 /* free INTF sub-blocks mem */ 207 for (i = 0; i < cat->pingpong_count; i++) 208 kfree(dpu_dbg->mdp_regs->pp[i]); 209 210 /* free SSPP sub-blocks mem */ 211 for (i = 0; i < cat->sspp_count; i++) 212 kfree(dpu_dbg->mdp_regs->sspp[i]); 213 214 /* free TOP sub-blocks mem */ 215 kfree(dpu_dbg->mdp_regs->top); 216 217 /* free DSI regs mem */ 218 for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) 219 kfree(dpu_dbg->dsi_ctrl_regs[i]); 220 221 /* free DP regs mem */ 222 kfree(dpu_dbg->dp_ctrl_regs); 223 } 224 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 54383 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers @ 2021-04-09 6:41 ` kernel test robot 0 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-04-09 6:41 UTC (permalink / raw) To: Abhinav Kumar, dri-devel Cc: kbuild-all, linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno [-- Attachment #1: Type: text/plain, Size: 3727 bytes --] Hi Abhinav, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.12-rc6 next-20210408] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20210409-103135 git checkout 8ea10a48ffac0ff4eeda3e65ed93d8e5c2f0d4d4 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_free_blk_mem': >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:183:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 183 | struct dpu_hw_mdp *top; | ^~~ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c: In function 'dpu_dbg_init_blk_info': drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:230:21: warning: variable 'top' set but not used [-Wunused-but-set-variable] 230 | struct dpu_hw_mdp *top; | ^~~ vim +/top +183 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c 177 178 void dpu_dbg_free_blk_mem(struct drm_device *drm_dev) 179 { 180 struct msm_drm_private *priv; 181 struct dpu_kms *dpu_kms; 182 struct dpu_mdss_cfg *cat; > 183 struct dpu_hw_mdp *top; 184 struct dpu_dbg_base *dpu_dbg; 185 int i; 186 187 priv = drm_dev->dev_private; 188 dpu_kms = to_dpu_kms(priv->kms); 189 dpu_dbg = dpu_kms->dpu_dbg; 190 191 cat = dpu_kms->catalog; 192 top = dpu_kms->hw_mdp; 193 194 /* free CTL sub-blocks mem */ 195 for (i = 0; i < cat->ctl_count; i++) 196 kfree(dpu_dbg->mdp_regs->ctl[i]); 197 198 /* free DSPP sub-blocks mem */ 199 for (i = 0; i < cat->dspp_count; i++) 200 kfree(dpu_dbg->mdp_regs->dspp[i]); 201 202 /* free INTF sub-blocks mem */ 203 for (i = 0; i < cat->intf_count; i++) 204 kfree(dpu_dbg->mdp_regs->intf[i]); 205 206 /* free INTF sub-blocks mem */ 207 for (i = 0; i < cat->pingpong_count; i++) 208 kfree(dpu_dbg->mdp_regs->pp[i]); 209 210 /* free SSPP sub-blocks mem */ 211 for (i = 0; i < cat->sspp_count; i++) 212 kfree(dpu_dbg->mdp_regs->sspp[i]); 213 214 /* free TOP sub-blocks mem */ 215 kfree(dpu_dbg->mdp_regs->top); 216 217 /* free DSI regs mem */ 218 for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) 219 kfree(dpu_dbg->dsi_ctrl_regs[i]); 220 221 /* free DP regs mem */ 222 kfree(dpu_dbg->dp_ctrl_regs); 223 } 224 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 54383 bytes --] [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers 2021-04-09 2:28 ` Abhinav Kumar @ 2021-04-09 20:38 ` Rob Clark -1 siblings, 0 replies; 15+ messages in thread From: Rob Clark @ 2021-04-09 20:38 UTC (permalink / raw) To: Abhinav Kumar Cc: dri-devel, linux-arm-msm, freedreno, Sean Paul, Stephen Boyd, nganji, aravindh, Kuogee Hsieh, Daniel Vetter On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar <abhinavk@codeaurora.org> wrote: > > Add the dpu_dbg module which adds supports to dump dpu registers > which can be used in case of error conditions. > > changes in v3: > - Get rid of registration mechanism for sub-modules and instead get > this information from the dpu catalog itself > - Get rid of global dpu_dbg struct and instead store it in dpu_kms > - delegate the power management of the sub-modules to the resp drivers > - refactor and remove the linked list logic and simplify it to have > just an array > > Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > drivers/gpu/drm/msm/Makefile | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 +++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 +++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + > drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + > drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + > drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dsi/dsi.c | 5 + > drivers/gpu/drm/msm/dsi/dsi.h | 4 + > drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ > drivers/gpu/drm/msm/msm_drv.c | 29 ++- > drivers/gpu/drm/msm/msm_drv.h | 2 + > 16 files changed, 889 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c > [snip] > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > new file mode 100644 > index 0000000..302205a > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > @@ -0,0 +1,200 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. > + */ > + > +#ifndef DPU_DBG_H_ > +#define DPU_DBG_H_ > + > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_device.h> > +#include "../../../drm_crtc_internal.h" > +#include <drm/drm_print.h> > +#include <drm/drm_atomic.h> > +#include <linux/debugfs.h> > +#include <linux/list.h> > +#include <linux/delay.h> > +#include <linux/spinlock.h> > +#include <linux/ktime.h> > +#include <linux/debugfs.h> > +#include <linux/uaccess.h> > +#include <linux/dma-buf.h> > +#include <linux/slab.h> > +#include <linux/list_sort.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > +#include <linux/kthread.h> > +#include <linux/devcoredump.h> > +#include <stdarg.h> > +#include "dpu_hw_catalog.h" > +#include "dpu_kms.h" > +#include "dsi.h" > + > +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) > + > +enum dpu_dbg_dump_flag { > + DPU_DBG_DUMP_IN_LOG = BIT(0), > + DPU_DBG_DUMP_IN_MEM = BIT(1), > + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), > +}; overall, I like this better, but.. I'm not completely convinced about the need for DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side of things, and the only case I can think of where it might be useful is if you can't boot far enough to get to some minimal userspace.. once you have at least some minimal userspace, you can just pull out and clear the devcore dump via sysfs. That said, if state snapshot and printing were better separated it would just take a few lines of code to use a different drm_printer to print the state snapshot to dmesg. [snip] > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index ab281cb..d1675ee 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -2489,3 +2489,28 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) > > return of_drm_find_bridge(msm_host->device_node); > } > + > +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) > +{ > + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > + struct drm_device *dev = msm_host->dev; > + struct dpu_dbg_base *dpu_dbg; > + > + dpu_dbg = dpu_dbg_get(dev); > + > + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && > + !dpu_dbg->dpu_dbg_printer) { > + pr_err("invalid drm printer\n"); > + return; > + } for example, here ^^^ why should the other blocks even care? All they should know is that they've been asked to snapshot their state.. > + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) > + pm_runtime_get_sync(&msm_host->pdev->dev); > + > + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], > + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), msm_host->ctrl_base, > + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); > + > + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) > + pm_runtime_put_sync(&msm_host->pdev->dev); > +} I'm thinking something more along the lines of (in drm/msm/disp so it is a little less weird for dsi/dp/etc to be #include'ing header with structs): struct msm_disp_state { .. maybe a bit of generic information like kernel version, time, etc ... struct list_head blocks; /* list of msm_disp_state_block */ }; struct msm_disp_state_block { const char *name; struct list_head node; /* node in msm_disp_state::blocks */ unsigned size; uint32_t state[]; }; struct msm_disp_state * msm_disp_snapshot_state(struct drm_device *dev) { struct msm_disp_state *state = ...; if (priv->kms) priv->kms->snapshot(priv->kms, state); for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { msm_dsi_snapshot(priv->dsi[i], state); if (priv->dp) msm_dp_snapshot(priv->dp, state); return state; } ... maybe add a helper that can be used everywhere to allocate and append a new state block.. Ie. pass it state/name/len/iomem void msm_disp_state_free(struct msm_disp_state *state) { list_for_each_entry_safe (block, ...) kfree(block); kfree(state) } void msm_disp_state_print(struct drm_disp_state *state, drm_printer *p) { .. print generic state and loop over state->blocks printing them .. } ... if you do need to support printing to dmesg, you could re-use msm_disp_state_print() with an drm_info printer in just a few lines of code, rather than spreading the logic far and wide. BR, -R ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers @ 2021-04-09 20:38 ` Rob Clark 0 siblings, 0 replies; 15+ messages in thread From: Rob Clark @ 2021-04-09 20:38 UTC (permalink / raw) To: Abhinav Kumar Cc: linux-arm-msm, dri-devel, Stephen Boyd, Kuogee Hsieh, Sean Paul, aravindh, freedreno On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar <abhinavk@codeaurora.org> wrote: > > Add the dpu_dbg module which adds supports to dump dpu registers > which can be used in case of error conditions. > > changes in v3: > - Get rid of registration mechanism for sub-modules and instead get > this information from the dpu catalog itself > - Get rid of global dpu_dbg struct and instead store it in dpu_kms > - delegate the power management of the sub-modules to the resp drivers > - refactor and remove the linked list logic and simplify it to have > just an array > > Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > drivers/gpu/drm/msm/Makefile | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 +++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 +++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 +++++++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + > drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + > drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + > drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dsi/dsi.c | 5 + > drivers/gpu/drm/msm/dsi/dsi.h | 4 + > drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ > drivers/gpu/drm/msm/msm_drv.c | 29 ++- > drivers/gpu/drm/msm/msm_drv.h | 2 + > 16 files changed, 889 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c > [snip] > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > new file mode 100644 > index 0000000..302205a > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h > @@ -0,0 +1,200 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. > + */ > + > +#ifndef DPU_DBG_H_ > +#define DPU_DBG_H_ > + > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_device.h> > +#include "../../../drm_crtc_internal.h" > +#include <drm/drm_print.h> > +#include <drm/drm_atomic.h> > +#include <linux/debugfs.h> > +#include <linux/list.h> > +#include <linux/delay.h> > +#include <linux/spinlock.h> > +#include <linux/ktime.h> > +#include <linux/debugfs.h> > +#include <linux/uaccess.h> > +#include <linux/dma-buf.h> > +#include <linux/slab.h> > +#include <linux/list_sort.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > +#include <linux/kthread.h> > +#include <linux/devcoredump.h> > +#include <stdarg.h> > +#include "dpu_hw_catalog.h" > +#include "dpu_kms.h" > +#include "dsi.h" > + > +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) > + > +enum dpu_dbg_dump_flag { > + DPU_DBG_DUMP_IN_LOG = BIT(0), > + DPU_DBG_DUMP_IN_MEM = BIT(1), > + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), > +}; overall, I like this better, but.. I'm not completely convinced about the need for DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side of things, and the only case I can think of where it might be useful is if you can't boot far enough to get to some minimal userspace.. once you have at least some minimal userspace, you can just pull out and clear the devcore dump via sysfs. That said, if state snapshot and printing were better separated it would just take a few lines of code to use a different drm_printer to print the state snapshot to dmesg. [snip] > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index ab281cb..d1675ee 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -2489,3 +2489,28 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) > > return of_drm_find_bridge(msm_host->device_node); > } > + > +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) > +{ > + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > + struct drm_device *dev = msm_host->dev; > + struct dpu_dbg_base *dpu_dbg; > + > + dpu_dbg = dpu_dbg_get(dev); > + > + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && > + !dpu_dbg->dpu_dbg_printer) { > + pr_err("invalid drm printer\n"); > + return; > + } for example, here ^^^ why should the other blocks even care? All they should know is that they've been asked to snapshot their state.. > + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) > + pm_runtime_get_sync(&msm_host->pdev->dev); > + > + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], > + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), msm_host->ctrl_base, > + dpu_dbg->reg_dump_method, dpu_dbg->dpu_dbg_printer); > + > + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) > + pm_runtime_put_sync(&msm_host->pdev->dev); > +} I'm thinking something more along the lines of (in drm/msm/disp so it is a little less weird for dsi/dp/etc to be #include'ing header with structs): struct msm_disp_state { .. maybe a bit of generic information like kernel version, time, etc ... struct list_head blocks; /* list of msm_disp_state_block */ }; struct msm_disp_state_block { const char *name; struct list_head node; /* node in msm_disp_state::blocks */ unsigned size; uint32_t state[]; }; struct msm_disp_state * msm_disp_snapshot_state(struct drm_device *dev) { struct msm_disp_state *state = ...; if (priv->kms) priv->kms->snapshot(priv->kms, state); for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { msm_dsi_snapshot(priv->dsi[i], state); if (priv->dp) msm_dp_snapshot(priv->dp, state); return state; } ... maybe add a helper that can be used everywhere to allocate and append a new state block.. Ie. pass it state/name/len/iomem void msm_disp_state_free(struct msm_disp_state *state) { list_for_each_entry_safe (block, ...) kfree(block); kfree(state) } void msm_disp_state_print(struct drm_disp_state *state, drm_printer *p) { .. print generic state and loop over state->blocks printing them .. } ... if you do need to support printing to dmesg, you could re-use msm_disp_state_print() with an drm_info printer in just a few lines of code, rather than spreading the logic far and wide. BR, -R _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers 2021-04-09 20:38 ` Rob Clark @ 2021-04-09 21:20 ` abhinavk -1 siblings, 0 replies; 15+ messages in thread From: abhinavk @ 2021-04-09 21:20 UTC (permalink / raw) To: Rob Clark Cc: dri-devel, linux-arm-msm, freedreno, Sean Paul, Stephen Boyd, nganji, aravindh, Kuogee Hsieh, Daniel Vetter Hi Rob Thank you for the review. On 2021-04-09 13:38, Rob Clark wrote: > On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar <abhinavk@codeaurora.org> > wrote: >> >> Add the dpu_dbg module which adds supports to dump dpu registers >> which can be used in case of error conditions. >> >> changes in v3: >> - Get rid of registration mechanism for sub-modules and instead get >> this information from the dpu catalog itself >> - Get rid of global dpu_dbg struct and instead store it in dpu_kms >> - delegate the power management of the sub-modules to the resp >> drivers >> - refactor and remove the linked list logic and simplify it to have >> just an array >> >> Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> >> --- >> drivers/gpu/drm/msm/Makefile | 2 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 >> +++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 >> +++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 >> +++++++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + >> drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + >> drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + >> drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ >> drivers/gpu/drm/msm/dp/dp_display.h | 1 + >> drivers/gpu/drm/msm/dsi/dsi.c | 5 + >> drivers/gpu/drm/msm/dsi/dsi.h | 4 + >> drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ >> drivers/gpu/drm/msm/msm_drv.c | 29 ++- >> drivers/gpu/drm/msm/msm_drv.h | 2 + >> 16 files changed, 889 insertions(+), 2 deletions(-) >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c >> > > [snip] > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> new file mode 100644 >> index 0000000..302205a >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> @@ -0,0 +1,200 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2020-2021, The Linux Foundation. All rights >> reserved. >> + */ >> + >> +#ifndef DPU_DBG_H_ >> +#define DPU_DBG_H_ >> + >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_device.h> >> +#include "../../../drm_crtc_internal.h" >> +#include <drm/drm_print.h> >> +#include <drm/drm_atomic.h> >> +#include <linux/debugfs.h> >> +#include <linux/list.h> >> +#include <linux/delay.h> >> +#include <linux/spinlock.h> >> +#include <linux/ktime.h> >> +#include <linux/debugfs.h> >> +#include <linux/uaccess.h> >> +#include <linux/dma-buf.h> >> +#include <linux/slab.h> >> +#include <linux/list_sort.h> >> +#include <linux/pm.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/kthread.h> >> +#include <linux/devcoredump.h> >> +#include <stdarg.h> >> +#include "dpu_hw_catalog.h" >> +#include "dpu_kms.h" >> +#include "dsi.h" >> + >> +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) >> + >> +enum dpu_dbg_dump_flag { >> + DPU_DBG_DUMP_IN_LOG = BIT(0), >> + DPU_DBG_DUMP_IN_MEM = BIT(1), >> + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), >> +}; > > overall, I like this better, but.. > > I'm not completely convinced about the need for > DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side > of things, and the only case I can think of where it might be useful > is if you can't boot far enough to get to some minimal userspace.. > once you have at least some minimal userspace, you can just pull out > and clear the devcore dump via sysfs. That said, if state snapshot > and printing were better separated it would just take a few lines of > code to use a different drm_printer to print the state snapshot to > dmesg. > > [snip] > Yes, dumping the registers to log is just a debug feature and also useful if CONFIG_DEVCOREDUMP is not enabled. Its also useful for developers who can just view the dumps in the logs. Yes, the way this is written it is already generic enough to just take a different drm_printer. For the log i use this: + if (DPU_DBG_DUMP_IN_CONSOLE) { + p = drm_info_printer(dpu_dbg->drm_dev->dev); + dpu_dbg->dpu_dbg_printer = &p; + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_LOG); + } For the coredump I use this, p = drm_coredump_printer(&iter); + + drm_printf(&p, "---\n"); + + drm_printf(&p, "module: " KBUILD_MODNAME "\n"); + drm_printf(&p, "dpu devcoredump\n"); + drm_printf(&p, "timestamp %lld\n", ktime_to_ns(dpu_dbg->timestamp)); + + dpu_dbg->dpu_dbg_printer = &p; + + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_COREDUMP); >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c >> b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index ab281cb..d1675ee 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -2489,3 +2489,28 @@ struct drm_bridge >> *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) >> >> return of_drm_find_bridge(msm_host->device_node); >> } >> + >> +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) >> +{ >> + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >> + struct drm_device *dev = msm_host->dev; >> + struct dpu_dbg_base *dpu_dbg; >> + >> + dpu_dbg = dpu_dbg_get(dev); >> + >> + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && >> + !dpu_dbg->dpu_dbg_printer) { >> + pr_err("invalid drm printer\n"); >> + return; >> + } > > for example, here ^^^ > > why should the other blocks even care? All they should know is that > they've been asked to snapshot their state.. > You are right, the printer check can be moved to one level up to dpu_dbg_print_regs(). That way it will avoid this redundant valid printer check in the dump_regs() function of the sub-modules. The name of the function is xxx_xxx_dump_regs() but maybe I can just rename this to capture_state(). The reason for having the same function to capture the state and also print it out is to avoid code duplication. >> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) >> + pm_runtime_get_sync(&msm_host->pdev->dev); >> + >> + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], >> + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), >> msm_host->ctrl_base, >> + dpu_dbg->reg_dump_method, >> dpu_dbg->dpu_dbg_printer); >> + >> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) >> + pm_runtime_put_sync(&msm_host->pdev->dev); >> +} > > I'm thinking something more along the lines of (in drm/msm/disp so it > is a little less weird for dsi/dp/etc to be #include'ing header with > structs): Are you suggesting we move the dpu_dbg to drm/msm/disp? I am okay with that. > > struct msm_disp_state { > .. maybe a bit of generic information like kernel version, time, > etc ... > > struct list_head blocks; /* list of msm_disp_state_block */ > }; > > struct msm_disp_state_block { > const char *name; > struct list_head node; /* node in msm_disp_state::blocks */ > unsigned size; > uint32_t state[]; > }; > > struct msm_disp_state * msm_disp_snapshot_state(struct drm_device *dev) > { > struct msm_disp_state *state = ...; > > if (priv->kms) > priv->kms->snapshot(priv->kms, state); > > for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { > msm_dsi_snapshot(priv->dsi[i], state); > > if (priv->dp) > msm_dp_snapshot(priv->dp, state); > > return state; > } This is essentially what the dpu_dbg_dump_regs does right now. It just delegates the dumping to the respective modules. +{ + int i; + + for (i = 0; i < DPU_DBG_BASE_MAX; i++) { + if (dpu_dbg->blk_names[i] != NULL) { + DRM_DEBUG("blk name is %s\n", dpu_dbg->blk_names[i]); + if (!strcmp(dpu_dbg->blk_names[i], "all")) { + dpu_dbg_dump_all_regs(dpu_dbg->drm_dev); + break; + } else if (!strcmp(dpu_dbg->blk_names[i], "mdp")) { + dpu_dbg_dump_mdp_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dsi")) { + dpu_dbg_dump_dsi_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dp")) { + dpu_dbg_dump_dp_regs(dpu_dbg->drm_dev); + } else { + DRM_ERROR("blk name not found %s\n", dpu_dbg->blk_names[i]); + } + } + } +} > > ... maybe add a helper that can be used everywhere to allocate and > append a new state block.. Ie. pass it state/name/len/iomem > > void msm_disp_state_free(struct msm_disp_state *state) > { > list_for_each_entry_safe (block, ...) > kfree(block); > kfree(state) > } > > void msm_disp_state_print(struct drm_disp_state *state, drm_printer *p) > { > .. print generic state and loop over state->blocks printing them .. > } > > ... if you do need to support printing to dmesg, you could re-use > msm_disp_state_print() with an drm_info printer in just a few lines of > code, rather than spreading the logic far and wide. > > BR, > -R ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers @ 2021-04-09 21:20 ` abhinavk 0 siblings, 0 replies; 15+ messages in thread From: abhinavk @ 2021-04-09 21:20 UTC (permalink / raw) To: Rob Clark Cc: linux-arm-msm, dri-devel, Stephen Boyd, Kuogee Hsieh, Sean Paul, aravindh, freedreno Hi Rob Thank you for the review. On 2021-04-09 13:38, Rob Clark wrote: > On Thu, Apr 8, 2021 at 7:28 PM Abhinav Kumar <abhinavk@codeaurora.org> > wrote: >> >> Add the dpu_dbg module which adds supports to dump dpu registers >> which can be used in case of error conditions. >> >> changes in v3: >> - Get rid of registration mechanism for sub-modules and instead get >> this information from the dpu catalog itself >> - Get rid of global dpu_dbg struct and instead store it in dpu_kms >> - delegate the power management of the sub-modules to the resp >> drivers >> - refactor and remove the linked list logic and simplify it to have >> just an array >> >> Change-Id: Ide975ecf5d7952ae44daaa6eb611e27d09630be5 >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> >> --- >> drivers/gpu/drm/msm/Makefile | 2 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 221 >> +++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 200 >> +++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 257 >> +++++++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 86 +++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 5 + >> drivers/gpu/drm/msm/dp/dp_catalog.c | 10 + >> drivers/gpu/drm/msm/dp/dp_catalog.h | 5 + >> drivers/gpu/drm/msm/dp/dp_display.c | 37 ++++ >> drivers/gpu/drm/msm/dp/dp_display.h | 1 + >> drivers/gpu/drm/msm/dsi/dsi.c | 5 + >> drivers/gpu/drm/msm/dsi/dsi.h | 4 + >> drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++ >> drivers/gpu/drm/msm/msm_drv.c | 29 ++- >> drivers/gpu/drm/msm/msm_drv.h | 2 + >> 16 files changed, 889 insertions(+), 2 deletions(-) >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c >> > > [snip] > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> new file mode 100644 >> index 0000000..302205a >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h >> @@ -0,0 +1,200 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2020-2021, The Linux Foundation. All rights >> reserved. >> + */ >> + >> +#ifndef DPU_DBG_H_ >> +#define DPU_DBG_H_ >> + >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_device.h> >> +#include "../../../drm_crtc_internal.h" >> +#include <drm/drm_print.h> >> +#include <drm/drm_atomic.h> >> +#include <linux/debugfs.h> >> +#include <linux/list.h> >> +#include <linux/delay.h> >> +#include <linux/spinlock.h> >> +#include <linux/ktime.h> >> +#include <linux/debugfs.h> >> +#include <linux/uaccess.h> >> +#include <linux/dma-buf.h> >> +#include <linux/slab.h> >> +#include <linux/list_sort.h> >> +#include <linux/pm.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/kthread.h> >> +#include <linux/devcoredump.h> >> +#include <stdarg.h> >> +#include "dpu_hw_catalog.h" >> +#include "dpu_kms.h" >> +#include "dsi.h" >> + >> +#define DPU_DBG_DUMP_DATA_LIMITER (NULL) >> + >> +enum dpu_dbg_dump_flag { >> + DPU_DBG_DUMP_IN_LOG = BIT(0), >> + DPU_DBG_DUMP_IN_MEM = BIT(1), >> + DPU_DBG_DUMP_IN_COREDUMP = BIT(2), >> +}; > > overall, I like this better, but.. > > I'm not completely convinced about the need for > DUMP_IN_LOG/DUMP_IN_MEM.. we haven't really needed it on the GPU side > of things, and the only case I can think of where it might be useful > is if you can't boot far enough to get to some minimal userspace.. > once you have at least some minimal userspace, you can just pull out > and clear the devcore dump via sysfs. That said, if state snapshot > and printing were better separated it would just take a few lines of > code to use a different drm_printer to print the state snapshot to > dmesg. > > [snip] > Yes, dumping the registers to log is just a debug feature and also useful if CONFIG_DEVCOREDUMP is not enabled. Its also useful for developers who can just view the dumps in the logs. Yes, the way this is written it is already generic enough to just take a different drm_printer. For the log i use this: + if (DPU_DBG_DUMP_IN_CONSOLE) { + p = drm_info_printer(dpu_dbg->drm_dev->dev); + dpu_dbg->dpu_dbg_printer = &p; + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_LOG); + } For the coredump I use this, p = drm_coredump_printer(&iter); + + drm_printf(&p, "---\n"); + + drm_printf(&p, "module: " KBUILD_MODNAME "\n"); + drm_printf(&p, "dpu devcoredump\n"); + drm_printf(&p, "timestamp %lld\n", ktime_to_ns(dpu_dbg->timestamp)); + + dpu_dbg->dpu_dbg_printer = &p; + + dpu_dbg_print_regs(dpu_dbg->drm_dev, DPU_DBG_DUMP_IN_COREDUMP); >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c >> b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index ab281cb..d1675ee 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -2489,3 +2489,28 @@ struct drm_bridge >> *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) >> >> return of_drm_find_bridge(msm_host->device_node); >> } >> + >> +void msm_dsi_host_dump_regs(struct mipi_dsi_host *host) >> +{ >> + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >> + struct drm_device *dev = msm_host->dev; >> + struct dpu_dbg_base *dpu_dbg; >> + >> + dpu_dbg = dpu_dbg_get(dev); >> + >> + if (dpu_dbg_is_drm_printer_needed(dpu_dbg) && >> + !dpu_dbg->dpu_dbg_printer) { >> + pr_err("invalid drm printer\n"); >> + return; >> + } > > for example, here ^^^ > > why should the other blocks even care? All they should know is that > they've been asked to snapshot their state.. > You are right, the printer check can be moved to one level up to dpu_dbg_print_regs(). That way it will avoid this redundant valid printer check in the dump_regs() function of the sub-modules. The name of the function is xxx_xxx_dump_regs() but maybe I can just rename this to capture_state(). The reason for having the same function to capture the state and also print it out is to avoid code duplication. >> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) >> + pm_runtime_get_sync(&msm_host->pdev->dev); >> + >> + dpu_dbg_dump_regs(&dpu_dbg->dsi_ctrl_regs[msm_host->id], >> + msm_iomap_size(msm_host->pdev, "dsi_ctrl"), >> msm_host->ctrl_base, >> + dpu_dbg->reg_dump_method, >> dpu_dbg->dpu_dbg_printer); >> + >> + if (dpu_dbg->reg_dump_method == DPU_DBG_DUMP_IN_MEM) >> + pm_runtime_put_sync(&msm_host->pdev->dev); >> +} > > I'm thinking something more along the lines of (in drm/msm/disp so it > is a little less weird for dsi/dp/etc to be #include'ing header with > structs): Are you suggesting we move the dpu_dbg to drm/msm/disp? I am okay with that. > > struct msm_disp_state { > .. maybe a bit of generic information like kernel version, time, > etc ... > > struct list_head blocks; /* list of msm_disp_state_block */ > }; > > struct msm_disp_state_block { > const char *name; > struct list_head node; /* node in msm_disp_state::blocks */ > unsigned size; > uint32_t state[]; > }; > > struct msm_disp_state * msm_disp_snapshot_state(struct drm_device *dev) > { > struct msm_disp_state *state = ...; > > if (priv->kms) > priv->kms->snapshot(priv->kms, state); > > for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) { > msm_dsi_snapshot(priv->dsi[i], state); > > if (priv->dp) > msm_dp_snapshot(priv->dp, state); > > return state; > } This is essentially what the dpu_dbg_dump_regs does right now. It just delegates the dumping to the respective modules. +{ + int i; + + for (i = 0; i < DPU_DBG_BASE_MAX; i++) { + if (dpu_dbg->blk_names[i] != NULL) { + DRM_DEBUG("blk name is %s\n", dpu_dbg->blk_names[i]); + if (!strcmp(dpu_dbg->blk_names[i], "all")) { + dpu_dbg_dump_all_regs(dpu_dbg->drm_dev); + break; + } else if (!strcmp(dpu_dbg->blk_names[i], "mdp")) { + dpu_dbg_dump_mdp_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dsi")) { + dpu_dbg_dump_dsi_regs(dpu_dbg->drm_dev); + } else if (!strcmp(dpu_dbg->blk_names[i], "dp")) { + dpu_dbg_dump_dp_regs(dpu_dbg->drm_dev); + } else { + DRM_ERROR("blk name not found %s\n", dpu_dbg->blk_names[i]); + } + } + } +} > > ... maybe add a helper that can be used everywhere to allocate and > append a new state block.. Ie. pass it state/name/len/iomem > > void msm_disp_state_free(struct msm_disp_state *state) > { > list_for_each_entry_safe (block, ...) > kfree(block); > kfree(state) > } > > void msm_disp_state_print(struct drm_disp_state *state, drm_printer *p) > { > .. print generic state and loop over state->blocks printing them .. > } > > ... if you do need to support printing to dmesg, you could re-use > msm_disp_state_print() with an drm_info printer in just a few lines of > code, rather than spreading the logic far and wide. > > BR, > -R _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] drm/msm/dpu: add dpu_dbg points across dpu driver 2021-04-09 2:28 ` Abhinav Kumar @ 2021-04-09 2:28 ` Abhinav Kumar -1 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: Abhinav Kumar, linux-arm-msm, freedreno, robdclark, seanpaul, swboyd, nganji, aravindh, khsieh, daniel Add dpu_dbg points across dpu driver to trigger dumps when critical errors are hit. changes in v3: - change the callers to also pass the drm device while triggering the dump Change-Id: I351514afe2f1c85232a1562253e5671588075197 Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++++++++++++++--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 14 +++++++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 +++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 288e95e..d89c4e9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark <robdclark@gmail.com> */ @@ -26,6 +26,7 @@ #include "dpu_crtc.h" #include "dpu_trace.h" #include "dpu_core_irq.h" +#include "dpu_dbg.h" #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\ (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) @@ -1306,6 +1307,13 @@ static void dpu_encoder_underrun_callback(struct drm_encoder *drm_enc, DPU_ATRACE_BEGIN("encoder_underrun_callback"); atomic_inc(&phy_enc->underrun_cnt); + + /* trigger dump only on the first underrun */ + if (atomic_read(&phy_enc->underrun_cnt) == 1) { + pr_err("triggering dump\n"); + DPU_DBG_DUMP(drm_enc->dev, "all"); + } + trace_dpu_enc_underrun_cb(DRMID(drm_enc), atomic_read(&phy_enc->underrun_cnt)); DPU_ATRACE_END("encoder_underrun_callback"); @@ -1535,19 +1543,23 @@ static void dpu_encoder_helper_hw_reset(struct dpu_encoder_phys *phys_enc) struct dpu_encoder_virt *dpu_enc; struct dpu_hw_ctl *ctl; int rc; + struct drm_encoder *drm_enc; dpu_enc = to_dpu_encoder_virt(phys_enc->parent); ctl = phys_enc->hw_ctl; + drm_enc = phys_enc->parent; if (!ctl->ops.reset) return; - DRM_DEBUG_KMS("id:%u ctl %d reset\n", DRMID(phys_enc->parent), + DRM_DEBUG_KMS("id:%u ctl %d reset\n", DRMID(drm_enc), ctl->idx); rc = ctl->ops.reset(ctl); - if (rc) + if (rc) { DPU_ERROR_ENC(dpu_enc, "ctl %d reset failure\n", ctl->idx); + DPU_DBG_DUMP(drm_enc->dev, "all"); + } phys_enc->enable_state = DPU_ENC_ENABLED; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index b2be39b..fd5ddcb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2015-2018 The Linux Foundation. All rights reserved. + * Copyright (c) 2015-2018, 2020-2021 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ @@ -11,6 +11,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_CMDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->base.parent ? \ @@ -191,10 +192,13 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( to_dpu_encoder_phys_cmd(phys_enc); u32 frame_event = DPU_ENCODER_FRAME_EVENT_ERROR; bool do_log = false; + struct drm_encoder *drm_enc; if (!phys_enc->hw_pp) return -EINVAL; + drm_enc = phys_enc->parent; + cmd_enc->pp_timeout_report_cnt++; if (cmd_enc->pp_timeout_report_cnt == PP_TIMEOUT_MAX_TRIALS) { frame_event |= DPU_ENCODER_FRAME_EVENT_PANEL_DEAD; @@ -203,7 +207,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( do_log = true; } - trace_dpu_enc_phys_cmd_pdone_timeout(DRMID(phys_enc->parent), + trace_dpu_enc_phys_cmd_pdone_timeout(DRMID(drm_enc), phys_enc->hw_pp->idx - PINGPONG_0, cmd_enc->pp_timeout_report_cnt, atomic_read(&phys_enc->pending_kickoff_cnt), @@ -212,12 +216,12 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( /* to avoid flooding, only log first time, and "dead" time */ if (do_log) { DRM_ERROR("id:%d pp:%d kickoff timeout %d cnt %d koff_cnt %d\n", - DRMID(phys_enc->parent), + DRMID(drm_enc), phys_enc->hw_pp->idx - PINGPONG_0, phys_enc->hw_ctl->idx - CTL_0, cmd_enc->pp_timeout_report_cnt, atomic_read(&phys_enc->pending_kickoff_cnt)); - + DPU_DBG_DUMP(drm_enc->dev, "all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR); } @@ -228,7 +232,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( if (phys_enc->parent_ops->handle_frame_done) phys_enc->parent_ops->handle_frame_done( - phys_enc->parent, phys_enc, frame_event); + drm_enc, phys_enc, frame_event); return -ETIMEDOUT; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 9a69fad..4909e65 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020-2021 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ @@ -9,6 +9,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->parent ? \ @@ -468,6 +469,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) "update pending flush ctl %d intf %d\n", ctl->idx - CTL_0, phys_enc->hw_intf->idx); + atomic_set(&phys_enc->underrun_cnt, 0); /* ctl_flush & timing engine enable will be triggered by framework */ if (phys_enc->enable_state == DPU_ENC_DISABLED) @@ -537,6 +539,9 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( { struct dpu_hw_ctl *ctl; int rc; + struct drm_encoder *drm_enc; + + drm_enc = phys_enc->parent; ctl = phys_enc->hw_ctl; if (!ctl->ops.wait_reset_status) @@ -550,6 +555,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( if (rc) { DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n", ctl->idx, rc); + DPU_DBG_DUMP(drm_enc->dev, "all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC); } } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] drm/msm/dpu: add dpu_dbg points across dpu driver @ 2021-04-09 2:28 ` Abhinav Kumar 0 siblings, 0 replies; 15+ messages in thread From: Abhinav Kumar @ 2021-04-09 2:28 UTC (permalink / raw) To: dri-devel Cc: linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, aravindh, freedreno Add dpu_dbg points across dpu driver to trigger dumps when critical errors are hit. changes in v3: - change the callers to also pass the drm device while triggering the dump Change-Id: I351514afe2f1c85232a1562253e5671588075197 Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++++++++++++++--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 14 +++++++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 +++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 288e95e..d89c4e9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark <robdclark@gmail.com> */ @@ -26,6 +26,7 @@ #include "dpu_crtc.h" #include "dpu_trace.h" #include "dpu_core_irq.h" +#include "dpu_dbg.h" #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\ (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) @@ -1306,6 +1307,13 @@ static void dpu_encoder_underrun_callback(struct drm_encoder *drm_enc, DPU_ATRACE_BEGIN("encoder_underrun_callback"); atomic_inc(&phy_enc->underrun_cnt); + + /* trigger dump only on the first underrun */ + if (atomic_read(&phy_enc->underrun_cnt) == 1) { + pr_err("triggering dump\n"); + DPU_DBG_DUMP(drm_enc->dev, "all"); + } + trace_dpu_enc_underrun_cb(DRMID(drm_enc), atomic_read(&phy_enc->underrun_cnt)); DPU_ATRACE_END("encoder_underrun_callback"); @@ -1535,19 +1543,23 @@ static void dpu_encoder_helper_hw_reset(struct dpu_encoder_phys *phys_enc) struct dpu_encoder_virt *dpu_enc; struct dpu_hw_ctl *ctl; int rc; + struct drm_encoder *drm_enc; dpu_enc = to_dpu_encoder_virt(phys_enc->parent); ctl = phys_enc->hw_ctl; + drm_enc = phys_enc->parent; if (!ctl->ops.reset) return; - DRM_DEBUG_KMS("id:%u ctl %d reset\n", DRMID(phys_enc->parent), + DRM_DEBUG_KMS("id:%u ctl %d reset\n", DRMID(drm_enc), ctl->idx); rc = ctl->ops.reset(ctl); - if (rc) + if (rc) { DPU_ERROR_ENC(dpu_enc, "ctl %d reset failure\n", ctl->idx); + DPU_DBG_DUMP(drm_enc->dev, "all"); + } phys_enc->enable_state = DPU_ENC_ENABLED; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index b2be39b..fd5ddcb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2015-2018 The Linux Foundation. All rights reserved. + * Copyright (c) 2015-2018, 2020-2021 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ @@ -11,6 +11,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_CMDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->base.parent ? \ @@ -191,10 +192,13 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( to_dpu_encoder_phys_cmd(phys_enc); u32 frame_event = DPU_ENCODER_FRAME_EVENT_ERROR; bool do_log = false; + struct drm_encoder *drm_enc; if (!phys_enc->hw_pp) return -EINVAL; + drm_enc = phys_enc->parent; + cmd_enc->pp_timeout_report_cnt++; if (cmd_enc->pp_timeout_report_cnt == PP_TIMEOUT_MAX_TRIALS) { frame_event |= DPU_ENCODER_FRAME_EVENT_PANEL_DEAD; @@ -203,7 +207,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( do_log = true; } - trace_dpu_enc_phys_cmd_pdone_timeout(DRMID(phys_enc->parent), + trace_dpu_enc_phys_cmd_pdone_timeout(DRMID(drm_enc), phys_enc->hw_pp->idx - PINGPONG_0, cmd_enc->pp_timeout_report_cnt, atomic_read(&phys_enc->pending_kickoff_cnt), @@ -212,12 +216,12 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( /* to avoid flooding, only log first time, and "dead" time */ if (do_log) { DRM_ERROR("id:%d pp:%d kickoff timeout %d cnt %d koff_cnt %d\n", - DRMID(phys_enc->parent), + DRMID(drm_enc), phys_enc->hw_pp->idx - PINGPONG_0, phys_enc->hw_ctl->idx - CTL_0, cmd_enc->pp_timeout_report_cnt, atomic_read(&phys_enc->pending_kickoff_cnt)); - + DPU_DBG_DUMP(drm_enc->dev, "all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR); } @@ -228,7 +232,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( if (phys_enc->parent_ops->handle_frame_done) phys_enc->parent_ops->handle_frame_done( - phys_enc->parent, phys_enc, frame_event); + drm_enc, phys_enc, frame_event); return -ETIMEDOUT; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 9a69fad..4909e65 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020-2021 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ @@ -9,6 +9,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->parent ? \ @@ -468,6 +469,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) "update pending flush ctl %d intf %d\n", ctl->idx - CTL_0, phys_enc->hw_intf->idx); + atomic_set(&phys_enc->underrun_cnt, 0); /* ctl_flush & timing engine enable will be triggered by framework */ if (phys_enc->enable_state == DPU_ENC_DISABLED) @@ -537,6 +539,9 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( { struct dpu_hw_ctl *ctl; int rc; + struct drm_encoder *drm_enc; + + drm_enc = phys_enc->parent; ctl = phys_enc->hw_ctl; if (!ctl->ops.wait_reset_status) @@ -550,6 +555,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( if (rc) { DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n", ctl->idx, rc); + DPU_DBG_DUMP(drm_enc->dev, "all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC); } } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-04-09 21:21 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-09 2:28 [PATCH v3 0/3] Add devcoredump support for DPU Abhinav Kumar 2021-04-09 2:28 ` Abhinav Kumar 2021-04-09 2:28 ` [PATCH v3 1/3] drm: allow drm_atomic_print_state() to accept any drm_printer Abhinav Kumar 2021-04-09 2:28 ` Abhinav Kumar 2021-04-09 2:28 ` [PATCH v3 2/3] drm/msm/dpu: add support to dump dpu registers Abhinav Kumar 2021-04-09 2:28 ` Abhinav Kumar 2021-04-09 6:41 ` kernel test robot 2021-04-09 6:41 ` kernel test robot 2021-04-09 6:41 ` kernel test robot 2021-04-09 20:38 ` Rob Clark 2021-04-09 20:38 ` Rob Clark 2021-04-09 21:20 ` abhinavk 2021-04-09 21:20 ` abhinavk 2021-04-09 2:28 ` [PATCH v3 3/3] drm/msm/dpu: add dpu_dbg points across dpu driver Abhinav Kumar 2021-04-09 2:28 ` Abhinav Kumar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.