Maxime Ripard <mripard@xxxxxxxxxx> writes:
> On Mon, Mar 18, 2024 at 08:59:01AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 18.03.24 um 03:35 schrieb Zack Rusin:
>> > On Tue, Mar 12, 2024 at 11:48 AM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>> > > Framebuffer memory is allocated via vmalloc() from non-contiguous
>> > > physical pages. The physical framebuffer start address is therefore
>> > > meaningless. Do not set it.
>> > >
>> > > The value is not used within the kernel and only exported to userspace
>> > > on dedicated ARM configs. No functional change is expected.
>> > >
>> > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> > > Fixes: a5b44c4adb16 ("drm/fbdev-generic: Always use shadow buffering")
>> > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> > > Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>> > > Cc: Zack Rusin <zackr@xxxxxxxxxx>
>> > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> > > Cc: Maxime Ripard <mripard@xxxxxxxxxx>
>> > > Cc: <stable@xxxxxxxxxxxxxxx> # v6.4+
>> > > ---
>> > > drivers/gpu/drm/drm_fbdev_generic.c | 1 -
>> > > 1 file changed, 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
>> > > index d647d89764cb9..b4659cd6285ab 100644
>> > > --- a/drivers/gpu/drm/drm_fbdev_generic.c
>> > > +++ b/drivers/gpu/drm/drm_fbdev_generic.c
>> > > @@ -113,7 +113,6 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper,
>> > > /* screen */
>> > > info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
>> > > info->screen_buffer = screen_buffer;
>> > > - info->fix.smem_start = page_to_phys(vmalloc_to_page(info->screen_buffer));
>> > > info->fix.smem_len = screen_size;
>> > >
>> > > /* deferred I/O */
>> > > --
>> > > 2.44.0
>> > >
>> > Good idea. I think given that drm_leak_fbdev_smem is off by default we
>> > could remove the setting of smem_start by all of the in-tree drm
>> > drivers (they all have open source userspace that won't mess around
>> > with fbdev fb) - it will be reset to 0 anyway. Actually, I wonder if
>> > we still need drm_leak_fbdev_smem at all...
>>
>> All I know is that there's an embedded userspace driver that requires that
>> setting. I don't even know which hardware.
>
> The original Mali driver (ie, lima) used to require it, that's why we
> introduced it in the past.
>
> I'm not sure if the newer versions of that driver, or if newer Mali
> generations (ie, panfrost and panthor) closed source driver would
> require it, so it might be worth removing if it's easy enough to revert.
>
Agreed. The DRM_FBDEV_LEAK_PHYS_SMEM symbol already depends on EXPERT and
defaults to 'n', which implies that isn't enabled by most kernels AFAICT.
So dropping it and see if anyone complains sounds like a good idea to me.
> Maxime
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
