Re: [PATCH 7/8] drm/fourcc: Add DRM_FORMAT_R[124]

Pekka Paalanen
2022-02-17T10:03:08Z

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

On Tue, 15 Feb 2022 17:52:25 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Introduce fourcc codes for single-channel frame buffer formats with two,
> four, and sixteen intensity levels. Traditionally, the first channel
> has been called the "red" channel, but the fourcc can also be used for
> other light-on-dark displays.
>
> As the number of bits per pixel is less than eight, these rely on proper
> block handling for the calculation of bits per pixel and pitch.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> drivers/gpu/drm/drm_fourcc.c | 6 ++++++
> include/uapi/drm/drm_fourcc.h | 9 +++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 5c77ce10f53e3a64..c12e48ecb1ab8aad 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -151,6 +151,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
> { .format = DRM_FORMAT_C4, .depth = 4, .num_planes = 1,
> .char_per_block = { 1, }, .block_w = { 2, }, .block_h = { 1, }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_C8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_R1, .depth = 1, .num_planes = 1,
> + .char_per_block = { 1, }, .block_w = { 8, }, .block_h = { 1, }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_R2, .depth = 2, .num_planes = 1,
> + .char_per_block = { 1, }, .block_w = { 4, }, .block_h = { 1, }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_R4, .depth = 4, .num_planes = 1,
> + .char_per_block = { 1, }, .block_w = { 2, }, .block_h = { 1, }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_R8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_R10, .depth = 10, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_R12, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3f09174670b3cce6..8605a1acc6813e6c 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -104,6 +104,15 @@ extern "C" {
> #define DRM_FORMAT_C4 fourcc_code('C', '4', ' ', ' ') /* [3:0] C */
> #define DRM_FORMAT_C8 fourcc_code('C', '8', ' ', ' ') /* [7:0] C */
>
> +/* 1 bpp Red */
> +#define DRM_FORMAT_R1 fourcc_code('R', '1', ' ', ' ') /* [0] R */
> +
> +/* 2 bpp Red */
> +#define DRM_FORMAT_R2 fourcc_code('R', '2', ' ', ' ') /* [1:0] R */
> +
> +/* 4 bpp Red */
> +#define DRM_FORMAT_R4 fourcc_code('R', '4', ' ', ' ') /* [3:0] R */
> +
> /* 8 bpp Red */
> #define DRM_FORMAT_R8 fourcc_code('R', '8', ' ', ' ') /* [7:0] R */
>

Hi Geert,

I have the same comment here as for C1/C2/C4: these need to specify the
ordering inside a byte. Otherwise this reads as one byte of storage per
pixel, but using only 1/2/4 bits of each byte.

The idea of having Cx and Rx formats separately sounds good to me.

Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]