Regression on the sun4i-drm driver by cma helper change

[Daniel Vetter]

On Wed, Jul 11, 2018 at 4:52 PM, Noralf Trønnes <noralf at tronnes.org> wrote:
>
> Den 11.07.2018 16.06, skrev Maxime Ripard:
>>
>> CC'ing Noralf and Daniel,
>>
>> On Wed, Jul 11, 2018 at 09:47:53PM +0800, Icenowy Zheng wrote:
>>>
>>> Today, during testing the Banana Pi M2 Zero HDMI patch, I found a
>>> regression on the sun4i-drm driver, which happenes because of the cma
>>> helper change.
>>>
>>> The bad commit is 894a677f4b3e6d2ab8d01bb46c1fbd5f92e4591b ("drm/cma-
>>> helper: Use the generic fbdev emulation").
>>>
>>> If this commit is present, sun4i_framebuffer_init() will return
>>> -ENODEV, because of the call of drm_client_new(). In this case sun4i-
>>> drv will fail to probe, and all components will be unbound.
>>>
>>> By reverting the commit, sun4i-drv loads and HDMI output is correct.
>>>
>>> Could anyone investigate into this, please?
>>
>> Any ideas?
>
>
> I think it's because drm_fb_cma_fbdev_init() is called before
> drm_dev_register() and that this is what returns -ENODEV:
>
> int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
>            const char *name, const struct drm_client_funcs *funcs)
> {
> ...
>     mutex_lock(&dev->clientlist_mutex);
>     registered = dev->registered;
>     if (registered)
>         list_add(&client->list, &dev->clientlist);
>     mutex_unlock(&dev->clientlist_mutex);
>     if (!registered) {
>         ret = -ENODEV;
>         goto err_close;
>     }
>
> I would have been nice to get it confirmed that indeed this is the case.
>
> Maybe we should remove this check temporarily until all have converted to
> drm_fbdev_generic_setup() which will be called after drm_dev_register().
>
> I'll come back when I have looked at the other drivers to see in which
> order they register fbdev and DRM.

Yeah I don't think we can require that drivers have called
drm_dev_register before setting up the fbdev stuff, that will break
the world. What we might want to consider is a ->register callback
(for e.g. framebuffer_register, but not all the stuff leading up to
it), but for now I don't think there's a need for that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch