On Tue, Mar 26, 2019 at 06:55:30PM +0100, Noralf Trønnes wrote:
This moves the modesetting code from drm_fb_helper to drm_client so it can be shared by all internal clients.
I have also added a client display abstraction and a bootsplash example client to show where this might be heading. Hopefully Max Staudt will be able to pick up his bootsplash work now.
First a fairly unrelated thing that I noticed while reading stuff:
In drm_fbdev_generic_setup() we register the drm_client to the world (with drm_client_add) before it's fully set up. And the checks in the setup code aren't safe against a concurrent hotplug call from another thread. Which can happen, because usually by that point, and definitely by the time the driver called drm_dev_register() the hotplug handler is running.
Maybe good idea to rename drm_client_add to drm_client_register (to stay consistent with our naming scheme of _register() = others can start calling us from any thread).
We need to do the basic setup code _before_ we call drm_client_register. The kerneldoc for the various fbdev setup functions have explanations for when exactly it's ok to handle hotplug events.
The other bit is kinda the high-level review on the drm_client modeset api: - Allowing multiple different modeset clients per drm_client feels like overkill. I think we can just require a 1:1 mapping between drm_client and modeset config. If a client wants to have multiple different modeset configs per drm_device they can create more drm_clients.
- That also fixes your "do we need embedding" question, since drm_client supports that already.
- That means we could clean up the api considerably by embedding all the modeset stuff into drm_client, and e.g. allocating the modeset arrays at drm_client_init() time.
- Except that wouldn't work with the current fbdev emulation code, because that one isn't always using drm_client.
Hence my question/suggestion: Could we rework the fbdev emulation to always allocate a drm_client, but only use drm_client for buffer allocation for generic_setup(). That could also provide us with a smoother upgrade path for other drivers to generic_setup, e.g. we could ditch all the hotplug handling already.
I'm thinking of embedding a drm_client into drm_fb_helper, and calling drm_client_init() on it at the right time. But only call drm_client_add() for generic_setup(). At least as a first step.
Related question: What's the plan for drivers which don't support generic_setup()? If we eventually have stuff like kmscon running on top of drm_client, we'd have to somehow port them all ...
And finally the bikeshed: I thik drm_client_modeset would be a good prefix for all this (maybe even in a separate file): - we have a pretty clear split between basic drm stuff and kms - modeset means kms, display usually only means the actual physical display. drm_simple_display_pipe always gets me with using display instead of modeset, but a bit too late to rename that one :-)
Thoughts on this?
Cheers, Daniel
Noralf.
Noralf Trønnes (16): drm/fb-helper: Remove unused gamma_size variable drm/fb-helper: dpms_legacy(): Only set on connectors in use drm/atomic: Move __drm_atomic_helper_disable_plane/set_config() drm/fb-helper: No need to cache rotation and sw_rotations drm/fb-helper: Remove drm_fb_helper_crtc->{x,y,desired_mode} drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper drm/fb-helper: Remove drm_fb_helper_crtc drm/fb-helper: Prepare to move out commit code drm/fb-helper: Move out commit code drm/fb-helper: Remove drm_fb_helper_connector drm/fb-helper: Prepare to move out modeset config code drm/fb-helper: Move out modeset config code drm/fb-helper: Avoid race with DRM userspace drm/client: Add display abstraction drm/client: Hack: Add bootsplash example drm/vc4: Call drm_dev_register() after all setup is done
Documentation/gpu/todo.rst | 10 + drivers/gpu/drm/Kconfig | 5 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_atomic.c | 168 ++++ drivers/gpu/drm/drm_atomic_helper.c | 164 --- drivers/gpu/drm/drm_auth.c | 20 + drivers/gpu/drm/drm_bootsplash.c | 216 ++++ drivers/gpu/drm/drm_client.c | 1449 +++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_internal.h | 5 + drivers/gpu/drm/drm_drv.c | 4 + drivers/gpu/drm/drm_fb_helper.c | 1151 ++------------------- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/i915/intel_fbdev.c | 218 ---- drivers/gpu/drm/vc4/vc4_drv.c | 6 +- include/drm/drm_atomic_helper.h | 4 - include/drm/drm_client.h | 117 +++ include/drm/drm_fb_helper.h | 127 +-- 17 files changed, 2128 insertions(+), 1539 deletions(-) create mode 100644 drivers/gpu/drm/drm_bootsplash.c
-- 2.20.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx