Re: [PATCH 09/11] drm/tinydrm: Remove tinydrm_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Den 21.01.2019 10.29, skrev Daniel Vetter:
> On Sun, Jan 20, 2019 at 12:43:16PM +0100, Noralf Trønnes wrote:
>> No more users left so it can go alongside its helpers.
>> Update the tinydrm docs description and remove todo entry.
>>
>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
>> ---
>>  Documentation/gpu/tinydrm.rst                 |  26 +--
>>  Documentation/gpu/todo.rst                    |   4 -
>>  drivers/gpu/drm/tinydrm/core/Makefile         |   2 +-
>>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c   | 169 ------------------
>>  .../gpu/drm/tinydrm/core/tinydrm-helpers.c    |   2 +
>>  include/drm/tinydrm/tinydrm.h                 |  42 -----
>>  6 files changed, 10 insertions(+), 235 deletions(-)
>>  delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>>  delete mode 100644 include/drm/tinydrm/tinydrm.h
> 
> Looks great. Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

I'm glad that I've finally come to the point where I'm able to get rid
of the midlayer smell that you have talked about :-)

>>
>> diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst
>> index 1ca726474af4..19969b989efb 100644
>> --- a/Documentation/gpu/tinydrm.rst
>> +++ b/Documentation/gpu/tinydrm.rst
>> @@ -1,24 +1,12 @@
>> -==========================
>> -drm/tinydrm Driver library
>> -==========================
>> +============================
>> +drm/tinydrm Tiny DRM drivers
>> +============================
>>  
>> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>> -   :doc: overview
>> +tinydrm is a collection of DRM drivers that are so small they can fit in a
>> +single source file.
> 
> Since it's now just a collection of tiny drivers I think we should also
> rename the directory. Maybe in a follow-up patch, once this has all
> settled. I think just /tiny/ or /tinydrivers/ or so would be better.
>>  
>> -Core functionality
>> -==================
>> -
>> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>> -   :doc: core
>> -
>> -.. kernel-doc:: include/drm/tinydrm/tinydrm.h
>> -   :internal:
>> -
>> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>> -   :export:
> 
> Yay!
> 
>> -
>> -Additional helpers
>> -==================
>> +Helpers
>> +=======
>>  
>>  .. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h
>>     :internal:
> 
> Out of curiosity, what's left? From a quick look I think we could move the
> memcp/swab helpers into a drm_framebuffer_helper.c file (maybe move the fb
> argument first, since that's the main thing for ocd).

Yep.

> 
> And the spi stuff is kinda just spi helpers I guess? Could we move those
> into the spi subsystem, or was that idea already nacked?

Meghana started on this, but didn't finish. I don't think there was any
blockers, so it's just to pick it up where she left it.

There's also the mipi_dbi helper which should be moved along side
drm_mipi_dsi.

My plan is to do this when I'm done with the modesetting part of drm_client.

Noralf.

> 
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 38360ede1221..3495aec7a8d4 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -435,10 +435,6 @@ those drivers as simple as possible, so lots of room for refactoring:
>>    one of the ideas for having a shared dsi/dbi helper, abstracting away the
>>    transport details more.
>>  
>> -- Quick aside: The unregister devm stuff is kinda getting the lifetimes of
>> -  a drm_device wrong. Doesn't matter, since everyone else gets it wrong
>> -  too :-)
> 
> Hey I even wrote this already :-)
> 
>> -
>>  Contact: Noralf Trønnes, Daniel Vetter
>>  
>>  AMD DC Display Driver
>> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile
>> index bf2df7326df7..f88ea7ad302f 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-helpers.o
>> +tinydrm-y := tinydrm-helpers.o
>>  
>>  obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
>> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>> deleted file mode 100644
>> index e4a77feaacd6..000000000000
>> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>> +++ /dev/null
>> @@ -1,169 +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.h>
>> -#include <drm/drm_atomic_helper.h>
>> -#include <drm/drm_crtc_helper.h>
>> -#include <drm/drm_drv.h>
>> -#include <drm/drm_fb_helper.h>
>> -#include <drm/drm_gem_framebuffer_helper.h>
>> -#include <drm/drm_print.h>
>> -#include <drm/tinydrm/tinydrm.h>
>> -#include <linux/device.h>
>> -#include <linux/dma-buf.h>
>> -#include <linux/module.h>
>> -
>> -/**
>> - * DOC: overview
>> - *
>> - * This library provides driver helpers for very simple display hardware.
>> - *
>> - * It is based on &drm_simple_display_pipe coupled with a &drm_connector which
>> - * has only one fixed &drm_display_mode. The framebuffers are backed by the
>> - * cma helper and have support for framebuffer flushing (dirty).
>> - * fbdev support is also included.
>> - *
>> - */
>> -
>> -/**
>> - * DOC: core
>> - *
>> - * The driver allocates &tinydrm_device, initializes it using
>> - * devm_tinydrm_init(), sets up the pipeline using tinydrm_display_pipe_init()
>> - * and registers the DRM device using devm_tinydrm_register().
>> - */
>> -
>> -static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
>> -	.fb_create = drm_gem_fb_create_with_dirty,
>> -	.atomic_check = drm_atomic_helper_check,
>> -	.atomic_commit = drm_atomic_helper_commit,
>> -};
>> -
>> -static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
>> -			struct drm_driver *driver)
>> -{
>> -	struct drm_device *drm;
>> -
>> -	/*
>> -	 * We don't embed drm_device, because that prevent us from using
>> -	 * devm_kzalloc() to allocate tinydrm_device in the driver since
>> -	 * drm_dev_put() frees the structure. The devm_ functions provide
>> -	 * for easy error handling.
>> -	 */
>> -	drm = drm_dev_alloc(driver, parent);
>> -	if (IS_ERR(drm))
>> -		return PTR_ERR(drm);
>> -
>> -	tdev->drm = drm;
>> -	drm->dev_private = tdev;
>> -	drm_mode_config_init(drm);
>> -	drm->mode_config.funcs = &tinydrm_mode_config_funcs;
>> -	drm->mode_config.allow_fb_modifiers = true;
>> -
>> -	return 0;
>> -}
>> -
>> -static void tinydrm_fini(struct tinydrm_device *tdev)
>> -{
>> -	drm_mode_config_cleanup(tdev->drm);
>> -	tdev->drm->dev_private = NULL;
>> -	drm_dev_put(tdev->drm);
>> -}
>> -
>> -static void devm_tinydrm_release(void *data)
>> -{
>> -	tinydrm_fini(data);
>> -}
>> -
>> -/**
>> - * devm_tinydrm_init - Initialize tinydrm device
>> - * @parent: Parent device object
>> - * @tdev: tinydrm device
>> - * @driver: DRM driver
>> - *
>> - * This function initializes @tdev, the underlying DRM device and it's
>> - * mode_config. Resources will be automatically freed on driver detach (devres)
>> - * using drm_mode_config_cleanup() and drm_dev_put().
>> - *
>> - * Returns:
>> - * Zero on success, negative error code on failure.
>> - */
>> -int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
>> -		      struct drm_driver *driver)
>> -{
>> -	int ret;
>> -
>> -	ret = tinydrm_init(parent, tdev, driver);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = devm_add_action(parent, devm_tinydrm_release, tdev);
>> -	if (ret)
>> -		tinydrm_fini(tdev);
>> -
>> -	return ret;
>> -}
>> -EXPORT_SYMBOL(devm_tinydrm_init);
>> -
>> -static int tinydrm_register(struct tinydrm_device *tdev)
>> -{
>> -	struct drm_device *drm = tdev->drm;
>> -	int ret;
>> -
>> -	ret = drm_dev_register(tdev->drm, 0);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = drm_fbdev_generic_setup(drm, 0);
>> -	if (ret)
>> -		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
>> -
>> -	return 0;
>> -}
>> -
>> -static void tinydrm_unregister(struct tinydrm_device *tdev)
>> -{
>> -	drm_atomic_helper_shutdown(tdev->drm);
>> -	drm_dev_unregister(tdev->drm);
>> -}
>> -
>> -static void devm_tinydrm_register_release(void *data)
>> -{
>> -	tinydrm_unregister(data);
>> -}
>> -
>> -/**
>> - * devm_tinydrm_register - Register tinydrm device
>> - * @tdev: tinydrm device
>> - *
>> - * This function registers the underlying DRM device and fbdev.
>> - * These resources will be automatically unregistered on driver detach (devres)
>> - * and the display pipeline will be disabled.
>> - *
>> - * Returns:
>> - * Zero on success, negative error code on failure.
>> - */
>> -int devm_tinydrm_register(struct tinydrm_device *tdev)
>> -{
>> -	struct device *dev = tdev->drm->dev;
>> -	int ret;
>> -
>> -	ret = tinydrm_register(tdev);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = devm_add_action(dev, devm_tinydrm_register_release, tdev);
>> -	if (ret)
>> -		tinydrm_unregister(tdev);
>> -
>> -	return ret;
>> -}
>> -EXPORT_SYMBOL(devm_tinydrm_register);
>> -
>> -MODULE_LICENSE("GPL");
>> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> index 2737b6fdadc8..d7b38dfb6438 100644
>> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> @@ -365,3 +365,5 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
>>  EXPORT_SYMBOL(tinydrm_spi_transfer);
>>  
>>  #endif /* CONFIG_SPI */
>> +
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
>> deleted file mode 100644
>> index ee9b17759391..000000000000
>> --- a/include/drm/tinydrm/tinydrm.h
>> +++ /dev/null
>> @@ -1,42 +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.
>> - */
>> -
>> -#ifndef __LINUX_TINYDRM_H
>> -#define __LINUX_TINYDRM_H
>> -
>> -#include <drm/drm_simple_kms_helper.h>
>> -
>> -struct drm_driver;
>> -
>> -/**
>> - * struct tinydrm_device - tinydrm device
>> - */
>> -struct tinydrm_device {
>> -	/**
>> -	 * @drm: DRM device
>> -	 */
>> -	struct drm_device *drm;
>> -
>> -	/**
>> -	 * @pipe: Display pipe structure
>> -	 */
>> -	struct drm_simple_display_pipe pipe;
>> -};
>> -
>> -static inline struct tinydrm_device *
>> -pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
>> -{
>> -	return container_of(pipe, struct tinydrm_device, pipe);
>> -}
>> -
>> -int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
>> -		      struct drm_driver *driver);
>> -int devm_tinydrm_register(struct tinydrm_device *tdev);
>> -
>> -#endif /* __LINUX_TINYDRM_H */
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux