diff mbox

[RFC,v2,03/13] bootsplash: Flush framebuffer after drawing

Message ID 20171213194755.3409-4-mstaudt@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

'Max Staudt Dec. 13, 2017, 7:47 p.m. UTC
Framebuffers with deferred I/O need to be flushed to the screen
explicitly, since we use neither the mmap nor the file I/O abstractions
that handle this for userspace FB clients.

Example: xenfb

Some framebuffer drivers implement lazy access to the screen without
actually exposing a fbdefio interface - we also match some known ones,
currently:
 - ast
 - cirrus
 - mgag200

Signed-off-by: Max Staudt <mstaudt@suse.de>
Reviewed-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/video/fbdev/core/bootsplash.c          |  2 ++
 drivers/video/fbdev/core/bootsplash_internal.h |  1 +
 drivers/video/fbdev/core/bootsplash_render.c   | 33 ++++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

Comments

Daniel Vetter Dec. 13, 2017, 9:35 p.m. UTC | #1
On Wed, Dec 13, 2017 at 08:47:45PM +0100, Max Staudt wrote:
> Framebuffers with deferred I/O need to be flushed to the screen
> explicitly, since we use neither the mmap nor the file I/O abstractions
> that handle this for userspace FB clients.
> 
> Example: xenfb
> 
> Some framebuffer drivers implement lazy access to the screen without
> actually exposing a fbdefio interface - we also match some known ones,
> currently:
>  - ast
>  - cirrus
>  - mgag200
> 
> Signed-off-by: Max Staudt <mstaudt@suse.de>
> Reviewed-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/video/fbdev/core/bootsplash.c          |  2 ++
>  drivers/video/fbdev/core/bootsplash_internal.h |  1 +
>  drivers/video/fbdev/core/bootsplash_render.c   | 33 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/video/fbdev/core/bootsplash.c b/drivers/video/fbdev/core/bootsplash.c
> index 843c5400fefc..815b007f81ca 100644
> --- a/drivers/video/fbdev/core/bootsplash.c
> +++ b/drivers/video/fbdev/core/bootsplash.c
> @@ -112,6 +112,8 @@ void bootsplash_render_full(struct fb_info *info)
>  
>  	bootsplash_do_render_pictures(info, splash_state.file);
>  
> +	bootsplash_do_render_flush(info);
> +
>  out:
>  	mutex_unlock(&splash_state.data_lock);
>  }
> diff --git a/drivers/video/fbdev/core/bootsplash_internal.h b/drivers/video/fbdev/core/bootsplash_internal.h
> index 71e2a27ac0b8..0acb383aa4e3 100644
> --- a/drivers/video/fbdev/core/bootsplash_internal.h
> +++ b/drivers/video/fbdev/core/bootsplash_internal.h
> @@ -89,6 +89,7 @@ void bootsplash_do_render_background(struct fb_info *info,
>  				     const struct splash_file_priv *fp);
>  void bootsplash_do_render_pictures(struct fb_info *info,
>  				   const struct splash_file_priv *fp);
> +void bootsplash_do_render_flush(struct fb_info *info);
>  
>  
>  void bootsplash_free_file(struct splash_file_priv *fp);
> diff --git a/drivers/video/fbdev/core/bootsplash_render.c b/drivers/video/fbdev/core/bootsplash_render.c
> index 2ae36949d0e3..8c09c306ff67 100644
> --- a/drivers/video/fbdev/core/bootsplash_render.c
> +++ b/drivers/video/fbdev/core/bootsplash_render.c
> @@ -186,3 +186,36 @@ void bootsplash_do_render_pictures(struct fb_info *info,
>  				pp->pic_header->width, pp->pic_header->height);
>  	}
>  }
> +
> +
> +void bootsplash_do_render_flush(struct fb_info *info)
> +{
> +	/*
> +	 * FB drivers using deferred_io (such as Xen) need to sync the
> +	 * screen after modifying its contents. When the FB is mmap()ed
> +	 * from userspace, this happens via a dirty pages callback, but
> +	 * when modifying the FB from the kernel, there is no such thing.
> +	 *
> +	 * So let's issue a fake fb_copyarea (copying the FB onto itself)
> +	 * to trick the FB driver into syncing the screen.

Using drm directly would allow you to flush the contents without the fake
(and tbh, really expensive on most drivers) copy op. If you insist on
using fbdev for this stuff, then at least add a new hook to flush cpu
rendering.

> +	 *
> +	 * A few DRM drivers' FB implementations are broken by not using
> +	 * deferred_io when they really should - we match on the known
> +	 * bad ones manually for now.
> +	 */
> +	if (info->fbdefio
> +	    || !strcmp(info->fix.id, "astdrmfb")
> +	    || !strcmp(info->fix.id, "cirrusdrmfb")
> +	    || !strcmp(info->fix.id, "mgadrmfb")) {

We have a shared defio implementation now in drm_fb_helper.c, there's not
really many excuses to not fix up these drivers to just use those ...
-Daniel

> +		struct fb_copyarea area;
> +
> +		area.dx = 0;
> +		area.dy = 0;
> +		area.width = info->var.xres;
> +		area.height = info->var.yres;
> +		area.sx = 0;
> +		area.sy = 0;
> +
> +		info->fbops->fb_copyarea(info, &area);
> +	}
> +}
> -- 
> 2.12.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
'Max Staudt Dec. 14, 2017, 3:36 p.m. UTC | #2
On 12/13/2017 10:35 PM, Daniel Vetter wrote:
> Using drm directly would allow you to flush the contents without the fake
> (and tbh, really expensive on most drivers) copy op. If you insist on
> using fbdev for this stuff, then at least add a new hook to flush cpu
> rendering.

My reasoning is as follows:

1) The splash screen is meant to appear as early as possible in the boot process, and even on devices that don't have a DRM driver. For example, an ARM box with only efifb. Thus, the choice to work on top of FB.

2) We need to go out of the way when a graphical application starts, and come back when it's done. fbcon already has the logic for this, and fbcon is also the thing we're trying to hide. So it seems natural to add the splash on top of fbcon - at least for now.

3) I can't use DRM from the kernel, for the same reason for which there is no "drmcon" to supplant fbcon: There is no interface to reserve framebuffer memory from kernel space: To get memory for a framebuffer, one needs to have a struct file that is passed through the DRM stack down into the drivers.

If this interface existed, then there could be a generic "fb2drm" translation layer, and we would no longer need FB compatibility code in each KMS driver. Actually, I tried to implement this translation layer a year ago, and hit too many walls.

I've prepared the code for a future in which fbdev no longer exists: My sysfs interface is generically called "bootsplash", in the hope that it will one day move on top of KMS. The hooks into fbcon are minimal and the code is straightforward to port to KMS operations rather than FB. But that's for another day, as far as I can see.

4) I don't fully understand what you'd like me to do. Last time I tried to add a new entry to the fbops struct (namely fb_open_adj_file()), you told me not to touch the framebuffer subsystem anymore, as it is meant to die and driver developers shall use KMS instead. Have I misunderstood?

Something like fb->flush() to finish kernel space accesses would be nice to have, but would need to be implemented for all affected drivers separately. The copy op hack is ugly, but solves the problem generically.


What shall I do?

Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
As far as I can see, it would be needed for defio drivers only, is that correct?


>> +	 *
>> +	 * A few DRM drivers' FB implementations are broken by not using
>> +	 * deferred_io when they really should - we match on the known
>> +	 * bad ones manually for now.
>> +	 */
>> +	if (info->fbdefio
>> +	    || !strcmp(info->fix.id, "astdrmfb")
>> +	    || !strcmp(info->fix.id, "cirrusdrmfb")
>> +	    || !strcmp(info->fix.id, "mgadrmfb")) {
> 
> We have a shared defio implementation now in drm_fb_helper.c, there's not
> really many excuses to not fix up these drivers to just use those ...

I'll look into it.


Thanks!
Max
Daniel Vetter Dec. 19, 2017, 12:23 p.m. UTC | #3
On Thu, Dec 14, 2017 at 04:36:49PM +0100, Max Staudt wrote:
> On 12/13/2017 10:35 PM, Daniel Vetter wrote:
> > Using drm directly would allow you to flush the contents without the fake
> > (and tbh, really expensive on most drivers) copy op. If you insist on
> > using fbdev for this stuff, then at least add a new hook to flush cpu
> > rendering.
> 
> My reasoning is as follows:
> 
> 1) The splash screen is meant to appear as early as possible in the boot
> process, and even on devices that don't have a DRM driver. For example,
> an ARM box with only efifb. Thus, the choice to work on top of FB.
> 
> 2) We need to go out of the way when a graphical application starts, and
> come back when it's done. fbcon already has the logic for this, and
> fbcon is also the thing we're trying to hide. So it seems natural to add
> the splash on top of fbcon - at least for now.

And this "automatically disappear" semantics is horribly ill-defined
between fbdev and native kms. So you're not really solving a problem,
you're just not noticing the hacks because they're one layer removed (in
the fbdev emulation code).

> 3) I can't use DRM from the kernel, for the same reason for which there
> is no "drmcon" to supplant fbcon: There is no interface to reserve
> framebuffer memory from kernel space: To get memory for a framebuffer,
> one needs to have a struct file that is passed through the DRM stack
> down into the drivers.

On recent kernels you only need a struct drm_file, not a struct file. That
can be NULL. We've done this to make drmcon possible/easier.

> If this interface existed, then there could be a generic "fb2drm"
> translation layer, and we would no longer need FB compatibility code in
> each KMS driver. Actually, I tried to implement this translation layer a
> year ago, and hit too many walls.

We're pretty much there already I think. The reason it's not entirely gone
is that there's some nasty interactions between drm and the fbdev
emulation, and just having a pile of drivers that aren't too trivial to
convert.

> I've prepared the code for a future in which fbdev no longer exists: My
> sysfs interface is generically called "bootsplash", in the hope that it
> will one day move on top of KMS. The hooks into fbcon are minimal and
> the code is straightforward to port to KMS operations rather than FB.
> But that's for another day, as far as I can see.
> 
> 4) I don't fully understand what you'd like me to do. Last time I tried
> to add a new entry to the fbops struct (namely fb_open_adj_file()), you
> told me not to touch the framebuffer subsystem anymore, as it is meant
> to die and driver developers shall use KMS instead. Have I
> misunderstood?

I still don't like anyone adding features to fbdev :-)

> Something like fb->flush() to finish kernel space accesses would be nice
> to have, but would need to be implemented for all affected drivers
> separately. The copy op hack is ugly, but solves the problem
> generically.

Well, with defio being the hack it is (and because of that, a bunch of drm
drivers not really supporting it) I'm not sure things actually work better
without all this.

> What shall I do?
> 
> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
> As far as I can see, it would be needed for defio drivers only, is that correct?

Yes, which are kinda horrible anyway. I guess you could at least not do
all these hacks if it's not a defio driver.
-Daniel

> 
> 
> >> +	 *
> >> +	 * A few DRM drivers' FB implementations are broken by not using
> >> +	 * deferred_io when they really should - we match on the known
> >> +	 * bad ones manually for now.
> >> +	 */
> >> +	if (info->fbdefio
> >> +	    || !strcmp(info->fix.id, "astdrmfb")
> >> +	    || !strcmp(info->fix.id, "cirrusdrmfb")
> >> +	    || !strcmp(info->fix.id, "mgadrmfb")) {
> > 
> > We have a shared defio implementation now in drm_fb_helper.c, there's not
> > really many excuses to not fix up these drivers to just use those ...
> 
> I'll look into it.
> 
> 
> Thanks!
> Max
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
'Max Staudt Dec. 19, 2017, 1:34 p.m. UTC | #4
On 12/19/2017 01:23 PM, Daniel Vetter wrote:
> On Thu, Dec 14, 2017 at 04:36:49PM +0100, Max Staudt wrote:
>> 2) We need to go out of the way when a graphical application starts, and
>> come back when it's done. fbcon already has the logic for this, and
>> fbcon is also the thing we're trying to hide. So it seems natural to add
>> the splash on top of fbcon - at least for now.
> 
> And this "automatically disappear" semantics is horribly ill-defined
> between fbdev and native kms. So you're not really solving a problem,
> you're just not noticing the hacks because they're one layer removed (in
> the fbdev emulation code).
That's a general complaint about fbcon and/or the fbdev emulation in KMS drivers, right?

I can't see how it relates to my bootsplash, as I'm just replacing fbcon's output, wherever fbcon desires to draw at the given moment, and in no other case.

So when a graphical application sets the VT mode to KD_GRAPHICS, we get a call to do_blank_screen(), and then fbcon -and thus the bootsplash- is muted. The ioctl API has always been like this, and it's not specific to the patch in question.

Similarly, when a graphical application allocates a framebuffer via the KMS ioctl()s, and selects it for scanout, the driver will display that instead of the framebuffer it has allocated internally for the fbdev emulation.


>> 3) I can't use DRM from the kernel, for the same reason for which there
>> is no "drmcon" to supplant fbcon: There is no interface to reserve
>> framebuffer memory from kernel space: To get memory for a framebuffer,
>> one needs to have a struct file that is passed through the DRM stack
>> down into the drivers.
> 
> On recent kernels you only need a struct drm_file, not a struct file. That
> can be NULL. We've done this to make drmcon possible/easier.

Oh that's cool, I missed that. Thanks!

Maybe a fb2drm compat layer will become reality, after all.

The bootsplash code is fairly straightforward to port to a future drmcon, and I'm happy to make the changes once drmcon is available.
But for now, we only have fbcon. And a *lot* of FB drivers. And we want them to show a bootsplash instead of text. So that's where the bootsplash needs to hook into.


>> If this interface existed, then there could be a generic "fb2drm"
>> translation layer, and we would no longer need FB compatibility code in
>> each KMS driver. Actually, I tried to implement this translation layer a
>> year ago, and hit too many walls.
> 
> We're pretty much there already I think. The reason it's not entirely gone
> is that there's some nasty interactions between drm and the fbdev
> emulation, and just having a pile of drivers that aren't too trivial to
> convert.

Sounds like the state of the art last year - drm_file in most cases, but struct file deep in the drivers :(


>> 4) I don't fully understand what you'd like me to do. Last time I tried
>> to add a new entry to the fbops struct (namely fb_open_adj_file()), you
>> told me not to touch the framebuffer subsystem anymore, as it is meant
>> to die and driver developers shall use KMS instead. Have I
>> misunderstood?
> 
> I still don't like anyone adding features to fbdev :-)

So I must not touch fbops, correct?


>> Something like fb->flush() to finish kernel space accesses would be nice
>> to have, but would need to be implemented for all affected drivers
>> separately. The copy op hack is ugly, but solves the problem
>> generically.
> 
> Well, with defio being the hack it is (and because of that, a bunch of drm
> drivers not really supporting it) I'm not sure things actually work better
> without all this.

I don't understand what you mean.

What I do know is that fb_defio is here, and it's here to stay because some drivers need it.

What I also know is that I need to flush the screen after drawing my bootsplash.


>> What shall I do?
>>
>> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
>> As far as I can see, it would be needed for defio drivers only, is that correct?
> 
> Yes, which are kinda horrible anyway. I guess you could at least not do
> all these hacks if it's not a defio driver.

Again, I don't understand.

In my patch (see below), I explicitly check for info->fbdefio, as well as three known broken drmfb emulations. I don't do the copy hack on any other device.


So, what shall I do? As it is, the hack is already specific to devices that really, really need it.

Would you like me to extend the FB API or not?



Max


> -Daniel
> 
>>
>>
>>>> +	 *
>>>> +	 * A few DRM drivers' FB implementations are broken by not using
>>>> +	 * deferred_io when they really should - we match on the known
>>>> +	 * bad ones manually for now.
>>>> +	 */
>>>> +	if (info->fbdefio
>>>> +	    || !strcmp(info->fix.id, "astdrmfb")
>>>> +	    || !strcmp(info->fix.id, "cirrusdrmfb")
>>>> +	    || !strcmp(info->fix.id, "mgadrmfb")) {
Daniel Vetter Dec. 19, 2017, 1:57 p.m. UTC | #5
On Tue, Dec 19, 2017 at 02:34:22PM +0100, Max Staudt wrote:
> On 12/19/2017 01:23 PM, Daniel Vetter wrote:
> > On Thu, Dec 14, 2017 at 04:36:49PM +0100, Max Staudt wrote:
> >> 2) We need to go out of the way when a graphical application starts, and
> >> come back when it's done. fbcon already has the logic for this, and
> >> fbcon is also the thing we're trying to hide. So it seems natural to add
> >> the splash on top of fbcon - at least for now.
> > 
> > And this "automatically disappear" semantics is horribly ill-defined
> > between fbdev and native kms. So you're not really solving a problem,
> > you're just not noticing the hacks because they're one layer removed (in
> > the fbdev emulation code).
> That's a general complaint about fbcon and/or the fbdev emulation in KMS drivers, right?
> 
> I can't see how it relates to my bootsplash, as I'm just replacing
> fbcon's output, wherever fbcon desires to draw at the given moment, and
> in no other case.
> 
> So when a graphical application sets the VT mode to KD_GRAPHICS, we get
> a call to do_blank_screen(), and then fbcon -and thus the bootsplash- is
> muted. The ioctl API has always been like this, and it's not specific to
> the patch in question.
> 
> Similarly, when a graphical application allocates a framebuffer via the
> KMS ioctl()s, and selects it for scanout, the driver will display that
> instead of the framebuffer it has allocated internally for the fbdev
> emulation.
> 
> 
> >> 3) I can't use DRM from the kernel, for the same reason for which there
> >> is no "drmcon" to supplant fbcon: There is no interface to reserve
> >> framebuffer memory from kernel space: To get memory for a framebuffer,
> >> one needs to have a struct file that is passed through the DRM stack
> >> down into the drivers.
> > 
> > On recent kernels you only need a struct drm_file, not a struct file. That
> > can be NULL. We've done this to make drmcon possible/easier.
> 
> Oh that's cool, I missed that. Thanks!
> 
> Maybe a fb2drm compat layer will become reality, after all.
> 
> The bootsplash code is fairly straightforward to port to a future drmcon, and I'm happy to make the changes once drmcon is available.
> But for now, we only have fbcon. And a *lot* of FB drivers. And we want them to show a bootsplash instead of text. So that's where the bootsplash needs to hook into.
> 
> 
> >> If this interface existed, then there could be a generic "fb2drm"
> >> translation layer, and we would no longer need FB compatibility code in
> >> each KMS driver. Actually, I tried to implement this translation layer a
> >> year ago, and hit too many walls.
> > 
> > We're pretty much there already I think. The reason it's not entirely gone
> > is that there's some nasty interactions between drm and the fbdev
> > emulation, and just having a pile of drivers that aren't too trivial to
> > convert.
> 
> Sounds like the state of the art last year - drm_file in most cases, but
> struct file deep in the drivers :(

Where do drivers deal with struct file deep down?

> >> 4) I don't fully understand what you'd like me to do. Last time I tried
> >> to add a new entry to the fbops struct (namely fb_open_adj_file()), you
> >> told me not to touch the framebuffer subsystem anymore, as it is meant
> >> to die and driver developers shall use KMS instead. Have I
> >> misunderstood?
> > 
> > I still don't like anyone adding features to fbdev :-)
> 
> So I must not touch fbops, correct?

The problem is that defio is totally not how a real driver works. So
preferrably bootsplash would use kms directly, and use the explict dirtyfb
callback. But if you insist on using fbdev, then I think the beast course
here is to wire up a new fb_ops->flush callback.

Note that you only need to type the 1 trivial implementation for the drm
fbdev emulation, as long as the callback is optional. Trying to make defio
work correctly, as fbdev assumes it should work, in all cases, on top of
drm is imo an entirely pointless endeavour.

> >> Something like fb->flush() to finish kernel space accesses would be nice
> >> to have, but would need to be implemented for all affected drivers
> >> separately. The copy op hack is ugly, but solves the problem
> >> generically.
> > 
> > Well, with defio being the hack it is (and because of that, a bunch of drm
> > drivers not really supporting it) I'm not sure things actually work better
> > without all this.
> 
> I don't understand what you mean.
> 
> What I do know is that fb_defio is here, and it's here to stay because some drivers need it.
> 
> What I also know is that I need to flush the screen after drawing my bootsplash.

Yes, so lets ignore defio and do the flushing correctly, at least for kms
drivers.

> >> What shall I do?
> >>
> >> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
> >> As far as I can see, it would be needed for defio drivers only, is that correct?
> > 
> > Yes, which are kinda horrible anyway. I guess you could at least not do
> > all these hacks if it's not a defio driver.
> 
> Again, I don't understand.
> 
> In my patch (see below), I explicitly check for info->fbdefio, as well
> as three known broken drmfb emulations. I don't do the copy hack on any
> other device.

Yeah, and if we'd to the explicit flush, you wouldn't even need to check
for that. So

if (fbops->flush)
	fbops->flush(); /* this covers all drm drivers */
else if (fb->defio)
	copyarea hack, if you really still need to support some defio
	fbdev drivers, but really I think that's questionable
else
	; /* nothing */

> So, what shall I do? As it is, the hack is already specific to devices that really, really need it.
> 
> Would you like me to extend the FB API or not?

Yes. Well for real I'd like you to do kms, so maybe you need to explain
why exactly you absolutely have to use fbdev (aka which driver isn't
supported by drm that you want to enable this on).
-Daniel
Oliver Neukum Dec. 19, 2017, 2:07 p.m. UTC | #6
Am Dienstag, den 19.12.2017, 14:57 +0100 schrieb Daniel Vetter:
> > Would you like me to extend the FB API or not?
> 
> Yes. Well for real I'd like you to do kms, so maybe you need to explain
> why exactly you absolutely have to use fbdev (aka which driver isn't
> supported by drm that you want to enable this on).

Hi,

those would be at a minimum efifb, vesafb, xenfb
Those are obviously not sexy, but from a practical point of view
they are the minimum you need to support.

	Regards
		Oliver
'Max Staudt Dec. 19, 2017, 3:41 p.m. UTC | #7
On 12/19/2017 02:57 PM, Daniel Vetter wrote:
> Where do drivers deal with struct file deep down?

As an example, I remembered this to be the case in nouveau's code for allocating a framebuffer. So I checked, and it's much better now.

So I was mistaken about this - sorry.

Thanks a lot for cleaning up this part of DRM, I'm looking forward to a nicer future! Hopefully we can get unify the FB emulation in a single place at some point, but that's just my dreams and is going off-topic, so I'll stop here.


> The problem is that defio is totally not how a real driver works.

But they do exist and I can't ignore them.

I'm afraid I don't understand - why are those, such as xenfb, not real drivers?


> So
> preferrably bootsplash would use kms directly, and use the explict dirtyfb
> callback.

Sure, if I'd be hooking into drmcon, that would be great.

But drmcon doesn't exist yet, so it doesn't get us further to talk about a bootsplash on KMS :(

I'm hooking into the in-kernel terminal emulator, because the bootsplash is a functional extension of that. It just happens that fbcon sits on top of FB, so I work with what I get.

And the console in turn happens to work on all FB and KMS drivers, so it makes users of all kinds of drivers happy. In fact, that's why the FB emulation in KMS drivers came to be in the first place, if I remember right - to ensure fbcon continues to work.

Thus, once drmcon exists and becomes dominant over fbcon, moving the bootsplash to it makes sense. On the other hand, hooking into a raw video subsystem doesn't make sense as far as I can see, so a bootsplash on top of raw KMS is just as meaningless as a bootsplash on top of raw FB. So I have no choice but to work on top of fbcon, and thus use the FB subsystem.


> But if you insist on using fbdev, then I think the beast course
> here is to wire up a new fb_ops->flush callback.

Okay, makes sense. Thanks!


> Note that you only need to type the 1 trivial implementation for the drm
> fbdev emulation, as long as the callback is optional. Trying to make defio
> work correctly, as fbdev assumes it should work, in all cases, on top of
> drm is imo an entirely pointless endeavour.

I'll look into it.


> Yes, so lets ignore defio and do the flushing correctly, at least for kms
> drivers.

I agree.

In fact, if I define fbops->flush(), defio drivers can still add their own proper flushing function, so everybody wins. I like that, see below.


>>>> What shall I do?
>>>>
>>>> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
>>>> As far as I can see, it would be needed for defio drivers only, is that correct?
>>>
>>> Yes, which are kinda horrible anyway. I guess you could at least not do
>>> all these hacks if it's not a defio driver.
>>
>> Again, I don't understand.
>>
>> In my patch (see below), I explicitly check for info->fbdefio, as well
>> as three known broken drmfb emulations. I don't do the copy hack on any
>> other device.
> 
> Yeah, and if we'd to the explicit flush, you wouldn't even need to check
> for that. So
> 
> if (fbops->flush)
> 	fbops->flush(); /* this covers all drm drivers */
> else if (fb->defio)
> 	copyarea hack, if you really still need to support some defio
> 	fbdev drivers, but really I think that's questionable

I need to support xenfb, thus I might as well support all defio drivers.

Also, I like that your suggestion allows for affected drivers to implement their own, more efficient fbops->flush() directly, while ensuring that those that don't still have a fallback, so there is some performance to be gained.

I'll look into implementing this.


> else
> 	; /* nothing */
> 
>> So, what shall I do? As it is, the hack is already specific to devices that really, really need it.
>>
>> Would you like me to extend the FB API or not?
> 
> Yes. Well for real I'd like you to do kms, so maybe you need to explain
> why exactly you absolutely have to use fbdev (aka which driver isn't
> supported by drm that you want to enable this on).

See Oliver's reply - we have plenty of fb-only systems deployed in the real world. Think Xen. Think AArch64 with efifb. Think any system before the KMS driver is loaded (which is a case that the splash is supposed to handle).

Also, where would I hook into KMS, were I to implement it on top of KMS right now? I'm not working on top of FB per se, but on top of fbcon. So in a KMS world I wouldn't work on KMS itself, but on top of... drmcon, which doesn't exist.



Max
Daniel Vetter Dec. 19, 2017, 4:02 p.m. UTC | #8
On Tue, Dec 19, 2017 at 4:41 PM, Max Staudt <mstaudt@suse.de> wrote:
> On 12/19/2017 02:57 PM, Daniel Vetter wrote:
>> Where do drivers deal with struct file deep down?
>
> As an example, I remembered this to be the case in nouveau's code for allocating a framebuffer. So I checked, and it's much better now.
>
> So I was mistaken about this - sorry.
>
> Thanks a lot for cleaning up this part of DRM, I'm looking forward to a nicer future! Hopefully we can get unify the FB emulation in a single place at some point, but that's just my dreams and is going off-topic, so I'll stop here.
>
>
>> The problem is that defio is totally not how a real driver works.
>
> But they do exist and I can't ignore them.
>
> I'm afraid I don't understand - why are those, such as xenfb, not real drivers?

I mean kms drivers. The problem is that the magic mapping that fbdev
expects is real pain. Everyone else, including kms, expects an
explicit flush operation. So instead of hacking around even more with
the defio corner cases that don't work, I'm suggesting we just add
that flush operation. At least internally.

Fixing kms drivers to implement a better defio is probably not a
reasonable investement of time.

>> So
>> preferrably bootsplash would use kms directly, and use the explict dirtyfb
>> callback.
>
> Sure, if I'd be hooking into drmcon, that would be great.
>
> But drmcon doesn't exist yet, so it doesn't get us further to talk about a bootsplash on KMS :(
>
> I'm hooking into the in-kernel terminal emulator, because the bootsplash is a functional extension of that. It just happens that fbcon sits on top of FB, so I work with what I get.

Why do you need a console for a boot splash? You're not drawing
console output afaiui ... And even your current fbdev-based
implementation only interfaces with fbcon insofar as you're making
sure fbcon doesn't wreak your boot splash. Or I'm missing something
somewhere.

> And the console in turn happens to work on all FB and KMS drivers, so it makes users of all kinds of drivers happy. In fact, that's why the FB emulation in KMS drivers came to be in the first place, if I remember right - to ensure fbcon continues to work.
>
> Thus, once drmcon exists and becomes dominant over fbcon, moving the bootsplash to it makes sense. On the other hand, hooking into a raw video subsystem doesn't make sense as far as I can see, so a bootsplash on top of raw KMS is just as meaningless as a bootsplash on top of raw FB. So I have no choice but to work on top of fbcon, and thus use the FB subsystem.
>
>
>> But if you insist on using fbdev, then I think the beast course
>> here is to wire up a new fb_ops->flush callback.
>
> Okay, makes sense. Thanks!
>
>
>> Note that you only need to type the 1 trivial implementation for the drm
>> fbdev emulation, as long as the callback is optional. Trying to make defio
>> work correctly, as fbdev assumes it should work, in all cases, on top of
>> drm is imo an entirely pointless endeavour.
>
> I'll look into it.
>
>
>> Yes, so lets ignore defio and do the flushing correctly, at least for kms
>> drivers.
>
> I agree.
>
> In fact, if I define fbops->flush(), defio drivers can still add their own proper flushing function, so everybody wins. I like that, see below.

tbh I'd forget about ever touching any of the existing fbdev drivers.
Imo just not worth the time investement.

>>>>> What shall I do?
>>>>>
>>>>> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
>>>>> As far as I can see, it would be needed for defio drivers only, is that correct?
>>>>
>>>> Yes, which are kinda horrible anyway. I guess you could at least not do
>>>> all these hacks if it's not a defio driver.
>>>
>>> Again, I don't understand.
>>>
>>> In my patch (see below), I explicitly check for info->fbdefio, as well
>>> as three known broken drmfb emulations. I don't do the copy hack on any
>>> other device.
>>
>> Yeah, and if we'd to the explicit flush, you wouldn't even need to check
>> for that. So
>>
>> if (fbops->flush)
>>       fbops->flush(); /* this covers all drm drivers */
>> else if (fb->defio)
>>       copyarea hack, if you really still need to support some defio
>>       fbdev drivers, but really I think that's questionable
>
> I need to support xenfb, thus I might as well support all defio drivers.
>
> Also, I like that your suggestion allows for affected drivers to implement their own, more efficient fbops->flush() directly, while ensuring that those that don't still have a fallback, so there is some performance to be gained.
>
> I'll look into implementing this.
>
>
>> else
>>       ; /* nothing */
>>
>>> So, what shall I do? As it is, the hack is already specific to devices that really, really need it.
>>>
>>> Would you like me to extend the FB API or not?
>>
>> Yes. Well for real I'd like you to do kms, so maybe you need to explain
>> why exactly you absolutely have to use fbdev (aka which driver isn't
>> supported by drm that you want to enable this on).
>
> See Oliver's reply - we have plenty of fb-only systems deployed in the real world. Think Xen. Think AArch64 with efifb. Think any system before the KMS driver is loaded (which is a case that the splash is supposed to handle).

And you need a real pretty boot-splash on those? That sounds all like
servers, and I haven't yet seen a request for real pretty&fast boot
splash for servers.

> Also, where would I hook into KMS, were I to implement it on top of KMS right now? I'm not working on top of FB per se, but on top of fbcon. So in a KMS world I wouldn't work on KMS itself, but on top of... drmcon, which doesn't exist.

Hm, I guess I need to double check again, but I don't get why you need
to sit on top of a console for the boot splash. I mean I understand
that you need to shut up the console when the boot splash is on, but
from a quick look you're not using fbcon to render anything or
otherwise tie into it. Where's the connection?
-Daniel
Daniel Vetter Dec. 19, 2017, 4:09 p.m. UTC | #9
btw the reason drmcon didn't move is that David Herrmann moved on from
hacking on graphics stuff, and no one needs it. There's nothing
fundamentally wrong with his patches for a basic emergency console on
plain drm, or the simpledrm driver to get a basic drm framebuffer up
on vesafb/efifb and friends. Just wanted to bring this in since you
sound like you're expecting this to magically have happened somehow.
We don't merge code without real use-cases.
-Daniel


On Tue, Dec 19, 2017 at 4:41 PM, Max Staudt <mstaudt@suse.de> wrote:
> On 12/19/2017 02:57 PM, Daniel Vetter wrote:
>> Where do drivers deal with struct file deep down?
>
> As an example, I remembered this to be the case in nouveau's code for allocating a framebuffer. So I checked, and it's much better now.
>
> So I was mistaken about this - sorry.
>
> Thanks a lot for cleaning up this part of DRM, I'm looking forward to a nicer future! Hopefully we can get unify the FB emulation in a single place at some point, but that's just my dreams and is going off-topic, so I'll stop here.
>
>
>> The problem is that defio is totally not how a real driver works.
>
> But they do exist and I can't ignore them.
>
> I'm afraid I don't understand - why are those, such as xenfb, not real drivers?
>
>
>> So
>> preferrably bootsplash would use kms directly, and use the explict dirtyfb
>> callback.
>
> Sure, if I'd be hooking into drmcon, that would be great.
>
> But drmcon doesn't exist yet, so it doesn't get us further to talk about a bootsplash on KMS :(
>
> I'm hooking into the in-kernel terminal emulator, because the bootsplash is a functional extension of that. It just happens that fbcon sits on top of FB, so I work with what I get.
>
> And the console in turn happens to work on all FB and KMS drivers, so it makes users of all kinds of drivers happy. In fact, that's why the FB emulation in KMS drivers came to be in the first place, if I remember right - to ensure fbcon continues to work.
>
> Thus, once drmcon exists and becomes dominant over fbcon, moving the bootsplash to it makes sense. On the other hand, hooking into a raw video subsystem doesn't make sense as far as I can see, so a bootsplash on top of raw KMS is just as meaningless as a bootsplash on top of raw FB. So I have no choice but to work on top of fbcon, and thus use the FB subsystem.
>
>
>> But if you insist on using fbdev, then I think the beast course
>> here is to wire up a new fb_ops->flush callback.
>
> Okay, makes sense. Thanks!
>
>
>> Note that you only need to type the 1 trivial implementation for the drm
>> fbdev emulation, as long as the callback is optional. Trying to make defio
>> work correctly, as fbdev assumes it should work, in all cases, on top of
>> drm is imo an entirely pointless endeavour.
>
> I'll look into it.
>
>
>> Yes, so lets ignore defio and do the flushing correctly, at least for kms
>> drivers.
>
> I agree.
>
> In fact, if I define fbops->flush(), defio drivers can still add their own proper flushing function, so everybody wins. I like that, see below.
>
>
>>>>> What shall I do?
>>>>>
>>>>> Shall I add a new FB op for flushing when writing to the raw memory from the kernel?
>>>>> As far as I can see, it would be needed for defio drivers only, is that correct?
>>>>
>>>> Yes, which are kinda horrible anyway. I guess you could at least not do
>>>> all these hacks if it's not a defio driver.
>>>
>>> Again, I don't understand.
>>>
>>> In my patch (see below), I explicitly check for info->fbdefio, as well
>>> as three known broken drmfb emulations. I don't do the copy hack on any
>>> other device.
>>
>> Yeah, and if we'd to the explicit flush, you wouldn't even need to check
>> for that. So
>>
>> if (fbops->flush)
>>       fbops->flush(); /* this covers all drm drivers */
>> else if (fb->defio)
>>       copyarea hack, if you really still need to support some defio
>>       fbdev drivers, but really I think that's questionable
>
> I need to support xenfb, thus I might as well support all defio drivers.
>
> Also, I like that your suggestion allows for affected drivers to implement their own, more efficient fbops->flush() directly, while ensuring that those that don't still have a fallback, so there is some performance to be gained.
>
> I'll look into implementing this.
>
>
>> else
>>       ; /* nothing */
>>
>>> So, what shall I do? As it is, the hack is already specific to devices that really, really need it.
>>>
>>> Would you like me to extend the FB API or not?
>>
>> Yes. Well for real I'd like you to do kms, so maybe you need to explain
>> why exactly you absolutely have to use fbdev (aka which driver isn't
>> supported by drm that you want to enable this on).
>
> See Oliver's reply - we have plenty of fb-only systems deployed in the real world. Think Xen. Think AArch64 with efifb. Think any system before the KMS driver is loaded (which is a case that the splash is supposed to handle).
>
> Also, where would I hook into KMS, were I to implement it on top of KMS right now? I'm not working on top of FB per se, but on top of fbcon. So in a KMS world I wouldn't work on KMS itself, but on top of... drmcon, which doesn't exist.
>
>
>
> Max
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
'Max Staudt Dec. 19, 2017, 4:23 p.m. UTC | #10
On 12/19/2017 05:02 PM, Daniel Vetter wrote:
> On Tue, Dec 19, 2017 at 4:41 PM, Max Staudt <mstaudt@suse.de> wrote:
>> On 12/19/2017 02:57 PM, Daniel Vetter wrote:
>>> The problem is that defio is totally not how a real driver works.
>>
>> But they do exist and I can't ignore them.
>>
>> I'm afraid I don't understand - why are those, such as xenfb, not real drivers?
> 
> I mean kms drivers. The problem is that the magic mapping that fbdev
> expects is real pain. Everyone else, including kms, expects an
> explicit flush operation. So instead of hacking around even more with
> the defio corner cases that don't work, I'm suggesting we just add
> that flush operation. At least internally.
> 
> Fixing kms drivers to implement a better defio is probably not a
> reasonable investement of time.

Ah yes, I understand now, you mean that KMS drivers have explicit flush, and defio is a hack to retrofit such drivers to an API that never supported a flush operation (the fbdev API), but always used to expose the video memory directly. Right?

If yes, then I agree. Fixing the defio in the KMS drivers wouldn't even solve my problem - I'd still need to implement flush. So might as well care about the flush straight away, yep!


>>> So
>>> preferrably bootsplash would use kms directly, and use the explict dirtyfb
>>> callback.
>>
>> Sure, if I'd be hooking into drmcon, that would be great.
>>
>> But drmcon doesn't exist yet, so it doesn't get us further to talk about a bootsplash on KMS :(
>>
>> I'm hooking into the in-kernel terminal emulator, because the bootsplash is a functional extension of that. It just happens that fbcon sits on top of FB, so I work with what I get.
> 
> Why do you need a console for a boot splash? You're not drawing
> console output afaiui ... And even your current fbdev-based
> implementation only interfaces with fbcon insofar as you're making
> sure fbcon doesn't wreak your boot splash. Or I'm missing something
> somewhere.

Errr... true. I'll answer it below, where you ask again.


>> In fact, if I define fbops->flush(), defio drivers can still add their own proper flushing function, so everybody wins. I like that, see below.
> 
> tbh I'd forget about ever touching any of the existing fbdev drivers.
> Imo just not worth the time investement.

Fair point. It's optional anyway, and can still be done (quickly and painlessly) on demand.

Since my goal here is making a nice bootsplash, I'll touch as few drivers as I can.


>>>> So, what shall I do? As it is, the hack is already specific to devices that really, really need it.
>>>>
>>>> Would you like me to extend the FB API or not?
>>>
>>> Yes. Well for real I'd like you to do kms, so maybe you need to explain
>>> why exactly you absolutely have to use fbdev (aka which driver isn't
>>> supported by drm that you want to enable this on).
>>
>> See Oliver's reply - we have plenty of fb-only systems deployed in the real world. Think Xen. Think AArch64 with efifb. Think any system before the KMS driver is loaded (which is a case that the splash is supposed to handle).
> 
> And you need a real pretty boot-splash on those? That sounds all like
> servers, and I haven't yet seen a request for real pretty&fast boot
> splash for servers.

Yeah, every little helps.

And the vesafb/efifb case is valid for all of the desktop/laptop machines as well.


>> Also, where would I hook into KMS, were I to implement it on top of KMS right now? I'm not working on top of FB per se, but on top of fbcon. So in a KMS world I wouldn't work on KMS itself, but on top of... drmcon, which doesn't exist.
> 
> Hm, I guess I need to double check again, but I don't get why you need
> to sit on top of a console for the boot splash. I mean I understand
> that you need to shut up the console when the boot splash is on, but
> from a quick look you're not using fbcon to render anything or
> otherwise tie into it. Where's the connection?
Fair point.

So the case you're looking at is someone who wants to have a bootsplash, yet doesn't want to have fbcon. Correct?

I agree, this is a case that is not covered with the current code. However such a generic solution would require the definition of new semantics of both fbcon and the bootsplash fighting for the same FB device - well, as long as no graphical application uses it. Urgh... It is a lot simpler to just dual-purpose fbcon, since it knows when to shut up on its own.

And I simply assume that those who load a bootsplash file into their initramfs won't be short a few bytes to compile in fbcon as well.

So... I've hooked into fbcon for simplicity's sake, so I don't have up to three parties fighting for the same device, and so I don't have to define semantics and interfaces to solve that conflict.



Max
'Max Staudt Dec. 19, 2017, 4:26 p.m. UTC | #11
On 12/19/2017 05:09 PM, Daniel Vetter wrote:
> btw the reason drmcon didn't move is that David Herrmann moved on from
> hacking on graphics stuff, and no one needs it. There's nothing
> fundamentally wrong with his patches for a basic emergency console on
> plain drm, or the simpledrm driver to get a basic drm framebuffer up
> on vesafb/efifb and friends. Just wanted to bring this in since you
> sound like you're expecting this to magically have happened somehow.
> We don't merge code without real use-cases.

Don't worry, I'm not expecting this to have magically happened. It will happen when it will happen, or maybe never.

I'm just working with that I've got right now, and once a successor to fbcon takes it's throne, I'm very happy to help move the bootsplash over.



Max
Ray Strode Dec. 19, 2017, 9:01 p.m. UTC | #12
Hi,

On Tue, Dec 19, 2017 at 10:41 AM, Max Staudt <mstaudt@suse.de> wrote:
> I'm hooking into the in-kernel terminal emulator, because the bootsplash is a
> functional extension of that. It just happens that fbcon sits on top of FB, so I
> work with what I get.
>
> And the console in turn happens to work on all FB and KMS drivers, so it
> makes users of all kinds of drivers happy. In fact, that's why the FB emulation
> in KMS drivers came to be in the first place, if I remember right - to ensure
> fbcon continues to work.

But what about multi-monitor? what about hidpi screens?  Ideally you want
each monitor to show the splash in a way that best fits that monitor.

You can't do that if you're drawing all over fbcon... and it's not like multiple
monitors and 4k screens are niche these days.

--Ray
Daniel Vetter Dec. 20, 2017, 9:45 a.m. UTC | #13
On Tue, Dec 19, 2017 at 05:23:52PM +0100, Max Staudt wrote:
> On 12/19/2017 05:02 PM, Daniel Vetter wrote:
> > On Tue, Dec 19, 2017 at 4:41 PM, Max Staudt <mstaudt@suse.de> wrote:
> >> On 12/19/2017 02:57 PM, Daniel Vetter wrote:
> >>> The problem is that defio is totally not how a real driver works.
> >>
> >> But they do exist and I can't ignore them.
> >>
> >> I'm afraid I don't understand - why are those, such as xenfb, not real drivers?
> > 
> > I mean kms drivers. The problem is that the magic mapping that fbdev
> > expects is real pain. Everyone else, including kms, expects an
> > explicit flush operation. So instead of hacking around even more with
> > the defio corner cases that don't work, I'm suggesting we just add
> > that flush operation. At least internally.
> > 
> > Fixing kms drivers to implement a better defio is probably not a
> > reasonable investement of time.
> 
> Ah yes, I understand now, you mean that KMS drivers have explicit flush,
> and defio is a hack to retrofit such drivers to an API that never
> supported a flush operation (the fbdev API), but always used to expose
> the video memory directly. Right?
> 
> If yes, then I agree. Fixing the defio in the KMS drivers wouldn't even
> solve my problem - I'd still need to implement flush. So might as well
> care about the flush straight away, yep!

Yup.

I'll leave the more fundamental discussion to the other thread on the
cover letter.
-Daniel
'Max Staudt Dec. 20, 2017, 1:14 p.m. UTC | #14
On 12/19/2017 10:01 PM, Ray Strode wrote:
> Hi,
> 
> On Tue, Dec 19, 2017 at 10:41 AM, Max Staudt <mstaudt@suse.de> wrote:
>> I'm hooking into the in-kernel terminal emulator, because the bootsplash is a
>> functional extension of that. It just happens that fbcon sits on top of FB, so I
>> work with what I get.
>>
>> And the console in turn happens to work on all FB and KMS drivers, so it
>> makes users of all kinds of drivers happy. In fact, that's why the FB emulation
>> in KMS drivers came to be in the first place, if I remember right - to ensure
>> fbcon continues to work.
> 
> But what about multi-monitor? what about hidpi screens?  Ideally you want
> each monitor to show the splash in a way that best fits that monitor.

Actually, I don't want that :)

This was a design decision that I've made to keep the code small and simple to audit.
As it is, the simple bootsplash code will make 99% of people happy. It includes positioning of objects in corners and in the center, and a background color, and thus can render something that doesn't look funny in all but the most extreme cases.

I've made this decision from the point of view of someone who wants to ship a general purpose distribution. If you have a look around and compare this to e.g. the Windows or Mac OS X bootsplashes, the possibilities that my kernel code provides already surpasses them.

If you really want such sophisticated features, supplanting the initial bootsplash  with Plymouth (or not using it at all) is a better solution. In my opinion, it is overengineering, at least for kernel code.


So... I'll just take what the fbdev emulation gives me. In most cases (single screen systems), this looks good. In most remaining cases, you have two identical monitors that show exactly the same thing. And whatever remains can live with whatever mode the DRM driver decides to set for the fbdev emulation.


As for HiDPI, if you're working on an embedded device with a fixed screen size - say a phone - you can easily include a huge picture the size of the screen in the bootsplash.


Again, it's feature creep now. Apple just centers a, well, an apple in the middle of the screen. Windows may even boot in 640x480 and then do a mode switch when showing the desktop (not sure whether this is still true with the latest version). Neither of them scales for HiDPI, and neither cares about multiple monitors. People are happy.


So in the end, it's a matter of taste. I agree that in user space, exploring these features is fun. But in kernel space, it's definitely important to keep the code short and simple. I'm convinced that I've found a good balance :)


Max
Ray Strode Dec. 20, 2017, 3:35 p.m. UTC | #15
Hi,

> Actually, I don't want that :)
>
> This was a design decision that I've made to keep the code small and simple to audit.
> As it is, the simple bootsplash code will make 99% of people happy.
You think only 1% of linux users have more than one monitor or a 4k screen?

> I've made this decision from the point of view of someone who wants to ship a general
> purpose distribution. If you have a look around and compare this to e.g. the Windows or
> Mac OS X bootsplashes, the possibilities that my kernel code provides already
> surpasses them.
I haven't looked specifically, but I don't believe you :-)  You're
telling me the apple boot
splash isn't scaled up on machines with retina displays?  I don't use
OSX (or windows),
so I don't know, but I'd be really surprised.

> If you really want such sophisticated features, supplanting the initial bootsplash  with
> Plymouth (or not using it at all) is a better solution. In my opinion, it is overengineering,
> at least for kernel code.
disagree..it's support for basic, commodity hardware these days.

> As for HiDPI, if you're working on an embedded device with a fixed screen size -
> say a phone - you can easily include a huge picture the size of the screen in the
> bootsplash.
I'm talking about a situation where you have a dell xps or whatever
with an external
monitor attached.  Each monitor should be able to show the splash
without deforming
it, and without making it huge or microscopic. pretty basic stuff...

--Ray
'Max Staudt Dec. 20, 2017, 4:52 p.m. UTC | #16
On 12/20/2017 04:35 PM, Ray Strode wrote:
> Hi,
> 
>> Actually, I don't want that :)
>>
>> This was a design decision that I've made to keep the code small and simple to audit.
>> As it is, the simple bootsplash code will make 99% of people happy.
> You think only 1% of linux users have more than one monitor or a 4k screen?

No, I think 99% will be glad to see a splash at all, and it's a 99% probabililty that the splash will look alright on at least one screen, if not more.

By the simple virtue that dual-monitor setups tend to use identical screens, it's okay if these are cloned while the splash is shown. So maybe 95% of these users are fine, as well.

As for 4k screens - a logo that looks okay on 800x600 will probably look fine, even though it's physically smaller, on a 4k screen.


If there really, really, really is a need, HiDPI can be retrofitted to the format later on.


>> I've made this decision from the point of view of someone who wants to ship a general
>> purpose distribution. If you have a look around and compare this to e.g. the Windows or
>> Mac OS X bootsplashes, the possibilities that my kernel code provides already
>> surpasses them.
> I haven't looked specifically, but I don't believe you :-)  You're
> telling me the apple boot
> splash isn't scaled up on machines with retina displays?  I don't use
> OSX (or windows),
> so I don't know, but I'd be really surprised.

Admittedly, my knowledge is aging as well. My pre-retina MacBook certainly didn't scale anything.

And Windows XP booted in 640x480, then went black, switched modes, and drew the desktop.


>> If you really want such sophisticated features, supplanting the initial bootsplash  with
>> Plymouth (or not using it at all) is a better solution. In my opinion, it is overengineering,
>> at least for kernel code.
> disagree..it's support for basic, commodity hardware these days.

Hmm. I guess it's a matter of taste, and we have to agree to disagree on this one. See above.

Also, like pointed out in the other email, it's meant as an alternative to Plymouth. Maybe I should have made this clear from the beginning - it's an option, not a replacement. We're in the FOSS ecosystem after all, where it's all about choice, and that choice exists because we admit that sometimes, not everyone can be made happy with the same solution.


>> As for HiDPI, if you're working on an embedded device with a fixed screen size -
>> say a phone - you can easily include a huge picture the size of the screen in the
>> bootsplash.
> I'm talking about a situation where you have a dell xps or whatever
> with an external
> monitor attached.  Each monitor should be able to show the splash
> without deforming
> it, and without making it huge or microscopic. pretty basic stuff...
See above. I doubt any other system cares.

And we can't even care before native driver KMS comes up, which may take a while, depending on the system.

I'd like to have this as an alternative to Plymouth in such setups.



Max
Alan Cox Dec. 31, 2017, 12:53 p.m. UTC | #17
On Tue, 19 Dec 2017 15:07:53 +0100
Oliver Neukum <oneukum@suse.com> wrote:

> Am Dienstag, den 19.12.2017, 14:57 +0100 schrieb Daniel Vetter:
> > > Would you like me to extend the FB API or not?  
> > 
> > Yes. Well for real I'd like you to do kms, so maybe you need to explain
> > why exactly you absolutely have to use fbdev (aka which driver isn't
> > supported by drm that you want to enable this on).  
> 
> Hi,
> 
> those would be at a minimum efifb, vesafb, xenfb
> Those are obviously not sexy, but from a practical point of view
> they are the minimum you need to support.

I think it's more constructive to look at it the other way around. What
drivers do we have that actually need to be used which don't have DRM
equivalents - and how do we fix that instead ?

Alan
'Max Staudt Jan. 3, 2018, 6:04 p.m. UTC | #18
On 12/31/2017 01:53 PM, Alan Cox wrote:
> On Tue, 19 Dec 2017 15:07:53 +0100
> Oliver Neukum <oneukum@suse.com> wrote:
> 
>> Am Dienstag, den 19.12.2017, 14:57 +0100 schrieb Daniel Vetter:
>>>> Would you like me to extend the FB API or not?  
>>>
>>> Yes. Well for real I'd like you to do kms, so maybe you need to explain
>>> why exactly you absolutely have to use fbdev (aka which driver isn't
>>> supported by drm that you want to enable this on).  
>>
>> Hi,
>>
>> those would be at a minimum efifb, vesafb, xenfb
>> Those are obviously not sexy, but from a practical point of view
>> they are the minimum you need to support.
> 
> I think it's more constructive to look at it the other way around. What
> drivers do we have that actually need to be used which don't have DRM
> equivalents - and how do we fix that instead ?
It's *at least* the above named drivers: efifb, vesafb, xenfb.


Max
diff mbox

Patch

diff --git a/drivers/video/fbdev/core/bootsplash.c b/drivers/video/fbdev/core/bootsplash.c
index 843c5400fefc..815b007f81ca 100644
--- a/drivers/video/fbdev/core/bootsplash.c
+++ b/drivers/video/fbdev/core/bootsplash.c
@@ -112,6 +112,8 @@  void bootsplash_render_full(struct fb_info *info)
 
 	bootsplash_do_render_pictures(info, splash_state.file);
 
+	bootsplash_do_render_flush(info);
+
 out:
 	mutex_unlock(&splash_state.data_lock);
 }
diff --git a/drivers/video/fbdev/core/bootsplash_internal.h b/drivers/video/fbdev/core/bootsplash_internal.h
index 71e2a27ac0b8..0acb383aa4e3 100644
--- a/drivers/video/fbdev/core/bootsplash_internal.h
+++ b/drivers/video/fbdev/core/bootsplash_internal.h
@@ -89,6 +89,7 @@  void bootsplash_do_render_background(struct fb_info *info,
 				     const struct splash_file_priv *fp);
 void bootsplash_do_render_pictures(struct fb_info *info,
 				   const struct splash_file_priv *fp);
+void bootsplash_do_render_flush(struct fb_info *info);
 
 
 void bootsplash_free_file(struct splash_file_priv *fp);
diff --git a/drivers/video/fbdev/core/bootsplash_render.c b/drivers/video/fbdev/core/bootsplash_render.c
index 2ae36949d0e3..8c09c306ff67 100644
--- a/drivers/video/fbdev/core/bootsplash_render.c
+++ b/drivers/video/fbdev/core/bootsplash_render.c
@@ -186,3 +186,36 @@  void bootsplash_do_render_pictures(struct fb_info *info,
 				pp->pic_header->width, pp->pic_header->height);
 	}
 }
+
+
+void bootsplash_do_render_flush(struct fb_info *info)
+{
+	/*
+	 * FB drivers using deferred_io (such as Xen) need to sync the
+	 * screen after modifying its contents. When the FB is mmap()ed
+	 * from userspace, this happens via a dirty pages callback, but
+	 * when modifying the FB from the kernel, there is no such thing.
+	 *
+	 * So let's issue a fake fb_copyarea (copying the FB onto itself)
+	 * to trick the FB driver into syncing the screen.
+	 *
+	 * A few DRM drivers' FB implementations are broken by not using
+	 * deferred_io when they really should - we match on the known
+	 * bad ones manually for now.
+	 */
+	if (info->fbdefio
+	    || !strcmp(info->fix.id, "astdrmfb")
+	    || !strcmp(info->fix.id, "cirrusdrmfb")
+	    || !strcmp(info->fix.id, "mgadrmfb")) {
+		struct fb_copyarea area;
+
+		area.dx = 0;
+		area.dy = 0;
+		area.width = info->var.xres;
+		area.height = info->var.yres;
+		area.sx = 0;
+		area.sy = 0;
+
+		info->fbops->fb_copyarea(info, &area);
+	}
+}