Re: [PATCH 8/8] drm/fourcc: Add DRM_FORMAT_D1

[Pekka Paalanen]

On Thu, 17 Feb 2022 11:42:29 +0100
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> Hi Pekka,
> 
> On Thu, Feb 17, 2022 at 11:10 AM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote:
> > On Tue, 15 Feb 2022 17:52:26 +0100
> > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:  
> > > Introduce a fourcc code for a single-channel frame buffer format with two
> > > darkness levels.  This can be used for two-level dark-on-light displays.
> > >
> > > As the number of bits per pixel is less than eight, this relies on
> > > proper block handling for the calculation of bits per pixel and pitch.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>  
> 
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -151,6 +151,8 @@ 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_D1,              .depth = 1,  .num_planes = 1,
> > > +               .char_per_block = { 1, }, .block_w = { 8, }, .block_h = { 1, }, .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,
> > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > > index 8605a1acc6813e6c..c15c6efcc65e5827 100644
> > > --- a/include/uapi/drm/drm_fourcc.h
> > > +++ b/include/uapi/drm/drm_fourcc.h
> > > @@ -104,6 +104,9 @@ 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 Darkness */
> > > +#define DRM_FORMAT_D1                fourcc_code('D', '1', ' ', ' ') /* [0] D */
> > > +  
> >
> > the same comment here as for C1 and R1 formats, need to specify pixel
> > ordering inside a byte.  
> 
> Right, will do.

Btw. does endianess of anything have any effect on these pixel formats?

That's probably a weird question, but I recall Pixman (the pixel
handling library of the X server nowadays known as Xorg) having pixel
formats where CPU endianess affects whether the first pixel in a byte
is found at the MSB or LSB.

> > I think it would also be good to explain the rationale why C1 and R1
> > are not suitable for this case and we need yet another 1-bit format in
> > the commit message.
> >
> > For posterity, of course. I roughly remember the discussions.  
> 
> C1 is color-indexed, which can be any two colors.
> R1 is light-on-dark.
> D1 is dark-on-light.
> 
> > I also wonder if anyone would actually use D1. Should it be added
> > anyway? There is no rule that a pixel format must be used inside the
> > kernel AFAIK, but is there even a prospective userspace wanting this?
> >
> > Exposing R1 and inverting bits while copying to hardware might be
> > enough?  
> 
> That's an option.  The repaper driver does that:
> 
>     drm_fb_xrgb8888_to_gray8(buf, 0, cma_obj->vaddr, fb, &clip);
>     repaper_gray8_to_mono_reversed(buf, fb->width, fb->height);
> 
> Can drm_framebuffer objects be backed by graphics memory, i.e.
> can they be displayed without copying?

Yes, they can. That is actually their primary purpose. So the invert
bits approach only works with drivers that need to manually shovel the
data, but not with direct hardware scanout.

D1 might be useful on hardware that:
- can scanout the buffer directly, and
- does not have an optional inverter in its hardware pipeline, and
- does not benefit from a shadow buffer.

Do you happen to know any that fits the description?


Thanks,
pq

Attachment: pgpySrB4UU_qM.pgp
Description: OpenPGP digital signature