linux-kernel - Re: [RFC PATCH v4 02/11] drm/fb-helper: Set FBINFO_MISC_FIRMWARE flag for DRIVER

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8671b156-af04-c728-d7bb-1019badbd13a@redhat.com>
Date:   Fri, 29 Apr 2022 12:50:20 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org
Cc:     David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel@...ts.freedesktop.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [RFC PATCH v4 02/11] drm/fb-helper: Set FBINFO_MISC_FIRMWARE flag
 for DRIVER_FIRMWARE fb

On 4/29/22 12:20, Thomas Zimmermann wrote:
> Hi Javier

[snip]

>>   
>>> We can do this with DRIVER_FIRMWARE. Alternatively, I'd suggest to we
>>> could also used the existing final parameter of
>>> drm_fbdev_generic_setup() to pass a flag that designates a firmware device.
>>>
>>
>> By existing final parameter you mean @preferred_bpp ? That doesn't seem
>> correct. I also like that by using DRIVER_FIRMWARE it is completely data
>> driven and transparent to the DRM driver.
> 
> DRIVER_FIRMWARE is an indirection and only used here. (Just like 
> FBINFO_MISC_FIRMWARE is a bad interface for marking framebuffers that 
> can be unplugged.) If a driver supports hot-unplugging, it should simply 
> register itself with aperture helpers, regardless of whether it's a 
> firmware framebuffer or not.
>

That's fair, and if in practice will only be used by one driver (simpledrm)
then that would also allow us to drop patches 1 and 2 from this series.

IOW, we wouldn't really need a DRIVER_FIRMWARE capability flag.

> Of preferred_bpp, we really only use the lowest byte. All other bits are 
> up for grabbing.  The argument is a workaround for handling 
> mode_config.prefered_depth correctly.
>

Yeah, but I didn't want to abuse that argument or package data in an int.

> Eventually, preferred_depth should be replaced by something like 
> 'preferred_format', which will hold the driver's preferred format in 
> 4CC.  We won't need preferred_bpp then. So we could turn preferred_bpp 
> into a flags argument.
>

That's a good point, maybe we could start from there and do this cleanup
as a preparatory change of this series ? Then the patches would only be
1) renaming preferred_bpp (that would be unused at this point) to flags
and 2) make simpledrm to set FBDEV_FIRMWARE flag or something like that.

Another option is to add a third flags param to drm_fbdev_generic_setup()
and make all drivers to set 0 besides simpledrm. That way marking the fb
as FBINFO_MISC_FIRMWARE won't be blocked by the preferred_depth cleanup.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat