| Message ID | 20240517233801.4071868-2-quic_abhinavk@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/4] drm/msm: register a fault handler for display mmu faults | 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);
On 5/19/2024 1:31 AM, Dmitry Baryshkov wrote: > 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? Ack. > >> drm_helper_move_panel_connectors_to_head(ddev); >> >> drm_for_each_crtc(crtc, ddev) { >> -- >> 2.44.0 >> >
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(+)