| Message ID | b6e27bc6a54f5cb340658fa5969f7b48fbfbf1b7.1526514457.git.rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > This commit adds the essential infrastructure for around CRTCs which > is composed of: a new data struct for output data information, a > function for creating planes, and a simple encoder attached to the > connector. Finally, due to the introduction of a new initialization > function, connectors were moved from vkms_drv.c to vkms_display.c. > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > --- > drivers/gpu/drm/vkms/Makefile | 2 +- > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > 6 files changed, 211 insertions(+), 47 deletions(-) > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > index 2aef948d3a34..3f774a6a9c58 100644 > --- a/drivers/gpu/drm/vkms/Makefile > +++ b/drivers/gpu/drm/vkms/Makefile > @@ -1,3 +1,3 @@ > -vkms-y := vkms_drv.o > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > obj-$(CONFIG_DRM_VKMS) += vkms.o > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > new file mode 100644 > index 000000000000..bf76cd39ece7 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > @@ -0,0 +1,35 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include "vkms_drv.h" > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc_helper.h> > + > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > + .set_config = drm_atomic_helper_set_config, > + .destroy = drm_crtc_cleanup, > + .page_flip = drm_atomic_helper_page_flip, > + .reset = drm_atomic_helper_crtc_reset, > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > +}; > + > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > + struct drm_plane *primary, struct drm_plane *cursor) > +{ > + int ret; > + Just as a follow up, I have noticed that vkms breaks when inspecting its state in the debugfs, also when running igt tests with kms and core filters, and adding the following fixed that issue: 1) a drm_crtc_helper_funcs with dummy atomic_check. > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > + &vkms_crtc_funcs, NULL); > + if (ret) { > + DRM_ERROR("Failed to init CRTC\n"); > + return ret; > + } > + > + return ret; > +} > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index aec3f180f96d..070613e32934 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -6,7 +6,6 @@ > */ > > #include <linux/module.h> > -#include <drm/drmP.h> > #include <drm/drm_gem.h> > #include <drm/drm_crtc_helper.h> > #include <drm/drm_atomic_helper.h> > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > .minor = DRIVER_MINOR, > }; > > -static const u32 vkms_formats[] = { > - DRM_FORMAT_XRGB8888, > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > + .atomic_check = drm_atomic_helper_check, > + .atomic_commit = drm_atomic_helper_commit, > }; > > -static void vkms_connector_destroy(struct drm_connector *connector) > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > { > - drm_connector_unregister(connector); > - drm_connector_cleanup(connector); > -} > + struct drm_device *dev = &vkmsdev->drm; > > -static const struct drm_connector_funcs vkms_connector_funcs = { > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = vkms_connector_destroy, > -}; > + drm_mode_config_init(dev); > + dev->mode_config.funcs = &vkms_mode_funcs; > + dev->mode_config.min_width = XRES_MIN; > + dev->mode_config.min_height = YRES_MIN; > + dev->mode_config.max_width = XRES_MAX; > + dev->mode_config.max_height = YRES_MAX; > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > - .atomic_check = drm_atomic_helper_check, > - .atomic_commit = drm_atomic_helper_commit, > -}; > + return vkms_output_init(vkmsdev); > +} > > static int __init vkms_init(void) > { > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > goto out_fini; > } > > - drm_mode_config_init(&vkms_device->drm); > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > - vkms_device->drm.mode_config.min_width = XRES_MIN; > - vkms_device->drm.mode_config.min_height = YRES_MIN; > - vkms_device->drm.mode_config.max_width = XRES_MAX; > - vkms_device->drm.mode_config.max_height = YRES_MAX; > - > - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, > - &vkms_connector_funcs, > - DRM_MODE_CONNECTOR_VIRTUAL); > - if (ret < 0) { > - DRM_ERROR("Failed to init connector\n"); > - goto out_unregister; > - } > - > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > - &vkms_device->pipe, > - NULL, > - vkms_formats, > - ARRAY_SIZE(vkms_formats), > - NULL, > - &vkms_device->connector); > - if (ret < 0) { > - DRM_ERROR("Cannot setup simple display pipe\n"); > + ret = vkms_modeset_init(vkms_device); > + if (ret) > goto out_unregister; > - } > > ret = drm_dev_register(&vkms_device->drm, 0); > if (ret) > goto out_unregister; > > - drm_connector_register(&vkms_device->connector); > - > return 0; > > out_unregister: > platform_device_unregister(vkms_device->platform); > + > out_fini: > drm_dev_fini(&vkms_device->drm); > + > out_free: > kfree(vkms_device); > - > return ret; > } > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index c77c5bf5032a..b0f9d2e61a42 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -1,13 +1,31 @@ > #ifndef _VKMS_DRV_H_ > #define _VKMS_DRV_H_ > > -#include <drm/drm_simple_kms_helper.h> > +#include <drm/drmP.h> > +#include <drm/drm.h> > +#include <drm/drm_encoder.h> > + > +static const u32 vkms_formats[] = { > + DRM_FORMAT_XRGB8888, > +}; > + > +struct vkms_output { > + struct drm_crtc crtc; > + struct drm_encoder encoder; > + struct drm_connector connector; > +}; > > struct vkms_device { > struct drm_device drm; > struct platform_device *platform; > - struct drm_simple_display_pipe pipe; > - struct drm_connector connector; > + struct vkms_output output; > }; > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > + struct drm_plane *primary, struct drm_plane *cursor); > + > +int vkms_output_init(struct vkms_device *vkmsdev); > + > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > + > #endif /* _VKMS_DRV_H_ */ > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > new file mode 100644 > index 000000000000..48143eac3c12 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include "vkms_drv.h" > +#include <drm/drm_crtc_helper.h> > + > +static void vkms_connector_destroy(struct drm_connector *connector) > +{ > + drm_connector_unregister(connector); > + drm_connector_cleanup(connector); > +} > + > +static const struct drm_connector_funcs vkms_connector_funcs = { > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = vkms_connector_destroy, 2) adding the following hooks with their drm_atomic_helper_connector_* counterpart to the vkms_connector_funcs: atomic_duplicate_state, atomic_destroy_state, and reset. > +}; > + > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +int vkms_output_init(struct vkms_device *vkmsdev) > +{ > + struct vkms_output *output = &vkmsdev->output; > + struct drm_device *dev = &vkmsdev->drm; > + struct drm_connector *connector = &output->connector; > + struct drm_encoder *encoder = &output->encoder; > + struct drm_crtc *crtc = &output->crtc; > + struct drm_plane *primary; > + int ret; > + > + primary = vkms_plane_init(vkmsdev); > + if (IS_ERR(primary)) > + return PTR_ERR(primary); > + > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > + if (ret) > + goto err_crtc; > + 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > + DRM_MODE_CONNECTOR_VIRTUAL); > + if (ret) { > + DRM_ERROR("Failed to init connector\n"); > + goto err_connector; > + } > + > + ret = drm_connector_register(connector); > + if (ret) { > + DRM_ERROR("Failed to register connector\n"); > + goto err_connector_register; > + } > + > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > + DRM_MODE_ENCODER_VIRTUAL, NULL); > + if (ret) { > + DRM_ERROR("Failed to init encoder\n"); > + goto err_encoder; > + } > + encoder->possible_crtcs = 1; > + > + ret = drm_mode_connector_attach_encoder(connector, encoder); > + if (ret) { > + DRM_ERROR("Failed to attach connector to encoder\n"); > + goto err_attach; > + } > + > + drm_mode_config_reset(dev); > + > + return 0; > + > +err_attach: > + drm_encoder_cleanup(encoder); > + > +err_encoder: > + drm_connector_unregister(connector); > + > +err_connector_register: > + drm_connector_cleanup(connector); > + > +err_connector: > + drm_crtc_cleanup(crtc); > + > +err_crtc: > + drm_plane_cleanup(primary); > + return ret; > +} > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > new file mode 100644 > index 000000000000..2c25b1d6ab5b > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -0,0 +1,46 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include "vkms_drv.h" > +#include <drm/drm_plane_helper.h> > +#include <drm/drm_atomic_helper.h> > + > +static const struct drm_plane_funcs vkms_plane_funcs = { > + .update_plane = drm_atomic_helper_update_plane, > + .disable_plane = drm_atomic_helper_disable_plane, > + .destroy = drm_plane_cleanup, > + .reset = drm_atomic_helper_plane_reset, > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > +}; > + > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > +{ > + struct drm_device *dev = &vkmsdev->drm; > + struct drm_plane *plane; > + const u32 *formats; > + int ret, nformats; > + > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > + if (!plane) > + return ERR_PTR(-ENOMEM); > + > + formats = vkms_formats; > + nformats = ARRAY_SIZE(vkms_formats); > + > + ret = drm_universal_plane_init(dev, plane, 0, > + &vkms_plane_funcs, > + formats, nformats, > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > + if (ret) { > + kfree(plane); > + return ERR_PTR(ret); > + } > + 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}. > + return plane; > +} > -- > 2.17.0 > - Haneen
Hi Haneen, Thanks for the feedback :) On 05/20, Haneen Mohammed wrote: > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > > This commit adds the essential infrastructure for around CRTCs which > > is composed of: a new data struct for output data information, a > > function for creating planes, and a simple encoder attached to the > > connector. Finally, due to the introduction of a new initialization > > function, connectors were moved from vkms_drv.c to vkms_display.c. > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > --- > > drivers/gpu/drm/vkms/Makefile | 2 +- > > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > > drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ > > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > > 6 files changed, 211 insertions(+), 47 deletions(-) > > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > index 2aef948d3a34..3f774a6a9c58 100644 > > --- a/drivers/gpu/drm/vkms/Makefile > > +++ b/drivers/gpu/drm/vkms/Makefile > > @@ -1,3 +1,3 @@ > > -vkms-y := vkms_drv.o > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > > new file mode 100644 > > index 000000000000..bf76cd39ece7 > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > @@ -0,0 +1,35 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_atomic_helper.h> > > +#include <drm/drm_crtc_helper.h> > > + > > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > > + .set_config = drm_atomic_helper_set_config, > > + .destroy = drm_crtc_cleanup, > > + .page_flip = drm_atomic_helper_page_flip, > > + .reset = drm_atomic_helper_crtc_reset, > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > +}; > > + > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > + struct drm_plane *primary, struct drm_plane *cursor) > > +{ > > + int ret; > > + > > Just as a follow up, I have noticed that vkms breaks when inspecting > its state in the debugfs, also when running igt tests with kms and core > filters, and adding the following fixed that issue: Could you explain me how you tested the state with debugfs? I will add the fixes suggested in your comments. Thanks > 1) a drm_crtc_helper_funcs with dummy atomic_check. > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > > + &vkms_crtc_funcs, NULL); > > + if (ret) { > > + DRM_ERROR("Failed to init CRTC\n"); > > + return ret; > > + } > > + > > + return ret; > > +} > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index aec3f180f96d..070613e32934 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -6,7 +6,6 @@ > > */ > > > > #include <linux/module.h> > > -#include <drm/drmP.h> > > #include <drm/drm_gem.h> > > #include <drm/drm_crtc_helper.h> > > #include <drm/drm_atomic_helper.h> > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > > .minor = DRIVER_MINOR, > > }; > > > > -static const u32 vkms_formats[] = { > > - DRM_FORMAT_XRGB8888, > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > + .atomic_check = drm_atomic_helper_check, > > + .atomic_commit = drm_atomic_helper_commit, > > }; > > > > -static void vkms_connector_destroy(struct drm_connector *connector) > > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > > { > > - drm_connector_unregister(connector); > > - drm_connector_cleanup(connector); > > -} > > + struct drm_device *dev = &vkmsdev->drm; > > > > -static const struct drm_connector_funcs vkms_connector_funcs = { > > - .fill_modes = drm_helper_probe_single_connector_modes, > > - .destroy = vkms_connector_destroy, > > -}; > > + drm_mode_config_init(dev); > > + dev->mode_config.funcs = &vkms_mode_funcs; > > + dev->mode_config.min_width = XRES_MIN; > > + dev->mode_config.min_height = YRES_MIN; > > + dev->mode_config.max_width = XRES_MAX; > > + dev->mode_config.max_height = YRES_MAX; > > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > > - .atomic_check = drm_atomic_helper_check, > > - .atomic_commit = drm_atomic_helper_commit, > > -}; > > + return vkms_output_init(vkmsdev); > > +} > > > > static int __init vkms_init(void) > > { > > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > > goto out_fini; > > } > > > > - drm_mode_config_init(&vkms_device->drm); > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > > - vkms_device->drm.mode_config.min_width = XRES_MIN; > > - vkms_device->drm.mode_config.min_height = YRES_MIN; > > - vkms_device->drm.mode_config.max_width = XRES_MAX; > > - vkms_device->drm.mode_config.max_height = YRES_MAX; > > - > > - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, > > - &vkms_connector_funcs, > > - DRM_MODE_CONNECTOR_VIRTUAL); > > - if (ret < 0) { > > - DRM_ERROR("Failed to init connector\n"); > > - goto out_unregister; > > - } > > - > > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > > - &vkms_device->pipe, > > - NULL, > > - vkms_formats, > > - ARRAY_SIZE(vkms_formats), > > - NULL, > > - &vkms_device->connector); > > - if (ret < 0) { > > - DRM_ERROR("Cannot setup simple display pipe\n"); > > + ret = vkms_modeset_init(vkms_device); > > + if (ret) > > goto out_unregister; > > - } > > > > ret = drm_dev_register(&vkms_device->drm, 0); > > if (ret) > > goto out_unregister; > > > > - drm_connector_register(&vkms_device->connector); > > - > > return 0; > > > > out_unregister: > > platform_device_unregister(vkms_device->platform); > > + > > out_fini: > > drm_dev_fini(&vkms_device->drm); > > + > > out_free: > > kfree(vkms_device); > > - > > return ret; > > } > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > index c77c5bf5032a..b0f9d2e61a42 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > @@ -1,13 +1,31 @@ > > #ifndef _VKMS_DRV_H_ > > #define _VKMS_DRV_H_ > > > > -#include <drm/drm_simple_kms_helper.h> > > +#include <drm/drmP.h> > > +#include <drm/drm.h> > > +#include <drm/drm_encoder.h> > > + > > +static const u32 vkms_formats[] = { > > + DRM_FORMAT_XRGB8888, > > +}; > > + > > +struct vkms_output { > > + struct drm_crtc crtc; > > + struct drm_encoder encoder; > > + struct drm_connector connector; > > +}; > > > > struct vkms_device { > > struct drm_device drm; > > struct platform_device *platform; > > - struct drm_simple_display_pipe pipe; > > - struct drm_connector connector; > > + struct vkms_output output; > > }; > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > + struct drm_plane *primary, struct drm_plane *cursor); > > + > > +int vkms_output_init(struct vkms_device *vkmsdev); > > + > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > > + > > #endif /* _VKMS_DRV_H_ */ > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > new file mode 100644 > > index 000000000000..48143eac3c12 > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_crtc_helper.h> > > + > > +static void vkms_connector_destroy(struct drm_connector *connector) > > +{ > > + drm_connector_unregister(connector); > > + drm_connector_cleanup(connector); > > +} > > + > > +static const struct drm_connector_funcs vkms_connector_funcs = { > > + .fill_modes = drm_helper_probe_single_connector_modes, > > + .destroy = vkms_connector_destroy, > > 2) adding the following hooks with their drm_atomic_helper_connector_* > counterpart to the vkms_connector_funcs: > atomic_duplicate_state, atomic_destroy_state, and reset. > > > +}; > > + > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > > + .destroy = drm_encoder_cleanup, > > +}; > > + > > +int vkms_output_init(struct vkms_device *vkmsdev) > > +{ > > + struct vkms_output *output = &vkmsdev->output; > > + struct drm_device *dev = &vkmsdev->drm; > > + struct drm_connector *connector = &output->connector; > > + struct drm_encoder *encoder = &output->encoder; > > + struct drm_crtc *crtc = &output->crtc; > > + struct drm_plane *primary; > > + int ret; > > + > > + primary = vkms_plane_init(vkmsdev); > > + if (IS_ERR(primary)) > > + return PTR_ERR(primary); > > + > > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > > + if (ret) > > + goto err_crtc; > > + > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > > + DRM_MODE_CONNECTOR_VIRTUAL); > > + if (ret) { > > + DRM_ERROR("Failed to init connector\n"); > > + goto err_connector; > > + } > > + > > + ret = drm_connector_register(connector); > > + if (ret) { > > + DRM_ERROR("Failed to register connector\n"); > > + goto err_connector_register; > > + } > > + > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > > + DRM_MODE_ENCODER_VIRTUAL, NULL); > > + if (ret) { > > + DRM_ERROR("Failed to init encoder\n"); > > + goto err_encoder; > > + } > > + encoder->possible_crtcs = 1; > > + > > + ret = drm_mode_connector_attach_encoder(connector, encoder); > > + if (ret) { > > + DRM_ERROR("Failed to attach connector to encoder\n"); > > + goto err_attach; > > + } > > + > > + drm_mode_config_reset(dev); > > + > > + return 0; > > + > > +err_attach: > > + drm_encoder_cleanup(encoder); > > + > > +err_encoder: > > + drm_connector_unregister(connector); > > + > > +err_connector_register: > > + drm_connector_cleanup(connector); > > + > > +err_connector: > > + drm_crtc_cleanup(crtc); > > + > > +err_crtc: > > + drm_plane_cleanup(primary); > > + return ret; > > +} > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > new file mode 100644 > > index 000000000000..2c25b1d6ab5b > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -0,0 +1,46 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_plane_helper.h> > > +#include <drm/drm_atomic_helper.h> > > + > > +static const struct drm_plane_funcs vkms_plane_funcs = { > > + .update_plane = drm_atomic_helper_update_plane, > > + .disable_plane = drm_atomic_helper_disable_plane, > > + .destroy = drm_plane_cleanup, > > + .reset = drm_atomic_helper_plane_reset, > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > > +}; > > + > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > > +{ > > + struct drm_device *dev = &vkmsdev->drm; > > + struct drm_plane *plane; > > + const u32 *formats; > > + int ret, nformats; > > + > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > > + if (!plane) > > + return ERR_PTR(-ENOMEM); > > + > > + formats = vkms_formats; > > + nformats = ARRAY_SIZE(vkms_formats); > > + > > + ret = drm_universal_plane_init(dev, plane, 0, > > + &vkms_plane_funcs, > > + formats, nformats, > > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > > + if (ret) { > > + kfree(plane); > > + return ERR_PTR(ret); > > + } > > + > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}. > > > + return plane; > > +} > > -- > > 2.17.0 > > > > - Haneen
On Sun, May 20, 2018 at 11:28:37AM -0300, Rodrigo Siqueira wrote: > Hi Haneen, > > Thanks for the feedback :) > > On 05/20, Haneen Mohammed wrote: > > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > > > This commit adds the essential infrastructure for around CRTCs which > > > is composed of: a new data struct for output data information, a > > > function for creating planes, and a simple encoder attached to the > > > connector. Finally, due to the introduction of a new initialization > > > function, connectors were moved from vkms_drv.c to vkms_display.c. > > > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > --- > > > drivers/gpu/drm/vkms/Makefile | 2 +- > > > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > > > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > > > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > > > drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > > > 6 files changed, 211 insertions(+), 47 deletions(-) > > > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > > > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > > > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > > index 2aef948d3a34..3f774a6a9c58 100644 > > > --- a/drivers/gpu/drm/vkms/Makefile > > > +++ b/drivers/gpu/drm/vkms/Makefile > > > @@ -1,3 +1,3 @@ > > > -vkms-y := vkms_drv.o > > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > > > new file mode 100644 > > > index 000000000000..bf76cd39ece7 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > > @@ -0,0 +1,35 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_atomic_helper.h> > > > +#include <drm/drm_crtc_helper.h> > > > + > > > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > > > + .set_config = drm_atomic_helper_set_config, > > > + .destroy = drm_crtc_cleanup, > > > + .page_flip = drm_atomic_helper_page_flip, > > > + .reset = drm_atomic_helper_crtc_reset, > > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > > +}; > > > + > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > + struct drm_plane *primary, struct drm_plane *cursor) > > > +{ > > > + int ret; > > > + > > > > Just as a follow up, I have noticed that vkms breaks when inspecting > > its state in the debugfs, also when running igt tests with kms and core > > filters, and adding the following fixed that issue: > > Could you explain me how you tested the state with debugfs? > > I will add the fixes suggested in your comments. > > Thanks > I just tried to read this file: /sys/kernel/debug/dri/0/state, which then caused a null pointer dereference error. > > 1) a drm_crtc_helper_funcs with dummy atomic_check. > > > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > > > + &vkms_crtc_funcs, NULL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init CRTC\n"); > > > + return ret; > > > + } > > > + > > > + return ret; > > > +} > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > index aec3f180f96d..070613e32934 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -6,7 +6,6 @@ > > > */ > > > > > > #include <linux/module.h> > > > -#include <drm/drmP.h> > > > #include <drm/drm_gem.h> > > > #include <drm/drm_crtc_helper.h> > > > #include <drm/drm_atomic_helper.h> > > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > > > .minor = DRIVER_MINOR, > > > }; > > > > > > -static const u32 vkms_formats[] = { > > > - DRM_FORMAT_XRGB8888, > > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > + .atomic_check = drm_atomic_helper_check, > > > + .atomic_commit = drm_atomic_helper_commit, > > > }; > > > > > > -static void vkms_connector_destroy(struct drm_connector *connector) > > > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > { > > > - drm_connector_unregister(connector); > > > - drm_connector_cleanup(connector); > > > -} > > > + struct drm_device *dev = &vkmsdev->drm; > > > > > > -static const struct drm_connector_funcs vkms_connector_funcs = { > > > - .fill_modes = drm_helper_probe_single_connector_modes, > > > - .destroy = vkms_connector_destroy, > > > -}; > > > + drm_mode_config_init(dev); > > > + dev->mode_config.funcs = &vkms_mode_funcs; > > > + dev->mode_config.min_width = XRES_MIN; > > > + dev->mode_config.min_height = YRES_MIN; > > > + dev->mode_config.max_width = XRES_MAX; > > > + dev->mode_config.max_height = YRES_MAX; > > > > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > - .atomic_check = drm_atomic_helper_check, > > > - .atomic_commit = drm_atomic_helper_commit, > > > -}; > > > + return vkms_output_init(vkmsdev); > > > +} > > > > > > static int __init vkms_init(void) > > > { > > > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > > > goto out_fini; > > > } > > > > > > - drm_mode_config_init(&vkms_device->drm); > > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > > > - vkms_device->drm.mode_config.min_width = XRES_MIN; > > > - vkms_device->drm.mode_config.min_height = YRES_MIN; > > > - vkms_device->drm.mode_config.max_width = XRES_MAX; > > > - vkms_device->drm.mode_config.max_height = YRES_MAX; > > > - > > > - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, > > > - &vkms_connector_funcs, > > > - DRM_MODE_CONNECTOR_VIRTUAL); > > > - if (ret < 0) { > > > - DRM_ERROR("Failed to init connector\n"); > > > - goto out_unregister; > > > - } > > > - > > > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > > > - &vkms_device->pipe, > > > - NULL, > > > - vkms_formats, > > > - ARRAY_SIZE(vkms_formats), > > > - NULL, > > > - &vkms_device->connector); > > > - if (ret < 0) { > > > - DRM_ERROR("Cannot setup simple display pipe\n"); > > > + ret = vkms_modeset_init(vkms_device); > > > + if (ret) > > > goto out_unregister; > > > - } > > > > > > ret = drm_dev_register(&vkms_device->drm, 0); > > > if (ret) > > > goto out_unregister; > > > > > > - drm_connector_register(&vkms_device->connector); > > > - > > > return 0; > > > > > > out_unregister: > > > platform_device_unregister(vkms_device->platform); > > > + > > > out_fini: > > > drm_dev_fini(&vkms_device->drm); > > > + > > > out_free: > > > kfree(vkms_device); > > > - > > > return ret; > > > } > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > > index c77c5bf5032a..b0f9d2e61a42 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > > @@ -1,13 +1,31 @@ > > > #ifndef _VKMS_DRV_H_ > > > #define _VKMS_DRV_H_ > > > > > > -#include <drm/drm_simple_kms_helper.h> > > > +#include <drm/drmP.h> > > > +#include <drm/drm.h> > > > +#include <drm/drm_encoder.h> > > > + > > > +static const u32 vkms_formats[] = { > > > + DRM_FORMAT_XRGB8888, > > > +}; > > > + > > > +struct vkms_output { > > > + struct drm_crtc crtc; > > > + struct drm_encoder encoder; > > > + struct drm_connector connector; > > > +}; > > > > > > struct vkms_device { > > > struct drm_device drm; > > > struct platform_device *platform; > > > - struct drm_simple_display_pipe pipe; > > > - struct drm_connector connector; > > > + struct vkms_output output; > > > }; > > > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > + struct drm_plane *primary, struct drm_plane *cursor); > > > + > > > +int vkms_output_init(struct vkms_device *vkmsdev); > > > + > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > > > + > > > #endif /* _VKMS_DRV_H_ */ > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > > new file mode 100644 > > > index 000000000000..48143eac3c12 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > > @@ -0,0 +1,91 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_crtc_helper.h> > > > + > > > +static void vkms_connector_destroy(struct drm_connector *connector) > > > +{ > > > + drm_connector_unregister(connector); > > > + drm_connector_cleanup(connector); > > > +} > > > + > > > +static const struct drm_connector_funcs vkms_connector_funcs = { > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > + .destroy = vkms_connector_destroy, > > > > 2) adding the following hooks with their drm_atomic_helper_connector_* > > counterpart to the vkms_connector_funcs: > > atomic_duplicate_state, atomic_destroy_state, and reset. > > > > > +}; > > > + > > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > > > + .destroy = drm_encoder_cleanup, > > > +}; > > > + > > > +int vkms_output_init(struct vkms_device *vkmsdev) > > > +{ > > > + struct vkms_output *output = &vkmsdev->output; > > > + struct drm_device *dev = &vkmsdev->drm; > > > + struct drm_connector *connector = &output->connector; > > > + struct drm_encoder *encoder = &output->encoder; > > > + struct drm_crtc *crtc = &output->crtc; > > > + struct drm_plane *primary; > > > + int ret; > > > + > > > + primary = vkms_plane_init(vkmsdev); > > > + if (IS_ERR(primary)) > > > + return PTR_ERR(primary); > > > + > > > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > > > + if (ret) > > > + goto err_crtc; > > > + > > > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > > > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > > > + DRM_MODE_CONNECTOR_VIRTUAL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init connector\n"); > > > + goto err_connector; > > > + } > > > + > > > + ret = drm_connector_register(connector); > > > + if (ret) { > > > + DRM_ERROR("Failed to register connector\n"); > > > + goto err_connector_register; > > > + } > > > + > > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > > > + DRM_MODE_ENCODER_VIRTUAL, NULL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init encoder\n"); > > > + goto err_encoder; > > > + } > > > + encoder->possible_crtcs = 1; > > > + > > > + ret = drm_mode_connector_attach_encoder(connector, encoder); > > > + if (ret) { > > > + DRM_ERROR("Failed to attach connector to encoder\n"); > > > + goto err_attach; > > > + } > > > + > > > + drm_mode_config_reset(dev); > > > + > > > + return 0; > > > + > > > +err_attach: > > > + drm_encoder_cleanup(encoder); > > > + > > > +err_encoder: > > > + drm_connector_unregister(connector); > > > + > > > +err_connector_register: > > > + drm_connector_cleanup(connector); > > > + > > > +err_connector: > > > + drm_crtc_cleanup(crtc); > > > + > > > +err_crtc: > > > + drm_plane_cleanup(primary); > > > + return ret; > > > +} > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > > new file mode 100644 > > > index 000000000000..2c25b1d6ab5b > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > @@ -0,0 +1,46 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_plane_helper.h> > > > +#include <drm/drm_atomic_helper.h> > > > + > > > +static const struct drm_plane_funcs vkms_plane_funcs = { > > > + .update_plane = drm_atomic_helper_update_plane, > > > + .disable_plane = drm_atomic_helper_disable_plane, > > > + .destroy = drm_plane_cleanup, > > > + .reset = drm_atomic_helper_plane_reset, > > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > > > +}; > > > + > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > > > +{ > > > + struct drm_device *dev = &vkmsdev->drm; > > > + struct drm_plane *plane; > > > + const u32 *formats; > > > + int ret, nformats; > > > + > > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > > > + if (!plane) > > > + return ERR_PTR(-ENOMEM); > > > + > > > + formats = vkms_formats; > > > + nformats = ARRAY_SIZE(vkms_formats); > > > + > > > + ret = drm_universal_plane_init(dev, plane, 0, > > > + &vkms_plane_funcs, > > > + formats, nformats, > > > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > > > + if (ret) { > > > + kfree(plane); > > > + return ERR_PTR(ret); > > > + } > > > + > > > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}. > > > > > + return plane; > > > +} > > > -- > > > 2.17.0 > > > > > > > - Haneen
Ahhhh, I got it! Thanks a lot Haneen :) On Sun, May 20, 2018 at 1:44 PM, Haneen Mohammed <hamohammed.sa@gmail.com> wrote: > On Sun, May 20, 2018 at 11:28:37AM -0300, Rodrigo Siqueira wrote: > > Hi Haneen, > > > > Thanks for the feedback :) > > > > On 05/20, Haneen Mohammed wrote: > > > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > > > > This commit adds the essential infrastructure for around CRTCs which > > > > is composed of: a new data struct for output data information, a > > > > function for creating planes, and a simple encoder attached to the > > > > connector. Finally, due to the introduction of a new initialization > > > > function, connectors were moved from vkms_drv.c to vkms_display.c. > > > > > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > --- > > > > drivers/gpu/drm/vkms/Makefile | 2 +- > > > > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > > > > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > > > > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > > > > drivers/gpu/drm/vkms/vkms_output.c | 91 > ++++++++++++++++++++++++++++++ > > > > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > > > > 6 files changed, 211 insertions(+), 47 deletions(-) > > > > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > > > > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > > > > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > > > > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/ > Makefile > > > > index 2aef948d3a34..3f774a6a9c58 100644 > > > > --- a/drivers/gpu/drm/vkms/Makefile > > > > +++ b/drivers/gpu/drm/vkms/Makefile > > > > @@ -1,3 +1,3 @@ > > > > -vkms-y := vkms_drv.o > > > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > > > > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c > b/drivers/gpu/drm/vkms/vkms_crtc.c > > > > new file mode 100644 > > > > index 000000000000..bf76cd39ece7 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > > > @@ -0,0 +1,35 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * This program is free software; you can redistribute it and/or > modify > > > > + * it under the terms of the GNU General Public License as > published by > > > > + * the Free Software Foundation; either version 2 of the License, or > > > > + * (at your option) any later version. > > > > + */ > > > > + > > > > +#include "vkms_drv.h" > > > > +#include <drm/drm_atomic_helper.h> > > > > +#include <drm/drm_crtc_helper.h> > > > > + > > > > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > > > > + .set_config = drm_atomic_helper_set_config, > > > > + .destroy = drm_crtc_cleanup, > > > > + .page_flip = drm_atomic_helper_page_flip, > > > > + .reset = drm_atomic_helper_crtc_reset, > > > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > > > > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > > > +}; > > > > + > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > > + struct drm_plane *primary, struct drm_plane *cursor) > > > > +{ > > > > + int ret; > > > > + > > > > > > Just as a follow up, I have noticed that vkms breaks when inspecting > > > its state in the debugfs, also when running igt tests with kms and core > > > filters, and adding the following fixed that issue: > > > > Could you explain me how you tested the state with debugfs? > > > > I will add the fixes suggested in your comments. > > > > Thanks > > > > I just tried to read this file: /sys/kernel/debug/dri/0/state, which > then caused a null pointer dereference error. > > > > 1) a drm_crtc_helper_funcs with dummy atomic_check. > > > > > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > > > > + &vkms_crtc_funcs, NULL); > > > > + if (ret) { > > > > + DRM_ERROR("Failed to init CRTC\n"); > > > > + return ret; > > > > + } > > > > + > > > > + return ret; > > > > +} > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c > b/drivers/gpu/drm/vkms/vkms_drv.c > > > > index aec3f180f96d..070613e32934 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > > @@ -6,7 +6,6 @@ > > > > */ > > > > > > > > #include <linux/module.h> > > > > -#include <drm/drmP.h> > > > > #include <drm/drm_gem.h> > > > > #include <drm/drm_crtc_helper.h> > > > > #include <drm/drm_atomic_helper.h> > > > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > > > > .minor = DRIVER_MINOR, > > > > }; > > > > > > > > -static const u32 vkms_formats[] = { > > > > - DRM_FORMAT_XRGB8888, > > > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > > + .atomic_check = drm_atomic_helper_check, > > > > + .atomic_commit = drm_atomic_helper_commit, > > > > }; > > > > > > > > -static void vkms_connector_destroy(struct drm_connector *connector) > > > > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > > { > > > > - drm_connector_unregister(connector); > > > > - drm_connector_cleanup(connector); > > > > -} > > > > + struct drm_device *dev = &vkmsdev->drm; > > > > > > > > -static const struct drm_connector_funcs vkms_connector_funcs = { > > > > - .fill_modes = drm_helper_probe_single_connector_modes, > > > > - .destroy = vkms_connector_destroy, > > > > -}; > > > > + drm_mode_config_init(dev); > > > > + dev->mode_config.funcs = &vkms_mode_funcs; > > > > + dev->mode_config.min_width = XRES_MIN; > > > > + dev->mode_config.min_height = YRES_MIN; > > > > + dev->mode_config.max_width = XRES_MAX; > > > > + dev->mode_config.max_height = YRES_MAX; > > > > > > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > > - .atomic_check = drm_atomic_helper_check, > > > > - .atomic_commit = drm_atomic_helper_commit, > > > > -}; > > > > + return vkms_output_init(vkmsdev); > > > > +} > > > > > > > > static int __init vkms_init(void) > > > > { > > > > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > > > > goto out_fini; > > > > } > > > > > > > > - drm_mode_config_init(&vkms_device->drm); > > > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > > > > - vkms_device->drm.mode_config.min_width = XRES_MIN; > > > > - vkms_device->drm.mode_config.min_height = YRES_MIN; > > > > - vkms_device->drm.mode_config.max_width = XRES_MAX; > > > > - vkms_device->drm.mode_config.max_height = YRES_MAX; > > > > - > > > > - ret = drm_connector_init(&vkms_device->drm, > &vkms_device->connector, > > > > - &vkms_connector_funcs, > > > > - DRM_MODE_CONNECTOR_VIRTUAL); > > > > - if (ret < 0) { > > > > - DRM_ERROR("Failed to init connector\n"); > > > > - goto out_unregister; > > > > - } > > > > - > > > > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > > > > - &vkms_device->pipe, > > > > - NULL, > > > > - vkms_formats, > > > > - ARRAY_SIZE(vkms_formats), > > > > - NULL, > > > > - &vkms_device->connector); > > > > - if (ret < 0) { > > > > - DRM_ERROR("Cannot setup simple display pipe\n"); > > > > + ret = vkms_modeset_init(vkms_device); > > > > + if (ret) > > > > goto out_unregister; > > > > - } > > > > > > > > ret = drm_dev_register(&vkms_device->drm, 0); > > > > if (ret) > > > > goto out_unregister; > > > > > > > > - drm_connector_register(&vkms_device->connector); > > > > - > > > > return 0; > > > > > > > > out_unregister: > > > > platform_device_unregister(vkms_device->platform); > > > > + > > > > out_fini: > > > > drm_dev_fini(&vkms_device->drm); > > > > + > > > > out_free: > > > > kfree(vkms_device); > > > > - > > > > return ret; > > > > } > > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h > b/drivers/gpu/drm/vkms/vkms_drv.h > > > > index c77c5bf5032a..b0f9d2e61a42 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > > > @@ -1,13 +1,31 @@ > > > > #ifndef _VKMS_DRV_H_ > > > > #define _VKMS_DRV_H_ > > > > > > > > -#include <drm/drm_simple_kms_helper.h> > > > > +#include <drm/drmP.h> > > > > +#include <drm/drm.h> > > > > +#include <drm/drm_encoder.h> > > > > + > > > > +static const u32 vkms_formats[] = { > > > > + DRM_FORMAT_XRGB8888, > > > > +}; > > > > + > > > > +struct vkms_output { > > > > + struct drm_crtc crtc; > > > > + struct drm_encoder encoder; > > > > + struct drm_connector connector; > > > > +}; > > > > > > > > struct vkms_device { > > > > struct drm_device drm; > > > > struct platform_device *platform; > > > > - struct drm_simple_display_pipe pipe; > > > > - struct drm_connector connector; > > > > + struct vkms_output output; > > > > }; > > > > > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > > + struct drm_plane *primary, struct drm_plane *cursor); > > > > + > > > > +int vkms_output_init(struct vkms_device *vkmsdev); > > > > + > > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > > > > + > > > > #endif /* _VKMS_DRV_H_ */ > > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c > b/drivers/gpu/drm/vkms/vkms_output.c > > > > new file mode 100644 > > > > index 000000000000..48143eac3c12 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > > > @@ -0,0 +1,91 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * This program is free software; you can redistribute it and/or > modify > > > > + * it under the terms of the GNU General Public License as > published by > > > > + * the Free Software Foundation; either version 2 of the License, or > > > > + * (at your option) any later version. > > > > + */ > > > > + > > > > +#include "vkms_drv.h" > > > > +#include <drm/drm_crtc_helper.h> > > > > + > > > > +static void vkms_connector_destroy(struct drm_connector *connector) > > > > +{ > > > > + drm_connector_unregister(connector); > > > > + drm_connector_cleanup(connector); > > > > +} > > > > + > > > > +static const struct drm_connector_funcs vkms_connector_funcs = { > > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > > + .destroy = vkms_connector_destroy, > > > > > > 2) adding the following hooks with their drm_atomic_helper_connector_* > > > counterpart to the vkms_connector_funcs: > > > atomic_duplicate_state, atomic_destroy_state, and reset. > > > > > > > +}; > > > > + > > > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > > > > + .destroy = drm_encoder_cleanup, > > > > +}; > > > > + > > > > +int vkms_output_init(struct vkms_device *vkmsdev) > > > > +{ > > > > + struct vkms_output *output = &vkmsdev->output; > > > > + struct drm_device *dev = &vkmsdev->drm; > > > > + struct drm_connector *connector = &output->connector; > > > > + struct drm_encoder *encoder = &output->encoder; > > > > + struct drm_crtc *crtc = &output->crtc; > > > > + struct drm_plane *primary; > > > > + int ret; > > > > + > > > > + primary = vkms_plane_init(vkmsdev); > > > > + if (IS_ERR(primary)) > > > > + return PTR_ERR(primary); > > > > + > > > > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > > > > + if (ret) > > > > + goto err_crtc; > > > > + > > > > > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > > > > > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > > > > + DRM_MODE_CONNECTOR_VIRTUAL); > > > > + if (ret) { > > > > + DRM_ERROR("Failed to init connector\n"); > > > > + goto err_connector; > > > > + } > > > > + > > > > + ret = drm_connector_register(connector); > > > > + if (ret) { > > > > + DRM_ERROR("Failed to register connector\n"); > > > > + goto err_connector_register; > > > > + } > > > > + > > > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > > > > + DRM_MODE_ENCODER_VIRTUAL, NULL); > > > > + if (ret) { > > > > + DRM_ERROR("Failed to init encoder\n"); > > > > + goto err_encoder; > > > > + } > > > > + encoder->possible_crtcs = 1; > > > > + > > > > + ret = drm_mode_connector_attach_encoder(connector, encoder); > > > > + if (ret) { > > > > + DRM_ERROR("Failed to attach connector to encoder\n"); > > > > + goto err_attach; > > > > + } > > > > + > > > > + drm_mode_config_reset(dev); > > > > + > > > > + return 0; > > > > + > > > > +err_attach: > > > > + drm_encoder_cleanup(encoder); > > > > + > > > > +err_encoder: > > > > + drm_connector_unregister(connector); > > > > + > > > > +err_connector_register: > > > > + drm_connector_cleanup(connector); > > > > + > > > > +err_connector: > > > > + drm_crtc_cleanup(crtc); > > > > + > > > > +err_crtc: > > > > + drm_plane_cleanup(primary); > > > > + return ret; > > > > +} > > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c > b/drivers/gpu/drm/vkms/vkms_plane.c > > > > new file mode 100644 > > > > index 000000000000..2c25b1d6ab5b > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > > @@ -0,0 +1,46 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * This program is free software; you can redistribute it and/or > modify > > > > + * it under the terms of the GNU General Public License as > published by > > > > + * the Free Software Foundation; either version 2 of the License, or > > > > + * (at your option) any later version. > > > > + */ > > > > + > > > > +#include "vkms_drv.h" > > > > +#include <drm/drm_plane_helper.h> > > > > +#include <drm/drm_atomic_helper.h> > > > > + > > > > +static const struct drm_plane_funcs vkms_plane_funcs = { > > > > + .update_plane = drm_atomic_helper_update_plane, > > > > + .disable_plane = drm_atomic_helper_disable_plane, > > > > + .destroy = drm_plane_cleanup, > > > > + .reset = drm_atomic_helper_plane_reset, > > > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > > > > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > > > > +}; > > > > + > > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > > > > +{ > > > > + struct drm_device *dev = &vkmsdev->drm; > > > > + struct drm_plane *plane; > > > > + const u32 *formats; > > > > + int ret, nformats; > > > > + > > > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > > > > + if (!plane) > > > > + return ERR_PTR(-ENOMEM); > > > > + > > > > + formats = vkms_formats; > > > > + nformats = ARRAY_SIZE(vkms_formats); > > > > + > > > > + ret = drm_universal_plane_init(dev, plane, 0, > > > > + &vkms_plane_funcs, > > > > + formats, nformats, > > > > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > > > > + if (ret) { > > > > + kfree(plane); > > > > + return ERR_PTR(ret); > > > > + } > > > > + > > > > > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, > cleanup_fb}. > > > > > > > + return plane; > > > > +} > > > > -- > > > > 2.17.0 > > > > > > > > > > - Haneen >
On Sun, May 20, 2018 at 09:22:31AM +0300, Haneen Mohammed wrote: > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > > This commit adds the essential infrastructure for around CRTCs which > > is composed of: a new data struct for output data information, a > > function for creating planes, and a simple encoder attached to the > > connector. Finally, due to the introduction of a new initialization > > function, connectors were moved from vkms_drv.c to vkms_display.c. > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > --- > > drivers/gpu/drm/vkms/Makefile | 2 +- > > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > > drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ > > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > > 6 files changed, 211 insertions(+), 47 deletions(-) > > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > index 2aef948d3a34..3f774a6a9c58 100644 > > --- a/drivers/gpu/drm/vkms/Makefile > > +++ b/drivers/gpu/drm/vkms/Makefile > > @@ -1,3 +1,3 @@ > > -vkms-y := vkms_drv.o > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > > new file mode 100644 > > index 000000000000..bf76cd39ece7 > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > @@ -0,0 +1,35 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_atomic_helper.h> > > +#include <drm/drm_crtc_helper.h> > > + > > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > > + .set_config = drm_atomic_helper_set_config, > > + .destroy = drm_crtc_cleanup, > > + .page_flip = drm_atomic_helper_page_flip, > > + .reset = drm_atomic_helper_crtc_reset, > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > +}; > > + > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > + struct drm_plane *primary, struct drm_plane *cursor) > > +{ > > + int ret; > > + > > Just as a follow up, I have noticed that vkms breaks when inspecting > its state in the debugfs, also when running igt tests with kms and core > filters, and adding the following fixed that issue: > > 1) a drm_crtc_helper_funcs with dummy atomic_check. We're trying to make callbacks as optional as possible, this was probably just an oversight. Can you try to find the place where this is called, and add suitable checks for NULL _funcs pointer? Thanks, Daniel > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > > + &vkms_crtc_funcs, NULL); > > + if (ret) { > > + DRM_ERROR("Failed to init CRTC\n"); > > + return ret; > > + } > > + > > + return ret; > > +} > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index aec3f180f96d..070613e32934 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -6,7 +6,6 @@ > > */ > > > > #include <linux/module.h> > > -#include <drm/drmP.h> > > #include <drm/drm_gem.h> > > #include <drm/drm_crtc_helper.h> > > #include <drm/drm_atomic_helper.h> > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > > .minor = DRIVER_MINOR, > > }; > > > > -static const u32 vkms_formats[] = { > > - DRM_FORMAT_XRGB8888, > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > + .atomic_check = drm_atomic_helper_check, > > + .atomic_commit = drm_atomic_helper_commit, > > }; > > > > -static void vkms_connector_destroy(struct drm_connector *connector) > > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > > { > > - drm_connector_unregister(connector); > > - drm_connector_cleanup(connector); > > -} > > + struct drm_device *dev = &vkmsdev->drm; > > > > -static const struct drm_connector_funcs vkms_connector_funcs = { > > - .fill_modes = drm_helper_probe_single_connector_modes, > > - .destroy = vkms_connector_destroy, > > -}; > > + drm_mode_config_init(dev); > > + dev->mode_config.funcs = &vkms_mode_funcs; > > + dev->mode_config.min_width = XRES_MIN; > > + dev->mode_config.min_height = YRES_MIN; > > + dev->mode_config.max_width = XRES_MAX; > > + dev->mode_config.max_height = YRES_MAX; > > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > > - .atomic_check = drm_atomic_helper_check, > > - .atomic_commit = drm_atomic_helper_commit, > > -}; > > + return vkms_output_init(vkmsdev); > > +} > > > > static int __init vkms_init(void) > > { > > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > > goto out_fini; > > } > > > > - drm_mode_config_init(&vkms_device->drm); > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > > - vkms_device->drm.mode_config.min_width = XRES_MIN; > > - vkms_device->drm.mode_config.min_height = YRES_MIN; > > - vkms_device->drm.mode_config.max_width = XRES_MAX; > > - vkms_device->drm.mode_config.max_height = YRES_MAX; > > - > > - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, > > - &vkms_connector_funcs, > > - DRM_MODE_CONNECTOR_VIRTUAL); > > - if (ret < 0) { > > - DRM_ERROR("Failed to init connector\n"); > > - goto out_unregister; > > - } > > - > > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > > - &vkms_device->pipe, > > - NULL, > > - vkms_formats, > > - ARRAY_SIZE(vkms_formats), > > - NULL, > > - &vkms_device->connector); > > - if (ret < 0) { > > - DRM_ERROR("Cannot setup simple display pipe\n"); > > + ret = vkms_modeset_init(vkms_device); > > + if (ret) > > goto out_unregister; > > - } > > > > ret = drm_dev_register(&vkms_device->drm, 0); > > if (ret) > > goto out_unregister; > > > > - drm_connector_register(&vkms_device->connector); > > - > > return 0; > > > > out_unregister: > > platform_device_unregister(vkms_device->platform); > > + > > out_fini: > > drm_dev_fini(&vkms_device->drm); > > + > > out_free: > > kfree(vkms_device); > > - > > return ret; > > } > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > index c77c5bf5032a..b0f9d2e61a42 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > @@ -1,13 +1,31 @@ > > #ifndef _VKMS_DRV_H_ > > #define _VKMS_DRV_H_ > > > > -#include <drm/drm_simple_kms_helper.h> > > +#include <drm/drmP.h> > > +#include <drm/drm.h> > > +#include <drm/drm_encoder.h> > > + > > +static const u32 vkms_formats[] = { > > + DRM_FORMAT_XRGB8888, > > +}; > > + > > +struct vkms_output { > > + struct drm_crtc crtc; > > + struct drm_encoder encoder; > > + struct drm_connector connector; > > +}; > > > > struct vkms_device { > > struct drm_device drm; > > struct platform_device *platform; > > - struct drm_simple_display_pipe pipe; > > - struct drm_connector connector; > > + struct vkms_output output; > > }; > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > + struct drm_plane *primary, struct drm_plane *cursor); > > + > > +int vkms_output_init(struct vkms_device *vkmsdev); > > + > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > > + > > #endif /* _VKMS_DRV_H_ */ > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > new file mode 100644 > > index 000000000000..48143eac3c12 > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_crtc_helper.h> > > + > > +static void vkms_connector_destroy(struct drm_connector *connector) > > +{ > > + drm_connector_unregister(connector); > > + drm_connector_cleanup(connector); > > +} > > + > > +static const struct drm_connector_funcs vkms_connector_funcs = { > > + .fill_modes = drm_helper_probe_single_connector_modes, > > + .destroy = vkms_connector_destroy, > > 2) adding the following hooks with their drm_atomic_helper_connector_* > counterpart to the vkms_connector_funcs: > atomic_duplicate_state, atomic_destroy_state, and reset. > > > +}; > > + > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > > + .destroy = drm_encoder_cleanup, > > +}; > > + > > +int vkms_output_init(struct vkms_device *vkmsdev) > > +{ > > + struct vkms_output *output = &vkmsdev->output; > > + struct drm_device *dev = &vkmsdev->drm; > > + struct drm_connector *connector = &output->connector; > > + struct drm_encoder *encoder = &output->encoder; > > + struct drm_crtc *crtc = &output->crtc; > > + struct drm_plane *primary; > > + int ret; > > + > > + primary = vkms_plane_init(vkmsdev); > > + if (IS_ERR(primary)) > > + return PTR_ERR(primary); > > + > > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > > + if (ret) > > + goto err_crtc; > > + > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > > + DRM_MODE_CONNECTOR_VIRTUAL); > > + if (ret) { > > + DRM_ERROR("Failed to init connector\n"); > > + goto err_connector; > > + } > > + > > + ret = drm_connector_register(connector); > > + if (ret) { > > + DRM_ERROR("Failed to register connector\n"); > > + goto err_connector_register; > > + } > > + > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > > + DRM_MODE_ENCODER_VIRTUAL, NULL); > > + if (ret) { > > + DRM_ERROR("Failed to init encoder\n"); > > + goto err_encoder; > > + } > > + encoder->possible_crtcs = 1; > > + > > + ret = drm_mode_connector_attach_encoder(connector, encoder); > > + if (ret) { > > + DRM_ERROR("Failed to attach connector to encoder\n"); > > + goto err_attach; > > + } > > + > > + drm_mode_config_reset(dev); > > + > > + return 0; > > + > > +err_attach: > > + drm_encoder_cleanup(encoder); > > + > > +err_encoder: > > + drm_connector_unregister(connector); > > + > > +err_connector_register: > > + drm_connector_cleanup(connector); > > + > > +err_connector: > > + drm_crtc_cleanup(crtc); > > + > > +err_crtc: > > + drm_plane_cleanup(primary); > > + return ret; > > +} > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > new file mode 100644 > > index 000000000000..2c25b1d6ab5b > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -0,0 +1,46 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include "vkms_drv.h" > > +#include <drm/drm_plane_helper.h> > > +#include <drm/drm_atomic_helper.h> > > + > > +static const struct drm_plane_funcs vkms_plane_funcs = { > > + .update_plane = drm_atomic_helper_update_plane, > > + .disable_plane = drm_atomic_helper_disable_plane, > > + .destroy = drm_plane_cleanup, > > + .reset = drm_atomic_helper_plane_reset, > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > > +}; > > + > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > > +{ > > + struct drm_device *dev = &vkmsdev->drm; > > + struct drm_plane *plane; > > + const u32 *formats; > > + int ret, nformats; > > + > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > > + if (!plane) > > + return ERR_PTR(-ENOMEM); > > + > > + formats = vkms_formats; > > + nformats = ARRAY_SIZE(vkms_formats); > > + > > + ret = drm_universal_plane_init(dev, plane, 0, > > + &vkms_plane_funcs, > > + formats, nformats, > > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > > + if (ret) { > > + kfree(plane); > > + return ERR_PTR(ret); > > + } > > + > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}. > > > + return plane; > > +} > > -- > > 2.17.0 > > > > - Haneen
On Wed, May 23, 2018 at 10:53:33AM +0200, Daniel Vetter wrote: > On Sun, May 20, 2018 at 09:22:31AM +0300, Haneen Mohammed wrote: > > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote: > > > This commit adds the essential infrastructure for around CRTCs which > > > is composed of: a new data struct for output data information, a > > > function for creating planes, and a simple encoder attached to the > > > connector. Finally, due to the introduction of a new initialization > > > function, connectors were moved from vkms_drv.c to vkms_display.c. > > > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > --- > > > drivers/gpu/drm/vkms/Makefile | 2 +- > > > drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ > > > drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- > > > drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- > > > drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ > > > 6 files changed, 211 insertions(+), 47 deletions(-) > > > create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c > > > create mode 100644 drivers/gpu/drm/vkms/vkms_output.c > > > create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c > > > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > > index 2aef948d3a34..3f774a6a9c58 100644 > > > --- a/drivers/gpu/drm/vkms/Makefile > > > +++ b/drivers/gpu/drm/vkms/Makefile > > > @@ -1,3 +1,3 @@ > > > -vkms-y := vkms_drv.o > > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o > > > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > > > new file mode 100644 > > > index 000000000000..bf76cd39ece7 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > > @@ -0,0 +1,35 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_atomic_helper.h> > > > +#include <drm/drm_crtc_helper.h> > > > + > > > +static const struct drm_crtc_funcs vkms_crtc_funcs = { > > > + .set_config = drm_atomic_helper_set_config, > > > + .destroy = drm_crtc_cleanup, > > > + .page_flip = drm_atomic_helper_page_flip, > > > + .reset = drm_atomic_helper_crtc_reset, > > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > > +}; > > > + > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > + struct drm_plane *primary, struct drm_plane *cursor) > > > +{ > > > + int ret; > > > + > > > > Just as a follow up, I have noticed that vkms breaks when inspecting > > its state in the debugfs, also when running igt tests with kms and core > > filters, and adding the following fixed that issue: > > > > 1) a drm_crtc_helper_funcs with dummy atomic_check. > > We're trying to make callbacks as optional as possible, this was probably > just an oversight. Can you try to find the place where this is called, and > add suitable checks for NULL _funcs pointer? > > Thanks, Daniel Sure, I'll look for that. > > > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, > > > + &vkms_crtc_funcs, NULL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init CRTC\n"); > > > + return ret; > > > + } > > > + > > > + return ret; > > > +} > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > index aec3f180f96d..070613e32934 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -6,7 +6,6 @@ > > > */ > > > > > > #include <linux/module.h> > > > -#include <drm/drmP.h> > > > #include <drm/drm_gem.h> > > > #include <drm/drm_crtc_helper.h> > > > #include <drm/drm_atomic_helper.h> > > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { > > > .minor = DRIVER_MINOR, > > > }; > > > > > > -static const u32 vkms_formats[] = { > > > - DRM_FORMAT_XRGB8888, > > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > + .atomic_check = drm_atomic_helper_check, > > > + .atomic_commit = drm_atomic_helper_commit, > > > }; > > > > > > -static void vkms_connector_destroy(struct drm_connector *connector) > > > +static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > { > > > - drm_connector_unregister(connector); > > > - drm_connector_cleanup(connector); > > > -} > > > + struct drm_device *dev = &vkmsdev->drm; > > > > > > -static const struct drm_connector_funcs vkms_connector_funcs = { > > > - .fill_modes = drm_helper_probe_single_connector_modes, > > > - .destroy = vkms_connector_destroy, > > > -}; > > > + drm_mode_config_init(dev); > > > + dev->mode_config.funcs = &vkms_mode_funcs; > > > + dev->mode_config.min_width = XRES_MIN; > > > + dev->mode_config.min_height = YRES_MIN; > > > + dev->mode_config.max_width = XRES_MAX; > > > + dev->mode_config.max_height = YRES_MAX; > > > > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = { > > > - .atomic_check = drm_atomic_helper_check, > > > - .atomic_commit = drm_atomic_helper_commit, > > > -}; > > > + return vkms_output_init(vkmsdev); > > > +} > > > > > > static int __init vkms_init(void) > > > { > > > @@ -98,48 +96,24 @@ static int __init vkms_init(void) > > > goto out_fini; > > > } > > > > > > - drm_mode_config_init(&vkms_device->drm); > > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; > > > - vkms_device->drm.mode_config.min_width = XRES_MIN; > > > - vkms_device->drm.mode_config.min_height = YRES_MIN; > > > - vkms_device->drm.mode_config.max_width = XRES_MAX; > > > - vkms_device->drm.mode_config.max_height = YRES_MAX; > > > - > > > - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, > > > - &vkms_connector_funcs, > > > - DRM_MODE_CONNECTOR_VIRTUAL); > > > - if (ret < 0) { > > > - DRM_ERROR("Failed to init connector\n"); > > > - goto out_unregister; > > > - } > > > - > > > - ret = drm_simple_display_pipe_init(&vkms_device->drm, > > > - &vkms_device->pipe, > > > - NULL, > > > - vkms_formats, > > > - ARRAY_SIZE(vkms_formats), > > > - NULL, > > > - &vkms_device->connector); > > > - if (ret < 0) { > > > - DRM_ERROR("Cannot setup simple display pipe\n"); > > > + ret = vkms_modeset_init(vkms_device); > > > + if (ret) > > > goto out_unregister; > > > - } > > > > > > ret = drm_dev_register(&vkms_device->drm, 0); > > > if (ret) > > > goto out_unregister; > > > > > > - drm_connector_register(&vkms_device->connector); > > > - > > > return 0; > > > > > > out_unregister: > > > platform_device_unregister(vkms_device->platform); > > > + > > > out_fini: > > > drm_dev_fini(&vkms_device->drm); > > > + > > > out_free: > > > kfree(vkms_device); > > > - > > > return ret; > > > } > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > > index c77c5bf5032a..b0f9d2e61a42 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > > @@ -1,13 +1,31 @@ > > > #ifndef _VKMS_DRV_H_ > > > #define _VKMS_DRV_H_ > > > > > > -#include <drm/drm_simple_kms_helper.h> > > > +#include <drm/drmP.h> > > > +#include <drm/drm.h> > > > +#include <drm/drm_encoder.h> > > > + > > > +static const u32 vkms_formats[] = { > > > + DRM_FORMAT_XRGB8888, > > > +}; > > > + > > > +struct vkms_output { > > > + struct drm_crtc crtc; > > > + struct drm_encoder encoder; > > > + struct drm_connector connector; > > > +}; > > > > > > struct vkms_device { > > > struct drm_device drm; > > > struct platform_device *platform; > > > - struct drm_simple_display_pipe pipe; > > > - struct drm_connector connector; > > > + struct vkms_output output; > > > }; > > > > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > > + struct drm_plane *primary, struct drm_plane *cursor); > > > + > > > +int vkms_output_init(struct vkms_device *vkmsdev); > > > + > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); > > > + > > > #endif /* _VKMS_DRV_H_ */ > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > > new file mode 100644 > > > index 000000000000..48143eac3c12 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > > @@ -0,0 +1,91 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_crtc_helper.h> > > > + > > > +static void vkms_connector_destroy(struct drm_connector *connector) > > > +{ > > > + drm_connector_unregister(connector); > > > + drm_connector_cleanup(connector); > > > +} > > > + > > > +static const struct drm_connector_funcs vkms_connector_funcs = { > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > + .destroy = vkms_connector_destroy, > > > > 2) adding the following hooks with their drm_atomic_helper_connector_* > > counterpart to the vkms_connector_funcs: > > atomic_duplicate_state, atomic_destroy_state, and reset. > > > > > +}; > > > + > > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > > > + .destroy = drm_encoder_cleanup, > > > +}; > > > + > > > +int vkms_output_init(struct vkms_device *vkmsdev) > > > +{ > > > + struct vkms_output *output = &vkmsdev->output; > > > + struct drm_device *dev = &vkmsdev->drm; > > > + struct drm_connector *connector = &output->connector; > > > + struct drm_encoder *encoder = &output->encoder; > > > + struct drm_crtc *crtc = &output->crtc; > > > + struct drm_plane *primary; > > > + int ret; > > > + > > > + primary = vkms_plane_init(vkmsdev); > > > + if (IS_ERR(primary)) > > > + return PTR_ERR(primary); > > > + > > > + ret = vkms_crtc_init(dev, crtc, primary, NULL); > > > + if (ret) > > > + goto err_crtc; > > > + > > > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}. > > > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, > > > + DRM_MODE_CONNECTOR_VIRTUAL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init connector\n"); > > > + goto err_connector; > > > + } > > > + > > > + ret = drm_connector_register(connector); > > > + if (ret) { > > > + DRM_ERROR("Failed to register connector\n"); > > > + goto err_connector_register; > > > + } > > > + > > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > > > + DRM_MODE_ENCODER_VIRTUAL, NULL); > > > + if (ret) { > > > + DRM_ERROR("Failed to init encoder\n"); > > > + goto err_encoder; > > > + } > > > + encoder->possible_crtcs = 1; > > > + > > > + ret = drm_mode_connector_attach_encoder(connector, encoder); > > > + if (ret) { > > > + DRM_ERROR("Failed to attach connector to encoder\n"); > > > + goto err_attach; > > > + } > > > + > > > + drm_mode_config_reset(dev); > > > + > > > + return 0; > > > + > > > +err_attach: > > > + drm_encoder_cleanup(encoder); > > > + > > > +err_encoder: > > > + drm_connector_unregister(connector); > > > + > > > +err_connector_register: > > > + drm_connector_cleanup(connector); > > > + > > > +err_connector: > > > + drm_crtc_cleanup(crtc); > > > + > > > +err_crtc: > > > + drm_plane_cleanup(primary); > > > + return ret; > > > +} > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > > new file mode 100644 > > > index 000000000000..2c25b1d6ab5b > > > --- /dev/null > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > @@ -0,0 +1,46 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License as published by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "vkms_drv.h" > > > +#include <drm/drm_plane_helper.h> > > > +#include <drm/drm_atomic_helper.h> > > > + > > > +static const struct drm_plane_funcs vkms_plane_funcs = { > > > + .update_plane = drm_atomic_helper_update_plane, > > > + .disable_plane = drm_atomic_helper_disable_plane, > > > + .destroy = drm_plane_cleanup, > > > + .reset = drm_atomic_helper_plane_reset, > > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > > > +}; > > > + > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) > > > +{ > > > + struct drm_device *dev = &vkmsdev->drm; > > > + struct drm_plane *plane; > > > + const u32 *formats; > > > + int ret, nformats; > > > + > > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL); > > > + if (!plane) > > > + return ERR_PTR(-ENOMEM); > > > + > > > + formats = vkms_formats; > > > + nformats = ARRAY_SIZE(vkms_formats); > > > + > > > + ret = drm_universal_plane_init(dev, plane, 0, > > > + &vkms_plane_funcs, > > > + formats, nformats, > > > + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); > > > + if (ret) { > > > + kfree(plane); > > > + return ERR_PTR(ret); > > > + } > > > + > > > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}. > > > > > + return plane; > > > +} > > > -- > > > 2.17.0 > > > > > > > - Haneen > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Rodrigo, On Wed, 16 May 2018, Rodrigo Siqueira wrote: All files added by this commit have inconsistent license information: > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > @@ -0,0 +1,35 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -0,0 +1,46 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ The SPDX license identifier says GPL v2 only, but the license notice says GPL v2 or later. This is just wrong. Please decide which one to use and please also remove the boiler plate text. It is redundant. See Documentation/process/license-rules.rst Please fix ASAP, add a Fixes tag and cc stable. Thanks, tglx
On 01/18, Thomas Gleixner wrote: > Rodrigo, > > On Wed, 16 May 2018, Rodrigo Siqueira wrote: > > All files added by this commit have inconsistent license information: > > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > @@ -0,0 +1,35 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -0,0 +1,46 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > The SPDX license identifier says GPL v2 only, but the license notice says > GPL v2 or later. This is just wrong. > > Please decide which one to use and please also remove the boiler plate > text. It is redundant. > > See Documentation/process/license-rules.rst > > Please fix ASAP, add a Fixes tag and cc stable. Hi Thomas, Thanks for your feedback. I'll try to send a patch for fix this issue today. Best Regards Rodrigo Siqueira > Thanks, > > tglx
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile index 2aef948d3a34..3f774a6a9c58 100644 --- a/drivers/gpu/drm/vkms/Makefile +++ b/drivers/gpu/drm/vkms/Makefile @@ -1,3 +1,3 @@ -vkms-y := vkms_drv.o +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o obj-$(CONFIG_DRM_VKMS) += vkms.o diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c new file mode 100644 index 000000000000..bf76cd39ece7 --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include "vkms_drv.h" +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> + +static const struct drm_crtc_funcs vkms_crtc_funcs = { + .set_config = drm_atomic_helper_set_config, + .destroy = drm_crtc_cleanup, + .page_flip = drm_atomic_helper_page_flip, + .reset = drm_atomic_helper_crtc_reset, + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, +}; + +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, + struct drm_plane *primary, struct drm_plane *cursor) +{ + int ret; + + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor, + &vkms_crtc_funcs, NULL); + if (ret) { + DRM_ERROR("Failed to init CRTC\n"); + return ret; + } + + return ret; +} diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index aec3f180f96d..070613e32934 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -6,7 +6,6 @@ */ #include <linux/module.h> -#include <drm/drmP.h> #include <drm/drm_gem.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_atomic_helper.h> @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = { .minor = DRIVER_MINOR, }; -static const u32 vkms_formats[] = { - DRM_FORMAT_XRGB8888, +static const struct drm_mode_config_funcs vkms_mode_funcs = { + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, }; -static void vkms_connector_destroy(struct drm_connector *connector) +static int vkms_modeset_init(struct vkms_device *vkmsdev) { - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} + struct drm_device *dev = &vkmsdev->drm; -static const struct drm_connector_funcs vkms_connector_funcs = { - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = vkms_connector_destroy, -}; + drm_mode_config_init(dev); + dev->mode_config.funcs = &vkms_mode_funcs; + dev->mode_config.min_width = XRES_MIN; + dev->mode_config.min_height = YRES_MIN; + dev->mode_config.max_width = XRES_MAX; + dev->mode_config.max_height = YRES_MAX; -static const struct drm_mode_config_funcs vkms_mode_funcs = { - .atomic_check = drm_atomic_helper_check, - .atomic_commit = drm_atomic_helper_commit, -}; + return vkms_output_init(vkmsdev); +} static int __init vkms_init(void) { @@ -98,48 +96,24 @@ static int __init vkms_init(void) goto out_fini; } - drm_mode_config_init(&vkms_device->drm); - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs; - vkms_device->drm.mode_config.min_width = XRES_MIN; - vkms_device->drm.mode_config.min_height = YRES_MIN; - vkms_device->drm.mode_config.max_width = XRES_MAX; - vkms_device->drm.mode_config.max_height = YRES_MAX; - - ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector, - &vkms_connector_funcs, - DRM_MODE_CONNECTOR_VIRTUAL); - if (ret < 0) { - DRM_ERROR("Failed to init connector\n"); - goto out_unregister; - } - - ret = drm_simple_display_pipe_init(&vkms_device->drm, - &vkms_device->pipe, - NULL, - vkms_formats, - ARRAY_SIZE(vkms_formats), - NULL, - &vkms_device->connector); - if (ret < 0) { - DRM_ERROR("Cannot setup simple display pipe\n"); + ret = vkms_modeset_init(vkms_device); + if (ret) goto out_unregister; - } ret = drm_dev_register(&vkms_device->drm, 0); if (ret) goto out_unregister; - drm_connector_register(&vkms_device->connector); - return 0; out_unregister: platform_device_unregister(vkms_device->platform); + out_fini: drm_dev_fini(&vkms_device->drm); + out_free: kfree(vkms_device); - return ret; } diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index c77c5bf5032a..b0f9d2e61a42 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -1,13 +1,31 @@ #ifndef _VKMS_DRV_H_ #define _VKMS_DRV_H_ -#include <drm/drm_simple_kms_helper.h> +#include <drm/drmP.h> +#include <drm/drm.h> +#include <drm/drm_encoder.h> + +static const u32 vkms_formats[] = { + DRM_FORMAT_XRGB8888, +}; + +struct vkms_output { + struct drm_crtc crtc; + struct drm_encoder encoder; + struct drm_connector connector; +}; struct vkms_device { struct drm_device drm; struct platform_device *platform; - struct drm_simple_display_pipe pipe; - struct drm_connector connector; + struct vkms_output output; }; +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, + struct drm_plane *primary, struct drm_plane *cursor); + +int vkms_output_init(struct vkms_device *vkmsdev); + +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev); + #endif /* _VKMS_DRV_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c new file mode 100644 index 000000000000..48143eac3c12 --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include "vkms_drv.h" +#include <drm/drm_crtc_helper.h> + +static void vkms_connector_destroy(struct drm_connector *connector) +{ + drm_connector_unregister(connector); + drm_connector_cleanup(connector); +} + +static const struct drm_connector_funcs vkms_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = vkms_connector_destroy, +}; + +static const struct drm_encoder_funcs vkms_encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +int vkms_output_init(struct vkms_device *vkmsdev) +{ + struct vkms_output *output = &vkmsdev->output; + struct drm_device *dev = &vkmsdev->drm; + struct drm_connector *connector = &output->connector; + struct drm_encoder *encoder = &output->encoder; + struct drm_crtc *crtc = &output->crtc; + struct drm_plane *primary; + int ret; + + primary = vkms_plane_init(vkmsdev); + if (IS_ERR(primary)) + return PTR_ERR(primary); + + ret = vkms_crtc_init(dev, crtc, primary, NULL); + if (ret) + goto err_crtc; + + ret = drm_connector_init(dev, connector, &vkms_connector_funcs, + DRM_MODE_CONNECTOR_VIRTUAL); + if (ret) { + DRM_ERROR("Failed to init connector\n"); + goto err_connector; + } + + ret = drm_connector_register(connector); + if (ret) { + DRM_ERROR("Failed to register connector\n"); + goto err_connector_register; + } + + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, + DRM_MODE_ENCODER_VIRTUAL, NULL); + if (ret) { + DRM_ERROR("Failed to init encoder\n"); + goto err_encoder; + } + encoder->possible_crtcs = 1; + + ret = drm_mode_connector_attach_encoder(connector, encoder); + if (ret) { + DRM_ERROR("Failed to attach connector to encoder\n"); + goto err_attach; + } + + drm_mode_config_reset(dev); + + return 0; + +err_attach: + drm_encoder_cleanup(encoder); + +err_encoder: + drm_connector_unregister(connector); + +err_connector_register: + drm_connector_cleanup(connector); + +err_connector: + drm_crtc_cleanup(crtc); + +err_crtc: + drm_plane_cleanup(primary); + return ret; +} diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c new file mode 100644 index 000000000000..2c25b1d6ab5b --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include "vkms_drv.h" +#include <drm/drm_plane_helper.h> +#include <drm/drm_atomic_helper.h> + +static const struct drm_plane_funcs vkms_plane_funcs = { + .update_plane = drm_atomic_helper_update_plane, + .disable_plane = drm_atomic_helper_disable_plane, + .destroy = drm_plane_cleanup, + .reset = drm_atomic_helper_plane_reset, + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, +}; + +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev) +{ + struct drm_device *dev = &vkmsdev->drm; + struct drm_plane *plane; + const u32 *formats; + int ret, nformats; + + plane = kzalloc(sizeof(*plane), GFP_KERNEL); + if (!plane) + return ERR_PTR(-ENOMEM); + + formats = vkms_formats; + nformats = ARRAY_SIZE(vkms_formats); + + ret = drm_universal_plane_init(dev, plane, 0, + &vkms_plane_funcs, + formats, nformats, + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); + if (ret) { + kfree(plane); + return ERR_PTR(ret); + } + + return plane; +}
This commit adds the essential infrastructure for around CRTCs which is composed of: a new data struct for output data information, a function for creating planes, and a simple encoder attached to the connector. Finally, due to the introduction of a new initialization function, connectors were moved from vkms_drv.c to vkms_display.c. Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> --- drivers/gpu/drm/vkms/Makefile | 2 +- drivers/gpu/drm/vkms/vkms_crtc.c | 35 ++++++++++++ drivers/gpu/drm/vkms/vkms_drv.c | 60 ++++++-------------- drivers/gpu/drm/vkms/vkms_drv.h | 24 +++++++- drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_plane.c | 46 +++++++++++++++ 6 files changed, 211 insertions(+), 47 deletions(-) create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c create mode 100644 drivers/gpu/drm/vkms/vkms_output.c create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c