Subject: Re: [RFC PATCH v2 03/13] bootsplash: Flush framebuffer after drawing
To: b.zolnierkie@samsung.com, linux-fbdev@vger.kernel.org,
        michal@markovi.net, sndirsch@suse.com, oneukum@suse.com,
        tiwai@suse.com, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, bernhard.rosenkranzer@linaro.org,
        philm@manjaro.org
References: <20171213194755.3409-1-mstaudt@suse.de>
 <20171213194755.3409-4-mstaudt@suse.de>
 <20171213213506.GD26573@phenom.ffwll.local>
From: Max Staudt <mstaudt@suse.de>
Message-ID: <d66a994a-cb3a-c1fb-8f91-adbd475a40f0@suse.de>
Date: Thu, 14 Dec 2017 16:36:49 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <20171213213506.GD26573@phenom.ffwll.local>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2900
Lines: 47

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