linux-kernel - Re: [PATCH] video/aperture: fix typos

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]

Date:   Wed, 5 Apr 2023 18:36:53 +0800
From:   Sui Jingfeng <15330273260@....cn>
To:     Javier Martinez Canillas <javierm@...hat.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Maxime Ripard <mripard@...nel.org>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, Li Yi <liyi@...ngson.cn>,
        Christian Koenig <christian.koenig@....com>,
        Helge Deller <deller@....de>,
        Lucas De Marchi <lucas.demarchi@...el.com>
Cc:     linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] video/aperture: fix typos


On 2023/4/5 17:55, Sui Jingfeng wrote:
> Hi,
>
> thanks you for the time and effort  for reviewing.
>
> On 2023/4/4 19:03, Javier Martinez Canillas wrote:
>> Javier Martinez Canillas <javierm@...hat.com> writes:
>>
>> [...]
>>
>>>>>        /*
>>>>>         * Remove the device from the device hierarchy. This is the 
>>>>> right thing
>>>>> -     * to do for firmware-based DRM drivers, such as EFI, VESA or 
>>>>> VGA. After
>>>>> +     * to do for firmware-based fb drivers, such as EFI, VESA or 
>>>>> VGA. After
>>>> That sentences is not well phrased. Maybe say 'This is required for
>>>> firmware-provided graphics, such as EFI, VESA or VGA.'
>>>>
>>> Graphic drivers or display drivers would indeed be more accurate 
>>> here. But
>>> I think that "fb drivers" is still well pharsed since the are other 
>>> places
>>> where either fbdev or DRM drivers for firmware-provided framebuffers 
>>> are
>>> named like that.
>>>
>> Sui,
>>
>> Maybe you could post a follow-up patch to improve the comment as 
>> suggested
>> by Thomas?
>>
> Yes, certainly.
>
>
> This is the right thing to do for conflicting drivers takes over the 
> hardware resource required.
>

While the `drivers` include both drm driver and the real device 
dependent framebuffer driver,

They are typically build as kernel module as oppose to the efifb and 
simplefb which is built

into kernel kernel.  Firmware framebuffer is conflict with the drm 
driver is because the memory

region they use overlaps.  If there no overlap, then no `taken over`  
will be happen.


By the way,  I'm asked to made efifb and simplefb works on LoongArch and 
Mips platform in the past.

We are using downstream kernel(linux-4.19)  at that time. efifb is ony 
works for platform with uefi

firmware support. By ensure that framebuffer locate at the address range 
of the on-board video ram(VRAM)

and passing screen_info parameters to kernel correctly, 
drm_aperture_remove_conflicting_framebuffers

will success. This required the uefi firmware engineer understand this, 
for loongson bridge chip, this need

a small trick.  Simplefb is only tested on loongson SoC platform by 
providing fb parameters in DT which

match the PMON firmware's setting. As the framebuffer may located at 
anywhere in the physical address

space, there no aperture for SoC anymore.  Should call 
aperture_remove_conflicting_devices(0, ~0, false, "drmfb")

to remove them all.


>
> But the comments is actually nearly perfect in overall, it has some 
> difficulty to improve
>
> the perfection.  Below is my personal understanding toward the above 
> sentence.
>
>
> efifb and simplefb belong to the class of firmware based framebuffer 
> driver.
>
> They are generic and platform agnostic, yet they have to relay on the 
> firmware
>
> to passing fb format, fb size, fb base address, fb resolution and fb 
> stride etc to the kernel.
>
> Linux kernel using those information to fill the global screen_info 
> structure.
>
> sysfb_init() then using the global screen_info to  create a platform 
> device,
>
> the device will be claimed by efifb or simplefb driver finally. This 
> is a hand over solution.
>
> It relay on the firmware setup such a framebuffer and hand over the 
> state(this is
>
> actually a kind of modeset state) to kernel.
>
>
> efifb only own the potential hardware resource for a very short time if a
>
> conflicting drm driver probe successfully.
>
>
> For the platform/graphics without  a drm driver, developers may choose to
>
> use efifb driver as a replacement.  So, there are no conflicting 
> happen on
>
> such a case. The `nomodeset` kernel cmd options can also be used for
>
> debugging and testing purpose if the more intelligent drm driver is 
> broken
>
> due to bugs.
>