Re: [PATCH v2 00/11] drm: Introduce DRM client library

[Jocelyn Falempe <jfalempe@redhat.com>]

On 02/10/2024 15:04, Thomas Zimmermann wrote:
> With the next DRM client coming soon (drm_log) and most of DRM's
> fbdev emulation consolidated in a few places, it's time to provide
> a single place for the clients.
> 
> The new module drm_client_lib.ko stores most of the in-kernel client
> code. It is designed such that drivers can opt into client support,
> but the presence of the client module depends on the user's kernel
> configuration. Without selected clients, no client module will be
> build.

I think I would rather rename drm_client.c to drm_client_lib.c, and have 
a drm_client.ko. Users may need to interact with module name more than 
with C file.

> 
> Version 2 of this patchset is a significant rework of the patches,
> so there are no R-bs kept. The base client code and client event
> handling remains in the DRM core. This simplifies module dependencies.
> But suspend and resume are now handled in client helpers instead of
> direct calls to fbdev emulation. This breaks a cyclic dependency
> among the involved modules. It also allows any client to process
> suspend and resume events.

Thanks a lot for this work. I rebased my drm_log proof of concept on top 
on this v2, and it works great.
I think I can add a suspend/resume callback to drm_log, just marking the 
console as CON_SUSPENDED should work, I will test that shortly.


Best regards,

-- 

Jocelyn

> 
> v2:
> - rework of the overall design
> - keep base client code in DRM core.
> 
> Thomas Zimmermann (11):
>    drm/i915: Select DRM_CLIENT_SELECTION
>    drm/xe: Select DRM_CLIENT_SELECTION
>    drm/fbdev: Select fbdev I/O helpers from modules that require them
>    drm/fbdev: Store fbdev module parameters in separate file
>    drm/client: Move client event handlers to drm_client_event.c
>    drm/client: Move suspend/resume into DRM client callbacks
>    drm/amdgpu: Suspend and resume internal clients with client helpers
>    drm/nouveau: Suspend and resume clients with client helpers
>    drm/radeon: Suspend and resume clients with client helpers
>    drm/client: Make client support optional
>    drm/client: Add client-lib module
> 
>   Documentation/gpu/drm-client.rst              |   3 +
>   drivers/gpu/drm/Kconfig                       |  39 +++-
>   drivers/gpu/drm/Makefile                      |  20 +-
>   drivers/gpu/drm/amd/amdgpu/Kconfig            |   1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  22 +-
>   drivers/gpu/drm/drm_client.c                  | 121 -----------
>   drivers/gpu/drm/drm_client_event.c            | 195 ++++++++++++++++++
>   drivers/gpu/drm/drm_client_setup.c            |   3 +
>   drivers/gpu/drm/drm_drv.c                     |   2 +-
>   drivers/gpu/drm/drm_fb_helper.c               |  31 ---
>   drivers/gpu/drm/drm_fbdev_client.c            |  30 ++-
>   drivers/gpu/drm/drm_file.c                    |   2 +-
>   drivers/gpu/drm/drm_internal.h                |   7 +
>   drivers/gpu/drm/drm_kms_helper_common.c       |  38 ++++
>   drivers/gpu/drm/drm_modeset_helper.c          |  14 +-
>   drivers/gpu/drm/drm_probe_helper.c            |   2 +-
>   drivers/gpu/drm/i915/Kconfig                  |   1 +
>   .../drm/i915/display/intel_display_driver.c   |   2 +-
>   drivers/gpu/drm/nouveau/nouveau_display.c     |   8 +-
>   drivers/gpu/drm/nouveau/nouveau_vga.c         |   2 +-
>   drivers/gpu/drm/radeon/radeon_device.c        |  19 +-
>   drivers/gpu/drm/radeon/radeon_fbdev.c         |   6 -
>   drivers/gpu/drm/radeon/radeon_mode.h          |   3 -
>   drivers/gpu/drm/xe/Kconfig                    |   1 +
>   include/drm/drm_client.h                      |  39 +++-
>   include/drm/drm_client_event.h                |  27 +++
>   26 files changed, 423 insertions(+), 215 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_client_event.c
>   create mode 100644 include/drm/drm_client_event.h
>