Re: [PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings
On 12/24/23 22:51, Vegard Nossum wrote:
> As of commit b77fdd6a48e6 ("scripts/kernel-doc: restore warning for
> Excess struct/union"), we see the following warnings when running 'make
> htmldocs':
>
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
> 'DRM_NOUVEAU_VM_BIND_OP_MAP' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
> 'DRM_NOUVEAU_VM_BIND_OP_UNMAP' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
> 'DRM_NOUVEAU_VM_BIND_SPARSE' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:336: warning: Excess struct member
> 'DRM_NOUVEAU_VM_BIND_RUN_ASYNC' description in 'drm_nouveau_vm_bind'
>
> The problem is that these values are #define constants, but had kerneldoc
> comments attached to them as if they were actual struct members.
>
> There are a number of ways we could fix this, but I chose to draw
> inspiration from include/uapi/drm/i915_drm.h, which pulls them into the
> corresponding kerneldoc comment for the struct member that they are
> intended to be used with.
>
> To keep the diff readable, there are a number of things I _didn't_ do in
> this patch, but which we should also consider:
>
> - This is pretty good documentation, but it ends up in gpu/driver-uapi,
> which is part of subsystem-apis/ when it really ought to display under
> userspace-api/ (the "Linux kernel user-space API guide" book of the
> documentation).
>
> - More generally, we might want a warning if include/uapi/ files are
> kerneldoc'd outside userspace-api/.
>
> - I'd consider it cleaner if the #defines appeared between the kerneldoc
> for the member and the member itself (which is something other DRM-
> related UAPI docs do).
>
> - The %IDENTIFIER kerneldoc syntax is intended for "constants", and is
> more appropriate in this context than ``IDENTIFIER`` or
> The DRM docs aren't very consistent on this.
>
> Cc: Randy Dunlap
> Cc: Jonathan Corbet
> Signed-off-by: Vegard Nossum
This all looks good to me. Thanks for your help.
Reviewed-by: Randy Dunlap
Tested-by: Randy Dunlap
I do see one thing that I don't like in the generated html output.
It's not a problem with this patch.
The #defines for DRM_NOUVEAU_VM_BIND_OP_MAP etc. have a ';' at the
end of each line:
struct drm_nouveau_vm_bind_op {
__u32 op;
#define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0;
#define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1;
__u32 flags;
#define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8);
__u32 handle;
__u32 pad;
__u64 addr;
__u64 bo_offset;
__u64 range;
};
so something else to look into one of these days
> ---
> include/uapi/drm/nouveau_drm.h | 56 --
> 1 file changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
> index 0bade1592f34..c95ef8a4d94a 100644
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
[PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings
As of commit b77fdd6a48e6 ("scripts/kernel-doc: restore warning for
Excess struct/union"), we see the following warnings when running 'make
htmldocs':
./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
'DRM_NOUVEAU_VM_BIND_OP_MAP' description in 'drm_nouveau_vm_bind_op'
./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
'DRM_NOUVEAU_VM_BIND_OP_UNMAP' description in 'drm_nouveau_vm_bind_op'
./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member
'DRM_NOUVEAU_VM_BIND_SPARSE' description in 'drm_nouveau_vm_bind_op'
./include/uapi/drm/nouveau_drm.h:336: warning: Excess struct member
'DRM_NOUVEAU_VM_BIND_RUN_ASYNC' description in 'drm_nouveau_vm_bind'
The problem is that these values are #define constants, but had kerneldoc
comments attached to them as if they were actual struct members.
There are a number of ways we could fix this, but I chose to draw
inspiration from include/uapi/drm/i915_drm.h, which pulls them into the
corresponding kerneldoc comment for the struct member that they are
intended to be used with.
To keep the diff readable, there are a number of things I _didn't_ do in
this patch, but which we should also consider:
- This is pretty good documentation, but it ends up in gpu/driver-uapi,
which is part of subsystem-apis/ when it really ought to display under
userspace-api/ (the "Linux kernel user-space API guide" book of the
documentation).
- More generally, we might want a warning if include/uapi/ files are
kerneldoc'd outside userspace-api/.
- I'd consider it cleaner if the #defines appeared between the kerneldoc
for the member and the member itself (which is something other DRM-
related UAPI docs do).
- The %IDENTIFIER kerneldoc syntax is intended for "constants", and is
more appropriate in this context than ``IDENTIFIER`` or
The DRM docs aren't very consistent on this.
Cc: Randy Dunlap
Cc: Jonathan Corbet
Signed-off-by: Vegard Nossum
---
include/uapi/drm/nouveau_drm.h | 56 --
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 0bade1592f34..c95ef8a4d94a 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -238,34 +238,32 @@ struct drm_nouveau_vm_init {
struct drm_nouveau_vm_bind_op {
/**
* @op: the operation type
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_OP_MAP - Map a GEM object to the GPU's VA
+* space. Optionally, the _NOUVEAU_VM_BIND_SPARSE flag can be
+* passed to instruct the kernel to create sparse mappings for the
+* given range.
+*
+* %DRM_NOUVEAU_VM_BIND_OP_UNMAP - Unmap an existing mapping in the
+* GPU's VA space. If the region the mapping is located in is a
+* sparse region, new sparse mappings are created where the unmapped
+* (memory backed) mapping was mapped previously. To remove a sparse
+* region the _NOUVEAU_VM_BIND_SPARSE must be set.
*/
__u32 op;
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_MAP:
- *
- * Map a GEM object to the GPU's VA space. Optionally, the
- * _NOUVEAU_VM_BIND_SPARSE flag can be passed to instruct the kernel to
- * create sparse mappings for the given range.
- */
#define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_UNMAP:
- *
- * Unmap an existing mapping in the GPU's VA space. If the region the mapping
- * is located in is a sparse region, new sparse mappings are created where the
- * unmapped (memory backed) mapping was mapped previously. To remove a sparse
- * region the _NOUVEAU_VM_BIND_SPARSE must be set.
- */
#define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1
/**
* @flags: the flags for a _nouveau_vm_bind_op
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_SPARSE - Indicates that an allocated VA
+* space region should be sparse.
*/
__u32 flags;
-/**
- * @DRM_NOUVEAU_VM_BIND_SPARSE:
- *
- * Indicates that an allocated VA space region should be sparse.
- */
#define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8)
/**
* @handle: the handle of the DRM GEM object to map
@@ -301,17 +299,17 @@ struct drm_nouveau_vm_bind {
__u32 op_count;
/**
* @flags: the flags for a _nouveau_vm_bind ioctl
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_RUN_ASYNC - Indicates that the given VM_BIND
+* operation should be executed asynchronously by the kernel.
+*
+* If this flag is not supplied the kernel executes the associated
+* operations synchronously and doesn't accept any _nouveau_sync
+* objects.
*/
__u32 flags;
-/**
- * @DRM_NOUVEAU_VM_BIND_RUN_ASYNC:
- *
- * Indicates that the given VM_BIND operation should be executed asynchronously
- * by the kernel.
- *
- * If this flag is not supplied the
Re: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl
Hi Julia,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.7-rc6]
[also build test WARNING on linus/master]
[cannot apply to drm-misc/drm-misc-next drm/drm-next next-20231222]
[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/Julia-Zhang/drm-virtio-Implement-RESOURCE_GET_LAYOUT-ioctl/20231222-182142
base: v6.7-rc6
patch link:
https://lore.kernel.org/r/20231221100016.4022353-2-julia.zhang%40amd.com
patch subject: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl
config: i386-randconfig-004-20231225
(https://download.01.org/0day-ci/archive/20231225/202312250806.svsnk275-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231225/202312250806.svsnk275-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202312250806.svsnk275-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/virtio/virtgpu_ioctl.c:712:1: warning: unused label 'valid'
>> [-Wunused-label]
valid:
^~
1 warning generated.
vim +/valid +712 drivers/gpu/drm/virtio/virtgpu_ioctl.c
673
674 static int virtio_gpu_resource_query_layout_ioctl(struct drm_device
*dev,
675void *data,
676struct drm_file *file)
677 {
678 struct drm_virtgpu_resource_query_layout *args = data;
679 struct virtio_gpu_device *vgdev = dev->dev_private;
680 struct drm_gem_object *obj = NULL;
681 struct virtio_gpu_object *bo = NULL;
682 struct virtio_gpu_query_info bo_info = {0};
683 int ret = 0;
684 int i;
685
686 if (!vgdev->has_resource_query_layout) {
687 DRM_ERROR("failing: no RQL on host\n");
688 return -EINVAL;
689 }
690
691 if (args->handle > 0) {
692 obj = drm_gem_object_lookup(file, args->handle);
693 if (obj == NULL) {
694 DRM_ERROR("invalid handle 0x%x\n",
args->handle);
695 return -ENOENT;
696 }
697 bo = gem_to_virtio_gpu_obj(obj);
698 }
699
700 ret = virtio_gpu_cmd_get_resource_layout(vgdev, _info,
args->width,
701 args->height,
args->format,
702 args->bind, bo ?
bo->hw_res_handle : 0);
703 if (ret)
704 goto out;
705
706 ret = wait_event_timeout(vgdev->resp_wq,
707 atomic_read(_info.is_valid),
708 5 * HZ);
709 if (!ret)
710 goto out;
711
> 712 valid:
713 smp_rmb();
714 WARN_ON(atomic_read(_info.is_valid));
715 args->num_planes = bo_info.num_planes;
716 args->modifier = bo_info.modifier;
717 for (i = 0; i < args->num_planes; i++) {
718 args->planes[i].offset = bo_info.planes[i].offset;
719 args->planes[i].stride = bo_info.planes[i].stride;
720 }
721 for (; i < VIRTIO_GPU_MAX_RESOURCE_PLANES; i++) {
722 args->planes[i].offset = 0;
723 args->planes[i].stride = 0;
724 }
725 ret = 0;
726
727 out:
728 if (obj)
729 drm_gem_object_put(obj);
730 return ret;
731 }
732
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] drm/rockchip: vop2: Avoid use regmap_reinit_cache at runtime
On Sun, 17 Dec 2023 16:44:15 +0800, Andy Yan wrote:
> From: Andy Yan
>
> Marek Report a possible irq lock inversion dependency warning when
> commit 81a06f1d02e5 ("Revert "drm/rockchip: vop2: Use regcache_sync()
> to fix suspend/resume"") lands linux-next.
>
> I can reproduce this warning with:
> CONFIG_PROVE_LOCKING=y
> CONFIG_DEBUG_LOCKDEP=y
>
> [...]
Applied, thanks!
[1/1] drm/rockchip: vop2: Avoid use regmap_reinit_cache at runtime
commit: 3ee348eb36f14e9303a7e9757efb91b0bbf3f7a9
Best regards,
--
Heiko Stuebner
Re: [PATCH] drm/rockchip: vop2: clean up some inconsistent indenting
On Tue, 19 Dec 2023 14:26:35 +0800, Jiapeng Chong wrote: > No functional modification involved. > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:1708 rk3588_calc_cru_cfg() warn: > inconsistent indenting. > > Applied, thanks! [1/1] drm/rockchip: vop2: clean up some inconsistent indenting commit: f40e61eb538d35661d6dda1de92867954d776c4a Best regards, -- Heiko Stuebner
Re: [PATCH] MAINTAINERS: Change vmware.com addresses to broadcom.com
On 12/24/2023 6:20 AM, Zack Rusin wrote: Update the email addresses for vmwgfx and vmmouse to reflect the fact that VMware is now part of Broadcom. Add a .mailmap entry because the vmware.com address will start bouncing soon. Signed-off-by: Zack Rusin Cc: Andrew Morton Cc: Ian Forbes Cc: Martin Krastev Cc: Maaz Mombasawala Cc: Broadcom internal kernel review list Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Acked-by: Florian Fainelli -- Florian smime.p7s Description: S/MIME Cryptographic Signature
[PATCH] drm/amd/pm/smu7: fix a memleak in smu7_hwmgr_backend_init
The hwmgr->backend, (i.e. data) allocated by kzalloc is not freed in
the error-handling paths of smu7_get_evv_voltages and
smu7_update_edc_leakage_table. However, it did be freed in the
error-handling of phm_initializa_dynamic_state_adjustment_rule_settings,
by smu7_hwmgr_backend_fini. So the lack of free in smu7_get_evv_voltages
and smu7_update_edc_leakage_table is considered a memleak in this patch.
Fixes: 599a7e9fe1b6 ("drm/amd/powerplay: implement smu7 hwmgr to manager asics
with smu ip version 7.")
Fixes: 8f0804c6b7d0 ("drm/amd/pm: add edc leakage controller setting")
Signed-off-by: Zhipeng Lu
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index 11372fcc59c8..b1a8799e2dee 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -2974,6 +2974,8 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
result = smu7_get_evv_voltages(hwmgr);
if (result) {
pr_info("Get EVV Voltage Failed. Abort Driver
loading!\n");
+ kfree(hwmgr->backend);
+ hwmgr->backend = NULL;
return -EINVAL;
}
} else {
@@ -3019,8 +3021,10 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr
*hwmgr)
}
result = smu7_update_edc_leakage_table(hwmgr);
- if (result)
+ if (result) {
+ smu7_hwmgr_backend_fini(hwmgr);
return result;
+ }
return 0;
}
--
2.34.1
