Re: [PATCH 1/3] drm/vkms: Add basic CRTC initialization
Now I got it! I will split the patch and apply your suggestions :) Thanks On 05/16, Daniel Vetter wrote: > On Wed, May 16, 2018 at 5:40 PM, Daniel Vetterwrote: > > On Wed, May 16, 2018 at 4:51 PM, Rodrigo Siqueira > > wrote: > >> Hi Daniel, > >> > >> Thanks for the feedback :) > >> > >> You can find my comments below: > >> > >> On 05/16, Daniel Vetter wrote: > >>> On Wed, May 16, 2018 at 12:06:54AM -0300, Rodrigo Siqueira wrote: > >>> > This commit adds the essential infrastructure for managing CRTCs which > >>> > is composed of: a new data struct for output data information, a > >>> > function for basic modeset initialization, and the operation to create > >>> > planes. 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 > >>> > --- > >>> > drivers/gpu/drm/vkms/Makefile | 2 +- > >>> > drivers/gpu/drm/vkms/vkms_display.c | 108 > >>> > drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- > >>> > drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- > >>> > drivers/gpu/drm/vkms/vkms_plane.c | 46 > >>> > 5 files changed, 180 insertions(+), 43 deletions(-) > >>> > create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o > >>> > > >>> > obj-$(CONFIG_DRM_VKMS) += vkms.o > >>> > diff --git a/drivers/gpu/drm/vkms/vkms_display.c > >>> > b/drivers/gpu/drm/vkms/vkms_display.c > >>> > new file mode 100644 > >>> > index ..b20b41f9590b > >>> > --- /dev/null > >>> > +++ b/drivers/gpu/drm/vkms/vkms_display.c > >>> > @@ -0,0 +1,108 @@ > >>> > +// 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 > >>> > +#include > >>> > + > >>> > +#define XRES_MIN32 > >>> > +#define YRES_MIN32 > >>> > + > >>> > +#define XRES_DEF 1024 > >>> > +#define YRES_DEF 768 > >>> > >>> These seem unused. > >> > >> I created these defines because the documentation says: > >> > >> "[..]Once done, mode configuration must be setup by initializing the > >> following fields." > >> (https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#c.drm_mode_config_init) > >> > >> Finally, I used them in "mode_config" fields from drm_device (code below). > >> > >> Is it ok to not setup these values on mode_config? I looked at virtio as > >> an inspiration for this. > > > > XYRES_MIN and _MAX you need, but _DEF seems unused. That's why I > > asked, sorry for not making this clear. > > btw could make sense to split this fix into a separate patch, since > the min/max setup is indeed missing and required. > -Daniel > > > -Daniel > > > >> > >>> > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > >>> > + .atomic_check = drm_atomic_helper_check, > >>> > + .atomic_commit = drm_atomic_helper_commit, > >>> > +}; > >>> > + > >>> > +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, > >>> > +}; > >>> > + > >>> > +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 int vkms_output_init(struct vkms_device *vkmsdev) > >>> > +{ > >>> > + struct vkms_output *output = >output; > >>> > + struct drm_device *dev = >drm; > >>> > + struct drm_connector *connector = >connector; > >>> > + struct drm_crtc *crtc = >crtc; > >>> > + struct drm_plane *primary; > >>> > + int ret; > >>> > + > >>> > + primary = vkms_plane_init(vkmsdev); > >>> > + if
Re: [PATCH 1/3] drm/vkms: Add basic CRTC initialization
On Wed, May 16, 2018 at 5:40 PM, Daniel Vetterwrote: > On Wed, May 16, 2018 at 4:51 PM, Rodrigo Siqueira > wrote: >> Hi Daniel, >> >> Thanks for the feedback :) >> >> You can find my comments below: >> >> On 05/16, Daniel Vetter wrote: >>> On Wed, May 16, 2018 at 12:06:54AM -0300, Rodrigo Siqueira wrote: >>> > This commit adds the essential infrastructure for managing CRTCs which >>> > is composed of: a new data struct for output data information, a >>> > function for basic modeset initialization, and the operation to create >>> > planes. 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 >>> > --- >>> > drivers/gpu/drm/vkms/Makefile | 2 +- >>> > drivers/gpu/drm/vkms/vkms_display.c | 108 >>> > drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- >>> > drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- >>> > drivers/gpu/drm/vkms/vkms_plane.c | 46 >>> > 5 files changed, 180 insertions(+), 43 deletions(-) >>> > create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o >>> > >>> > obj-$(CONFIG_DRM_VKMS) += vkms.o >>> > diff --git a/drivers/gpu/drm/vkms/vkms_display.c >>> > b/drivers/gpu/drm/vkms/vkms_display.c >>> > new file mode 100644 >>> > index ..b20b41f9590b >>> > --- /dev/null >>> > +++ b/drivers/gpu/drm/vkms/vkms_display.c >>> > @@ -0,0 +1,108 @@ >>> > +// 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 >>> > +#include >>> > + >>> > +#define XRES_MIN32 >>> > +#define YRES_MIN32 >>> > + >>> > +#define XRES_DEF 1024 >>> > +#define YRES_DEF 768 >>> >>> These seem unused. >> >> I created these defines because the documentation says: >> >> "[..]Once done, mode configuration must be setup by initializing the >> following fields." >> (https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#c.drm_mode_config_init) >> >> Finally, I used them in "mode_config" fields from drm_device (code below). >> >> Is it ok to not setup these values on mode_config? I looked at virtio as >> an inspiration for this. > > XYRES_MIN and _MAX you need, but _DEF seems unused. That's why I > asked, sorry for not making this clear. btw could make sense to split this fix into a separate patch, since the min/max setup is indeed missing and required. -Daniel > -Daniel > >> >>> > +static const struct drm_mode_config_funcs vkms_mode_funcs = { >>> > + .atomic_check = drm_atomic_helper_check, >>> > + .atomic_commit = drm_atomic_helper_commit, >>> > +}; >>> > + >>> > +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, >>> > +}; >>> > + >>> > +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 int vkms_output_init(struct vkms_device *vkmsdev) >>> > +{ >>> > + struct vkms_output *output = >output; >>> > + struct drm_device *dev = >drm; >>> > + struct drm_connector *connector = >connector; >>> > + struct drm_crtc *crtc = >crtc; >>> > + struct drm_plane *primary; >>> > + int ret; >>> > + >>> > + primary = vkms_plane_init(vkmsdev); >>> > + if (IS_ERR(primary)) >>> > + return PTR_ERR(primary); >>> > + >>> > + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, >>> > + _crtc_funcs, NULL); >>> > + if (ret) { >>> > + DRM_ERROR("Failed to init CRTC\n"); >>> > + goto err_crtc; >>> > + } >>> > + primary->crtc = crtc; >>> >>> If you want
Re: [PATCH 1/3] drm/vkms: Add basic CRTC initialization
On Wed, May 16, 2018 at 4:51 PM, Rodrigo Siqueirawrote: > Hi Daniel, > > Thanks for the feedback :) > > You can find my comments below: > > On 05/16, Daniel Vetter wrote: >> On Wed, May 16, 2018 at 12:06:54AM -0300, Rodrigo Siqueira wrote: >> > This commit adds the essential infrastructure for managing CRTCs which >> > is composed of: a new data struct for output data information, a >> > function for basic modeset initialization, and the operation to create >> > planes. 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 >> > --- >> > drivers/gpu/drm/vkms/Makefile | 2 +- >> > drivers/gpu/drm/vkms/vkms_display.c | 108 >> > drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- >> > drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- >> > drivers/gpu/drm/vkms/vkms_plane.c | 46 >> > 5 files changed, 180 insertions(+), 43 deletions(-) >> > create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o >> > >> > obj-$(CONFIG_DRM_VKMS) += vkms.o >> > diff --git a/drivers/gpu/drm/vkms/vkms_display.c >> > b/drivers/gpu/drm/vkms/vkms_display.c >> > new file mode 100644 >> > index ..b20b41f9590b >> > --- /dev/null >> > +++ b/drivers/gpu/drm/vkms/vkms_display.c >> > @@ -0,0 +1,108 @@ >> > +// 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 >> > +#include >> > + >> > +#define XRES_MIN32 >> > +#define YRES_MIN32 >> > + >> > +#define XRES_DEF 1024 >> > +#define YRES_DEF 768 >> >> These seem unused. > > I created these defines because the documentation says: > > "[..]Once done, mode configuration must be setup by initializing the > following fields." > (https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#c.drm_mode_config_init) > > Finally, I used them in "mode_config" fields from drm_device (code below). > > Is it ok to not setup these values on mode_config? I looked at virtio as > an inspiration for this. XYRES_MIN and _MAX you need, but _DEF seems unused. That's why I asked, sorry for not making this clear. -Daniel > >> > +static const struct drm_mode_config_funcs vkms_mode_funcs = { >> > + .atomic_check = drm_atomic_helper_check, >> > + .atomic_commit = drm_atomic_helper_commit, >> > +}; >> > + >> > +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, >> > +}; >> > + >> > +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 int vkms_output_init(struct vkms_device *vkmsdev) >> > +{ >> > + struct vkms_output *output = >output; >> > + struct drm_device *dev = >drm; >> > + struct drm_connector *connector = >connector; >> > + struct drm_crtc *crtc = >crtc; >> > + struct drm_plane *primary; >> > + int ret; >> > + >> > + primary = vkms_plane_init(vkmsdev); >> > + if (IS_ERR(primary)) >> > + return PTR_ERR(primary); >> > + >> > + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, >> > + _crtc_funcs, NULL); >> > + if (ret) { >> > + DRM_ERROR("Failed to init CRTC\n"); >> > + goto err_crtc; >> > + } >> > + primary->crtc = crtc; >> >> If you want to split stuff up a bit, I think putting the crtc stuff into >> vkms_crtc.c, and then renaming this file here to vkms_output.c would make >> sense. > > Nice, make a lot of sense to me. I will do it on v2. > >> > + >> > + ret = drm_connector_init(dev, connector, _connector_funcs, >> > +
Re: [PATCH 1/3] drm/vkms: Add basic CRTC initialization
Hi Daniel, Thanks for the feedback :) You can find my comments below: On 05/16, Daniel Vetter wrote: > On Wed, May 16, 2018 at 12:06:54AM -0300, Rodrigo Siqueira wrote: > > This commit adds the essential infrastructure for managing CRTCs which > > is composed of: a new data struct for output data information, a > > function for basic modeset initialization, and the operation to create > > planes. 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> > --- > > drivers/gpu/drm/vkms/Makefile | 2 +- > > drivers/gpu/drm/vkms/vkms_display.c | 108 > > drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- > > drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- > > drivers/gpu/drm/vkms/vkms_plane.c | 46 > > 5 files changed, 180 insertions(+), 43 deletions(-) > > create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > diff --git a/drivers/gpu/drm/vkms/vkms_display.c > > b/drivers/gpu/drm/vkms/vkms_display.c > > new file mode 100644 > > index ..b20b41f9590b > > --- /dev/null > > +++ b/drivers/gpu/drm/vkms/vkms_display.c > > @@ -0,0 +1,108 @@ > > +// 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 > > +#include > > + > > +#define XRES_MIN32 > > +#define YRES_MIN32 > > + > > +#define XRES_DEF 1024 > > +#define YRES_DEF 768 > > These seem unused. I created these defines because the documentation says: "[..]Once done, mode configuration must be setup by initializing the following fields." (https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#c.drm_mode_config_init) Finally, I used them in "mode_config" fields from drm_device (code below). Is it ok to not setup these values on mode_config? I looked at virtio as an inspiration for this. > > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > > + .atomic_check = drm_atomic_helper_check, > > + .atomic_commit = drm_atomic_helper_commit, > > +}; > > + > > +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, > > +}; > > + > > +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 int vkms_output_init(struct vkms_device *vkmsdev) > > +{ > > + struct vkms_output *output = >output; > > + struct drm_device *dev = >drm; > > + struct drm_connector *connector = >connector; > > + struct drm_crtc *crtc = >crtc; > > + struct drm_plane *primary; > > + int ret; > > + > > + primary = vkms_plane_init(vkmsdev); > > + if (IS_ERR(primary)) > > + return PTR_ERR(primary); > > + > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, > > + _crtc_funcs, NULL); > > + if (ret) { > > + DRM_ERROR("Failed to init CRTC\n"); > > + goto err_crtc; > > + } > > + primary->crtc = crtc; > > If you want to split stuff up a bit, I think putting the crtc stuff into > vkms_crtc.c, and then renaming this file here to vkms_output.c would make > sense. Nice, make a lot of sense to me. I will do it on v2. > > + > > + ret = drm_connector_init(dev, connector, _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; > > + } > > + > >
Re: [PATCH 1/3] drm/vkms: Add basic CRTC initialization
On Wed, May 16, 2018 at 12:06:54AM -0300, Rodrigo Siqueira wrote: > This commit adds the essential infrastructure for managing CRTCs which > is composed of: a new data struct for output data information, a > function for basic modeset initialization, and the operation to create > planes. 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> --- > drivers/gpu/drm/vkms/Makefile | 2 +- > drivers/gpu/drm/vkms/vkms_display.c | 108 > drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- > drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- > drivers/gpu/drm/vkms/vkms_plane.c | 46 > 5 files changed, 180 insertions(+), 43 deletions(-) > create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o > > obj-$(CONFIG_DRM_VKMS) += vkms.o > diff --git a/drivers/gpu/drm/vkms/vkms_display.c > b/drivers/gpu/drm/vkms/vkms_display.c > new file mode 100644 > index ..b20b41f9590b > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_display.c > @@ -0,0 +1,108 @@ > +// 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 > +#include > + > +#define XRES_MIN32 > +#define YRES_MIN32 > + > +#define XRES_DEF 1024 > +#define YRES_DEF 768 These seem unused. > +#define XRES_MAX 8192 > +#define YRES_MAX 8192 > + > +static const struct drm_mode_config_funcs vkms_mode_funcs = { > + .atomic_check = drm_atomic_helper_check, > + .atomic_commit = drm_atomic_helper_commit, > +}; > + > +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, > +}; > + > +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 int vkms_output_init(struct vkms_device *vkmsdev) > +{ > + struct vkms_output *output = >output; > + struct drm_device *dev = >drm; > + struct drm_connector *connector = >connector; > + struct drm_crtc *crtc = >crtc; > + struct drm_plane *primary; > + int ret; > + > + primary = vkms_plane_init(vkmsdev); > + if (IS_ERR(primary)) > + return PTR_ERR(primary); > + > + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, > + _crtc_funcs, NULL); > + if (ret) { > + DRM_ERROR("Failed to init CRTC\n"); > + goto err_crtc; > + } > + primary->crtc = crtc; If you want to split stuff up a bit, I think putting the crtc stuff into vkms_crtc.c, and then renaming this file here to vkms_output.c would make sense. > + > + ret = drm_connector_init(dev, connector, _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; > + } > + > + drm_mode_config_reset(dev); > + > + return 0; > + > +err_connector_register: > + drm_connector_cleanup(connector); > + > +err_connector: > + drm_crtc_cleanup(crtc); > + > +err_crtc: > + drm_plane_cleanup(primary); > + return ret; > +} > + > +int vkms_modeset_init(struct vkms_device *vkmsdev) Plus keeping vkms_modeset_init in vkms_drv.c, vkms is 100% about modesetting and nothing else, so splitting that out is a bit overkill imo. > +{ > + struct drm_device *dev = >drm; > + > + drm_mode_config_init(dev); > + dev->mode_config.funcs = _mode_funcs; > + dev->mode_config.min_width = XRES_MIN; > +
[PATCH 1/3] drm/vkms: Add basic CRTC initialization
This commit adds the essential infrastructure for managing CRTCs which is composed of: a new data struct for output data information, a function for basic modeset initialization, and the operation to create planes. 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--- drivers/gpu/drm/vkms/Makefile | 2 +- drivers/gpu/drm/vkms/vkms_display.c | 108 drivers/gpu/drm/vkms/vkms_drv.c | 41 +-- drivers/gpu/drm/vkms/vkms_drv.h | 26 ++- drivers/gpu/drm/vkms/vkms_plane.c | 46 5 files changed, 180 insertions(+), 43 deletions(-) create mode 100644 drivers/gpu/drm/vkms/vkms_display.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..f747e2a3a6f4 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_display.o obj-$(CONFIG_DRM_VKMS) += vkms.o diff --git a/drivers/gpu/drm/vkms/vkms_display.c b/drivers/gpu/drm/vkms/vkms_display.c new file mode 100644 index ..b20b41f9590b --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_display.c @@ -0,0 +1,108 @@ +// 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 +#include + +#define XRES_MIN32 +#define YRES_MIN32 + +#define XRES_DEF 1024 +#define YRES_DEF 768 + +#define XRES_MAX 8192 +#define YRES_MAX 8192 + +static const struct drm_mode_config_funcs vkms_mode_funcs = { + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +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, +}; + +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 int vkms_output_init(struct vkms_device *vkmsdev) +{ + struct vkms_output *output = >output; + struct drm_device *dev = >drm; + struct drm_connector *connector = >connector; + struct drm_crtc *crtc = >crtc; + struct drm_plane *primary; + int ret; + + primary = vkms_plane_init(vkmsdev); + if (IS_ERR(primary)) + return PTR_ERR(primary); + + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, + _crtc_funcs, NULL); + if (ret) { + DRM_ERROR("Failed to init CRTC\n"); + goto err_crtc; + } + primary->crtc = crtc; + + ret = drm_connector_init(dev, connector, _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; + } + + drm_mode_config_reset(dev); + + return 0; + +err_connector_register: + drm_connector_cleanup(connector); + +err_connector: + drm_crtc_cleanup(crtc); + +err_crtc: + drm_plane_cleanup(primary); + return ret; +} + +int vkms_modeset_init(struct vkms_device *vkmsdev) +{ + struct drm_device *dev = >drm; + + drm_mode_config_init(dev); + dev->mode_config.funcs = _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; + + return vkms_output_init(vkmsdev); +} diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 35517b09538e..11e0569807bd 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -6,9 +6,7 @@ */ #include -#include #include -#include #include "vkms_drv.h" #define DRIVER_NAME"vkms" @@ -52,21 +50,6 @@ static struct drm_driver vkms_driver = {