Re: [PATCH 04/11] drm/tinydrm: Remove tinydrm_display_pipe_init()

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Jan 20, 2019 at 12:43:11PM +0100, Noralf Trønnes wrote:
> Further strip down tinydrm.ko and switch to drm_simple_connector_create().
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  Documentation/gpu/tinydrm.rst               |   3 -
>  drivers/gpu/drm/tinydrm/core/Makefile       |   2 +-
>  drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 183 --------------------
>  drivers/gpu/drm/tinydrm/mipi-dbi.c          |  24 ++-
>  drivers/gpu/drm/tinydrm/repaper.c           |  16 +-
>  drivers/gpu/drm/tinydrm/st7586.c            |  19 +-
>  include/drm/tinydrm/tinydrm.h               |   9 -
>  7 files changed, 43 insertions(+), 213 deletions(-)
>  delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> 
> diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst
> index a913644bfc19..1ca726474af4 100644
> --- a/Documentation/gpu/tinydrm.rst
> +++ b/Documentation/gpu/tinydrm.rst
> @@ -17,9 +17,6 @@ Core functionality
>  .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>     :export:
>  
> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> -   :export:
> -
>  Additional helpers
>  ==================
>  
> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile
> index fb221e6f8885..bf2df7326df7 100644
> --- a/drivers/gpu/drm/tinydrm/core/Makefile
> +++ b/drivers/gpu/drm/tinydrm/core/Makefile
> @@ -1,3 +1,3 @@
> -tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
> +tinydrm-y := tinydrm-core.o tinydrm-helpers.o
>  
>  obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> deleted file mode 100644
> index 323564329535..000000000000
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> +++ /dev/null
> @@ -1,183 +0,0 @@
> -/*
> - * Copyright (C) 2016 Noralf Trønnes
> - *
> - * 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 <drm/drm_atomic_helper.h>
> -#include <drm/drm_crtc_helper.h>
> -#include <drm/drm_drv.h>
> -#include <drm/drm_gem_framebuffer_helper.h>
> -#include <drm/drm_modes.h>
> -#include <drm/drm_print.h>
> -#include <drm/tinydrm/tinydrm.h>
> -
> -struct tinydrm_connector {
> -	struct drm_connector base;
> -	struct drm_display_mode mode;
> -};
> -
> -static inline struct tinydrm_connector *
> -to_tinydrm_connector(struct drm_connector *connector)
> -{
> -	return container_of(connector, struct tinydrm_connector, base);
> -}
> -
> -static int tinydrm_connector_get_modes(struct drm_connector *connector)
> -{
> -	struct tinydrm_connector *tconn = to_tinydrm_connector(connector);
> -	struct drm_display_mode *mode;
> -
> -	mode = drm_mode_duplicate(connector->dev, &tconn->mode);
> -	if (!mode) {
> -		DRM_ERROR("Failed to duplicate mode\n");
> -		return 0;
> -	}
> -
> -	if (mode->name[0] == '\0')
> -		drm_mode_set_name(mode);
> -
> -	mode->type |= DRM_MODE_TYPE_PREFERRED;
> -	drm_mode_probed_add(connector, mode);
> -
> -	if (mode->width_mm) {
> -		connector->display_info.width_mm = mode->width_mm;
> -		connector->display_info.height_mm = mode->height_mm;
> -	}
> -
> -	return 1;
> -}
> -
> -static const struct drm_connector_helper_funcs tinydrm_connector_hfuncs = {
> -	.get_modes = tinydrm_connector_get_modes,
> -};
> -
> -static enum drm_connector_status
> -tinydrm_connector_detect(struct drm_connector *connector, bool force)
> -{
> -	if (drm_dev_is_unplugged(connector->dev))
> -		return connector_status_disconnected;
> -
> -	return connector->status;
> -}
> -
> -static void tinydrm_connector_destroy(struct drm_connector *connector)
> -{
> -	struct tinydrm_connector *tconn = to_tinydrm_connector(connector);
> -
> -	drm_connector_cleanup(connector);
> -	kfree(tconn);
> -}
> -
> -static const struct drm_connector_funcs tinydrm_connector_funcs = {
> -	.reset = drm_atomic_helper_connector_reset,
> -	.detect = tinydrm_connector_detect,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = tinydrm_connector_destroy,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -struct drm_connector *
> -tinydrm_connector_create(struct drm_device *drm,
> -			 const struct drm_display_mode *mode,
> -			 int connector_type)
> -{
> -	struct tinydrm_connector *tconn;
> -	struct drm_connector *connector;
> -	int ret;
> -
> -	tconn = kzalloc(sizeof(*tconn), GFP_KERNEL);
> -	if (!tconn)
> -		return ERR_PTR(-ENOMEM);
> -
> -	drm_mode_copy(&tconn->mode, mode);
> -	connector = &tconn->base;
> -
> -	drm_connector_helper_add(connector, &tinydrm_connector_hfuncs);
> -	ret = drm_connector_init(drm, connector, &tinydrm_connector_funcs,
> -				 connector_type);
> -	if (ret) {
> -		kfree(tconn);
> -		return ERR_PTR(ret);
> -	}
> -
> -	connector->status = connector_status_connected;
> -
> -	return connector;
> -}
> -
> -static int tinydrm_rotate_mode(struct drm_display_mode *mode,
> -			       unsigned int rotation)
> -{
> -	if (rotation == 0 || rotation == 180) {
> -		return 0;
> -	} else if (rotation == 90 || rotation == 270) {
> -		swap(mode->hdisplay, mode->vdisplay);
> -		swap(mode->hsync_start, mode->vsync_start);
> -		swap(mode->hsync_end, mode->vsync_end);
> -		swap(mode->htotal, mode->vtotal);
> -		swap(mode->width_mm, mode->height_mm);
> -		return 0;
> -	} else {
> -		return -EINVAL;
> -	}
> -}
> -
> -/**
> - * tinydrm_display_pipe_init - Initialize display pipe
> - * @tdev: tinydrm device
> - * @funcs: Display pipe functions
> - * @connector_type: Connector type
> - * @formats: Array of supported formats (DRM_FORMAT\_\*)
> - * @format_count: Number of elements in @formats
> - * @mode: Supported mode
> - * @rotation: Initial @mode rotation in degrees Counter Clock Wise
> - *
> - * This function sets up a &drm_simple_display_pipe with a &drm_connector that
> - * has one fixed &drm_display_mode which is rotated according to @rotation.
> - *
> - * Returns:
> - * Zero on success, negative error code on failure.
> - */
> -int
> -tinydrm_display_pipe_init(struct tinydrm_device *tdev,
> -			  const struct drm_simple_display_pipe_funcs *funcs,
> -			  int connector_type,
> -			  const uint32_t *formats,
> -			  unsigned int format_count,
> -			  const struct drm_display_mode *mode,
> -			  unsigned int rotation)
> -{
> -	struct drm_device *drm = tdev->drm;
> -	struct drm_display_mode mode_copy;
> -	struct drm_connector *connector;
> -	int ret;
> -	static const uint64_t modifiers[] = {
> -		DRM_FORMAT_MOD_LINEAR,
> -		DRM_FORMAT_MOD_INVALID
> -	};
> -
> -	drm_mode_copy(&mode_copy, mode);
> -	ret = tinydrm_rotate_mode(&mode_copy, rotation);
> -	if (ret) {
> -		DRM_ERROR("Illegal rotation value %u\n", rotation);
> -		return -EINVAL;
> -	}
> -
> -	drm->mode_config.min_width = mode_copy.hdisplay;
> -	drm->mode_config.max_width = mode_copy.hdisplay;
> -	drm->mode_config.min_height = mode_copy.vdisplay;
> -	drm->mode_config.max_height = mode_copy.vdisplay;
> -
> -	connector = tinydrm_connector_create(drm, &mode_copy, connector_type);
> -	if (IS_ERR(connector))
> -		return PTR_ERR(connector);
> -
> -	return drm_simple_display_pipe_init(drm, &tdev->pipe, funcs, formats,
> -					    format_count, modifiers, connector);
> -}
> -EXPORT_SYMBOL(tinydrm_display_pipe_init);
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index 918f77c7de34..d1d546f3a664 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -391,7 +391,13 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
>  {
>  	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
>  	struct tinydrm_device *tdev = &mipi->tinydrm;
> +	struct drm_connector *connector;
> +	struct drm_device *drm;
>  	int ret;
> +	static const uint64_t modifiers[] = {
> +		DRM_FORMAT_MOD_LINEAR,
> +		DRM_FORMAT_MOD_INVALID
> +	};
>  
>  	if (!mipi->command)
>  		return -EINVAL;
> @@ -406,18 +412,22 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
>  	if (ret)
>  		return ret;
>  
> -	/* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */
> -	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> -					DRM_MODE_CONNECTOR_VIRTUAL,
> -					mipi_dbi_formats,
> -					ARRAY_SIZE(mipi_dbi_formats), mode,
> -					rotation);
> +	drm = tdev->drm;
> +
> +	connector = drm_simple_connector_create(drm, DRM_MODE_CONNECTOR_VIRTUAL, mode, rotation);

You lost the todo here ... _VIRTUAL is kinda a pretty good lie, since this
is definitely not virtual hw :-)
-Daniel

> +	if (IS_ERR(connector))
> +		return PTR_ERR(connector);
> +
> +	ret = drm_simple_display_pipe_init(drm, &tdev->pipe, pipe_funcs,
> +					   mipi_dbi_formats, ARRAY_SIZE(mipi_dbi_formats),
> +					   modifiers, connector);
>  	if (ret)
>  		return ret;
>  
>  	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
>  
> -	tdev->drm->mode_config.preferred_depth = 16;
> +	drm_simple_connector_set_mode_config(connector);
> +	drm->mode_config.preferred_depth = 16;
>  	mipi->rotation = rotation;
>  
>  	drm_mode_config_reset(tdev->drm);
> diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
> index 72d30151ecd8..1d551744cc9b 100644
> --- a/drivers/gpu/drm/tinydrm/repaper.c
> +++ b/drivers/gpu/drm/tinydrm/repaper.c
> @@ -924,12 +924,14 @@ static int repaper_probe(struct spi_device *spi)
>  	const struct drm_display_mode *mode;
>  	const struct spi_device_id *spi_id;
>  	const struct of_device_id *match;
> +	struct drm_connector *connector;
>  	struct device *dev = &spi->dev;
>  	struct tinydrm_device *tdev;
>  	enum repaper_model model;
>  	const char *thermal_zone;
>  	struct repaper_epd *epd;
>  	size_t line_buffer_size;
> +	struct drm_device *drm;
>  	int ret;
>  
>  	match = of_match_device(repaper_of_match, dev);
> @@ -1069,13 +1071,19 @@ static int repaper_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs,
> -					DRM_MODE_CONNECTOR_VIRTUAL,
> -					repaper_formats,
> -					ARRAY_SIZE(repaper_formats), mode, 0);
> +	drm = tdev->drm;
> +
> +	connector = drm_simple_connector_create(drm, DRM_MODE_CONNECTOR_VIRTUAL, mode, 0);
> +	if (IS_ERR(connector))
> +		return PTR_ERR(connector);
> +
> +	ret = drm_simple_display_pipe_init(drm, &tdev->pipe, &repaper_pipe_funcs,
> +					   repaper_formats, ARRAY_SIZE(repaper_formats),
> +					   NULL, connector);
>  	if (ret)
>  		return ret;
>  
> +	drm_simple_connector_set_mode_config(connector);
>  	drm_mode_config_reset(tdev->drm);
>  	spi_set_drvdata(spi, tdev);
>  
> diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
> index 5ee7db561349..6cb68dd7e7b6 100644
> --- a/drivers/gpu/drm/tinydrm/st7586.c
> +++ b/drivers/gpu/drm/tinydrm/st7586.c
> @@ -271,6 +271,8 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
>  {
>  	size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;
>  	struct tinydrm_device *tdev = &mipi->tinydrm;
> +	struct drm_connector *connector;
> +	struct drm_device *drm;
>  	int ret;
>  
>  	mutex_init(&mipi->cmdlock);
> @@ -283,17 +285,22 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
>  	if (ret)
>  		return ret;
>  
> -	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> -					DRM_MODE_CONNECTOR_VIRTUAL,
> -					st7586_formats,
> -					ARRAY_SIZE(st7586_formats),
> -					mode, rotation);
> +	drm = tdev->drm;
> +
> +	connector = drm_simple_connector_create(drm, DRM_MODE_CONNECTOR_VIRTUAL, mode, rotation);
> +	if (IS_ERR(connector))
> +		return PTR_ERR(connector);
> +
> +	ret = drm_simple_display_pipe_init(drm, &tdev->pipe, pipe_funcs,
> +					   st7586_formats, ARRAY_SIZE(st7586_formats),
> +					   NULL, connector);
>  	if (ret)
>  		return ret;
>  
>  	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
>  
> -	tdev->drm->mode_config.preferred_depth = 32;
> +	drm_simple_connector_set_mode_config(connector);
> +	drm->mode_config.preferred_depth = 32;
>  	mipi->rotation = rotation;
>  
>  	drm_mode_config_reset(tdev->drm);
> diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
> index 87e7f9b93a37..69c4363fd88e 100644
> --- a/include/drm/tinydrm/tinydrm.h
> +++ b/include/drm/tinydrm/tinydrm.h
> @@ -40,13 +40,4 @@ int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
>  int devm_tinydrm_register(struct tinydrm_device *tdev);
>  void tinydrm_shutdown(struct tinydrm_device *tdev);
>  
> -int
> -tinydrm_display_pipe_init(struct tinydrm_device *tdev,
> -			  const struct drm_simple_display_pipe_funcs *funcs,
> -			  int connector_type,
> -			  const uint32_t *formats,
> -			  unsigned int format_count,
> -			  const struct drm_display_mode *mode,
> -			  unsigned int rotation);
> -
>  #endif /* __LINUX_TINYDRM_H */
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel