| Message ID | 20240517233801.4071868-2-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: add a display mmu fault handler | expand |
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 <quic_abhinavk@quicinc.com> > --- > 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 >
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 <quic_abhinavk@quicinc.com> > --- > 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 >
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); > +
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) {
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 <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/msm_kms.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)