On Thu, Oct 18, 2018 at 3:59 PM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote: > > Do some cleanups to the code related to debugfs files from across > the DPU driver including removing destroy functions, > tweeks to the initialization functions, removal of unused > static inline stubs and skipping unneeded null checks. > > Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> Reviewed-by: Bruce Wang <bzwang@xxxxxxxxxxxx> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 30 ++--------- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 9 +--- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 17 ------- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 30 ++--------- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 31 +++--------- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 50 ++++--------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 - > drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 3 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++----- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 11 +--- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 15 +----- > 11 files changed, 33 insertions(+), 182 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c > index f66070cb2f42..f188c78dd736 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c > @@ -307,10 +307,8 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) > unsigned long irq_flags; > int i, irq_count, enable_count, cb_count; > > - if (!irq_obj || !irq_obj->enable_counts || !irq_obj->irq_cb_tbl) { > - DPU_ERROR("invalid parameters\n"); > + if (WARN_ON(!irq_obj->enable_counts || !irq_obj->irq_cb_tbl)) > return 0; > - } > > for (i = 0; i < irq_obj->total_irqs; i++) { > spin_lock_irqsave(&irq_obj->cb_lock, irq_flags); > @@ -331,31 +329,11 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v) > > DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_core_irq); > > -int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, > - struct dentry *parent) > -{ > - dpu_kms->irq_obj.debugfs_file = debugfs_create_file("core_irq", 0600, > - parent, &dpu_kms->irq_obj, > - &dpu_debugfs_core_irq_fops); > - > - return 0; > -} > - > -void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms) > -{ > - debugfs_remove(dpu_kms->irq_obj.debugfs_file); > - dpu_kms->irq_obj.debugfs_file = NULL; > -} > - > -#else > -int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, > +void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, > struct dentry *parent) > { > - return 0; > -} > - > -void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms) > -{ > + debugfs_create_file("core_irq", 0600, parent, &dpu_kms->irq_obj, > + &dpu_debugfs_core_irq_fops); > } > #endif > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h > index 884f77fa3eb6..e9015a2b23fe 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h > @@ -132,15 +132,8 @@ int dpu_core_irq_unregister_callback( > * dpu_debugfs_core_irq_init - register core irq debugfs > * @dpu_kms: pointer to kms > * @parent: debugfs directory root > - * @Return: 0 on success > */ > -int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, > +void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms, > struct dentry *parent); > > -/** > - * dpu_debugfs_core_irq_destroy - deregister core irq debugfs > - * @dpu_kms: pointer to kms > - */ > -void dpu_debugfs_core_irq_destroy(struct dpu_kms *dpu_kms); > - > #endif /* __DPU_CORE_IRQ_H__ */ > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > index e68ccb7a898a..7bc887ed9d74 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > @@ -524,12 +524,6 @@ static const struct file_operations dpu_core_perf_mode_fops = { > .write = _dpu_core_perf_mode_write, > }; > > -static void dpu_core_perf_debugfs_destroy(struct dpu_core_perf *perf) > -{ > - debugfs_remove_recursive(perf->debugfs_root); > - perf->debugfs_root = NULL; > -} > - > int dpu_core_perf_debugfs_init(struct dpu_core_perf *perf, > struct dentry *parent) > { > @@ -578,16 +572,6 @@ int dpu_core_perf_debugfs_init(struct dpu_core_perf *perf, > > return 0; > } > -#else > -static void dpu_core_perf_debugfs_destroy(struct dpu_core_perf *perf) > -{ > -} > - > -int dpu_core_perf_debugfs_init(struct dpu_core_perf *perf, > - struct dentry *parent) > -{ > - return 0; > -} > #endif > > void dpu_core_perf_destroy(struct dpu_core_perf *perf) > @@ -597,7 +581,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf) > return; > } > > - dpu_core_perf_debugfs_destroy(perf); > perf->max_core_clk_rate = 0; > perf->core_clk = NULL; > perf->phandle = NULL; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index a8aa59b8caeb..890994106e39 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1246,9 +1246,6 @@ static int dpu_crtc_debugfs_status_show(struct seq_file *s, void *data) > > int i, out_width; > > - if (!s || !s->private) > - return -EINVAL; > - > dpu_crtc = s->private; > crtc = &dpu_crtc->base; > > @@ -1378,14 +1375,7 @@ DEFINE_SHOW_ATTRIBUTE(dpu_crtc_debugfs_state); > > static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc) > { > - struct dpu_crtc *dpu_crtc; > - struct dpu_kms *dpu_kms; > - > - if (!crtc) > - return -EINVAL; > - dpu_crtc = to_dpu_crtc(crtc); > - > - dpu_kms = _dpu_crtc_get_kms(crtc); > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > > dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name, > crtc->dev->primary->debugfs_root); > @@ -1403,25 +1393,11 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc) > > return 0; > } > - > -static void _dpu_crtc_destroy_debugfs(struct drm_crtc *crtc) > -{ > - struct dpu_crtc *dpu_crtc; > - > - if (!crtc) > - return; > - dpu_crtc = to_dpu_crtc(crtc); > - debugfs_remove_recursive(dpu_crtc->debugfs_root); > -} > #else > static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc) > { > return 0; > } > - > -static void _dpu_crtc_destroy_debugfs(struct drm_crtc *crtc) > -{ > -} > #endif /* CONFIG_DEBUG_FS */ > > static int dpu_crtc_late_register(struct drm_crtc *crtc) > @@ -1431,7 +1407,9 @@ static int dpu_crtc_late_register(struct drm_crtc *crtc) > > static void dpu_crtc_early_unregister(struct drm_crtc *crtc) > { > - _dpu_crtc_destroy_debugfs(crtc); > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > + > + debugfs_remove_recursive(dpu_crtc->debugfs_root); > } > > static const struct drm_crtc_funcs dpu_crtc_funcs = { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 5559e5d40142..2811860f2688 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1838,14 +1838,9 @@ void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc) > #ifdef CONFIG_DEBUG_FS > static int dpu_encoder_status_show(struct seq_file *s, void *data) > { > - struct dpu_encoder_virt *dpu_enc; > + struct dpu_encoder_virt *dpu_enc = s->private; > int i; > > - if (!s || !s->private) > - return -EINVAL; > - > - dpu_enc = s->private; > - > mutex_lock(&dpu_enc->enc_lock); > for (i = 0; i < dpu_enc->num_phys_encs; i++) { > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > @@ -1879,18 +1874,17 @@ DEFINE_SHOW_ATTRIBUTE(dpu_encoder_status); > > static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc) > { > - struct dpu_encoder_virt *dpu_enc; > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > struct msm_drm_private *priv; > struct dpu_kms *dpu_kms; > int i; > char name[DPU_NAME_SIZE]; > > - if (!drm_enc || !drm_enc->dev || !drm_enc->dev->dev_private) { > + if (!drm_enc->dev || !drm_enc->dev->dev_private) { > DPU_ERROR("invalid encoder or kms\n"); > return -EINVAL; > } > > - dpu_enc = to_dpu_encoder_virt(drm_enc); > priv = drm_enc->dev->dev_private; > dpu_kms = to_dpu_kms(priv->kms); > > @@ -1915,26 +1909,11 @@ static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc) > > return 0; > } > - > -static void _dpu_encoder_destroy_debugfs(struct drm_encoder *drm_enc) > -{ > - struct dpu_encoder_virt *dpu_enc; > - > - if (!drm_enc) > - return; > - > - dpu_enc = to_dpu_encoder_virt(drm_enc); > - debugfs_remove_recursive(dpu_enc->debugfs_root); > -} > #else > static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc) > { > return 0; > } > - > -static void _dpu_encoder_destroy_debugfs(struct drm_encoder *drm_enc) > -{ > -} > #endif > > static int dpu_encoder_late_register(struct drm_encoder *encoder) > @@ -1944,7 +1923,9 @@ static int dpu_encoder_late_register(struct drm_encoder *encoder) > > static void dpu_encoder_early_unregister(struct drm_encoder *encoder) > { > - _dpu_encoder_destroy_debugfs(encoder); > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder); > + > + debugfs_remove_recursive(dpu_enc->debugfs_root); > } > > static int dpu_encoder_virt_add_phys_encs( > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 2a91881048c8..01be305758c5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -81,7 +81,7 @@ static int _dpu_danger_signal_status(struct seq_file *s, > struct dpu_danger_safe_status status; > int i; > > - if (!kms || !kms->dev || !kms->dev->dev_private || !kms->hw_mdp) { > + if (!kms->dev || !kms->dev->dev_private || !kms->hw_mdp) { > DPU_ERROR("invalid arg(s)\n"); > return 0; > } > @@ -125,46 +125,31 @@ static int dpu_debugfs_safe_stats_show(struct seq_file *s, void *v) > } > DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_safe_stats); > > -static void dpu_debugfs_danger_destroy(struct dpu_kms *dpu_kms) > -{ > - debugfs_remove_recursive(dpu_kms->debugfs_danger); > - dpu_kms->debugfs_danger = NULL; > -} > - > -static int dpu_debugfs_danger_init(struct dpu_kms *dpu_kms, > +static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms, > struct dentry *parent) > { > dpu_kms->debugfs_danger = debugfs_create_dir("danger", > parent); > if (!dpu_kms->debugfs_danger) { > DPU_ERROR("failed to create danger debugfs\n"); > - return -EINVAL; > } > > debugfs_create_file("danger_status", 0600, dpu_kms->debugfs_danger, > dpu_kms, &dpu_debugfs_danger_stats_fops); > debugfs_create_file("safe_status", 0600, dpu_kms->debugfs_danger, > dpu_kms, &dpu_debugfs_safe_stats_fops); > - > - return 0; > } > > static int dpu_debugfs_regset32_show(struct seq_file *s, void *data) > { > - struct dpu_debugfs_regset32 *regset; > - struct dpu_kms *dpu_kms; > + struct dpu_debugfs_regset32 *regset = s->private; > + struct dpu_kms *dpu_kms = regset->dpu_kms; > struct drm_device *dev; > struct msm_drm_private *priv; > void __iomem *base; > uint32_t i, addr; > > - if (!s || !s->private) > - return 0; > - > - regset = s->private; > - > - dpu_kms = regset->dpu_kms; > - if (!dpu_kms || !dpu_kms->mmio) > + if (!dpu_kms->mmio) > return 0; > > dev = dpu_kms->dev; > @@ -231,7 +216,7 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms) > > p = dpu_hw_util_get_log_mask_ptr(); > > - if (!dpu_kms || !p) > + if (!p) > return -EINVAL; > > dpu_kms->debugfs_root = debugfs_create_dir("debug", > @@ -245,9 +230,9 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms) > /* allow root to be NULL */ > debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, dpu_kms->debugfs_root, p); > > - (void) dpu_debugfs_danger_init(dpu_kms, dpu_kms->debugfs_root); > - (void) dpu_debugfs_vbif_init(dpu_kms, dpu_kms->debugfs_root); > - (void) dpu_debugfs_core_irq_init(dpu_kms, dpu_kms->debugfs_root); > + dpu_debugfs_danger_init(dpu_kms, dpu_kms->debugfs_root); > + dpu_debugfs_vbif_init(dpu_kms, dpu_kms->debugfs_root); > + dpu_debugfs_core_irq_init(dpu_kms, dpu_kms->debugfs_root); > > rc = dpu_core_perf_debugfs_init(&dpu_kms->perf, dpu_kms->debugfs_root); > if (rc) { > @@ -257,21 +242,6 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms) > > return 0; > } > - > -static void _dpu_debugfs_destroy(struct dpu_kms *dpu_kms) > -{ > - /* don't need to NULL check debugfs_root */ > - if (dpu_kms) { > - dpu_debugfs_vbif_destroy(dpu_kms); > - dpu_debugfs_danger_destroy(dpu_kms); > - dpu_debugfs_core_irq_destroy(dpu_kms); > - debugfs_remove_recursive(dpu_kms->debugfs_root); > - } > -} > -#else > -static void _dpu_debugfs_destroy(struct dpu_kms *dpu_kms) > -{ > -} > #endif > > static int dpu_kms_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc) > @@ -626,7 +596,7 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms) > &dpu_kms->phandle, dpu_kms->power_event); > > /* safe to call these more than once during shutdown */ > - _dpu_debugfs_destroy(dpu_kms); > + debugfs_remove_recursive(dpu_kms->debugfs_root); > _dpu_kms_mmu_destroy(dpu_kms); > > if (dpu_kms->catalog) { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > index e7539c9870e4..33ed5779687f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > @@ -102,7 +102,6 @@ struct dpu_irq { > atomic_t *enable_counts; > atomic_t *irq_counts; > spinlock_t cb_lock; > - struct dentry *debugfs_file; > }; > > struct dpu_kms { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > index 19abf719811a..5ecc26fdc328 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > @@ -120,13 +120,12 @@ static int _dpu_mdss_irq_domain_add(struct dpu_mdss *dpu_mdss) > return 0; > } > > -static int _dpu_mdss_irq_domain_fini(struct dpu_mdss *dpu_mdss) > +static void _dpu_mdss_irq_domain_fini(struct dpu_mdss *dpu_mdss) > { > if (dpu_mdss->irq_controller.domain) { > irq_domain_remove(dpu_mdss->irq_controller.domain); > dpu_mdss->irq_controller.domain = NULL; > } > - return 0; > } > static int dpu_mdss_enable(struct msm_mdss *mdss) > { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index 98d8315f625d..3c5580c75581 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -1451,25 +1451,11 @@ static int _dpu_plane_init_debugfs(struct drm_plane *plane) > > return 0; > } > - > -static void _dpu_plane_destroy_debugfs(struct drm_plane *plane) > -{ > - struct dpu_plane *pdpu; > - > - if (!plane) > - return; > - pdpu = to_dpu_plane(plane); > - > - debugfs_remove_recursive(pdpu->debugfs_root); > -} > #else > static int _dpu_plane_init_debugfs(struct drm_plane *plane) > { > return 0; > } > -static void _dpu_plane_destroy_debugfs(struct drm_plane *plane) > -{ > -} > #endif > > static int dpu_plane_late_register(struct drm_plane *plane) > @@ -1479,7 +1465,9 @@ static int dpu_plane_late_register(struct drm_plane *plane) > > static void dpu_plane_early_unregister(struct drm_plane *plane) > { > - _dpu_plane_destroy_debugfs(plane); > + struct dpu_plane *pdpu = to_dpu_plane(plane); > + > + debugfs_remove_recursive(pdpu->debugfs_root); > } > > static const struct drm_plane_funcs dpu_plane_funcs = { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > index ff5091d2555d..98d8faf68057 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > @@ -310,13 +310,8 @@ void dpu_vbif_init_memtypes(struct dpu_kms *dpu_kms) > } > > #ifdef CONFIG_DEBUG_FS > -void dpu_debugfs_vbif_destroy(struct dpu_kms *dpu_kms) > -{ > - debugfs_remove_recursive(dpu_kms->debugfs_vbif); > - dpu_kms->debugfs_vbif = NULL; > -} > > -int dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > +void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > { > char vbif_name[32]; > struct dentry *debugfs_vbif; > @@ -325,7 +320,7 @@ int dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > dpu_kms->debugfs_vbif = debugfs_create_dir("vbif", debugfs_root); > if (!dpu_kms->debugfs_vbif) { > DPU_ERROR("failed to create vbif debugfs\n"); > - return -EINVAL; > + return; > } > > for (i = 0; i < dpu_kms->catalog->vbif_count; i++) { > @@ -376,7 +371,5 @@ int dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) > (u32 *)&cfg->ot_limit); > } > } > - > - return 0; > } > #endif > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > index f17af52dbbd5..6356876d7a66 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > @@ -78,17 +78,6 @@ void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms); > */ > void dpu_vbif_init_memtypes(struct dpu_kms *dpu_kms); > > -#ifdef CONFIG_DEBUG_FS > -int dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root); > -void dpu_debugfs_vbif_destroy(struct dpu_kms *dpu_kms); > -#else > -static inline int dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, > - struct dentry *debugfs_root) > -{ > - return 0; > -} > -static inline void dpu_debugfs_vbif_destroy(struct dpu_kms *dpu_kms) > -{ > -} > -#endif > +void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root); > + > #endif /* __DPU_VBIF_H__ */ > -- > 2.18.0 >