Re: [PATCH 00/13] drm/vkms: Allow to configure device

Louis Chauvet
2025-01-30T13:48:19Z

On 29/01/25 - 12:00, José Expósito wrote:
> Hi everyone,
>
> In preparation for ConfigFS support, a flexible way to configure VKMS device(s)
> is required.
> This series adds the required APIs to create a configuration, the code changes
> required to apply it and KUnit test validating the changes.

Hi José,

Thanks a lot!

This series is amazing and better than mine on many points. I have few
comments:
- a "strange" naming pair: add/destroy (I expect add/remove or
create/destroy like other function in DRM)
- usage of "complex" list accessors, can't we just create iterators?
- should we use pr_err in vkms_config_valid?

> Louis Chauvet and I are working on ConfigFS support. In this series I tried to
> merge his changes [1] with mine [2].
> I kept his Signed-off-by to reflect that, even if I show up as the author of
> some/most of the patches, this was a joint effort.

To avoid confusion, you should add the Co-developped-by tag, so it will be
clear that we worked together on this.

> I'm still polishing the ConfigFS code [3] and its IGT tests [4] (connector
> hot-add/remove bugs) but the IGT tests also exercise this series and can be used
> for additional test coverage.

I will take a look at those series. For the connector hot-add/remove, do
you have any example of usage in the kernel? I did not found anything in
the documentation explaining they are hot-addable.

Thanks again for this series,
Louis Chauvet

> Best wishes,
> José Expósito
>
> [1] https://patchwork.kernel.org/project/dri-devel/cover/20250121-google-remove-crtc-index-from-parameter-v3-0-cac00a3c3544@bootlin.com/
> [2] https://patchwork.kernel.org/project/dri-devel/cover/20240813105134.17439-1-jose.exposito89@gmail.com/
> [3] https://github.com/JoseExposito/linux/commits/patch-vkms-configfs/
> [4] https://gitlab.freedesktop.org/jexposit/igt-gpu-tools/-/commits/vkms-configfs
>
> José Expósito (12):
> drm/vkms: Extract vkms_connector header
> drm/vkms: Add KUnit test scaffolding
> drm/vkms: Extract vkms_config header
> drm/vkms: Move default_config creation to its own function
> drm/vkms: Set device name from vkms_config
> drm/vkms: Allow to configure multiple planes
> drm/vkms: Allow to configure multiple CRTCs
> drm/vkms: Allow to attach planes and CRTCs
> drm/vkms: Allow to configure multiple encoders
> drm/vkms: Allow to attach encoders and CRTCs
> drm/vkms: Allow to configure multiple connectors
> drm/vkms: Allow to attach connectors and encoders
>
> Louis Chauvet (1):
> drm/vkms: Add a validation function for VKMS configuration
>
> drivers/gpu/drm/vkms/Kconfig | 15 +
> drivers/gpu/drm/vkms/Makefile | 5 +-
> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
> drivers/gpu/drm/vkms/tests/Makefile | 3 +
> drivers/gpu/drm/vkms/tests/vkms_config_test.c | 782 +++++++++++++++++
> drivers/gpu/drm/vkms/vkms_config.c | 784 ++++++++++++++++++
> drivers/gpu/drm/vkms/vkms_config.h | 479 +++++++++++
> drivers/gpu/drm/vkms/vkms_connector.c | 61 ++
> drivers/gpu/drm/vkms/vkms_connector.h | 26 +
> drivers/gpu/drm/vkms/vkms_drv.c | 45 +-
> drivers/gpu/drm/vkms/vkms_drv.h | 17 +-
> drivers/gpu/drm/vkms/vkms_output.c | 255 ++++--
> 12 files changed, 2337 insertions(+), 139 deletions(-)
> create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
> create mode 100644 drivers/gpu/drm/vkms/tests/Makefile
> create mode 100644 drivers/gpu/drm/vkms/tests/vkms_config_test.c
> create mode 100644 drivers/gpu/drm/vkms/vkms_config.c
> create mode 100644 drivers/gpu/drm/vkms/vkms_config.h
> create mode 100644 drivers/gpu/drm/vkms/vkms_connector.c
> create mode 100644 drivers/gpu/drm/vkms/vkms_connector.h
>
> --
> 2.48.1
>