| Message ID | 20230130123008.287141-3-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/debugfs: Make the debugfs structure more generic | expand |
Hi Maíra,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip next-20230131]
[cannot apply to drm-intel/for-linux-next-fixes linus/master v6.2-rc6]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ma-ra-Canal/drm-debugfs-Introduce-wrapper-for-debugfs-list/20230130-203549
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230130123008.287141-3-mcanal%40igalia.com
patch subject: [PATCH v2 2/6] drm/debugfs: Make drm_device use the struct drm_debugfs_files
config: arc-randconfig-r043-20230129 (https://download.01.org/0day-ci/archive/20230201/202302010102.Zlw3VTYI-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.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/intel-lab-lkp/linux/commit/776660a442d9112d8fbc06b0b8b0b77852b18fca
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ma-ra-Canal/drm-debugfs-Introduce-wrapper-for-debugfs-list/20230130-203549
git checkout 776660a442d9112d8fbc06b0b8b0b77852b18fca
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_drv.c: In function 'drm_dev_init_release':
>> drivers/gpu/drm/drm_drv.c:602:9: error: implicit declaration of function 'drm_debugfs_files_destroy'; did you mean 'drm_debugfs_list_destroy'? [-Werror=implicit-function-declaration]
602 | drm_debugfs_files_destroy(dev->debugfs_files);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| drm_debugfs_list_destroy
drivers/gpu/drm/drm_drv.c: In function 'drm_dev_init':
>> drivers/gpu/drm/drm_drv.c:650:30: error: implicit declaration of function 'drm_debugfs_files_init'; did you mean 'drm_debugfs_list_init'? [-Werror=implicit-function-declaration]
650 | dev->debugfs_files = drm_debugfs_files_init();
| ^~~~~~~~~~~~~~~~~~~~~~
| drm_debugfs_list_init
>> drivers/gpu/drm/drm_drv.c:650:28: warning: assignment to 'struct drm_debugfs_files *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
650 | dev->debugfs_files = drm_debugfs_files_init();
| ^
cc1: some warnings being treated as errors
vim +602 drivers/gpu/drm/drm_drv.c
562
563 /**
564 * DOC: component helper usage recommendations
565 *
566 * DRM drivers that drive hardware where a logical device consists of a pile of
567 * independent hardware blocks are recommended to use the :ref:`component helper
568 * library<component>`. For consistency and better options for code reuse the
569 * following guidelines apply:
570 *
571 * - The entire device initialization procedure should be run from the
572 * &component_master_ops.master_bind callback, starting with
573 * devm_drm_dev_alloc(), then binding all components with
574 * component_bind_all() and finishing with drm_dev_register().
575 *
576 * - The opaque pointer passed to all components through component_bind_all()
577 * should point at &struct drm_device of the device instance, not some driver
578 * specific private structure.
579 *
580 * - The component helper fills the niche where further standardization of
581 * interfaces is not practical. When there already is, or will be, a
582 * standardized interface like &drm_bridge or &drm_panel, providing its own
583 * functions to find such components at driver load time, like
584 * drm_of_find_panel_or_bridge(), then the component helper should not be
585 * used.
586 */
587
588 static void drm_dev_init_release(struct drm_device *dev, void *res)
589 {
590 drm_legacy_ctxbitmap_cleanup(dev);
591 drm_legacy_remove_map_hash(dev);
592 drm_fs_inode_free(dev->anon_inode);
593
594 put_device(dev->dev);
595 /* Prevent use-after-free in drm_managed_release when debugging is
596 * enabled. Slightly awkward, but can't really be helped. */
597 dev->dev = NULL;
598 mutex_destroy(&dev->master_mutex);
599 mutex_destroy(&dev->clientlist_mutex);
600 mutex_destroy(&dev->filelist_mutex);
601 mutex_destroy(&dev->struct_mutex);
> 602 drm_debugfs_files_destroy(dev->debugfs_files);
603 drm_legacy_destroy_members(dev);
604 }
605
606 static int drm_dev_init(struct drm_device *dev,
607 const struct drm_driver *driver,
608 struct device *parent)
609 {
610 struct inode *inode;
611 int ret;
612
613 if (!drm_core_init_complete) {
614 DRM_ERROR("DRM core is not initialized\n");
615 return -ENODEV;
616 }
617
618 if (WARN_ON(!parent))
619 return -EINVAL;
620
621 kref_init(&dev->ref);
622 dev->dev = get_device(parent);
623 dev->driver = driver;
624
625 INIT_LIST_HEAD(&dev->managed.resources);
626 spin_lock_init(&dev->managed.lock);
627
628 /* no per-device feature limits by default */
629 dev->driver_features = ~0u;
630
631 if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
632 (drm_core_check_feature(dev, DRIVER_RENDER) ||
633 drm_core_check_feature(dev, DRIVER_MODESET))) {
634 DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
635 return -EINVAL;
636 }
637
638 drm_legacy_init_members(dev);
639 INIT_LIST_HEAD(&dev->filelist);
640 INIT_LIST_HEAD(&dev->filelist_internal);
641 INIT_LIST_HEAD(&dev->clientlist);
642 INIT_LIST_HEAD(&dev->vblank_event_list);
643
644 spin_lock_init(&dev->event_lock);
645 mutex_init(&dev->struct_mutex);
646 mutex_init(&dev->filelist_mutex);
647 mutex_init(&dev->clientlist_mutex);
648 mutex_init(&dev->master_mutex);
649
> 650 dev->debugfs_files = drm_debugfs_files_init();
651
652 ret = drmm_add_action_or_reset(dev, drm_dev_init_release, NULL);
653 if (ret)
654 return ret;
655
656 inode = drm_fs_inode_new();
657 if (IS_ERR(inode)) {
658 ret = PTR_ERR(inode);
659 DRM_ERROR("Cannot allocate anonymous inode: %d\n", ret);
660 goto err;
661 }
662
663 dev->anon_inode = inode;
664
665 if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
666 ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
667 if (ret)
668 goto err;
669 } else {
670 if (drm_core_check_feature(dev, DRIVER_RENDER)) {
671 ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
672 if (ret)
673 goto err;
674 }
675
676 ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
677 if (ret)
678 goto err;
679 }
680
681 ret = drm_legacy_create_map_hash(dev);
682 if (ret)
683 goto err;
684
685 drm_legacy_ctxbitmap_init(dev);
686
687 if (drm_core_check_feature(dev, DRIVER_GEM)) {
688 ret = drm_gem_init(dev);
689 if (ret) {
690 DRM_ERROR("Cannot initialize graphics execution manager (GEM)\n");
691 goto err;
692 }
693 }
694
695 ret = drm_dev_set_unique(dev, dev_name(parent));
696 if (ret)
697 goto err;
698
699 return 0;
700
701 err:
702 drm_managed_release(dev);
703
704 return ret;
705 }
706
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 8658d3929ea5..aa83f230c402 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -263,7 +263,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id, if (dev->driver->debugfs_init) dev->driver->debugfs_init(minor); - list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) { + list_for_each_entry_safe(entry, tmp, &dev->debugfs_files->list, list) { debugfs_create_file(entry->file.name, 0444, minor->debugfs_root, entry, &drm_debugfs_entry_fops); list_del(&entry->list); @@ -280,7 +280,7 @@ void drm_debugfs_late_register(struct drm_device *dev) if (!minor) return; - list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) { + list_for_each_entry_safe(entry, tmp, &dev->debugfs_files->list, list) { debugfs_create_file(entry->file.name, 0444, minor->debugfs_root, entry, &drm_debugfs_entry_fops); list_del(&entry->list); @@ -357,9 +357,9 @@ void drm_debugfs_add_file(struct drm_device *dev, const char *name, entry->file.data = data; entry->dev = dev; - mutex_lock(&dev->debugfs_mutex); - list_add(&entry->list, &dev->debugfs_list); - mutex_unlock(&dev->debugfs_mutex); + mutex_lock(&dev->debugfs_files->mutex); + list_add(&entry->list, &dev->debugfs_files->list); + mutex_unlock(&dev->debugfs_files->mutex); } EXPORT_SYMBOL(drm_debugfs_add_file); diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c6eb8972451a..50812cbe1d81 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -39,6 +39,7 @@ #include <drm/drm_cache.h> #include <drm/drm_client.h> #include <drm/drm_color_mgmt.h> +#include <drm/drm_debugfs.h> #include <drm/drm_drv.h> #include <drm/drm_file.h> #include <drm/drm_managed.h> @@ -598,7 +599,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) mutex_destroy(&dev->clientlist_mutex); mutex_destroy(&dev->filelist_mutex); mutex_destroy(&dev->struct_mutex); - mutex_destroy(&dev->debugfs_mutex); + drm_debugfs_files_destroy(dev->debugfs_files); drm_legacy_destroy_members(dev); } @@ -639,14 +640,14 @@ static int drm_dev_init(struct drm_device *dev, INIT_LIST_HEAD(&dev->filelist_internal); INIT_LIST_HEAD(&dev->clientlist); INIT_LIST_HEAD(&dev->vblank_event_list); - INIT_LIST_HEAD(&dev->debugfs_list); spin_lock_init(&dev->event_lock); mutex_init(&dev->struct_mutex); mutex_init(&dev->filelist_mutex); mutex_init(&dev->clientlist_mutex); mutex_init(&dev->master_mutex); - mutex_init(&dev->debugfs_mutex); + + dev->debugfs_files = drm_debugfs_files_init(); ret = drmm_add_action_or_reset(dev, drm_dev_init_release, NULL); if (ret) diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 7cf4afae2e79..77290f4a06ff 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -16,6 +16,7 @@ struct drm_vblank_crtc; struct drm_vma_offset_manager; struct drm_vram_mm; struct drm_fb_helper; +struct drm_debugfs_files; struct inode; @@ -312,19 +313,12 @@ struct drm_device { struct drm_fb_helper *fb_helper; /** - * @debugfs_mutex: - * - * Protects &debugfs_list access. - */ - struct mutex debugfs_mutex; - - /** - * @debugfs_list: + * @debugfs_files: * * List of debugfs files to be created by the DRM device. The files * must be added during drm_dev_register(). */ - struct list_head debugfs_list; + struct drm_debugfs_files *debugfs_files; /* Everything below here is for legacy driver, never use! */ /* private: */
The struct drm_debugfs_files encapsulates all the debugfs-related objects, so that they can be initialized and destroyed with two helpers. Therefore, make the struct drm_device use the struct drm_debugfs_files instead of instantiating the debugfs list and mutex separated. Signed-off-by: Maíra Canal <mcanal@igalia.com> --- drivers/gpu/drm/drm_debugfs.c | 10 +++++----- drivers/gpu/drm/drm_drv.c | 7 ++++--- include/drm/drm_device.h | 12 +++--------- 3 files changed, 12 insertions(+), 17 deletions(-)