In preparation to register a iommu fault handler for display
related modules, register a fault handler for the backing
mmu object of msm_kms.
Currently, the fault handler only captures the display snapshot
but we can expand this later if more information needs to be
added to debug display mmu faults.
Signed-off-by: Abhinav Kumar <[email protected]>
---
drivers/gpu/drm/msm/msm_kms.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index af6a6fcb1173..62c8e6163e81 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
return aspace;
}
+static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, void *data)
+{
+ struct msm_kms *kms = arg;
+ struct msm_disp_state *state;
+ int ret;
+
+ ret = mutex_lock_interruptible(&kms->dump_mutex);
+ if (ret)
+ return ret;
+
+ state = msm_disp_snapshot_state_sync(kms);
+
+ mutex_unlock(&kms->dump_mutex);
+
+ if (IS_ERR(state)) {
+ DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
+ return PTR_ERR(state);
+ }
+
+ return 0;
+}
+
void msm_drm_kms_uninit(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
goto err_msm_uninit;
}
+ if (kms->aspace)
+ msm_mmu_set_fault_handler(kms->aspace->mmu, kms, msm_kms_fault_handler);
+
drm_helper_move_panel_connectors_to_head(ddev);
drm_for_each_crtc(crtc, ddev) {
--
2.44.0
On Fri, May 17, 2024 at 04:37:56PM -0700, Abhinav Kumar wrote:
> In preparation to register a iommu fault handler for display
> related modules, register a fault handler for the backing
> mmu object of msm_kms.
>
> Currently, the fault handler only captures the display snapshot
> but we can expand this later if more information needs to be
> added to debug display mmu faults.
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_kms.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&kms->dump_mutex);
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(&kms->dump_mutex);
> +
> + if (IS_ERR(state)) {
> + DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
> + return PTR_ERR(state);
> + }
> +
> + return 0;
> +}
> +
> void msm_drm_kms_uninit(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> goto err_msm_uninit;
> }
>
> + if (kms->aspace)
> + msm_mmu_set_fault_handler(kms->aspace->mmu, kms, msm_kms_fault_handler);
> +
Can we move this to msm_kms_init_aspace() instead of checking for
kms->aspace?
> drm_helper_move_panel_connectors_to_head(ddev);
>
> drm_for_each_crtc(crtc, ddev) {
> --
> 2.44.0
>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 04:37:56PM -0700, Abhinav Kumar wrote:
> In preparation to register a iommu fault handler for display
> related modules, register a fault handler for the backing
> mmu object of msm_kms.
>
> Currently, the fault handler only captures the display snapshot
> but we can expand this later if more information needs to be
> added to debug display mmu faults.
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_kms.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&kms->dump_mutex);
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(&kms->dump_mutex);
> +
> + if (IS_ERR(state)) {
> + DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
> + return PTR_ERR(state);
> + }
> +
> + return 0;
Hmm, after reading the rest of the code, this means that we won't get
the error on the console. Could you please change this to -ENOSYS?
> +}
> +
> void msm_drm_kms_uninit(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> goto err_msm_uninit;
> }
>
> + if (kms->aspace)
> + msm_mmu_set_fault_handler(kms->aspace->mmu, kms, msm_kms_fault_handler);
> +
> drm_helper_move_panel_connectors_to_head(ddev);
>
> drm_for_each_crtc(crtc, ddev) {
> --
> 2.44.0
>
--
With best wishes
Dmitry
Quoting Abhinav Kumar (2024-05-17 16:37:56)
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&kms->dump_mutex);
From past experience I've seen the smmu fault handler called in hardirq
context, so it can't sleep. Is there some way to grab the register
contents without sleeping? Otherwise this will have to fork off
somewhere else that can take locks, runtime PM resume, etc.
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(&kms->dump_mutex);
> +