On Tue, Jun 21, 2016 at 03:09:40PM +0200, Benjamin Gaignard wrote:
> Use drm_dev_alloc() and drm_dev_register() instead of .load()
> To simplify init sequence only create fbdev when requested
> in output_poll_changed().
>
> version 2:
> remove call to drm_connector_unregister_all() and
> drm_dev_set_unique()
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> ---
> drivers/gpu/drm/sti/sti_drv.c | 137
> +++++++++++++++++++++++++++++------------
> drivers/gpu/drm/sti/sti_drv.h | 1 +
> drivers/gpu/drm/sti/sti_dvo.c | 7 ---
> drivers/gpu/drm/sti/sti_hda.c | 8 +--
> drivers/gpu/drm/sti/sti_hdmi.c | 21 +------
> 5 files changed, 100 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 26aa85d..96bd3d0 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -226,8 +226,28 @@ static int sti_atomic_commit(struct drm_device *drm,
> return 0;
> }
>
> +static void sti_output_poll_changed(struct drm_device *ddev)
> +{
> + struct sti_private *private = ddev->dev_private;
> +
> + if (!ddev->mode_config.num_connector)
> + return;
> +
> + if (private->fbdev) {
> + drm_fbdev_cma_hotplug_event(private->fbdev);
> + return;
> + }
> +
> + private->fbdev = drm_fbdev_cma_init(ddev, 32,
> + ddev->mode_config.num_crtc,
> + ddev->mode_config.num_connector);
> + if (IS_ERR(private->fbdev))
> + private->fbdev = NULL;
> +}
This creates another copy of the copy-pasted deferred fbdev init code.
Thierry Redding is working to clean that up, but since you've just created
more work for him here I think you've volunteered to review Thierry's
patches ;-)
All three patches merged to drm-misc, thanks.
-Daniel
> +
> static const struct drm_mode_config_funcs sti_mode_config_funcs = {
> .fb_create = drm_fb_cma_create,
> + .output_poll_changed = sti_output_poll_changed,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = sti_atomic_commit,
> };
> @@ -248,44 +268,6 @@ static void sti_mode_config_init(struct drm_device *dev)
> dev->mode_config.funcs = &sti_mode_config_funcs;
> }
>
> -static int sti_load(struct drm_device *dev, unsigned long flags)
> -{
> - struct sti_private *private;
> - int ret;
> -
> - private = kzalloc(sizeof(*private), GFP_KERNEL);
> - if (!private) {
> - DRM_ERROR("Failed to allocate private\n");
> - return -ENOMEM;
> - }
> - dev->dev_private = (void *)private;
> - private->drm_dev = dev;
> -
> - mutex_init(&private->commit.lock);
> - INIT_WORK(&private->commit.work, sti_atomic_work);
> -
> - drm_mode_config_init(dev);
> - drm_kms_helper_poll_init(dev);
> -
> - sti_mode_config_init(dev);
> -
> - ret = component_bind_all(dev->dev, dev);
> - if (ret) {
> - drm_kms_helper_poll_fini(dev);
> - drm_mode_config_cleanup(dev);
> - kfree(private);
> - return ret;
> - }
> -
> - drm_mode_config_reset(dev);
> -
> - drm_fbdev_cma_init(dev, 32,
> - dev->mode_config.num_crtc,
> - dev->mode_config.num_connector);
> -
> - return 0;
> -}
> -
> static const struct file_operations sti_driver_fops = {
> .owner = THIS_MODULE,
> .open = drm_open,
> @@ -302,7 +284,6 @@ static const struct file_operations sti_driver_fops = {
> static struct drm_driver sti_driver = {
> .driver_features = DRIVER_HAVE_IRQ | DRIVER_MODESET |
> DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
> - .load = sti_load,
> .gem_free_object_unlocked = drm_gem_cma_free_object,
> .gem_vm_ops = &drm_gem_cma_vm_ops,
> .dumb_create = drm_gem_cma_dumb_create,
> @@ -339,14 +320,88 @@ static int compare_of(struct device *dev, void *data)
> return dev->of_node == data;
> }
>
> +static int sti_init(struct drm_device *ddev)
> +{
> + struct sti_private *private;
> +
> + private = kzalloc(sizeof(*private), GFP_KERNEL);
> + if (!private)
> + return -ENOMEM;
> +
> + ddev->dev_private = (void *)private;
> + dev_set_drvdata(ddev->dev, ddev);
> + private->drm_dev = ddev;
> +
> + mutex_init(&private->commit.lock);
> + INIT_WORK(&private->commit.work, sti_atomic_work);
> +
> + drm_mode_config_init(ddev);
> +
> + sti_mode_config_init(ddev);
> +
> + drm_kms_helper_poll_init(ddev);
> +
> + return 0;
> +}
> +
> +static void sti_cleanup(struct drm_device *ddev)
> +{
> + struct sti_private *private = ddev->dev_private;
> +
> + if (private->fbdev) {
> + drm_fbdev_cma_fini(private->fbdev);
> + private->fbdev = NULL;
> + }
> +
> + drm_kms_helper_poll_fini(ddev);
> + drm_vblank_cleanup(ddev);
> + kfree(private);
> + ddev->dev_private = NULL;
> +}
> +
> static int sti_bind(struct device *dev)
> {
> - return drm_platform_init(&sti_driver, to_platform_device(dev));
> + struct drm_device *ddev;
> + int ret;
> +
> + ddev = drm_dev_alloc(&sti_driver, dev);
> + if (!ddev)
> + return -ENOMEM;
> +
> + ddev->platformdev = to_platform_device(dev);
> +
> + ret = sti_init(ddev);
> + if (ret)
> + goto err_drm_dev_unref;
> +
> + ret = component_bind_all(ddev->dev, ddev);
> + if (ret)
> + goto err_cleanup;
> +
> + ret = drm_dev_register(ddev, 0);
> + if (ret)
> + goto err_register;
> +
> + drm_mode_config_reset(ddev);
> +
> + return 0;
> +
> +err_register:
> + drm_mode_config_cleanup(ddev);
> +err_cleanup:
> + sti_cleanup(ddev);
> +err_drm_dev_unref:
> + drm_dev_unref(ddev);
> + return ret;
> }
>
> static void sti_unbind(struct device *dev)
> {
> - drm_put_dev(dev_get_drvdata(dev));
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + drm_dev_unregister(ddev);
> + sti_cleanup(ddev);
> + drm_dev_unref(ddev);
> }
>
> static const struct component_master_ops sti_ops = {
> diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
> index 30ddc20..78ebe5e 100644
> --- a/drivers/gpu/drm/sti/sti_drv.h
> +++ b/drivers/gpu/drm/sti/sti_drv.h
> @@ -24,6 +24,7 @@ struct sti_private {
> struct sti_compositor *compo;
> struct drm_property *plane_zorder_property;
> struct drm_device *drm_dev;
> + struct drm_fbdev_cma *fbdev;
>
> struct {
> struct drm_atomic_state *state;
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index 8a2b48f..ec31080 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -497,10 +497,6 @@ static int sti_dvo_bind(struct device *dev, struct
> device *master, void *data)
> drm_connector_helper_add(drm_connector,
> &sti_dvo_connector_helper_funcs);
>
> - err = drm_connector_register(drm_connector);
> - if (err)
> - goto err_connector;
> -
> err = drm_mode_connector_attach_encoder(drm_connector, encoder);
> if (err) {
> DRM_ERROR("Failed to attach a connector to a encoder\n");
> @@ -510,10 +506,7 @@ static int sti_dvo_bind(struct device *dev, struct
> device *master, void *data)
> return 0;
>
> err_sysfs:
> - drm_connector_unregister(drm_connector);
> -err_connector:
> drm_bridge_remove(bridge);
> - drm_connector_cleanup(drm_connector);
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index e31d52d..8505569 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -761,10 +761,6 @@ static int sti_hda_bind(struct device *dev, struct
> device *master, void *data)
> drm_connector_helper_add(drm_connector,
> &sti_hda_connector_helper_funcs);
>
> - err = drm_connector_register(drm_connector);
> - if (err)
> - goto err_connector;
> -
> err = drm_mode_connector_attach_encoder(drm_connector, encoder);
> if (err) {
> DRM_ERROR("Failed to attach a connector to a encoder\n");
> @@ -777,9 +773,7 @@ static int sti_hda_bind(struct device *dev, struct device
> *master, void *data)
> return 0;
>
> err_sysfs:
> - drm_connector_unregister(drm_connector);
> -err_connector:
> - drm_connector_cleanup(drm_connector);
> + drm_bridge_remove(bridge);
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index 9d9c2c5..8d1402b 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -915,16 +915,6 @@ sti_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
> return connector_status_disconnected;
> }
>
> -static void sti_hdmi_connector_destroy(struct drm_connector *connector)
> -{
> - struct sti_hdmi_connector *hdmi_connector
> - = to_sti_hdmi_connector(connector);
> -
> - drm_connector_unregister(connector);
> - drm_connector_cleanup(connector);
> - kfree(hdmi_connector);
> -}
> -
> static void sti_hdmi_connector_init_property(struct drm_device *drm_dev,
> struct drm_connector *connector)
> {
> @@ -1024,7 +1014,7 @@ static int sti_hdmi_late_register(struct drm_connector
> *connector)
> static const struct drm_connector_funcs sti_hdmi_connector_funcs = {
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = sti_hdmi_connector_detect,
> - .destroy = sti_hdmi_connector_destroy,
> + .destroy = drm_connector_cleanup,
> .reset = drm_atomic_helper_connector_reset,
> .set_property = drm_atomic_helper_connector_set_property,
> .atomic_set_property = sti_hdmi_connector_set_property,
> @@ -1092,10 +1082,6 @@ static int sti_hdmi_bind(struct device *dev, struct
> device *master, void *data)
> /* initialise property */
> sti_hdmi_connector_init_property(drm_dev, drm_connector);
>
> - err = drm_connector_register(drm_connector);
> - if (err)
> - goto err_connector;
> -
> err = drm_mode_connector_attach_encoder(drm_connector, encoder);
> if (err) {
> DRM_ERROR("Failed to attach a connector to a encoder\n");
> @@ -1108,10 +1094,7 @@ static int sti_hdmi_bind(struct device *dev, struct
> device *master, void *data)
> return 0;
>
> err_sysfs:
> - drm_connector_unregister(drm_connector);
> -err_connector:
> - drm_connector_cleanup(drm_connector);
> -
> + drm_bridge_remove(bridge);
> return -EINVAL;
> }
>
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
