dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* drm fb helpers hotplug/resize
@ 2022-10-05 19:49 Zack Rusin
  2022-10-06  8:01 ` Thomas Zimmermann
  0 siblings, 1 reply; 7+ messages in thread
From: Zack Rusin @ 2022-10-05 19:49 UTC (permalink / raw)
  To: tzimmermann; +Cc: dri-devel

Hi, Thomas.

Because you've been the one who's been working on drm_fb_helper.c the most the last
few years I wanted to pick your brain a bit.

I was porting vmwgfx to drm_fb_helper code which is largely trivial, just removing
all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But
drm_fb_helper.c code never deals with resizes which is a bit of a problem.

e.g. replacing the drm_sysfs_hotplug_event() call from
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up in 
drm_fb_helper_hotplug_event:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003

Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never resizes
drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized. 

In general I don't see drm_fb_helper code ever being able to deal with resizes. In
particular because the fbdev's xres_virtual/yres_virtual are sized exactly to the
initial xres/yres. 

It's definitely a lot bigger issue on virtualized environments where at boot we'll
have some very conservative size (800x600) on vmwgfx which is then usually resized
to the size of the window. drm_fb_helper breaks pretty bad in that case because it
can't deal with those resizes at all. 

Is this scenario something that drm_fb_helper should be able to handle or is it not
worth pursuing it? I don't think there's a trivial way of handling it so my guess is
that it would make drm_fb_helper quite a bit more complicated.

z

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-05 19:49 drm fb helpers hotplug/resize Zack Rusin
@ 2022-10-06  8:01 ` Thomas Zimmermann
  2022-10-07  2:16   ` Zack Rusin
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Zimmermann @ 2022-10-06  8:01 UTC (permalink / raw)
  To: Zack Rusin; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3902 bytes --]

Hi Zack

Am 05.10.22 um 21:49 schrieb Zack Rusin:
> Hi, Thomas.
> 
> Because you've been the one who's been working on drm_fb_helper.c the most the last
> few years I wanted to pick your brain a bit.
> 
> I was porting vmwgfx to drm_fb_helper code which is largely trivial, just removing
> all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But

Thanks a lot for this work. I have been looking into doing this 
conversion myself at some point, but never found the time to actually do 
it.

> drm_fb_helper.c code never deals with resizes which is a bit of a problem.
> 
> e.g. replacing the drm_sysfs_hotplug_event() call from
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
> with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up in
> drm_fb_helper_hotplug_event:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003
> 
> Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never resizes
> drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized.
> 
> In general I don't see drm_fb_helper code ever being able to deal with resizes. In
> particular because the fbdev's xres_virtual/yres_virtual are sized exactly to the
> initial xres/yres.
> 
> It's definitely a lot bigger issue on virtualized environments where at boot we'll
> have some very conservative size (800x600) on vmwgfx which is then usually resized
> to the size of the window. drm_fb_helper breaks pretty bad in that case because it
> can't deal with those resizes at all.
> 
> Is this scenario something that drm_fb_helper should be able to handle or is it not
> worth pursuing it? I don't think there's a trivial way of handling it so my guess is
> that it would make drm_fb_helper quite a bit more complicated.

I'm aware that resizing is missing. It's one of the few things I'd like 
to see being added to generic fbdev emulation. But as you say, it's not 
easy. The generic fbdev emulation has all kinds of code paths for the 
various drivers' memory managers. That makes it complicated.

The problem is that fbdev's mmap'ed memory cannot be reallocated. It is 
expected to behave like 'real video memory.' So either reserve a chunk 
of the video ram for fbdev's GEM objects or use deferred I/O, which 
provides mmaped pages from a shadow buffer in system memory. vmwgfx uses 
the latter IIRC.

But ideally, we'd get rid of most of the shadow buffering and try to 
mmap pages directly from GEM objects. For modesetting, this means that 
the new mode's framebuffer has to inherit the old framebuffer's buffer 
objects. Probably the easiest solution is to allocate a framebuffer once 
and reconfigure its parameters (width, height, pitch) on each modeset 
operation.

Switching to a higher resolution would require more video memory. 
Although we cannot reallocate, this problem can be solved with the 
drm_fbdev_overalloc parameter. It gives the percentage of allocated 
video memory. If you start with 800x600 with overalloc at 400, you'd get 
enough video memory for 2400 scanlines. This allows for fbdev panning 
(i.e., pageflipping). With that extra memory fbdev could switch to 
another display mode with a higher resolution. For example, changing to 
1024x786 would result in 1875 scanlines at the given overalloc of 400.

To implement this, I guess that some of fbdev's memory allocation needs 
to be changed. The check_var and set_par code needs an update to handle 
the modeset. And I suspect that there are other dark corners that need 
to be reworked as well.

Best regards
Thomas

> 
> z

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-06  8:01 ` Thomas Zimmermann
@ 2022-10-07  2:16   ` Zack Rusin
  2022-10-07  7:10     ` Thomas Zimmermann
  0 siblings, 1 reply; 7+ messages in thread
From: Zack Rusin @ 2022-10-07  2:16 UTC (permalink / raw)
  To: tzimmermann; +Cc: dri-devel

On Thu, 2022-10-06 at 10:01 +0200, Thomas Zimmermann wrote:
> Hi Zack
> 
> Am 05.10.22 um 21:49 schrieb Zack Rusin:
> > Hi, Thomas.
> > 
> > Because you've been the one who's been working on drm_fb_helper.c the most the last
> > few years I wanted to pick your brain a bit.
> > 
> > I was porting vmwgfx to drm_fb_helper code which is largely trivial, just removing
> > all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But
> 
> Thanks a lot for this work. I have been looking into doing this 
> conversion myself at some point, but never found the time to actually do 
> it.
> 
> > drm_fb_helper.c code never deals with resizes which is a bit of a problem.
> > 
> > e.g. replacing the drm_sysfs_hotplug_event() call from
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
> > with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up in
> > drm_fb_helper_hotplug_event:
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003
> > 
> > Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never resizes
> > drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized.
> > 
> > In general I don't see drm_fb_helper code ever being able to deal with resizes. In
> > particular because the fbdev's xres_virtual/yres_virtual are sized exactly to the
> > initial xres/yres.
> > 
> > It's definitely a lot bigger issue on virtualized environments where at boot we'll
> > have some very conservative size (800x600) on vmwgfx which is then usually resized
> > to the size of the window. drm_fb_helper breaks pretty bad in that case because it
> > can't deal with those resizes at all.
> > 
> > Is this scenario something that drm_fb_helper should be able to handle or is it not
> > worth pursuing it? I don't think there's a trivial way of handling it so my guess is
> > that it would make drm_fb_helper quite a bit more complicated.
> 
> I'm aware that resizing is missing. It's one of the few things I'd like 
> to see being added to generic fbdev emulation. But as you say, it's not 
> easy. The generic fbdev emulation has all kinds of code paths for the 
> various drivers' memory managers. That makes it complicated.
> 
> The problem is that fbdev's mmap'ed memory cannot be reallocated. It is 
> expected to behave like 'real video memory.' So either reserve a chunk 
> of the video ram for fbdev's GEM objects or use deferred I/O, which 
> provides mmaped pages from a shadow buffer in system memory. vmwgfx uses 
> the latter IIRC.
> 
> But ideally, we'd get rid of most of the shadow buffering and try to 
> mmap pages directly from GEM objects. For modesetting, this means that 
> the new mode's framebuffer has to inherit the old framebuffer's buffer 
> objects. Probably the easiest solution is to allocate a framebuffer once 
> and reconfigure its parameters (width, height, pitch) on each modeset 
> operation.
> 
> Switching to a higher resolution would require more video memory. 
> Although we cannot reallocate, this problem can be solved with the 
> drm_fbdev_overalloc parameter. It gives the percentage of allocated 
> video memory. If you start with 800x600 with overalloc at 400, you'd get 
> enough video memory for 2400 scanlines. This allows for fbdev panning 
> (i.e., pageflipping). With that extra memory fbdev could switch to 
> another display mode with a higher resolution. For example, changing to 
> 1024x786 would result in 1875 scanlines at the given overalloc of 400.
> 
> To implement this, I guess that some of fbdev's memory allocation needs 
> to be changed. The check_var and set_par code needs an update to handle 
> the modeset. And I suspect that there are other dark corners that need 
> to be reworked as well.

That sounds good. In a similar fashion to drm_fbdev_overalloc another, rather hacky
but vastly simpler approach, would be to basically allow the drivers to specify the
maximum size of fb to support in drm_fbdev_generic_setup. This would just directly
set the drm_fb_helper_surface_size::surface_width and surface_height with the end
result being that drm_client_framebuffer_create would be called with those values
and xres_virtual/yres_virtual would be set to them. Resizing would basically just
work then, right? Of course at the cost of possibly large allocation, e.g. 4k fb
even when only 800x600 is actually used.

Either way I'll send out the patch that ports vmwgfx to drm_fb_helpers because even
without resizing support, it removes ~1000loc from vmwgfx and I think is well worth
it and we can figure out how to handle drm_fb_helpers later.

z

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-07  2:16   ` Zack Rusin
@ 2022-10-07  7:10     ` Thomas Zimmermann
  2022-10-07  7:15       ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Zimmermann @ 2022-10-07  7:10 UTC (permalink / raw)
  To: Zack Rusin; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5971 bytes --]

Hi

Am 07.10.22 um 04:16 schrieb Zack Rusin:
> On Thu, 2022-10-06 at 10:01 +0200, Thomas Zimmermann wrote:
>> Hi Zack
>>
>> Am 05.10.22 um 21:49 schrieb Zack Rusin:
>>> Hi, Thomas.
>>>
>>> Because you've been the one who's been working on drm_fb_helper.c the most the last
>>> few years I wanted to pick your brain a bit.
>>>
>>> I was porting vmwgfx to drm_fb_helper code which is largely trivial, just removing
>>> all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But
>>
>> Thanks a lot for this work. I have been looking into doing this
>> conversion myself at some point, but never found the time to actually do
>> it.
>>
>>> drm_fb_helper.c code never deals with resizes which is a bit of a problem.
>>>
>>> e.g. replacing the drm_sysfs_hotplug_event() call from
>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
>>> with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up in
>>> drm_fb_helper_hotplug_event:
>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003
>>>
>>> Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never resizes
>>> drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized.
>>>
>>> In general I don't see drm_fb_helper code ever being able to deal with resizes. In
>>> particular because the fbdev's xres_virtual/yres_virtual are sized exactly to the
>>> initial xres/yres.
>>>
>>> It's definitely a lot bigger issue on virtualized environments where at boot we'll
>>> have some very conservative size (800x600) on vmwgfx which is then usually resized
>>> to the size of the window. drm_fb_helper breaks pretty bad in that case because it
>>> can't deal with those resizes at all.

The initial resolution of 800x600 is imposed by the driver, right? 
(VMW_MIN_INITIAL_{WIDTH,HEIGHT}) You can use video= on the kernel 
command line to select a resolution. That gives at least a workaround 
with fbdev emulation.

>>>
>>> Is this scenario something that drm_fb_helper should be able to handle or is it not
>>> worth pursuing it? I don't think there's a trivial way of handling it so my guess is
>>> that it would make drm_fb_helper quite a bit more complicated.
>>
>> I'm aware that resizing is missing. It's one of the few things I'd like
>> to see being added to generic fbdev emulation. But as you say, it's not
>> easy. The generic fbdev emulation has all kinds of code paths for the
>> various drivers' memory managers. That makes it complicated.
>>
>> The problem is that fbdev's mmap'ed memory cannot be reallocated. It is
>> expected to behave like 'real video memory.' So either reserve a chunk
>> of the video ram for fbdev's GEM objects or use deferred I/O, which
>> provides mmaped pages from a shadow buffer in system memory. vmwgfx uses
>> the latter IIRC.
>>
>> But ideally, we'd get rid of most of the shadow buffering and try to
>> mmap pages directly from GEM objects. For modesetting, this means that
>> the new mode's framebuffer has to inherit the old framebuffer's buffer
>> objects. Probably the easiest solution is to allocate a framebuffer once
>> and reconfigure its parameters (width, height, pitch) on each modeset
>> operation.
>>
>> Switching to a higher resolution would require more video memory.
>> Although we cannot reallocate, this problem can be solved with the
>> drm_fbdev_overalloc parameter. It gives the percentage of allocated
>> video memory. If you start with 800x600 with overalloc at 400, you'd get
>> enough video memory for 2400 scanlines. This allows for fbdev panning
>> (i.e., pageflipping). With that extra memory fbdev could switch to
>> another display mode with a higher resolution. For example, changing to
>> 1024x786 would result in 1875 scanlines at the given overalloc of 400.
>>
>> To implement this, I guess that some of fbdev's memory allocation needs
>> to be changed. The check_var and set_par code needs an update to handle
>> the modeset. And I suspect that there are other dark corners that need
>> to be reworked as well.
> 
> That sounds good. In a similar fashion to drm_fbdev_overalloc another, rather hacky
> but vastly simpler approach, would be to basically allow the drivers to specify the
> maximum size of fb to support in drm_fbdev_generic_setup. This would just directly
> set the drm_fb_helper_surface_size::surface_width and surface_height with the end
> result being that drm_client_framebuffer_create would be called with those values
> and xres_virtual/yres_virtual would be set to them. Resizing would basically just
> work then, right? Of course at the cost of possibly large allocation, e.g. 4k fb
> even when only 800x600 is actually used.

For the absolute size of fbdev memory, I think we should introduce a 
module parameter in drm_fb_helper, which an option to set a default 
value in the kernel config. It would benefit all drivers that use fbdev 
emulation and work how overalloc works.

If no size is given, the current approach would be used.

I don't think resizing would work immediately. There isn't anything in 
the check_var and set_par functions that implements the necessary atomic 
check and commit.

> 
> Either way I'll send out the patch that ports vmwgfx to drm_fb_helpers because even
> without resizing support, it removes ~1000loc from vmwgfx and I think is well worth
> it and we can figure out how to handle drm_fb_helpers later.

OK, if that isn't a problem for vmwgfx. I remember that Suse had a 
customer that uses X11 on top of fbdev *with* vmwgfx's display resizing. 
These users could certainly switch to using DRM though.

Best regards
Thomas

> 
> z

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-07  7:10     ` Thomas Zimmermann
@ 2022-10-07  7:15       ` Ville Syrjälä
  2022-10-07  7:58         ` Thomas Zimmermann
  0 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2022-10-07  7:15 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: dri-devel

On Fri, Oct 07, 2022 at 09:10:27AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 07.10.22 um 04:16 schrieb Zack Rusin:
> > On Thu, 2022-10-06 at 10:01 +0200, Thomas Zimmermann wrote:
> >> Hi Zack
> >>
> >> Am 05.10.22 um 21:49 schrieb Zack Rusin:
> >>> Hi, Thomas.
> >>>
> >>> Because you've been the one who's been working on drm_fb_helper.c the most the last
> >>> few years I wanted to pick your brain a bit.
> >>>
> >>> I was porting vmwgfx to drm_fb_helper code which is largely trivial, just removing
> >>> all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But
> >>
> >> Thanks a lot for this work. I have been looking into doing this
> >> conversion myself at some point, but never found the time to actually do
> >> it.
> >>
> >>> drm_fb_helper.c code never deals with resizes which is a bit of a problem.
> >>>
> >>> e.g. replacing the drm_sysfs_hotplug_event() call from
> >>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
> >>> with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up in
> >>> drm_fb_helper_hotplug_event:
> >>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003
> >>>
> >>> Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never resizes
> >>> drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized.
> >>>
> >>> In general I don't see drm_fb_helper code ever being able to deal with resizes. In
> >>> particular because the fbdev's xres_virtual/yres_virtual are sized exactly to the
> >>> initial xres/yres.
> >>>
> >>> It's definitely a lot bigger issue on virtualized environments where at boot we'll
> >>> have some very conservative size (800x600) on vmwgfx which is then usually resized
> >>> to the size of the window. drm_fb_helper breaks pretty bad in that case because it
> >>> can't deal with those resizes at all.
> 
> The initial resolution of 800x600 is imposed by the driver, right? 
> (VMW_MIN_INITIAL_{WIDTH,HEIGHT}) You can use video= on the kernel 
> command line to select a resolution. That gives at least a workaround 
> with fbdev emulation.
> 
> >>>
> >>> Is this scenario something that drm_fb_helper should be able to handle or is it not
> >>> worth pursuing it? I don't think there's a trivial way of handling it so my guess is
> >>> that it would make drm_fb_helper quite a bit more complicated.
> >>
> >> I'm aware that resizing is missing. It's one of the few things I'd like
> >> to see being added to generic fbdev emulation. But as you say, it's not
> >> easy. The generic fbdev emulation has all kinds of code paths for the
> >> various drivers' memory managers. That makes it complicated.
> >>
> >> The problem is that fbdev's mmap'ed memory cannot be reallocated. It is
> >> expected to behave like 'real video memory.' So either reserve a chunk
> >> of the video ram for fbdev's GEM objects or use deferred I/O, which
> >> provides mmaped pages from a shadow buffer in system memory. vmwgfx uses
> >> the latter IIRC.
> >>
> >> But ideally, we'd get rid of most of the shadow buffering and try to
> >> mmap pages directly from GEM objects. For modesetting, this means that
> >> the new mode's framebuffer has to inherit the old framebuffer's buffer
> >> objects. Probably the easiest solution is to allocate a framebuffer once
> >> and reconfigure its parameters (width, height, pitch) on each modeset
> >> operation.
> >>
> >> Switching to a higher resolution would require more video memory.
> >> Although we cannot reallocate, this problem can be solved with the
> >> drm_fbdev_overalloc parameter. It gives the percentage of allocated
> >> video memory. If you start with 800x600 with overalloc at 400, you'd get
> >> enough video memory for 2400 scanlines. This allows for fbdev panning
> >> (i.e., pageflipping). With that extra memory fbdev could switch to
> >> another display mode with a higher resolution. For example, changing to
> >> 1024x786 would result in 1875 scanlines at the given overalloc of 400.
> >>
> >> To implement this, I guess that some of fbdev's memory allocation needs
> >> to be changed. The check_var and set_par code needs an update to handle
> >> the modeset. And I suspect that there are other dark corners that need
> >> to be reworked as well.
> > 
> > That sounds good. In a similar fashion to drm_fbdev_overalloc another, rather hacky
> > but vastly simpler approach, would be to basically allow the drivers to specify the
> > maximum size of fb to support in drm_fbdev_generic_setup. This would just directly
> > set the drm_fb_helper_surface_size::surface_width and surface_height with the end
> > result being that drm_client_framebuffer_create would be called with those values
> > and xres_virtual/yres_virtual would be set to them. Resizing would basically just
> > work then, right? Of course at the cost of possibly large allocation, e.g. 4k fb
> > even when only 800x600 is actually used.
> 
> For the absolute size of fbdev memory, I think we should introduce a 
> module parameter in drm_fb_helper, which an option to set a default 
> value in the kernel config. It would benefit all drivers that use fbdev 
> emulation and work how overalloc works.
> 
> If no size is given, the current approach would be used.
> 
> I don't think resizing would work immediately. There isn't anything in 
> the check_var and set_par functions that implements the necessary atomic 
> check and commit.

set_par() is the thing tht does the commit.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-07  7:15       ` Ville Syrjälä
@ 2022-10-07  7:58         ` Thomas Zimmermann
  2022-10-07  8:19           ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Zimmermann @ 2022-10-07  7:58 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1603 bytes --]

Hi

Am 07.10.22 um 09:15 schrieb Ville Syrjälä:

>>
>> For the absolute size of fbdev memory, I think we should introduce a
>> module parameter in drm_fb_helper, which an option to set a default
>> value in the kernel config. It would benefit all drivers that use fbdev
>> emulation and work how overalloc works.
>>
>> If no size is given, the current approach would be used.
>>
>> I don't think resizing would work immediately. There isn't anything in
>> the check_var and set_par functions that implements the necessary atomic
>> check and commit.
> 
> set_par() is the thing tht does the commit.
> 

I know. There actually even is a commit statement in set_par, which can 
restore the initial mode. My point is that it has no means of changing 
the display mode. A full modeset would require us to convert the fb 
screeninfo into an atomic state and commit that instead. For the fbconv 
helper, I once made code to convert between the two. Leaving this here 
for reference. [1][2]

Similarly, in check_var we sort out and reject all mode changes. We'd 
have to change that as well.

I guess we can continue to ignore non-atomic modesetting.

Best regards
Thomas

[1] 
https://gitlab.freedesktop.org/tzimmermann/linux/-/commit/385161cd2d048b5cf80544bff8ced3da7a82dfa9
[2] 
https://gitlab.freedesktop.org/tzimmermann/linux/-/commit/a541c405a638f47ee80389b222fbde6e311e8220

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: drm fb helpers hotplug/resize
  2022-10-07  7:58         ` Thomas Zimmermann
@ 2022-10-07  8:19           ` Ville Syrjälä
  0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2022-10-07  8:19 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: dri-devel

On Fri, Oct 07, 2022 at 09:58:31AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 07.10.22 um 09:15 schrieb Ville Syrjälä:
> 
> >>
> >> For the absolute size of fbdev memory, I think we should introduce a
> >> module parameter in drm_fb_helper, which an option to set a default
> >> value in the kernel config. It would benefit all drivers that use fbdev
> >> emulation and work how overalloc works.
> >>
> >> If no size is given, the current approach would be used.
> >>
> >> I don't think resizing would work immediately. There isn't anything in
> >> the check_var and set_par functions that implements the necessary atomic
> >> check and commit.
> > 
> > set_par() is the thing tht does the commit.
> > 
> 
> I know. There actually even is a commit statement in set_par, which can 
> restore the initial mode. My point is that it has no means of changing 
> the display mode. A full modeset would require us to convert the fb 
> screeninfo into an atomic state and commit that instead. For the fbconv 
> helper, I once made code to convert between the two. Leaving this here 
> for reference. [1][2]

Uff. Right, we'd probably just want to properly implemnt set_par()
then. 

BTW I had a slightly different take on the bitfiled stuff.
Maybe a bit easier to extend for new formats, but full of macros:
https://patchwork.freedesktop.org/patch/203189/?series=37820&rev=1

> 
> Similarly, in check_var we sort out and reject all mode changes. We'd 
> have to change that as well.

I guess we could do a TEST_ONLY commit there. Though I think
not doing that and just failing from set_par() should be fine
too.

> 
> I guess we can continue to ignore non-atomic modesetting.
> 
> Best regards
> Thomas
> 
> [1] 
> https://gitlab.freedesktop.org/tzimmermann/linux/-/commit/385161cd2d048b5cf80544bff8ced3da7a82dfa9
> [2] 
> https://gitlab.freedesktop.org/tzimmermann/linux/-/commit/a541c405a638f47ee80389b222fbde6e311e8220
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-10-07  8:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05 19:49 drm fb helpers hotplug/resize Zack Rusin
2022-10-06  8:01 ` Thomas Zimmermann
2022-10-07  2:16   ` Zack Rusin
2022-10-07  7:10     ` Thomas Zimmermann
2022-10-07  7:15       ` Ville Syrjälä
2022-10-07  7:58         ` Thomas Zimmermann
2022-10-07  8:19           ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).