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@xxxxxxxxx>
> > ---
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
