Re: [PATCH v8 5/7] drm/tyr: add a kernel buffer object

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Trailing newlines in logging macros cause double newlines in the kernel log.
--

--- Patch [5]: [PATCH v8 5/7] drm/tyr: add a kernel buffer object ---
commit d331aa5f9ef1fb72722d9ab68ad7b2746d91080d
Author: Deborah Brouwer <deborah.brouwer@collabora.com>

drm/tyr: add a kernel buffer object

Introduce a buffer object type (KernelBo) for internal driver allocations
that are managed by the kernel rather than userspace. It wraps a GEM shmem
object and automatically handles GPU virtual address space mapping during
creation and unmapping on drop.

> diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
> index c28be61a01bb2..583c2a87b23b3 100644
> --- a/drivers/gpu/drm/tyr/gem.rs
> +++ b/drivers/gpu/drm/tyr/gem.rs
[ ... ]
> +impl<'bound> KernelBo<'bound> {
> +    /// Creates a new kernel-owned buffer object and maps it into GPU VA space.
[ ... ]
> +    #[expect(dead_code)]
> +    pub(crate) fn new(
> +        ddev: &TyrDrmDevice,
> +        vm: Arc<Vm<'bound>>,
> +        size: u64,
> +        va_alloc: KernelBoVaAlloc,
> +        flags: VmMapFlags,
> +    ) -> Result<Self> {
> +        if size == 0 {
> +            dev_err!(vm.dev(), "Cannot create KernelBo with size 0\n");

[Severity: Low]
This isn't a bug, but does this dev_err macro call need a trailing newline? 
The rust dev_err macro automatically appends a newline to the formatted 
output. Including an explicit newline at the end of the format string 
might result in an unnecessary blank line in the kernel log.

> +            return Err(EINVAL);
> +        }
[ ... ]
> +impl Drop for KernelBo<'_> {
> +    fn drop(&mut self) {
> +        let va = self.va_range.start;
> +        let size = self.va_range.end - self.va_range.start;
> +
> +        if let Err(e) = self.vm.unmap_range(va, size) {
> +            // If unmap_range fails, it is still safe to drop the
> +            // KernelBo and its ARef to the GEM buffer object because
> +            // GPUVM also holds a reference to the GEM buffer object.
> +            // The physical pages won't be freed or reallocated.
> +            dev_err!(
> +                self.vm.dev(),
> +                "Failed to unmap KernelBo range {:#x}..{:#x}: {:?}\n",

[Severity: Low]
Similarly here, does this format string need the trailing newline?

> +                self.va_range.start,
> +                self.va_range.end,
> +                e
> +            );
> +        }
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-fw-boot-b4-v8-0-b28f744581c9@collabora.com?part=5