[PATCH] drm/stm: Enable RPM during fbdev registration

[Yannick FERTRE]

Hi Marek,

On 11/5/20 10:45 AM, Marek Vasut wrote:
> On 11/5/20 10:39 AM, Daniel Vetter wrote:
>> On Wed, Nov 04, 2020 at 01:52:00PM +0100, Marek Vasut wrote:
>>> Enable runtime PM before registering the fbdev emulation and disable it
>>> afterward, otherwise register access to the LTDC IP during the fbdev
>>> emulation registration might hang the system.
>>>
>>> The problem happens because RPM is activated at the end of ltdc_load(),
>>> but the fbdev emulation registration happens only after that, and ends
>>> up calling ltdc_crtc_mode_set_nofb(), which checks whether RPM is active
>>> and only if it is not active, calls pm_runtime_get_sync() to enable the
>>> clock and so on. If the clock are not enabled, any register access in
>>> ltdc_crtc_mode_set_nofb() could hang the platform completely.
>>>
>>> This patch makes sure that ltdc_crtc_mode_set_nofb() is called within
>>> pm_runtime_get_sync(), so with clock enabled.
> 
> [...]
> 
>> This looks like you're papering over a bug in your modeset code. If
>> userspace later on does a setpar on the fbdev chardev, the exact same
>> thing could happen. You need to fix your modeset code to avoid this, not
>> sprinkle temporary rpm_get/put all over some top level entry points,
>> because you can't even patch those all.
> 
> I have a feeling all those pm_runtime_active() checks in the driver 
> might be the root cause of this ? I wonder why the code doesn't use 
> pm_runtime_{get,put}_sync() only when accessing registers. Thoughts?

First line of function ltdc_crtc_mode_set_nofb check the pm_runtime to 
avoid to access registers without clock enabled.



static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
{
...
	if (!pm_runtime_active(ddev->dev)) {
		ret = pm_runtime_get_sync(ddev->dev);

I test the fb with framebuffer console, & it works fine on my side.
Do you test fb on a old kernel?
How can I reproduce your issue?

Best regards

Yannick