Re: [PATCH v3 02/25] drm/dumb-buffers: Provide helper to set pitch and size

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 18.02.25 um 20:32 schrieb Geert Uytterhoeven:
[...]
> > +                                args->bpp);
> > +                       fallthrough;
> > +               case 12:
> > +               case 15:
> > +               case 30: /* see drm_gem_afbc_get_bpp() */
> > +               case 10:
> Perhaps keep them sorted numerically?
The first block comes from the afbc helper; the second block from Mesa; 
hence the odd order. I'll reorder and comment each case individually.
> > +               case 64: /* used by Mesa */
> > +                       pitch = args->width * DIV_ROUND_UP(args->bpp, SZ_8);
> > +                       break;
> > +               }
> > +       }
> > +
> > +       if (!pitch || pitch > U32_MAX)
> > +               return -EINVAL;
> > +
> > +       args->pitch = pitch;
> > +
> > +       return drm_mode_align_dumb(args, pitch_align, size_align);
> > +}
> > +EXPORT_SYMBOL(drm_mode_size_dumb);
> > +
> >   int drm_mode_create_dumb(struct drm_device *dev,
> >                           struct drm_mode_create_dumb *args,
> >                           struct drm_file *file_priv)
> > diff --git a/include/drm/drm_dumb_buffers.h b/include/drm/drm_dumb_buffers.h
> > new file mode 100644
> > index 000000000000..6fe36004b19d
> > --- /dev/null
> > +++ b/include/drm/drm_dumb_buffers.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: MIT */
> > +
> > +#ifndef __DRM_DUMB_BUFFERS_H__
> > +#define __DRM_DUMB_BUFFERS_H__
> > +
> > +struct drm_device;
> > +struct drm_mode_create_dumb;
> > +
> > +int drm_mode_size_dumb(struct drm_device *dev,
> > +                      struct drm_mode_create_dumb *args,
> > +                      unsigned long pitch_align,
> > +                      unsigned long size_align);
> > +
> > +#endif
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index c082810c08a8..eea09103b1a6 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -1058,7 +1058,7 @@ struct drm_mode_crtc_page_flip_target {
> >    * struct drm_mode_create_dumb - Create a KMS dumb buffer for scanout.
> >    * @height: buffer height in pixels
> >    * @width: buffer width in pixels
> > - * @bpp: bits per pixel
> > + * @bpp: color mode
> >    * @flags: must be zero
> >    * @handle: buffer object handle
> >    * @pitch: number of bytes between two consecutive lines
> > @@ -1066,6 +1066,50 @@ struct drm_mode_crtc_page_flip_target {
> >    *
> >    * User-space fills @height, @width, @bpp and @flags. If the IOCTL succeeds,
> >    * the kernel fills @handle, @pitch and @size.
> > + *
> > + * The value of @bpp is a color-mode number describing a specific format
> > + * or a variant thereof. The value often corresponds to the number of bits
> > + * per pixel for most modes, although there are exceptions. Each color mode
> > + * maps to a DRM format plus a number of modes with similar pixel layout.
> > + * Framebuffer layout is always linear.
> > + *
> > + * Support for all modes and formats is optional. Even if dumb-buffer
> > + * creation with a certain color mode succeeds, it is not guaranteed that
> > + * the DRM driver supports any of the related formats. Most drivers support
> > + * a color mode of 32 with a format of DRM_FORMAT_XRGB8888 on their primary
> > + * plane.
> > + *
> > + * +------------+------------------------+------------------------+
> > + * | Color mode | Framebuffer format     | Compatibles            |
> > + * +============+========================+========================+
> > + * |     32     |  * DRM_FORMAT_XRGB8888 |  * DRM_FORMAT_XBGR8888 |
> > + * |            |                        |  * DRM_FORMAT_RGBX8888 |
> > + * |            |                        |  * DRM_FORMAT_BGRX8888 |
> > + * +------------+------------------------+------------------------+
> > + * |     24     |  * DRM_FORMAT_RGB888   |  * DRM_FORMAT_BGR888   |
> > + * +------------+------------------------+------------------------+
> > + * |     16     |  * DRM_FORMAT_RGB565   |  * DRM_FORMAT_BGR565   |
> > + * +------------+------------------------+------------------------+
> > + * |     15     |  * DRM_FORMAT_XRGB1555 |  * DRM_FORMAT_XBGR1555 |
> > + * |            |                        |  * DRM_FORMAT_RGBX1555 |
> > + * |            |                        |  * DRM_FORMAT_BGRX1555 |
> > + * +------------+------------------------+------------------------+
> > + * |      8     |  * DRM_FORMAT_C8       |  * DRM_FORMAT_R8       |
> + DRM_FORMAT_D8? (and 4/2/1 below)
Right, missed that.

> And DRM_FORMAT_Y8, if/when Tomi's series introducing that is accepted...
Sure, if it is compatible, it can also go into the third column.

Best regards
Thomas

> > + * +------------+------------------------+------------------------+
> > + * |      4     |  * DRM_FORMAT_C4       |  * DRM_FORMAT_R4       |
> > + * +------------+------------------------+------------------------+
> > + * |      2     |  * DRM_FORMAT_C2       |  * DRM_FORMAT_R2       |
> > + * +------------+------------------------+------------------------+
> > + * |      1     |  * DRM_FORMAT_C1       |  * DRM_FORMAT_R1       |
> > + * +------------+------------------------+------------------------+
> > + *
> > + * Color modes of 10, 12, 15, 30 and 64 are only supported for use by
> > + * legacy user space. Please don't use them in new code. Other modes
> > + * are not support.
> > + *
> > + * Do not attempt to allocate anything but linear framebuffer memory
> > + * with single-plane RGB data. Allocation of other framebuffer
> > + * layouts requires dedicated ioctls in the respective DRM driver.
> >    */
> >   struct drm_mode_create_dumb {
> >          __u32 height;
> Gr{oetje,eeting}s,
>
>                          Geert
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)