linux-kernel - Re: [PATCH v3 01/10] drm/fourcc: Add drm_format_info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YvVYyKZNDGfhGjb5@ravnborg.org>
Date:   Thu, 11 Aug 2022 21:30:16 +0200
From:   Sam Ravnborg <sam@...nborg.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...ux.ie>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        Linux/m68k <linux-m68k@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Javier Martinez Canillas <javierm@...hat.com>
Subject: Re: [PATCH v3 01/10] drm/fourcc: Add drm_format_info_bpp() helper

Hi Geert.

> > >
> > > Yeah I don't think we should describe these with bpp or cpp or anything
> > > like that. bpp < 8 makes sense since that's how this has been done since
> > > decades, but trying to extend these to funny new formats is a bad idea.
> > > This is also why cpp and depth refuse to compute these (or at least
> > > should).
> >
> > Daniel and I discussed this on irc. Let me try to recap here.
> > Using the bits per pixel info from drm_format_info is something we shall
> > try to avoid as this is often a sign of the wrong abstraction/design (my
> > words based on the irc talk).
> > So we shall limit the use of drm_format_info_bpp() to what we need now,
> > thus blocky formats should not be supported - to try to avoid seeing
> > this used more than necessary.
> >
> > Daniel suggested a rename to drm_format_info_legacy_bpp() to make it
> > obvious that this is often/always the wrong solution. I did not jump on
> > doing the rename as I do not know stuff good enough to tell people what
> > to use when this is not the right solution. The rename is simple, it is
> > the follow-up that keep me away.
> >
> > On top of this there is a few formats in drm_drourcc that has a depth
> > field set which should be dropped. .depth is only for the few legacy
> > formats where it is used today.
> >
> > We would also like to convert the fbdev helpers to drm_format_info,
> > and doing so will likely teach us a bit more what we need and what we
> > can drop.
> >
> > Geert - can you give drm_format_info_bpp() a spin so it is limited to
> > the formats used now (not the blocky ones).
> 
> You mean return 0 if char_per_block[] > 1?
if char_per_block[] > 1 AND block_w[] > 0 AND block_h[] > 0 should be
enough.

> I'm not sure it's actually safe to do so (and make this change this late
> in the development cycle), as this is used in drm_client_buffer_create(),
> drm_client_buffer_addfb(), and drm_mode_getfb().

drm_client_buffer_create() and drm_client_buffer_addfb() both get their
format from  drm_mode_legacy_fb_format() which do not produce any blocky
formats - so they are good.

drm_mode_getfb() looks up a framebuffer originally created using one of
the above (I think), so here it should also be fine.
I do not see the need to push this to fixes, so it has a full cycle to
mature if it causes issues.

	Sam