Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] sysfb: Fix screen_info type check for VGA
From: Javier Martinez Canillas @ 2025-06-04 11:15 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann, Alex Deucher,
	Tzung-Bi Shih, Helge Deller, Uwe Kleine-König, Zsolt Kajtar,
	stable
In-Reply-To: <20250603154838.401882-1-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Use the helper screen_info_video_type() to get the framebuffer
> type from struct screen_info. Handle supported values in sorted
> switch statement.
>
> Reading orig_video_isVGA is unreliable. On most systems it is a
> VIDEO_TYPE_ constant. On some systems with VGA it is simply set
> to 1 to signal the presence of a VGA output. See vga_probe() for
> an example. Retrieving the screen_info type with the helper
> screen_info_video_type() detects these cases and returns the
> appropriate VIDEO_TYPE_ constant. For VGA, sysfb creates a device
> named "vga-framebuffer".
>
> The sysfb code has been taken from vga16fb, where it likely didn't
> work correctly either. With this bugfix applied, vga16fb loads for
> compatible vga-framebuffer devices.
>

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH] sysfb: Fix screen_info type check for VGA
From: Tzung-Bi Shih @ 2025-06-04 10:51 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: javierm, dri-devel, linux-fbdev, Alex Deucher, Helge Deller,
	Uwe Kleine-König, Zsolt Kajtar, stable
In-Reply-To: <20250603154838.401882-1-tzimmermann@suse.de>

On Tue, Jun 03, 2025 at 05:48:20PM +0200, Thomas Zimmermann wrote:
> Use the helper screen_info_video_type() to get the framebuffer
> type from struct screen_info. Handle supported values in sorted
> switch statement.
> 
> Reading orig_video_isVGA is unreliable. On most systems it is a
> VIDEO_TYPE_ constant. On some systems with VGA it is simply set
> to 1 to signal the presence of a VGA output. See vga_probe() for
> an example. Retrieving the screen_info type with the helper
> screen_info_video_type() detects these cases and returns the
> appropriate VIDEO_TYPE_ constant. For VGA, sysfb creates a device
> named "vga-framebuffer".
> 
> The sysfb code has been taken from vga16fb, where it likely didn't
> work correctly either. With this bugfix applied, vga16fb loads for
> compatible vga-framebuffer devices.
> 
> Fixes: 0db5b61e0dc0 ("fbdev/vga16fb: Create EGA/VGA devices in sysfb code")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Helge Deller <deller@gmx.de>
> Cc: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
> Cc: Zsolt Kajtar <soci@c64.rulez.org>
> Cc: <stable@vger.kernel.org> # v6.1+
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-04  8:12 UTC (permalink / raw)
  To: Michael Kelley, David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
	haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, akpm@linux-foundation.org
  Cc: weh@microsoft.com, hch@lst.de, dri-devel@lists.freedesktop.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB4157871127ED95AD24EDF96DD46DA@SN6PR02MB4157.namprd02.prod.outlook.com>

Hi

Am 03.06.25 um 19:50 schrieb Michael Kelley:
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Monday, June 2, 2025 11:25 PM
>> Hi
>>
>> Am 03.06.25 um 03:49 schrieb Michael Kelley:
>> [...]
>>>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>>>> horrible hack.
>>>>
>>>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>>>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
>>> The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
>>> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
>>> a wrong impression. vm_mixed_ok() does a thorough job of validating the
>>> use of __vm_insert_mixed(), and since what I did was allowed, I thought
>>> perhaps it was OK. Your feedback has set me straight, and that's what I
>>> needed. :-)
>>>
>>> But the whole approach is moot with Alistair Popple's patch set that
>>> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
>>> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
>>> it. If there's not one, I'll take a crack at adding it in the next version of my
>>> patch set.
>> What is the motivation behind this work? The driver or fbdev as a whole
>> does not have much of a future anyway.
>>
>> I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
>>
> Yes, I think that's the longer term direction. A couple months ago I had an
> email conversation with Saurabh Sengar from the Microsoft Linux team where
> he raised this idea. I think the Microsoft folks will need to drive the deprecation
> process, as they need to coordinate with the distro vendors who publish
> images for running on local Hyper-V and in the Azure cloud. And my
> understanding is that the Linux kernel process would want the driver to
> be available but marked "deprecated" for a year or so before it actually
> goes away.

We (DRM upstream) recently considered moving some fbdev drivers to 
drivers/staging or marking them with !DRM if a DRM driver is available. 
Hyverv_fb would be a candidate.

At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works 
well on the various generations of the hyperv system. Much of our 
userspace would not be able to use hyperv_fb anyway.

>
> I do have some concerns about the maturity of the hyperv_drm driver
> "around the edges". For example, somebody just recently submitted a
> patch to flush output on panic. I have less familiarity hyperv_drm vs.
> hyperv_fb, so some of my concern is probably due to that. We might
> need to do review of hyperv_drm and see if there's anything else to
> deal with before hyperv_fb goes away.

The panic output is a feature that we recently added to the kernel. It 
allows a DRM driver to display a final error message in the case of a 
kernel panic (think of blue screens on Windows). Drivers require a 
minimum of support to make it work. That's what the hypervdrm patches 
were about.

Best regards
Thomas

>
> This all got started when I was looking at a problem with hyperv_fb,
> and I found several other related problems, some of which also existed
> in hyperv_drm. You've seen several small'ish fixes from me and Saurabh
> as a result, and this issue with mmap()'ing /dev/fb0 is the last one of that
> set. This fix is definitely a bit bigger, but it's the right fix. On the flip side,
> if we really get on a path to deprecate hyperv_fb, there are hack fixes for
> the mmap problem that are smaller and contained to hyperv_fb. I would
> be OK with a hack fix in that case.
>
> Michael

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply

* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03 17:50 UTC (permalink / raw)
  To: Thomas Zimmermann, David Hildenbrand, simona@ffwll.ch,
	deller@gmx.de, haiyangz@microsoft.com, kys@microsoft.com,
	wei.liu@kernel.org, decui@microsoft.com,
	akpm@linux-foundation.org
  Cc: weh@microsoft.com, hch@lst.de, dri-devel@lists.freedesktop.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <c0b91a50-d3e7-44f9-b9c5-9c3b29639428@suse.de>

From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Monday, June 2, 2025 11:25 PM
> 
> Hi
> 
> Am 03.06.25 um 03:49 schrieb Michael Kelley:
> [...]
> >> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> >> horrible hack.
> >>
> >> In another thread, you mention that you use PFN_SPECIAL to bypass the
> >> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> > The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
> > VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> > a wrong impression. vm_mixed_ok() does a thorough job of validating the
> > use of __vm_insert_mixed(), and since what I did was allowed, I thought
> > perhaps it was OK. Your feedback has set me straight, and that's what I
> > needed. :-)
> >
> > But the whole approach is moot with Alistair Popple's patch set that
> > eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> > special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> > it. If there's not one, I'll take a crack at adding it in the next version of my
> > patch set.
> 
> What is the motivation behind this work? The driver or fbdev as a whole
> does not have much of a future anyway.
> 
> I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
> 

Yes, I think that's the longer term direction. A couple months ago I had an
email conversation with Saurabh Sengar from the Microsoft Linux team where
he raised this idea. I think the Microsoft folks will need to drive the deprecation
process, as they need to coordinate with the distro vendors who publish
images for running on local Hyper-V and in the Azure cloud. And my
understanding is that the Linux kernel process would want the driver to
be available but marked "deprecated" for a year or so before it actually
goes away.

I do have some concerns about the maturity of the hyperv_drm driver
"around the edges". For example, somebody just recently submitted a
patch to flush output on panic. I have less familiarity hyperv_drm vs.
hyperv_fb, so some of my concern is probably due to that. We might
need to do review of hyperv_drm and see if there's anything else to
deal with before hyperv_fb goes away.

This all got started when I was looking at a problem with hyperv_fb,
and I found several other related problems, some of which also existed
in hyperv_drm. You've seen several small'ish fixes from me and Saurabh
as a result, and this issue with mmap()'ing /dev/fb0 is the last one of that
set. This fix is definitely a bit bigger, but it's the right fix. On the flip side, 
if we really get on a path to deprecate hyperv_fb, there are hack fixes for
the mmap problem that are smaller and contained to hyperv_fb. I would
be OK with a hack fix in that case.

Michael

^ permalink raw reply

* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03 17:24 UTC (permalink / raw)
  To: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
	haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, akpm@linux-foundation.org
  Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-mm@kvack.org
In-Reply-To: <e069436f-764d-464d-98ac-36a086297632@redhat.com>

From: David Hildenbrand <david@redhat.com> Sent: Tuesday, June 3, 2025 12:55 AM
> 
> On 03.06.25 03:49, Michael Kelley wrote:
> > From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
> >>
> >> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> >>> From: Michael Kelley <mhklinux@outlook.com>
> >>>
> >>> Current defio code works only for framebuffer memory that is allocated
> >>> with vmalloc(). The code assumes that the underlying page refcount can
> >>> be used by the mm subsystem to manage each framebuffer page's lifecycle,
> >>> including freeing the page if the refcount goes to 0. This approach is
> >>> consistent with vmalloc'ed memory, but not with contiguous kernel memory
> >>> allocated via alloc_pages() or similar. The latter such memory pages
> >>> usually have a refcount of 0 when allocated, and would be incorrectly
> >>> freed page-by-page if used with defio. That free'ing corrupts the memory
> >>> free lists and Linux eventually panics. Simply bumping the refcount after
> >>> allocation doesn’t work because when the framebuffer memory is freed,
> >>> __free_pages() complains about non-zero refcounts.
> >>>
> >>> Commit 37b4837959cb ("video: deferred io with physically contiguous
> >>> memory") from the year 2008 purported to add support for contiguous
> >>> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> >>> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> >>> use alloc_pages(). It's unclear to me how this commit actually worked at
> >>> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> >>> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> >>> arm32 architecture on which sh_mobile_lcdcfb is used.
> >>>
> >>> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> >>> sufficient to support contiguous kernel memory framebuffers. The problem
> >>> can be seen with the hyperv_fb driver, which may allocate the framebuffer
> >>> memory using vmalloc() or alloc_pages(), depending on the configuration
> >>> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
> >>>
> >>> Fix this limitation by adding defio support for contiguous kernel memory
> >>> framebuffers. A driver with a framebuffer allocated from contiguous
> >>> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
> >>>
> >>> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> >>> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> >>> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> >>> that the older arm32 devices still work correctly, but the path for
> >>> vmalloc() framebuffers is essentially unchanged.
> >>>
> >>> Even with these changes, defio does not support framebuffers in MMIO
> >>> space, as defio code depends on framebuffer memory pages having
> >>> corresponding 'struct page's.
> >>>
> >>> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV
> Gen 1 VMs.")
> >>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> >>> ---
> >>> Changes in v3:
> >>> * Moved definition of FBINFO_KMEMFB flag to a separate patch
> >>>     preceeding this one in the patch set [Helge Deller]
> >>> Changes in v2:
> >>> * Tweaked code comments regarding framebuffers allocated with
> >>>     dma_alloc_coherent() [Christoph Hellwig]
> >>>
> >>>    drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
> >>>    1 file changed, 108 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> >>> index 4fc93f253e06..f8ae91a1c4df 100644
> >>> --- a/drivers/video/fbdev/core/fb_defio.c
> >>> +++ b/drivers/video/fbdev/core/fb_defio.c
> >>> @@ -8,11 +8,40 @@
> >>>     * for more details.
> >>>     */
> >>>
> >>> +/*
> >>> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> >>> + * to batch user space writes into periodic updates to the underlying
> >>> + * framebuffer hardware or other implementation (such as with a virtualized
> >>> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> >>> + * framebuffer's kernel driver, and the callback is supplied with a list of
> >>> + * pages that have been modified in the preceding interval. The callback can
> >>> + * use this information to update the framebuffer hardware as necessary. The
> >>> + * batching can improve performance and reduce the overhead of updating the
> >>> + * hardware.
> >>> + *
> >>> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> >>> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> >>> + * memory allocations all have corresponding "struct page"s. Framebuffers
> >>> + * allocated using dma_alloc_coherent() should not be used with defio.
> >>> + * Such allocations should be treated as a black box owned by the DMA
> >>> + * layer, and should not be deconstructed into individual pages as defio
> >>> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> >>> + * does not have corrresponding "struct page"s.
> >>> + *
> >>> + * For framebuffers allocated using vmalloc(), struct fb_info must have
> >>> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> >>> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> >>> + * be set, and "fix.smem_start" must be set to the physical address of the
> >>> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> >>> + * size in bytes.
> >>> + */
> >>> +
> >>>    #include <linux/module.h>
> >>>    #include <linux/kernel.h>
> >>>    #include <linux/errno.h>
> >>>    #include <linux/string.h>
> >>>    #include <linux/mm.h>
> >>> +#include <linux/pfn_t.h>
> >>>    #include <linux/vmalloc.h>
> >>>    #include <linux/delay.h>
> >>>    #include <linux/interrupt.h>
> >>> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
> >>>    	else if (info->fix.smem_start)
> >>>    		page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
> >>>
> >>> -	if (page)
> >>> +	if (page && !(info->flags & FBINFO_KMEMFB))
> >>>    		get_page(page);
> >>>
> >>>    	return page;
> >>> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
> >>>
> >>>    	BUG_ON(!info->fbdefio->mapping);
> >>>
> >>> +	if (info->flags & FBINFO_KMEMFB)
> >>> +		/*
> >>> +		 * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> >>> +		 * there is no struct page associated with the page. The
> >>> +		 * PFN must be directly inserted and the created PTE will be
> >>> +		 * marked "special".
> >>> +		 */
> >>> +		return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> >>> +
> >>>    	vmf->page = page;
> >>>    	return 0;
> >>>    }
> >>> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
> >>>
> >>>    /*
> >>>     * Adds a page to the dirty list. Call this from struct
> >>> - * vm_operations_struct.page_mkwrite.
> >>> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
> >>>     */
> >>> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> >>> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
> >>>    					    struct page *page)
> >>>    {
> >>>    	struct fb_deferred_io *fbdefio = info->fbdefio;
> >>>    	struct fb_deferred_io_pageref *pageref;
> >>> +	unsigned long offset = vmf->pgoff << PAGE_SHIFT;
> >>>    	vm_fault_t ret;
> >>>
> >>>    	/* protect against the workqueue changing the page list */
> >>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> >>>    	}
> >>>
> >>>    	/*
> >>> -	 * We want the page to remain locked from ->page_mkwrite until
> >>> -	 * the PTE is marked dirty to avoid mapping_wrprotect_range()
> >>> -	 * being called before the PTE is updated, which would leave
> >>> -	 * the page ignored by defio.
> >>> -	 * Do this by locking the page here and informing the caller
> >>> -	 * about it with VM_FAULT_LOCKED.
> >>> +	 * The PTE must be marked writable before the defio deferred work runs
> >>> +	 * again and potentially marks the PTE write-protected. If the order
> >>> +	 * should be switched, the PTE would become writable without defio
> >>> +	 * tracking the page, leaving the page forever ignored by defio.
> >>> +	 *
> >>> +	 * For vmalloc() framebuffers, the associated struct page is locked
> >>> +	 * before releasing the defio lock. mm will later mark the PTE writaable
> >>> +	 * and release the struct page lock. The struct page lock prevents
> >>> +	 * the page from being prematurely being marked write-protected.
> >>> +	 *
> >>> +	 * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> >>> +	 * so the PTE must be marked writable while the defio lock is held.
> >>>    	 */
> >>> -	lock_page(pageref->page);
> >>> +	if (info->flags & FBINFO_KMEMFB) {
> >>> +		unsigned long pfn = page_to_pfn(pageref->page);
> >>> +
> >>> +		ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> >>> +					       __pfn_to_pfn_t(pfn, PFN_SPECIAL));
> >>
> >> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> >> horrible hack.
> >>
> >> In another thread, you mention that you use PFN_SPECIAL to bypass the
> >> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> >
> > The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
> > VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> > a wrong impression.
> 
> VM_PFNMAP: nothing is refcounted except anon pages
> 
> VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted
> 
> pte_special() is a way for GUP-fast to distinguish these refcounted (can
> GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any
> locks or the VMA being available.
> 
> Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page"
> (pfn_valid()) is likely very bogus.

OK, good to know.

> 
> > vm_mixed_ok() does a thorough job of validating the
> > use of __vm_insert_mixed(), and since what I did was allowed, I thought
> > perhaps it was OK. Your feedback has set me straight, and that's what I
> > needed. :-)
> 
> What exactly are you trying to achieve? :)
> 
> If it's mapping a page with a "struct page" and *not* refcounting it,
> then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP
> mapping. It will set pte_special() automatically for you.
> 

Yes, that's what I'm using to initially create the special PTE in the
.fault callback.

> >
> > But the whole approach is moot with Alistair Popple's patch set that
> > eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> > special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> > it. If there's not one, I'll take a crack at adding it in the next version of my
> > patch set.
> 
> I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably
> vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe
> :) )

Ok, I'll look at that more closely. The sequence is that the special
PTE gets created with vmf_insert_pfn(). Then when the page is first
written to, the .pfn_mkwrite callback is invoked by mm. The question
is the best way for that callback to mark the existing PTE as writable.

Thanks,

Michael

^ permalink raw reply

* Re: [PATCH 2/2] video: Make global edid_info depend on CONFIG_FIRMWARE_EDID
From: Helge Deller @ 2025-06-03 16:42 UTC (permalink / raw)
  To: Thomas Zimmermann, arnd, javierm; +Cc: dri-devel, linux-fbdev
In-Reply-To: <cd3e5ccb-3bdb-4a2b-a67e-3e97f06df433@suse.de>

On 6/3/25 18:18, Thomas Zimmermann wrote:
> Hi
> 
> Am 03.06.25 um 18:08 schrieb Helge Deller:
>> On 6/2/25 09:51, Thomas Zimmermann wrote:
>>> Protect global edid_info behind CONFIG_FIRMWARE_EDID and remove
>>> the config tests for CONFIG_X86. Makes edid_info available iff
>>> its option has been enabled.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   arch/x86/kernel/setup.c         | 4 ++++
>>>   drivers/gpu/drm/sysfb/efidrm.c  | 2 +-
>>>   drivers/gpu/drm/sysfb/vesadrm.c | 2 +-
>>>   include/video/edid.h            | 3 ++-
>>>   4 files changed, 8 insertions(+), 3 deletions(-)
>>
>> Reviewed-by: Helge Deller <deller@gmx.de>
> 
> Thanks for reviewing. I'd like to merge the series via drm-misc for some other patches I have for DRM. Ok?

Sure.

Helge

^ permalink raw reply

* Re: [PATCH 2/2] video: Make global edid_info depend on CONFIG_FIRMWARE_EDID
From: Thomas Zimmermann @ 2025-06-03 16:18 UTC (permalink / raw)
  To: Helge Deller, arnd, javierm; +Cc: dri-devel, linux-fbdev
In-Reply-To: <4e3f9936-0d0f-4e93-888f-738daa345905@gmx.de>

Hi

Am 03.06.25 um 18:08 schrieb Helge Deller:
> On 6/2/25 09:51, Thomas Zimmermann wrote:
>> Protect global edid_info behind CONFIG_FIRMWARE_EDID and remove
>> the config tests for CONFIG_X86. Makes edid_info available iff
>> its option has been enabled.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   arch/x86/kernel/setup.c         | 4 ++++
>>   drivers/gpu/drm/sysfb/efidrm.c  | 2 +-
>>   drivers/gpu/drm/sysfb/vesadrm.c | 2 +-
>>   include/video/edid.h            | 3 ++-
>>   4 files changed, 8 insertions(+), 3 deletions(-)
>
> Reviewed-by: Helge Deller <deller@gmx.de>

Thanks for reviewing. I'd like to merge the series via drm-misc for some 
other patches I have for DRM. Ok?

Best regards
Thomas


-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply

* Re: [PATCH 2/2] video: Make global edid_info depend on CONFIG_FIRMWARE_EDID
From: Helge Deller @ 2025-06-03 16:08 UTC (permalink / raw)
  To: Thomas Zimmermann, arnd, javierm; +Cc: dri-devel, linux-fbdev
In-Reply-To: <20250602075537.137759-3-tzimmermann@suse.de>

On 6/2/25 09:51, Thomas Zimmermann wrote:
> Protect global edid_info behind CONFIG_FIRMWARE_EDID and remove
> the config tests for CONFIG_X86. Makes edid_info available iff
> its option has been enabled.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   arch/x86/kernel/setup.c         | 4 ++++
>   drivers/gpu/drm/sysfb/efidrm.c  | 2 +-
>   drivers/gpu/drm/sysfb/vesadrm.c | 2 +-
>   include/video/edid.h            | 3 ++-
>   4 files changed, 8 insertions(+), 3 deletions(-)

Reviewed-by: Helge Deller <deller@gmx.de>


^ permalink raw reply

* Re: [PATCH 1/2] video: Make CONFIG_FIRMWARE_EDID generally available
From: Helge Deller @ 2025-06-03 16:08 UTC (permalink / raw)
  To: Thomas Zimmermann, arnd, javierm; +Cc: dri-devel, linux-fbdev
In-Reply-To: <20250602075537.137759-2-tzimmermann@suse.de>

On 6/2/25 09:51, Thomas Zimmermann wrote:
> DRM drivers such as efidrm and vesadrm can export firmware EDID
> data to userspace. Make the related option CONFIG_FIRMWARE_EDID
> available without CONFIG_FB. Make it depend on X86, which is
> currently the only architecture providing EDID information.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/video/Kconfig            | 18 +++++++++++++++++-
>   drivers/video/fbdev/core/Kconfig | 15 ---------------
>   drivers/video/fbdev/core/fbmon.c |  3 +--
>   3 files changed, 18 insertions(+), 18 deletions(-)

Reviewed-by: Helge Deller <deller@gmx.de>


^ permalink raw reply

* [PATCH] sysfb: Fix screen_info type check for VGA
From: Thomas Zimmermann @ 2025-06-03 15:48 UTC (permalink / raw)
  To: javierm
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann, Alex Deucher,
	Tzung-Bi Shih, Helge Deller, Uwe Kleine-König, Zsolt Kajtar,
	stable

Use the helper screen_info_video_type() to get the framebuffer
type from struct screen_info. Handle supported values in sorted
switch statement.

Reading orig_video_isVGA is unreliable. On most systems it is a
VIDEO_TYPE_ constant. On some systems with VGA it is simply set
to 1 to signal the presence of a VGA output. See vga_probe() for
an example. Retrieving the screen_info type with the helper
screen_info_video_type() detects these cases and returns the
appropriate VIDEO_TYPE_ constant. For VGA, sysfb creates a device
named "vga-framebuffer".

The sysfb code has been taken from vga16fb, where it likely didn't
work correctly either. With this bugfix applied, vga16fb loads for
compatible vga-framebuffer devices.

Fixes: 0db5b61e0dc0 ("fbdev/vga16fb: Create EGA/VGA devices in sysfb code")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Helge Deller <deller@gmx.de>
Cc: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: Zsolt Kajtar <soci@c64.rulez.org>
Cc: <stable@vger.kernel.org> # v6.1+
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/sysfb.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 7c5c03f274b9..889e5b05c739 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -143,6 +143,7 @@ static __init int sysfb_init(void)
 {
 	struct screen_info *si = &screen_info;
 	struct device *parent;
+	unsigned int type;
 	struct simplefb_platform_data mode;
 	const char *name;
 	bool compatible;
@@ -170,17 +171,26 @@ static __init int sysfb_init(void)
 			goto put_device;
 	}
 
+	type = screen_info_video_type(si);
+
 	/* if the FB is incompatible, create a legacy framebuffer device */
-	if (si->orig_video_isVGA == VIDEO_TYPE_EFI)
-		name = "efi-framebuffer";
-	else if (si->orig_video_isVGA == VIDEO_TYPE_VLFB)
-		name = "vesa-framebuffer";
-	else if (si->orig_video_isVGA == VIDEO_TYPE_VGAC)
-		name = "vga-framebuffer";
-	else if (si->orig_video_isVGA == VIDEO_TYPE_EGAC)
+	switch (type) {
+	case VIDEO_TYPE_EGAC:
 		name = "ega-framebuffer";
-	else
+		break;
+	case VIDEO_TYPE_VGAC:
+		name = "vga-framebuffer";
+		break;
+	case VIDEO_TYPE_VLFB:
+		name = "vesa-framebuffer";
+		break;
+	case VIDEO_TYPE_EFI:
+		name = "efi-framebuffer";
+		break;
+	default:
 		name = "platform-framebuffer";
+		break;
+	}
 
 	pd = platform_device_alloc(name, 0);
 	if (!pd) {
-- 
2.49.0


^ permalink raw reply related

* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: David Hildenbrand @ 2025-06-03  7:55 UTC (permalink / raw)
  To: Michael Kelley, simona@ffwll.ch, deller@gmx.de,
	haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, akpm@linux-foundation.org
  Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-mm@kvack.org
In-Reply-To: <SN6PR02MB41573C075152ECD8428CAF5ED46DA@SN6PR02MB4157.namprd02.prod.outlook.com>

On 03.06.25 03:49, Michael Kelley wrote:
> From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
>>
>> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
>>> From: Michael Kelley <mhklinux@outlook.com>
>>>
>>> Current defio code works only for framebuffer memory that is allocated
>>> with vmalloc(). The code assumes that the underlying page refcount can
>>> be used by the mm subsystem to manage each framebuffer page's lifecycle,
>>> including freeing the page if the refcount goes to 0. This approach is
>>> consistent with vmalloc'ed memory, but not with contiguous kernel memory
>>> allocated via alloc_pages() or similar. The latter such memory pages
>>> usually have a refcount of 0 when allocated, and would be incorrectly
>>> freed page-by-page if used with defio. That free'ing corrupts the memory
>>> free lists and Linux eventually panics. Simply bumping the refcount after
>>> allocation doesn’t work because when the framebuffer memory is freed,
>>> __free_pages() complains about non-zero refcounts.
>>>
>>> Commit 37b4837959cb ("video: deferred io with physically contiguous
>>> memory") from the year 2008 purported to add support for contiguous
>>> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
>>> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
>>> use alloc_pages(). It's unclear to me how this commit actually worked at
>>> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
>>> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
>>> arm32 architecture on which sh_mobile_lcdcfb is used.
>>>
>>> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
>>> sufficient to support contiguous kernel memory framebuffers. The problem
>>> can be seen with the hyperv_fb driver, which may allocate the framebuffer
>>> memory using vmalloc() or alloc_pages(), depending on the configuration
>>> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
>>>
>>> Fix this limitation by adding defio support for contiguous kernel memory
>>> framebuffers. A driver with a framebuffer allocated from contiguous
>>> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
>>>
>>> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
>>> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
>>> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
>>> that the older arm32 devices still work correctly, but the path for
>>> vmalloc() framebuffers is essentially unchanged.
>>>
>>> Even with these changes, defio does not support framebuffers in MMIO
>>> space, as defio code depends on framebuffer memory pages having
>>> corresponding 'struct page's.
>>>
>>> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
>>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>>> ---
>>> Changes in v3:
>>> * Moved definition of FBINFO_KMEMFB flag to a separate patch
>>>     preceeding this one in the patch set [Helge Deller]
>>> Changes in v2:
>>> * Tweaked code comments regarding framebuffers allocated with
>>>     dma_alloc_coherent() [Christoph Hellwig]
>>>
>>>    drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
>>>    1 file changed, 108 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
>>> index 4fc93f253e06..f8ae91a1c4df 100644
>>> --- a/drivers/video/fbdev/core/fb_defio.c
>>> +++ b/drivers/video/fbdev/core/fb_defio.c
>>> @@ -8,11 +8,40 @@
>>>     * for more details.
>>>     */
>>>
>>> +/*
>>> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
>>> + * to batch user space writes into periodic updates to the underlying
>>> + * framebuffer hardware or other implementation (such as with a virtualized
>>> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
>>> + * framebuffer's kernel driver, and the callback is supplied with a list of
>>> + * pages that have been modified in the preceding interval. The callback can
>>> + * use this information to update the framebuffer hardware as necessary. The
>>> + * batching can improve performance and reduce the overhead of updating the
>>> + * hardware.
>>> + *
>>> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
>>> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
>>> + * memory allocations all have corresponding "struct page"s. Framebuffers
>>> + * allocated using dma_alloc_coherent() should not be used with defio.
>>> + * Such allocations should be treated as a black box owned by the DMA
>>> + * layer, and should not be deconstructed into individual pages as defio
>>> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
>>> + * does not have corrresponding "struct page"s.
>>> + *
>>> + * For framebuffers allocated using vmalloc(), struct fb_info must have
>>> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
>>> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
>>> + * be set, and "fix.smem_start" must be set to the physical address of the
>>> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
>>> + * size in bytes.
>>> + */
>>> +
>>>    #include <linux/module.h>
>>>    #include <linux/kernel.h>
>>>    #include <linux/errno.h>
>>>    #include <linux/string.h>
>>>    #include <linux/mm.h>
>>> +#include <linux/pfn_t.h>
>>>    #include <linux/vmalloc.h>
>>>    #include <linux/delay.h>
>>>    #include <linux/interrupt.h>
>>> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
>>>    	else if (info->fix.smem_start)
>>>    		page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
>>>
>>> -	if (page)
>>> +	if (page && !(info->flags & FBINFO_KMEMFB))
>>>    		get_page(page);
>>>
>>>    	return page;
>>> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
>>>
>>>    	BUG_ON(!info->fbdefio->mapping);
>>>
>>> +	if (info->flags & FBINFO_KMEMFB)
>>> +		/*
>>> +		 * In this path, the VMA is marked VM_PFNMAP, so mm assumes
>>> +		 * there is no struct page associated with the page. The
>>> +		 * PFN must be directly inserted and the created PTE will be
>>> +		 * marked "special".
>>> +		 */
>>> +		return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
>>> +
>>>    	vmf->page = page;
>>>    	return 0;
>>>    }
>>> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
>>>
>>>    /*
>>>     * Adds a page to the dirty list. Call this from struct
>>> - * vm_operations_struct.page_mkwrite.
>>> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
>>>     */
>>> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
>>> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
>>>    					    struct page *page)
>>>    {
>>>    	struct fb_deferred_io *fbdefio = info->fbdefio;
>>>    	struct fb_deferred_io_pageref *pageref;
>>> +	unsigned long offset = vmf->pgoff << PAGE_SHIFT;
>>>    	vm_fault_t ret;
>>>
>>>    	/* protect against the workqueue changing the page list */
>>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
>>>    	}
>>>
>>>    	/*
>>> -	 * We want the page to remain locked from ->page_mkwrite until
>>> -	 * the PTE is marked dirty to avoid mapping_wrprotect_range()
>>> -	 * being called before the PTE is updated, which would leave
>>> -	 * the page ignored by defio.
>>> -	 * Do this by locking the page here and informing the caller
>>> -	 * about it with VM_FAULT_LOCKED.
>>> +	 * The PTE must be marked writable before the defio deferred work runs
>>> +	 * again and potentially marks the PTE write-protected. If the order
>>> +	 * should be switched, the PTE would become writable without defio
>>> +	 * tracking the page, leaving the page forever ignored by defio.
>>> +	 *
>>> +	 * For vmalloc() framebuffers, the associated struct page is locked
>>> +	 * before releasing the defio lock. mm will later mark the PTE writaable
>>> +	 * and release the struct page lock. The struct page lock prevents
>>> +	 * the page from being prematurely being marked write-protected.
>>> +	 *
>>> +	 * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
>>> +	 * so the PTE must be marked writable while the defio lock is held.
>>>    	 */
>>> -	lock_page(pageref->page);
>>> +	if (info->flags & FBINFO_KMEMFB) {
>>> +		unsigned long pfn = page_to_pfn(pageref->page);
>>> +
>>> +		ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
>>> +					       __pfn_to_pfn_t(pfn, PFN_SPECIAL));
>>
>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>> horrible hack.
>>
>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> 
> The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> a wrong impression.

VM_PFNMAP: nothing is refcounted except anon pages

VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted


pte_special() is a way for GUP-fast to distinguish these refcounted (can 
GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any 
locks or the VMA being available.


Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page" 
(pfn_valid()) is likely very bogus.

> vm_mixed_ok() does a thorough job of validating the
> use of __vm_insert_mixed(), and since what I did was allowed, I thought
> perhaps it was OK. Your feedback has set me straight, and that's what I
> needed. :-)

What exactly are you trying to achieve? :)

If it's mapping a page with a "struct page" and *not* refcounting it, 
then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP 
mapping. It will set pte_special() automatically for you.

> 
> But the whole approach is moot with Alistair Popple's patch set that
> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> it. If there's not one, I'll take a crack at adding it in the next version of my
> patch set.

I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably 
vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe 
:) )

-- 
Cheers,

David / dhildenb


^ permalink raw reply

* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-03  6:25 UTC (permalink / raw)
  To: Michael Kelley, David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
	haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, akpm@linux-foundation.org
  Cc: weh@microsoft.com, hch@lst.de, dri-devel@lists.freedesktop.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB41573C075152ECD8428CAF5ED46DA@SN6PR02MB4157.namprd02.prod.outlook.com>

Hi

Am 03.06.25 um 03:49 schrieb Michael Kelley:
[...]
>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>> horrible hack.
>>
>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> a wrong impression. vm_mixed_ok() does a thorough job of validating the
> use of __vm_insert_mixed(), and since what I did was allowed, I thought
> perhaps it was OK. Your feedback has set me straight, and that's what I
> needed. :-)
>
> But the whole approach is moot with Alistair Popple's patch set that
> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> it. If there's not one, I'll take a crack at adding it in the next version of my
> patch set.

What is the motivation behind this work? The driver or fbdev as a whole 
does not have much of a future anyway.

I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?

Best regards
Thomas

>
> Michael

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply

* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03  1:49 UTC (permalink / raw)
  To: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
	haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, akpm@linux-foundation.org
  Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-mm@kvack.org
In-Reply-To: <de0f2cb8-aed6-436f-b55e-d3f7b3fe6d81@redhat.com>

From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
> 
> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > Current defio code works only for framebuffer memory that is allocated
> > with vmalloc(). The code assumes that the underlying page refcount can
> > be used by the mm subsystem to manage each framebuffer page's lifecycle,
> > including freeing the page if the refcount goes to 0. This approach is
> > consistent with vmalloc'ed memory, but not with contiguous kernel memory
> > allocated via alloc_pages() or similar. The latter such memory pages
> > usually have a refcount of 0 when allocated, and would be incorrectly
> > freed page-by-page if used with defio. That free'ing corrupts the memory
> > free lists and Linux eventually panics. Simply bumping the refcount after
> > allocation doesn’t work because when the framebuffer memory is freed,
> > __free_pages() complains about non-zero refcounts.
> >
> > Commit 37b4837959cb ("video: deferred io with physically contiguous
> > memory") from the year 2008 purported to add support for contiguous
> > kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> > dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> > use alloc_pages(). It's unclear to me how this commit actually worked at
> > the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> > of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> > arm32 architecture on which sh_mobile_lcdcfb is used.
> >
> > In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> > sufficient to support contiguous kernel memory framebuffers. The problem
> > can be seen with the hyperv_fb driver, which may allocate the framebuffer
> > memory using vmalloc() or alloc_pages(), depending on the configuration
> > of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
> >
> > Fix this limitation by adding defio support for contiguous kernel memory
> > framebuffers. A driver with a framebuffer allocated from contiguous
> > kernel memory must set the FBINFO_KMEMFB flag to indicate such.
> >
> > Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> > framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> > vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> > that the older arm32 devices still work correctly, but the path for
> > vmalloc() framebuffers is essentially unchanged.
> >
> > Even with these changes, defio does not support framebuffers in MMIO
> > space, as defio code depends on framebuffer memory pages having
> > corresponding 'struct page's.
> >
> > Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > ---
> > Changes in v3:
> > * Moved definition of FBINFO_KMEMFB flag to a separate patch
> >    preceeding this one in the patch set [Helge Deller]
> > Changes in v2:
> > * Tweaked code comments regarding framebuffers allocated with
> >    dma_alloc_coherent() [Christoph Hellwig]
> >
> >   drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
> >   1 file changed, 108 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> > index 4fc93f253e06..f8ae91a1c4df 100644
> > --- a/drivers/video/fbdev/core/fb_defio.c
> > +++ b/drivers/video/fbdev/core/fb_defio.c
> > @@ -8,11 +8,40 @@
> >    * for more details.
> >    */
> >
> > +/*
> > + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> > + * to batch user space writes into periodic updates to the underlying
> > + * framebuffer hardware or other implementation (such as with a virtualized
> > + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> > + * framebuffer's kernel driver, and the callback is supplied with a list of
> > + * pages that have been modified in the preceding interval. The callback can
> > + * use this information to update the framebuffer hardware as necessary. The
> > + * batching can improve performance and reduce the overhead of updating the
> > + * hardware.
> > + *
> > + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> > + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> > + * memory allocations all have corresponding "struct page"s. Framebuffers
> > + * allocated using dma_alloc_coherent() should not be used with defio.
> > + * Such allocations should be treated as a black box owned by the DMA
> > + * layer, and should not be deconstructed into individual pages as defio
> > + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> > + * does not have corrresponding "struct page"s.
> > + *
> > + * For framebuffers allocated using vmalloc(), struct fb_info must have
> > + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> > + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> > + * be set, and "fix.smem_start" must be set to the physical address of the
> > + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> > + * size in bytes.
> > + */
> > +
> >   #include <linux/module.h>
> >   #include <linux/kernel.h>
> >   #include <linux/errno.h>
> >   #include <linux/string.h>
> >   #include <linux/mm.h>
> > +#include <linux/pfn_t.h>
> >   #include <linux/vmalloc.h>
> >   #include <linux/delay.h>
> >   #include <linux/interrupt.h>
> > @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
> >   	else if (info->fix.smem_start)
> >   		page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
> >
> > -	if (page)
> > +	if (page && !(info->flags & FBINFO_KMEMFB))
> >   		get_page(page);
> >
> >   	return page;
> > @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
> >
> >   	BUG_ON(!info->fbdefio->mapping);
> >
> > +	if (info->flags & FBINFO_KMEMFB)
> > +		/*
> > +		 * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> > +		 * there is no struct page associated with the page. The
> > +		 * PFN must be directly inserted and the created PTE will be
> > +		 * marked "special".
> > +		 */
> > +		return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> > +
> >   	vmf->page = page;
> >   	return 0;
> >   }
> > @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
> >
> >   /*
> >    * Adds a page to the dirty list. Call this from struct
> > - * vm_operations_struct.page_mkwrite.
> > + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
> >    */
> > -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> > +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
> >   					    struct page *page)
> >   {
> >   	struct fb_deferred_io *fbdefio = info->fbdefio;
> >   	struct fb_deferred_io_pageref *pageref;
> > +	unsigned long offset = vmf->pgoff << PAGE_SHIFT;
> >   	vm_fault_t ret;
> >
> >   	/* protect against the workqueue changing the page list */
> > @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> >   	}
> >
> >   	/*
> > -	 * We want the page to remain locked from ->page_mkwrite until
> > -	 * the PTE is marked dirty to avoid mapping_wrprotect_range()
> > -	 * being called before the PTE is updated, which would leave
> > -	 * the page ignored by defio.
> > -	 * Do this by locking the page here and informing the caller
> > -	 * about it with VM_FAULT_LOCKED.
> > +	 * The PTE must be marked writable before the defio deferred work runs
> > +	 * again and potentially marks the PTE write-protected. If the order
> > +	 * should be switched, the PTE would become writable without defio
> > +	 * tracking the page, leaving the page forever ignored by defio.
> > +	 *
> > +	 * For vmalloc() framebuffers, the associated struct page is locked
> > +	 * before releasing the defio lock. mm will later mark the PTE writaable
> > +	 * and release the struct page lock. The struct page lock prevents
> > +	 * the page from being prematurely being marked write-protected.
> > +	 *
> > +	 * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> > +	 * so the PTE must be marked writable while the defio lock is held.
> >   	 */
> > -	lock_page(pageref->page);
> > +	if (info->flags & FBINFO_KMEMFB) {
> > +		unsigned long pfn = page_to_pfn(pageref->page);
> > +
> > +		ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> > +					       __pfn_to_pfn_t(pfn, PFN_SPECIAL));
> 
> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> horrible hack.
> 
> In another thread, you mention that you use PFN_SPECIAL to bypass the
> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?

The VMA has VM_PFNMAP set, not VM_MIXEDMAP.  It seemed like
VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
a wrong impression. vm_mixed_ok() does a thorough job of validating the
use of __vm_insert_mixed(), and since what I did was allowed, I thought
perhaps it was OK. Your feedback has set me straight, and that's what I 
needed. :-)

But the whole approach is moot with Alistair Popple's patch set that
eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
it. If there's not one, I'll take a crack at adding it in the next version of my
patch set.

Michael

^ permalink raw reply

* Re: [RFC PATCH 1/2] backlight: Rename duplicated devices to support dual-backlight setups
From: Pengyu Luo @ 2025-06-02 13:45 UTC (permalink / raw)
  To: jani.nikula
  Cc: danielt, deller, dri-devel, jingoohan1, lee, linux-fbdev,
	linux-kernel, mitltlatltl
In-Reply-To: <7dc6a9e5171bc70be23188ffd8c45168fa79aacb@intel.com>

On Mon, May 26, 2025 at 4:53 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Sun, 25 May 2025, Pengyu Luo <mitltlatltl@gmail.com> wrote:
> > When registering a backlight device, if a device with the same name
> > already exists, append "-secondary" to the new device's name. This is
> > useful for platforms with dual backlight drivers (e.g. some panels use
> > dual ktz8866), where both instances need to coexist.
> >
> > For now, only one secondary instance is supported. If more instances
> > are needed, this logic can be extended with auto-increment or a more
> > flexible naming scheme.
>
> I think for both patches you should consider adding a new interface for
> creating dual backlight scenarios.
>
> For example, this patch turns a driver error (registering two or more
> backlights with the same name) into a special use case, patch 2
> magically connecting the two, and hiding the problem.
>
> With i915, you could have multiple devices, each with multiple
> independent panels with independent backlights. I think accidentally
> trying to register more than one backlight with the same name should
> remain an error, *unless* you want the special case of combined
> backlights.
>
> Similarly, what if you encounter a device with two panels, and two
> *independent* ktz8866?
>
> Please be explicit rather than implicit.
>

For i915, I noticed that it renamed the device internally, so I tried
to rename it in the backlight driver.

Indeed, patch 2 may combine independent panels, I was playing with
android tablet, and I ignored it, you had well explained it. Thanks
for pointing out, I will consider adding a new interface.

Best wishes,
Pengyu

^ permalink raw reply

* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: David Hildenbrand @ 2025-06-02  9:47 UTC (permalink / raw)
  To: mhklinux, simona, deller, haiyangz, kys, wei.liu, decui, akpm
  Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
	linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-4-mhklinux@outlook.com>

On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> Current defio code works only for framebuffer memory that is allocated
> with vmalloc(). The code assumes that the underlying page refcount can
> be used by the mm subsystem to manage each framebuffer page's lifecycle,
> including freeing the page if the refcount goes to 0. This approach is
> consistent with vmalloc'ed memory, but not with contiguous kernel memory
> allocated via alloc_pages() or similar. The latter such memory pages
> usually have a refcount of 0 when allocated, and would be incorrectly
> freed page-by-page if used with defio. That free'ing corrupts the memory
> free lists and Linux eventually panics. Simply bumping the refcount after
> allocation doesn’t work because when the framebuffer memory is freed,
> __free_pages() complains about non-zero refcounts.
> 
> Commit 37b4837959cb ("video: deferred io with physically contiguous
> memory") from the year 2008 purported to add support for contiguous
> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> use alloc_pages(). It's unclear to me how this commit actually worked at
> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> arm32 architecture on which sh_mobile_lcdcfb is used.
> 
> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> sufficient to support contiguous kernel memory framebuffers. The problem
> can be seen with the hyperv_fb driver, which may allocate the framebuffer
> memory using vmalloc() or alloc_pages(), depending on the configuration
> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
> 
> Fix this limitation by adding defio support for contiguous kernel memory
> framebuffers. A driver with a framebuffer allocated from contiguous
> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
> 
> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> that the older arm32 devices still work correctly, but the path for
> vmalloc() framebuffers is essentially unchanged.
> 
> Even with these changes, defio does not support framebuffers in MMIO
> space, as defio code depends on framebuffer memory pages having
> corresponding 'struct page's.
> 
> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
> Changes in v3:
> * Moved definition of FBINFO_KMEMFB flag to a separate patch
>    preceeding this one in the patch set [Helge Deller]
> Changes in v2:
> * Tweaked code comments regarding framebuffers allocated with
>    dma_alloc_coherent() [Christoph Hellwig]
> 
>   drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
>   1 file changed, 108 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index 4fc93f253e06..f8ae91a1c4df 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -8,11 +8,40 @@
>    * for more details.
>    */
>   
> +/*
> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> + * to batch user space writes into periodic updates to the underlying
> + * framebuffer hardware or other implementation (such as with a virtualized
> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> + * framebuffer's kernel driver, and the callback is supplied with a list of
> + * pages that have been modified in the preceding interval. The callback can
> + * use this information to update the framebuffer hardware as necessary. The
> + * batching can improve performance and reduce the overhead of updating the
> + * hardware.
> + *
> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> + * memory allocations all have corresponding "struct page"s. Framebuffers
> + * allocated using dma_alloc_coherent() should not be used with defio.
> + * Such allocations should be treated as a black box owned by the DMA
> + * layer, and should not be deconstructed into individual pages as defio
> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> + * does not have corrresponding "struct page"s.
> + *
> + * For framebuffers allocated using vmalloc(), struct fb_info must have
> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> + * be set, and "fix.smem_start" must be set to the physical address of the
> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> + * size in bytes.
> + */
> +
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/errno.h>
>   #include <linux/string.h>
>   #include <linux/mm.h>
> +#include <linux/pfn_t.h>
>   #include <linux/vmalloc.h>
>   #include <linux/delay.h>
>   #include <linux/interrupt.h>
> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
>   	else if (info->fix.smem_start)
>   		page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
>   
> -	if (page)
> +	if (page && !(info->flags & FBINFO_KMEMFB))
>   		get_page(page);
>   
>   	return page;
> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
>   
>   	BUG_ON(!info->fbdefio->mapping);
>   
> +	if (info->flags & FBINFO_KMEMFB)
> +		/*
> +		 * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> +		 * there is no struct page associated with the page. The
> +		 * PFN must be directly inserted and the created PTE will be
> +		 * marked "special".
> +		 */
> +		return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> +
>   	vmf->page = page;
>   	return 0;
>   }
> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
>   
>   /*
>    * Adds a page to the dirty list. Call this from struct
> - * vm_operations_struct.page_mkwrite.
> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
>    */
> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
>   					    struct page *page)
>   {
>   	struct fb_deferred_io *fbdefio = info->fbdefio;
>   	struct fb_deferred_io_pageref *pageref;
> +	unsigned long offset = vmf->pgoff << PAGE_SHIFT;
>   	vm_fault_t ret;
>   
>   	/* protect against the workqueue changing the page list */
> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
>   	}
>   
>   	/*
> -	 * We want the page to remain locked from ->page_mkwrite until
> -	 * the PTE is marked dirty to avoid mapping_wrprotect_range()
> -	 * being called before the PTE is updated, which would leave
> -	 * the page ignored by defio.
> -	 * Do this by locking the page here and informing the caller
> -	 * about it with VM_FAULT_LOCKED.
> +	 * The PTE must be marked writable before the defio deferred work runs
> +	 * again and potentially marks the PTE write-protected. If the order
> +	 * should be switched, the PTE would become writable without defio
> +	 * tracking the page, leaving the page forever ignored by defio.
> +	 *
> +	 * For vmalloc() framebuffers, the associated struct page is locked
> +	 * before releasing the defio lock. mm will later mark the PTE writaable
> +	 * and release the struct page lock. The struct page lock prevents
> +	 * the page from being prematurely being marked write-protected.
> +	 *
> +	 * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> +	 * so the PTE must be marked writable while the defio lock is held.
>   	 */
> -	lock_page(pageref->page);
> +	if (info->flags & FBINFO_KMEMFB) {
> +		unsigned long pfn = page_to_pfn(pageref->page);
> +
> +		ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> +					       __pfn_to_pfn_t(pfn, PFN_SPECIAL));

Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a 
horrible hack.

In another thread, you mention that you use PFN_SPECIAL to bypass the 
check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?

It's all a mess ...

-- 
Cheers,

David / dhildenb


^ permalink raw reply

* [PATCH 2/2] video: Make global edid_info depend on CONFIG_FIRMWARE_EDID
From: Thomas Zimmermann @ 2025-06-02  7:51 UTC (permalink / raw)
  To: arnd, javierm, deller; +Cc: dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250602075537.137759-1-tzimmermann@suse.de>

Protect global edid_info behind CONFIG_FIRMWARE_EDID and remove
the config tests for CONFIG_X86. Makes edid_info available iff
its option has been enabled.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 arch/x86/kernel/setup.c         | 4 ++++
 drivers/gpu/drm/sysfb/efidrm.c  | 2 +-
 drivers/gpu/drm/sysfb/vesadrm.c | 2 +-
 include/video/edid.h            | 3 ++-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d2a13b37833..cfe501d323d5 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -217,8 +217,10 @@ arch_initcall(init_x86_sysctl);
  */
 struct screen_info screen_info;
 EXPORT_SYMBOL(screen_info);
+#if defined(CONFIG_FIRMWARE_EDID)
 struct edid_info edid_info;
 EXPORT_SYMBOL_GPL(edid_info);
+#endif
 
 extern int root_mountflags;
 
@@ -503,7 +505,9 @@ static void __init parse_boot_params(void)
 {
 	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
 	screen_info = boot_params.screen_info;
+#if defined(CONFIG_FIRMWARE_EDID)
 	edid_info = boot_params.edid_info;
+#endif
 #ifdef CONFIG_X86_32
 	apm_info.bios = boot_params.apm_bios_info;
 	ist_info = boot_params.ist_info;
diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 46912924636a..a8b1305b6e13 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -202,7 +202,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d bytes\n",
 		&format->format, width, height, stride);
 
-#ifdef CONFIG_X86
+#if defined(CONFIG_FIRMWARE_EDID)
 	if (drm_edid_header_is_valid(edid_info.dummy) == 8)
 		sysfb->edid = edid_info.dummy;
 #endif
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 7945544ba73e..c5216dbe21ec 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -344,7 +344,7 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
 #endif
 	}
 
-#ifdef CONFIG_X86
+#if defined(CONFIG_FIRMWARE_EDID)
 	if (drm_edid_header_is_valid(edid_info.dummy) == 8)
 		sysfb->edid = edid_info.dummy;
 #endif
diff --git a/include/video/edid.h b/include/video/edid.h
index f614371e9116..c2b186b1933a 100644
--- a/include/video/edid.h
+++ b/include/video/edid.h
@@ -4,7 +4,8 @@
 
 #include <uapi/video/edid.h>
 
-#ifdef CONFIG_X86
+#if defined(CONFIG_FIRMWARE_EDID)
 extern struct edid_info edid_info;
 #endif
+
 #endif /* __linux_video_edid_h__ */
-- 
2.49.0


^ permalink raw reply related

* [PATCH 1/2] video: Make CONFIG_FIRMWARE_EDID generally available
From: Thomas Zimmermann @ 2025-06-02  7:51 UTC (permalink / raw)
  To: arnd, javierm, deller; +Cc: dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250602075537.137759-1-tzimmermann@suse.de>

DRM drivers such as efidrm and vesadrm can export firmware EDID
data to userspace. Make the related option CONFIG_FIRMWARE_EDID
available without CONFIG_FB. Make it depend on X86, which is
currently the only architecture providing EDID information.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/Kconfig            | 18 +++++++++++++++++-
 drivers/video/fbdev/core/Kconfig | 15 ---------------
 drivers/video/fbdev/core/fbmon.c |  3 +--
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 5df981920a94..c3da6c0bfca6 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -61,6 +61,23 @@ config HDMI
 
 endif # HAS_IOMEM
 
+config FIRMWARE_EDID
+	bool "Enable firmware EDID"
+	depends on X86
+	help
+	  This enables access to the EDID transferred from the firmware.
+	  On x86, this is from the VESA BIOS. DRM display drivers will
+	  be able to export the information to userspace.
+
+	  Also enable this if DDC/I2C transfers do not work for your driver
+	  and if you are using nvidiafb, i810fb or savagefb.
+
+	  In general, choosing Y for this option is safe.  If you
+	  experience extremely long delays while booting before you get
+	  something on your display, try setting this to N.  Matrox cards in
+	  combination with certain motherboards and monitors are known to
+	  suffer from this problem.
+
 if VT
 	source "drivers/video/console/Kconfig"
 endif
@@ -70,5 +87,4 @@ if FB_CORE || SGI_NEWPORT_CONSOLE
 
 endif
 
-
 endmenu
diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
index 4abe12db7594..b38c3b776bce 100644
--- a/drivers/video/fbdev/core/Kconfig
+++ b/drivers/video/fbdev/core/Kconfig
@@ -10,21 +10,6 @@ config FB_CORE
 config FB_NOTIFY
 	bool
 
-config FIRMWARE_EDID
-	bool "Enable firmware EDID"
-	depends on FB
-	help
-	  This enables access to the EDID transferred from the firmware.
-	  On the i386, this is from the Video BIOS. Enable this if DDC/I2C
-	  transfers do not work for your driver and if you are using
-	  nvidiafb, i810fb or savagefb.
-
-	  In general, choosing Y for this option is safe.  If you
-	  experience extremely long delays while booting before you get
-	  something on your display, try setting this to N.  Matrox cards in
-	  combination with certain motherboards and monitors are known to
-	  suffer from this problem.
-
 config FB_DEVICE
 	bool "Provide legacy /dev/fb* device"
 	depends on FB_CORE
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 0a26399dbc89..7762ad0284fa 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1482,13 +1482,12 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info)
 		-EINVAL : 0;
 }
 
-#if defined(CONFIG_FIRMWARE_EDID) && defined(CONFIG_X86)
-
 /*
  * We need to ensure that the EDID block is only returned for
  * the primary graphics adapter.
  */
 
+#if defined(CONFIG_FIRMWARE_EDID)
 const unsigned char *fb_firmware_edid(struct device *device)
 {
 	struct pci_dev *dev = NULL;
-- 
2.49.0


^ permalink raw reply related

* [PATCH 0/2] video: Make edid_info generally available in x86
From: Thomas Zimmermann @ 2025-06-02  7:51 UTC (permalink / raw)
  To: arnd, javierm, deller; +Cc: dri-devel, linux-fbdev, Thomas Zimmermann

The global variable edid_info provides the system framebuffer's EDID
information, if any. Make it available on x86 without dependencies on
fbdev. DRM drivers for system framebuffers export the information to
userspace.

Besides cleaning up, this series prepares support for firmware EDID on
EFI-based systems.

Thomas Zimmermann (2):
  video: Make CONFIG_FIRMWARE_EDID generally available
  video: Make global edid_info depend on CONFIG_FIRMWARE_EDID

 arch/x86/kernel/setup.c          |  4 ++++
 drivers/gpu/drm/sysfb/efidrm.c   |  2 +-
 drivers/gpu/drm/sysfb/vesadrm.c  |  2 +-
 drivers/video/Kconfig            | 18 +++++++++++++++++-
 drivers/video/fbdev/core/Kconfig | 15 ---------------
 drivers/video/fbdev/core/fbmon.c |  3 +--
 include/video/edid.h             |  3 ++-
 7 files changed, 26 insertions(+), 21 deletions(-)

-- 
2.49.0


^ permalink raw reply

* Re: Issues with vgacon with kernels >= 6.13
From: Thomas Zimmermann @ 2025-06-02  7:29 UTC (permalink / raw)
  To: Adam Stylinski, linux-fbdev
In-Reply-To: <aDt4pp9NE85BuAec@thinkpad>

Hi

Am 31.05.25 um 23:46 schrieb Adam Stylinski:
> I apologize in advance if this is the wrong tree to be barking up but I'm hitting a wall on this and I'm hoping someone on this ML can point me in the right direction.  Ever since kernel 6.13 (I've not yet bisected but that may be my next step) I've been unable to get my potato system booting up via the any of the simple framebuffer consoles.  The kernel simply hangs right after loading the disk IO schedulers (which, looking at my dmesg logs from 6.12 is _just_ before the vga framebuffer is supposed to take over).  The system is definitely in the realm of antique, with a Core 2 Quad Q9650 and a pre EFI boot stage. I've tried simplefb set as the default console, I've tried vesafb, and I've tried vga=normal, all of these seem to fail just before the GPU handoff.
>
> The major change I do see around 6.13 was a change to support the DRM friendly panic message and I'm tempted to maybe think that's where the regression lives. I also realize I may be one of maybe 6 people affected by this by keeping this system limping along, but I do use it regularly to test SIMD related performance, as it's the last CPU family in Intel's line that actually has the unaligned load penalty.
>
> This is the last kernel message I see prior to boot (hopefully LKML doesn't flag me for adding a URL, but I figured attaching an image would be worse):
> https://imgur.com/a/3QkVs2O

Try booting the kernel with fb.lockless_register_fb=1 on the kernel 
command line. With might give more output when the display driver loads.

Best regards
Thomas

>
> Does anyone know of any major changes that might have caused this and anything I should try? My previous configuration is actually to use uvesafb, of which has been failing due to v86d being missing from my distribution for some time now, but it would gracefully fallback. I have an nvidia GPU in this system so the nicer fb options aren't really a thing.  The builtin nvidia modeset is a thing and it does work but that doesn't happen until much later in the boot process. For all intents and purposes, I'm relying on VBE right until that module is loaded, but it hangs much much earlier than that.
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply

* Re: [GIT PULL] fbdev updates for v6.16-rc1
From: pr-tracker-bot @ 2025-06-01  2:46 UTC (permalink / raw)
  To: Helge Deller; +Cc: Linus Torvalds, linux-kernel, linux-fbdev, dri-devel
In-Reply-To: <aDs4uwcxU_M4mpVE@carbonx1>

The pull request you sent on Sat, 31 May 2025 19:13:31 +0200:

> http://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git tags/fbdev-for-6.16-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b42966552bb8d3027b66782fc1b53ce570e4d356

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply

* Re: Issues with vgacon with kernels >= 6.13
From: Adam Stylinski @ 2025-05-31 23:39 UTC (permalink / raw)
  To: Helge Deller; +Cc: linux-fbdev
In-Reply-To: <aDuLW5Zwt1uGhrXH@thinkpad>

On Sat, May 31, 2025 at 07:06:09PM -0400, Adam Stylinski wrote:
> On Sun, Jun 01, 2025 at 12:48:51AM +0200, Helge Deller wrote:
> > What is your .config?
> > 
> > You could try to:
> > - disable DRM:
> > # CONFIG_DRM is not set
> > 
> > - enable:
> > CONFIG_FB=y
> > CONFIG_VGA_CONSOLE=y
> > CONFIG_DUMMY_CONSOLE=y
> > CONFIG_DUMMY_CONSOLE_COLUMNS=80
> > CONFIG_DUMMY_CONSOLE_ROWS=25
> > CONFIG_FRAMEBUFFER_CONSOLE=y
> > CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION=y
> > 
> > Helge
> > 
> > On 5/31/25 23:46, Adam Stylinski wrote:
> > > I apologize in advance if this is the wrong tree to be barking up but I'm hitting a wall on this and I'm hoping someone on this ML can point me in the right direction.  Ever since kernel 6.13 (I've not yet bisected but that may be my next step) I've been unable to get my potato system booting up via the any of the simple framebuffer consoles.  The kernel simply hangs right after loading the disk IO schedulers (which, looking at my dmesg logs from 6.12 is _just_ before the vga framebuffer is supposed to take over).  The system is definitely in the realm of antique, with a Core 2 Quad Q9650 and a pre EFI boot stage. I've tried simplefb set as the default console, I've tried vesafb, and I've tried vga=normal, all of these seem to fail just before the GPU handoff.
> > > 
> > > The major change I do see around 6.13 was a change to support the DRM friendly panic message and I'm tempted to maybe think that's where the regression lives. I also realize I may be one of maybe 6 people affected by this by keeping this system limping along, but I do use it regularly to test SIMD related performance, as it's the last CPU family in Intel's line that actually has the unaligned load penalty.
> > > 
> > > This is the last kernel message I see prior to boot (hopefully LKML doesn't flag me for adding a URL, but I figured attaching an image would be worse):
> > > https://imgur.com/a/3QkVs2O
> > > 
> > > Does anyone know of any major changes that might have caused this and anything I should try? My previous configuration is actually to use uvesafb, of which has been failing due to v86d being missing from my distribution for some time now, but it would gracefully fallback. I have an nvidia GPU in this system so the nicer fb options aren't really a thing.  The builtin nvidia modeset is a thing and it does work but that doesn't happen until much later in the boot process. For all intents and purposes, I'm relying on VBE right until that module is loaded, but it hangs much much earlier than that.
> > > 
> > 
> 
> So, DRM I'm building as a module with 2 or 3 devices I'm not using (strictly it was required for the proprietary nvidia drivers). These modules aren't being loaded though.  Here's what those options are in my config (along with a few others):
> 
> CONFIG_FB=y
> CONFIG_FB_UVESA=y
> CONFIG_FB_VESA=y
> CONFIG_FB_CORE=y
> CONFIG_FB_NOTIFY=y
> CONFIG_FB_DEVICE=y
> CONFIG_FB_CFB_FILLRECT=y
> CONFIG_FB_CFB_COPYAREA=y
> CONFIG_FB_CFB_IMAGEBLIT=y
> CONFIG_FB_SYS_FILLRECT=y
> CONFIG_FB_SYS_COPYAREA=y
> CONFIG_FB_SYS_IMAGEBLIT=y
> CONFIG_FB_SYSMEM_FOPS=y
> CONFIG_FB_DEFERRED_IO=y
> CONFIG_FB_IOMEM_FOPS=y
> CONFIG_FB_IOMEM_HELPERS=y
> CONFIG_FB_SYSMEM_HELPERS=y
> CONFIG_FB_SYSMEM_HELPERS_DEFERRED=y
> CONFIG_FB_MODE_HELPERS=y
> CONFIG_FB_TILEBLITTING=y
> CONFIG_VGA_CONSOLE=y
> CONFIG_DUMMY_CONSOLE=y
> CONFIG_DUMMY_CONSOLE_COLUMNS=80
> CONFIG_DUMMY_CONSOLE_ROWS=25
> CONFIG_FRAMEBUFFER_CONSOLE=y
> # CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION is not set
> CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> # CONFIG_FRAMEBUFFER_CONSOLE_ROTATION is not set
> # CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER is not se
> CONFIG_DRM=m
> # CONFIG_DRM_DEBUG_MM is not set
> CONFIG_DRM_KMS_HELPER=m
> # CONFIG_DRM_PANIC is not set
> CONFIG_DRM_CLIENT=y
> CONFIG_DRM_CLIENT_LIB=m
> CONFIG_DRM_CLIENT_SELECTION=m
> CONFIG_DRM_CLIENT_SETUP=y
> CONFIG_DRM_FBDEV_EMULATION=y
> CONFIG_DRM_FBDEV_OVERALLOC=100
> # CONFIG_DRM_CLIENT_LOG is not set
> CONFIG_DRM_CLIENT_DEFAULT_FBDEV=y
> CONFIG_DRM_CLIENT_DEFAULT="fbdev"
> # CONFIG_DRM_LOAD_EDID_FIRMWARE is not set
> CONFIG_DRM_TTM=m
> CONFIG_DRM_EXEC=m
> CONFIG_DRM_TTM_HELPER=m
> CONFIG_DRM_GEM_SHMEM_HELPER=m
> # CONFIG_DRM_RADEON is not set
> # CONFIG_DRM_AMDGPU is not set
> # CONFIG_DRM_NOUVEAU is not set
> # CONFIG_DRM_I915 is not set
> # CONFIG_DRM_XE is not set
> CONFIG_DRM_VGEM=m
> # CONFIG_DRM_VKMS is not set
> # CONFIG_DRM_GMA500 is not set
> # CONFIG_DRM_UDL is not set
> # CONFIG_DRM_AST is not set
> # CONFIG_DRM_MGAG200 is not set
> CONFIG_DRM_QXL=m
> 
> The "TTM" bit is what was needed for nvidia drivers if I remember correctly.

Oh and here are the boot messages on a kernel that can boot, right around those timestamps:

[    0.663455] io scheduler mq-deadline registered
[    0.663496] io scheduler kyber registered
[    0.663539] io scheduler bfq registered
[    0.663851] pcieport 0000:00:1c.0: enabling device (0106 -> 0107)
[    0.664474] uvesafb: failed to execute /sbin/v86d
[    0.664521] uvesafb: make sure that the v86d helper is installed and executable                                                                                                
[    0.664570] uvesafb: Getting VBE info block failed (eax=0x4f00, err=-2)                                                                                                        
[    0.664613] uvesafb: vbe_init() failed with -22
[    0.664654] uvesafb uvesafb.0: probe with driver uvesafb failed with error -22
[    0.664783] Monitor-Mwait will be used to enter C-1 state
[    0.664818] Monitor-Mwait will be used to enter C-2 state
[    0.664853] Monitor-Mwait will be used to enter C-3 state
[    0.665084] tsc: Marking TSC unstable due to TSC halts in idle
[    0.665222] clocksource: Switched to clocksource hpet
[    0.665573] input: Power Button as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input0
[    0.665679] ACPI: button: Power Button [PWRB]
[    0.665775] input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input1
[    0.665923] ACPI: button: Power Button [PWRF]
[    0.681553] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.681772] 00:07: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
[    0.682710] Non-volatile memory driver v1.3

I'll CC the list as well, as I think that's the proper etiquette.

^ permalink raw reply

* Issues with vgacon with kernels >= 6.13
From: Adam Stylinski @ 2025-05-31 21:46 UTC (permalink / raw)
  To: linux-fbdev

I apologize in advance if this is the wrong tree to be barking up but I'm hitting a wall on this and I'm hoping someone on this ML can point me in the right direction.  Ever since kernel 6.13 (I've not yet bisected but that may be my next step) I've been unable to get my potato system booting up via the any of the simple framebuffer consoles.  The kernel simply hangs right after loading the disk IO schedulers (which, looking at my dmesg logs from 6.12 is _just_ before the vga framebuffer is supposed to take over).  The system is definitely in the realm of antique, with a Core 2 Quad Q9650 and a pre EFI boot stage. I've tried simplefb set as the default console, I've tried vesafb, and I've tried vga=normal, all of these seem to fail just before the GPU handoff.

The major change I do see around 6.13 was a change to support the DRM friendly panic message and I'm tempted to maybe think that's where the regression lives. I also realize I may be one of maybe 6 people affected by this by keeping this system limping along, but I do use it regularly to test SIMD related performance, as it's the last CPU family in Intel's line that actually has the unaligned load penalty.

This is the last kernel message I see prior to boot (hopefully LKML doesn't flag me for adding a URL, but I figured attaching an image would be worse):
https://imgur.com/a/3QkVs2O

Does anyone know of any major changes that might have caused this and anything I should try? My previous configuration is actually to use uvesafb, of which has been failing due to v86d being missing from my distribution for some time now, but it would gracefully fallback. I have an nvidia GPU in this system so the nicer fb options aren't really a thing.  The builtin nvidia modeset is a thing and it does work but that doesn't happen until much later in the boot process. For all intents and purposes, I'm relying on VBE right until that module is loaded, but it hangs much much earlier than that.

^ permalink raw reply

* [PATCH 1/1] staging: sm750fb: convert CamelCase function names to snake_case
From: khalid.datamax @ 2025-05-31 21:11 UTC (permalink / raw)
  To: sudipm.mukherjee
  Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
	Khalid Faisal
In-Reply-To: <20250531211319.55682-1-khalid.datamax@gmail.com>

From: Khalid Faisal <khalid.datamax@gmail.com>

This patch converts function names in the sm750fb driver from CamelCase to
snake_case to comply with Linux kernel coding style.

No functional changes.

Signed-off-by: Khalid Faisal <khalid.datamax@gmail.com>
---
 drivers/staging/sm750fb/ddk750_dvi.c    | 16 +++++------
 drivers/staging/sm750fb/ddk750_sii164.c | 38 ++++++++++++-------------
 drivers/staging/sm750fb/ddk750_sii164.h | 16 +++++------
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
index 8b81e8642..6fef1ab48 100644
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ b/drivers/staging/sm750fb/ddk750_dvi.c
@@ -16,15 +16,15 @@ static struct dvi_ctrl_device dcft_supported_dvi_controller[] = {
 	{
 		.init = sii164_init_chip,
 		.get_vendor_id = sii164_get_vendor_id,
-		.get_device_id = sii164GetDeviceID,
+		.get_device_id = sii164_get_device_id,
 #ifdef SII164_FULL_FUNCTIONS
-		.reset_chip = sii164ResetChip,
-		.get_chip_string = sii164GetChipString,
-		.set_power = sii164SetPower,
-		.enable_hot_plug_detection = sii164EnableHotPlugDetection,
-		.is_connected = sii164IsConnected,
-		.check_interrupt = sii164CheckInterrupt,
-		.clear_interrupt = sii164ClearInterrupt,
+		.reset_chip = sii164_reset_chip,
+		.get_chip_string = sii164_get_chip_string,
+		.set_power = sii164_set_power,
+		.enable_hot_plug_detection = sii164_enable_hot_plug_detection,
+		.is_connected = sii164_is_connected,
+		.check_interrupt = sii164_check_interrupt,
+		.clear_interrupt = sii164_clear_interrupt,
 #endif
 	},
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 2532b6024..d6bfd7c4e 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -48,13 +48,13 @@ unsigned short sii164_get_vendor_id(void)
 }
 
 /*
- *  sii164GetDeviceID
+ *  sii164_get_device_id
  *      This function gets the device ID of the DVI controller chip.
  *
  *  Output:
  *      Device ID
  */
-unsigned short sii164GetDeviceID(void)
+unsigned short sii164_get_device_id(void)
 {
 	unsigned short deviceID;
 
@@ -141,7 +141,7 @@ long sii164_init_chip(unsigned char edge_select,
 
 	/* Check if SII164 Chip exists */
 	if ((sii164_get_vendor_id() == SII164_VENDOR_ID) &&
-	    (sii164GetDeviceID() == SII164_DEVICE_ID)) {
+	    (sii164_get_device_id() == SII164_DEVICE_ID)) {
 		/*
 		 *  Initialize SII164 controller chip.
 		 */
@@ -250,36 +250,36 @@ long sii164_init_chip(unsigned char edge_select,
 #ifdef SII164_FULL_FUNCTIONS
 
 /*
- *  sii164ResetChip
+ *  sii164_reset_chip
  *      This function resets the DVI Controller Chip.
  */
-void sii164ResetChip(void)
+void sii164_reset_chip(void)
 {
 	/* Power down */
-	sii164SetPower(0);
-	sii164SetPower(1);
+	sii164_set_power(0);
+	sii164_set_power(1);
 }
 
 /*
- * sii164GetChipString
+ * sii164_get_chip_string
  *      This function returns a char string name of the current DVI Controller
  *      chip.
  *
  *      It's convenient for application need to display the chip name.
  */
-char *sii164GetChipString(void)
+char *sii164_get_chip_string(void)
 {
 	return gDviCtrlChipName;
 }
 
 /*
- *  sii164SetPower
+ *  sii164_set_power
  *      This function sets the power configuration of the DVI Controller Chip.
  *
  *  Input:
  *      powerUp - Flag to set the power down or up
  */
-void sii164SetPower(unsigned char powerUp)
+void sii164_set_power(unsigned char powerUp)
 {
 	unsigned char config;
 
@@ -329,12 +329,12 @@ void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
 }
 
 /*
- *  sii164EnableHotPlugDetection
+ *  sii164_enable_hot_plug_detection
  *      This function enables the Hot Plug detection.
  *
  *  enableHotPlug   - Enable (=1) / disable (=0) Hot Plug detection
  */
-void sii164EnableHotPlugDetection(unsigned char enableHotPlug)
+void sii164_enable_hot_plug_detection(unsigned char enableHotPlug)
 {
 	unsigned char detectReg;
 
@@ -350,14 +350,14 @@ void sii164EnableHotPlugDetection(unsigned char enableHotPlug)
 }
 
 /*
- *  sii164IsConnected
+ *  sii164_is_connected
  *      Check if the DVI Monitor is connected.
  *
  *  Output:
  *      0   - Not Connected
  *      1   - Connected
  */
-unsigned char sii164IsConnected(void)
+unsigned char sii164_is_connected(void)
 {
 	unsigned char hotPlugValue;
 
@@ -370,14 +370,14 @@ unsigned char sii164IsConnected(void)
 }
 
 /*
- *  sii164CheckInterrupt
+ *  sii164_check_interrupt
  *      Checks if interrupt has occurred.
  *
  *  Output:
  *      0   - No interrupt
  *      1   - Interrupt occurs
  */
-unsigned char sii164CheckInterrupt(void)
+unsigned char sii164_check_interrupt(void)
 {
 	unsigned char detectReg;
 
@@ -390,10 +390,10 @@ unsigned char sii164CheckInterrupt(void)
 }
 
 /*
- *  sii164ClearInterrupt
+ *  sii164_clear_interrupt
  *      Clear the hot plug interrupt.
  */
-void sii164ClearInterrupt(void)
+void sii164_clear_interrupt(void)
 {
 	unsigned char detectReg;
 
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index 71a7c1cb4..005473ca2 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -28,16 +28,16 @@ long sii164_init_chip(unsigned char edgeSelect,
 		      unsigned char pllFilterValue);
 
 unsigned short sii164_get_vendor_id(void);
-unsigned short sii164GetDeviceID(void);
+unsigned short sii164_get_device_id(void);
 
 #ifdef SII164_FULL_FUNCTIONS
-void sii164ResetChip(void);
-char *sii164GetChipString(void);
-void sii164SetPower(unsigned char powerUp);
-void sii164EnableHotPlugDetection(unsigned char enableHotPlug);
-unsigned char sii164IsConnected(void);
-unsigned char sii164CheckInterrupt(void);
-void sii164ClearInterrupt(void);
+void sii164_reset_chip(void);
+char *sii164_get_chip_string(void);
+void sii164_set_power(unsigned char powerUp);
+void sii164_enable_hot_plug_detection(unsigned char enableHotPlug);
+unsigned char sii164_is_connected(void);
+unsigned char sii164_check_interrupt(void);
+void sii164_clear_interrupt(void);
 #endif
 /*
  * below register definition is used for
-- 
2.43.0


^ permalink raw reply related

* [PATCH 0/1] staging: sm750fb: convert CamelCase function names to snake_case
From: khalid.datamax @ 2025-05-31 21:11 UTC (permalink / raw)
  To: sudipm.mukherjee
  Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
	Khalid Faisal
In-Reply-To: <20250531211319.55682-1-khalid.datamax@gmail.com>

From: Khalid Faisal <khalid.datamax@gmail.com>

This patch updates various function names in the sm750fb driver to follow
the Linux kernel coding style by converting CamelCase names to snake_case.

These changes were identified using checkpatch.pl, which recommends
using snake_case for function and variable names.

This patch is part of the Kernel Janitors cleanup effort.

-- 
Khalid Faisal


^ permalink raw reply

* [PATCH 0/1] staging: sm750fb: convert CamelCase function names to snake_case
From: khalid.datamax @ 2025-05-31 21:11 UTC (permalink / raw)
  To: sudipm.mukherjee
  Cc: teddy.wang, gregkh, linux-fbdev, linux-staging, linux-kernel,
	Khalid Faisal

This patch cleans up the staging driver sm750fb by converting function names
that are currently in CamelCase to the preferred snake_case style, following
Linux kernel coding guidelines.

Specifically, it renames the following functions for consistency and readability:
- sii164GetDeviceID		-> sii164_get_device_id
- sii164ResetChip		-> sii164_reset_chip
- sii164GetChipString		-> sii164_get_chip_string
- sii164SetPower		-> sii164_set_power
- sii164EnableHotPlugDetection	-> sii164_enable_hot_plug_detection
- sii164IsConnected		-> sii164_is_connected
- sii164CheckInterrupt		-> sii164_check_interrupt
- sii164ClearInterrupt		-> sii164_clear_interrupt

This helps maintain uniformity with the rest of the kernel codebase and
improves maintainability.

Signed-off-by: Khalid Faisal <khalid.datamax@gmail.com>



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox