Re: [PATCH RFC v1 2/6] drm/sprd: add Unisoc's drm kms master

From: Thomas Zimmermann
Date: Tue Jan 07 2020 - 07:23:05 EST

Next message: Florian Weimer: "Re: [RFC PATCH glibc 09/13] glibc: Perform rseq(2) registration at C startup and thread creation (v13)"
Previous message: Thomas Richter: "[PATCH] perf test: Fix test case Merge cpu map"
In reply to: Thomas Zimmermann: "Re: [PATCH RFC v1 2/6] drm/sprd: add Unisoc's drm kms master"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi

Am 07.01.20 um 12:36 schrieb tang pengchuan:
> Hi Thomas,
> Our soc needs to support both cma and iommu, but our iommu is not ready
> for upload, so i remove it from sprd_gem.c
> SoÂcan i upload the cma code firstï and add iommu support later

This might be possible, but I cannot make the decision. I'd suggest a
different strategy.

1) Convert to generic CMA handlers and submit. You'd want the least
amount of code in your patches, so they go in quickly and easily.

2) Duplicate CMA handlers into your driver and add IOMMU support.
Submit this patchset at a later point when it's ready.

Just an untested idea: Is it a generic IOMMU or a GART? With a generic
IOMMU it could be possible to implement dma_map_ops for your platform.
CMA helpers will use the IOMMU in their call to dma_alloc_wc(). [1] If
this works, you would not have to maintain you own GEM code and can
merge IOMMU support at any time.

Best regards
Thomas

[1] https://elixir.bootlin.com/linux/latest/source/kernel/dma/mapping.c#L298

>
> On Mon, Jan 6, 2020 at 6:11 PM Thomas Zimmermann <tzimmermann@xxxxxxx
> <mailto:tzimmermann@xxxxxxx>> wrote:
>
> Hi Kevin
>
> Am 16.12.19 um 12:40 schrieb Kevin Tang:
> > From: Kevin Tang <kevin.tang@xxxxxxxxxx
> <mailto:kevin.tang@xxxxxxxxxx>>
> >
> > Adds drm support for the Unisoc's display subsystem.
> >
> > This is drm device and gem driver. This driver provides support
> for the
> > Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> >
> > Cc: Orson Zhai <orsonzhai@xxxxxxxxx <mailto:orsonzhai@xxxxxxxxx>>
> > Cc: Baolin Wang <baolin.wang@xxxxxxxxxx
> <mailto:baolin.wang@xxxxxxxxxx>>
> > Cc: Chunyan Zhang <zhang.lyra@xxxxxxxxx <mailto:zhang.lyra@xxxxxxxxx>>
> > Signed-off-by: Kevin Tang <kevin.tang@xxxxxxxxxx
> <mailto:kevin.tang@xxxxxxxxxx>>
> > ---
> >Â drivers/gpu/drm/KconfigÂ Â Â Â Â|Â Â2 +
> >Â drivers/gpu/drm/MakefileÂ Â Â Â |Â Â1 +
> >Â drivers/gpu/drm/sprd/KconfigÂ Â |Â 14 ++
> >Â drivers/gpu/drm/sprd/MakefileÂ Â|Â Â8 ++
> >Â drivers/gpu/drm/sprd/sprd_drm.c | 286
> ++++++++++++++++++++++++++++++++++++++++
> >Â drivers/gpu/drm/sprd/sprd_drm.h |Â 16 +++
> >Â drivers/gpu/drm/sprd/sprd_gem.c | 178 +++++++++++++++++++++++++
> >Â drivers/gpu/drm/sprd/sprd_gem.h |Â 30 +++++
> >Â 8 files changed, 535 insertions(+)
> >Â create mode 100644 drivers/gpu/drm/sprd/Kconfig
> >Â create mode 100644 drivers/gpu/drm/sprd/Makefile
> >Â create mode 100644 drivers/gpu/drm/sprd/sprd_drm.c
> >Â create mode 100644 drivers/gpu/drm/sprd/sprd_drm.h
> >Â create mode 100644 drivers/gpu/drm/sprd/sprd_gem.c
> >Â create mode 100644 drivers/gpu/drm/sprd/sprd_gem.h
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index bfdadc3..cead12c 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -387,6 +387,8 @@ source "drivers/gpu/drm/aspeed/Kconfig"
> >Â
> >Â source "drivers/gpu/drm/mcde/Kconfig"
> >Â
> > +source "drivers/gpu/drm/sprd/Kconfig"
> > +
> >Â # Keep legacy drivers last
> >Â
> >Â menuconfig DRM_LEGACY
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 9f1c7c4..85ca211 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -122,3 +122,4 @@ obj-$(CONFIG_DRM_LIMA)Â += lima/
> >Â obj-$(CONFIG_DRM_PANFROST) += panfrost/
> >Â obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
> >Â obj-$(CONFIG_DRM_MCDE) += mcde/
> > +obj-$(CONFIG_DRM_SPRD) += sprd/
> > diff --git a/drivers/gpu/drm/sprd/Kconfig
> b/drivers/gpu/drm/sprd/Kconfig
> > new file mode 100644
> > index 0000000..79f286b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/Kconfig
> > @@ -0,0 +1,14 @@
> > +config DRM_SPRD
> > +Â Â Âtristate "DRM Support for Unisoc SoCs Platform"
> > +Â Â Âdepends on ARCH_SPRD
> > +Â Â Âdepends on DRM && OF
> > +Â Â Âselect DRM_KMS_HELPER
> > +Â Â Âselect DRM_GEM_CMA_HELPER
> > +Â Â Âselect DRM_KMS_CMA_HELPER
> > +Â Â Âselect DRM_MIPI_DSI
> > +Â Â Âselect DRM_PANEL
> > +Â Â Âselect VIDEOMODE_HELPERS
> > +Â Â Âselect BACKLIGHT_CLASS_DEVICE
> > +Â Â Âhelp
> > +Â Â Â ÂChoose this option if you have a Unisoc chipsets.
> > +Â Â Â ÂIf M is selected the module will be called sprd-drm.
> > \ No newline at end of file
> > diff --git a/drivers/gpu/drm/sprd/Makefile
> b/drivers/gpu/drm/sprd/Makefile
> > new file mode 100644
> > index 0000000..df0b316
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/Makefile
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +ccflags-y += -Iinclude/drm
> > +
> > +subdir-ccflags-y += -I$(src)
> > +
> > +obj-y := sprd_drm.o \
> > +Â Â Âsprd_gem.o
> > \ No newline at end of file
> > diff --git a/drivers/gpu/drm/sprd/sprd_drm.c
> b/drivers/gpu/drm/sprd/sprd_drm.c
> > new file mode 100644
> > index 0000000..4aee25fa4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/sprd_drm.c
> > @@ -0,0 +1,286 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 Unisoc Inc.
> > + */
> > +
> > +#include <linux/component.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/of_platform.h>
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_gem_cma_helper.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/drm_vblank.h>
> > +
> > +#include "sprd_drm.h"
> > +#include "sprd_gem.h"
> > +
> > +#define DRIVER_NAMEÂ "sprd"
> > +#define DRIVER_DESCÂ "Spreadtrum SoCs' DRM Driver"
> > +#define DRIVER_DATEÂ "20191101"
> > +#define DRIVER_MAJOR 1
> > +#define DRIVER_MINOR 0
> > +
> > +static const struct drm_mode_config_helper_funcs
> sprd_drm_mode_config_helper = {
> > +Â Â Â.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> > +};
> > +
> > +static const struct drm_mode_config_funcs
> sprd_drm_mode_config_funcs = {
> > +Â Â Â.fb_create = drm_gem_fb_create,
> > +Â Â Â.atomic_check = drm_atomic_helper_check,
> > +Â Â Â.atomic_commit = drm_atomic_helper_commit,
> > +};
> > +
> > +static void sprd_drm_mode_config_init(struct drm_device *drm)
> > +{
> > +Â Â Âdrm_mode_config_init(drm);
> > +
> > +Â Â Âdrm->mode_config.min_width = 0;
> > +Â Â Âdrm->mode_config.min_height = 0;
> > +Â Â Âdrm->mode_config.max_width = 8192;
> > +Â Â Âdrm->mode_config.max_height = 8192;
> > +Â Â Âdrm->mode_config.allow_fb_modifiers = true;
> > +
> > +Â Â Âdrm->mode_config.funcs = &sprd_drm_mode_config_funcs;
> > +Â Â Âdrm->mode_config.helper_private = &sprd_drm_mode_config_helper;
> > +}
> > +
> > +static const struct file_operations sprd_drm_fops = {
> > +Â Â Â.ownerÂ Â Â Â Â = THIS_MODULE,
> > +Â Â Â.openÂ Â Â Â Â Â= drm_open,
> > +Â Â Â.releaseÂ Â Â Â = drm_release,
> > +Â Â Â.unlocked_ioctl = drm_ioctl,
> > +Â Â Â.compat_ioctlÂ Â= drm_compat_ioctl,
> > +Â Â Â.pollÂ Â Â Â Â Â= drm_poll,
> > +Â Â Â.readÂ Â Â Â Â Â= drm_read,
> > +Â Â Â.llseekÂ Â Â Â Â= no_llseek,
> > +Â Â Â.mmapÂ Â Â Â Â Â= sprd_gem_mmap,
> > +};
> > +
> > +static struct drm_driver sprd_drm_drv = {
> > +Â Â Â.driver_featuresÂ Â Â Â = DRIVER_GEM | DRIVER_ATOMIC,
> > +Â Â Â.fopsÂ Â Â Â Â Â Â Â Â Â= &sprd_drm_fops,
> > +Â Â Â.gem_vm_opsÂ Â Â Â Â Â Â= &drm_gem_cma_vm_ops,
> > +Â Â Â.gem_free_object_unlockedÂ Â Â Â= sprd_gem_free_object,
> > +Â Â Â.dumb_createÂ Â Â Â Â Â = sprd_gem_dumb_create,
> > +Â Â Â.prime_fd_to_handleÂ Â Â= drm_gem_prime_fd_to_handle,
> > +Â Â Â.gem_prime_importÂ Â Â Â= drm_gem_prime_import,
> > +Â Â Â.gem_prime_import_sg_table = sprd_gem_prime_import_sg_table,
> > +
> > +Â Â Â.nameÂ Â Â Â Â Â Â Â Â Â= DRIVER_NAME,
> > +Â Â Â.descÂ Â Â Â Â Â Â Â Â Â= DRIVER_DESC,
> > +Â Â Â.dateÂ Â Â Â Â Â Â Â Â Â= DRIVER_DATE,
> > +Â Â Â.majorÂ Â Â Â Â Â Â Â Â = DRIVER_MAJOR,
> > +Â Â Â.minorÂ Â Â Â Â Â Â Â Â = DRIVER_MINOR,
> > +};
> > +
> > +static int sprd_drm_bind(struct device *dev)
> > +{
> > +Â Â Âstruct drm_device *drm;
> > +Â Â Âstruct sprd_drm *sprd;
> > +Â Â Âint err;
> > +
> > +Â Â Âdrm = drm_dev_alloc(&sprd_drm_drv, dev);
> > +Â Â Âif (IS_ERR(drm))
> > +Â Â Â Â Â Â Âreturn PTR_ERR(drm);
> > +
> > +Â Â Âdev_set_drvdata(dev, drm);
> > +
> > +Â Â Âsprd = devm_kzalloc(drm->dev, sizeof(*sprd), GFP_KERNEL);
> > +Â Â Âif (!sprd) {
> > +Â Â Â Â Â Â Âerr = -ENOMEM;
> > +Â Â Â Â Â Â Âgoto err_free_drm;
> > +Â Â Â}
> > +Â Â Âdrm->dev_private = sprd;
> > +
> > +Â Â Âsprd_drm_mode_config_init(drm);
> > +
> > +Â Â Â/* bind and init sub drivers */
> > +Â Â Âerr = component_bind_all(drm->dev, drm);
> > +Â Â Âif (err) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("failed to bind all component.\n");
> > +Â Â Â Â Â Â Âgoto err_dc_cleanup;
> > +Â Â Â}
> > +
> > +Â Â Â/* vblank init */
> > +Â Â Âerr = drm_vblank_init(drm, drm->mode_config.num_crtc);
> > +Â Â Âif (err) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("failed to initialize vblank.\n");
> > +Â Â Â Â Â Â Âgoto err_unbind_all;
> > +Â Â Â}
> > +Â Â Â/* with irq_enabled = true, we can use the vblank feature. */
> > +Â Â Âdrm->irq_enabled = true;
> > +
> > +Â Â Â/* reset all the states of crtc/plane/encoder/connector */
> > +Â Â Âdrm_mode_config_reset(drm);
> > +
> > +Â Â Â/* init kms poll for handling hpd */
> > +Â Â Âdrm_kms_helper_poll_init(drm);
> > +
> > +Â Â Âerr = drm_dev_register(drm, 0);
> > +Â Â Âif (err < 0)
> > +Â Â Â Â Â Â Âgoto err_kms_helper_poll_fini;
> > +
> > +Â Â Âreturn 0;
> > +
> > +err_kms_helper_poll_fini:
> > +Â Â Âdrm_kms_helper_poll_fini(drm);
> > +err_unbind_all:
> > +Â Â Âcomponent_unbind_all(drm->dev, drm);
> > +err_dc_cleanup:
> > +Â Â Âdrm_mode_config_cleanup(drm);
> > +err_free_drm:
> > +Â Â Âdrm_dev_put(drm);
> > +Â Â Âreturn err;
> > +}
> > +
> > +static void sprd_drm_unbind(struct device *dev)
> > +{
> > +Â Â Âdrm_put_dev(dev_get_drvdata(dev));
> > +}
> > +
> > +static const struct component_master_ops sprd_drm_component_ops = {
> > +Â Â Â.bind = sprd_drm_bind,
> > +Â Â Â.unbind = sprd_drm_unbind,
> > +};
> > +
> > +static int compare_of(struct device *dev, void *data)
> > +{
> > +Â Â Âstruct device_node *np = data;
> > +
> > +Â Â ÂDRM_DEBUG("compare %s\n", np->full_name);
> > +
> > +Â Â Âreturn dev->of_node == np;
> > +}
> > +
> > +static int sprd_drm_component_probe(struct device *dev,
> > +Â Â Â Â Â Â Â Â Â Â Â Â const struct component_master_ops *m_ops)
> > +{
> > +Â Â Âstruct device_node *ep, *port, *remote;
> > +Â Â Âstruct component_match *match = NULL;
> > +Â Â Âint i;
> > +
> > +Â Â Âif (!dev->of_node)
> > +Â Â Â Â Â Â Âreturn -EINVAL;
> > +
> > +Â Â Â/*
> > +Â Â Â * Bind the crtc's ports first, so that
> drm_of_find_possible_crtcs()
> > +Â Â Â * called from encoder's .bind callbacks works as expected
> > +Â Â Â */
> > +Â Â Âfor (i = 0; ; i++) {
> > +Â Â Â Â Â Â Âport = of_parse_phandle(dev->of_node, "ports", i);
> > +Â Â Â Â Â Â Âif (!port)
> > +Â Â Â Â Â Â Â Â Â Â Âbreak;
> > +
> > +Â Â Â Â Â Â Âif (!of_device_is_available(port->parent)) {
> > +Â Â Â Â Â Â Â Â Â Â Âof_node_put(port);
> > +Â Â Â Â Â Â Â Â Â Â Âcontinue;
> > +Â Â Â Â Â Â Â}
> > +
> > +Â Â Â Â Â Â Âcomponent_match_add(dev, &match, compare_of,
> port->parent);
> > +Â Â Â Â Â Â Âof_node_put(port);
> > +Â Â Â}
> > +
> > +Â Â Âif (i == 0) {
> > +Â Â Â Â Â Â Âdev_err(dev, "missing 'ports' property\n");
> > +Â Â Â Â Â Â Âreturn -ENODEV;
> > +Â Â Â}
> > +
> > +Â Â Âif (!match) {
> > +Â Â Â Â Â Â Âdev_err(dev, "no available port\n");
> > +Â Â Â Â Â Â Âreturn -ENODEV;
> > +Â Â Â}
> > +
> > +Â Â Â/*
> > +Â Â Â * For bound crtcs, bind the encoders attached to their
> remote endpoint
> > +Â Â Â */
> > +Â Â Âfor (i = 0; ; i++) {
> > +Â Â Â Â Â Â Âport = of_parse_phandle(dev->of_node, "ports", i);
> > +Â Â Â Â Â Â Âif (!port)
> > +Â Â Â Â Â Â Â Â Â Â Âbreak;
> > +
> > +Â Â Â Â Â Â Âif (!of_device_is_available(port->parent)) {
> > +Â Â Â Â Â Â Â Â Â Â Âof_node_put(port);
> > +Â Â Â Â Â Â Â Â Â Â Âcontinue;
> > +Â Â Â Â Â Â Â}
> > +
> > +Â Â Â Â Â Â Âfor_each_child_of_node(port, ep) {
> > +Â Â Â Â Â Â Â Â Â Â Âremote = of_graph_get_remote_port_parent(ep);
> > +Â Â Â Â Â Â Â Â Â Â Âif (!remote ||
> !of_device_is_available(remote)) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âof_node_put(remote);
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
> > +Â Â Â Â Â Â Â Â Â Â Â} else if
> (!of_device_is_available(remote->parent)) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdev_warn(dev, "parent device of %s
> is not available\n",
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â remote->full_name);
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âof_node_put(remote);
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
> > +Â Â Â Â Â Â Â Â Â Â Â}
> > +
> > +Â Â Â Â Â Â Â Â Â Â Âcomponent_match_add(dev, &match, compare_of,
> remote);
> > +Â Â Â Â Â Â Â Â Â Â Âof_node_put(remote);
> > +Â Â Â Â Â Â Â}
> > +Â Â Â Â Â Â Âof_node_put(port);
> > +Â Â Â}
> > +
> > +Â Â Âreturn component_master_add_with_match(dev, m_ops, match);
> > +}
> > +
> > +static int sprd_drm_probe(struct platform_device *pdev)
> > +{
> > +Â Â Âint ret;
> > +
> > +Â Â Âret = dma_set_mask_and_coherent(&pdev->dev, ~0);
> > +Â Â Âif (ret) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("dma_set_mask_and_coherent failed (%d)\n",
> ret);
> > +Â Â Â Â Â Â Âreturn ret;
> > +Â Â Â}
> > +
> > +Â Â Âreturn sprd_drm_component_probe(&pdev->dev,
> &sprd_drm_component_ops);
> > +}
> > +
> > +static int sprd_drm_remove(struct platform_device *pdev)
> > +{
> > +Â Â Âcomponent_master_del(&pdev->dev, &sprd_drm_component_ops);
> > +Â Â Âreturn 0;
> > +}
> > +
> > +static void sprd_drm_shutdown(struct platform_device *pdev)
> > +{
> > +Â Â Âstruct drm_device *drm = platform_get_drvdata(pdev);
> > +
> > +Â Â Âif (!drm) {
> > +Â Â Â Â Â Â ÂDRM_WARN("drm device is not available, no shutdown\n");
> > +Â Â Â Â Â Â Âreturn;
> > +Â Â Â}
> > +
> > +Â Â Âdrm_atomic_helper_shutdown(drm);
> > +}
> > +
> > +static const struct of_device_id sprd_drm_match_table[] = {
> > +Â Â Â{ .compatible = "sprd,display-subsystem",},
> > +Â Â Â{},
> > +};
> > +MODULE_DEVICE_TABLE(of, sprd_drm_match_table);
> > +
> > +static struct platform_driver sprd_drm_driver = {
> > +Â Â Â.probe = sprd_drm_probe,
> > +Â Â Â.remove = sprd_drm_remove,
> > +Â Â Â.shutdown = sprd_drm_shutdown,
> > +Â Â Â.driver = {
> > +Â Â Â Â Â Â Â.name = "sprd-drm-drv",
> > +Â Â Â Â Â Â Â.of_match_table = sprd_drm_match_table,
> > +Â Â Â},
> > +};
> > +
> > +module_platform_driver(sprd_drm_driver);
> > +
> > +MODULE_AUTHOR("Leon He <leon.he@xxxxxxxxxx
> <mailto:leon.he@xxxxxxxxxx>>");
> > +MODULE_AUTHOR("Kevin Tang <kevin.tang@xxxxxxxxxx
> <mailto:kevin.tang@xxxxxxxxxx>>");
> > +MODULE_DESCRIPTION("Unisoc DRM KMS Master Driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/gpu/drm/sprd/sprd_drm.h
> b/drivers/gpu/drm/sprd/sprd_drm.h
> > new file mode 100644
> > index 0000000..137cb27
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/sprd_drm.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019 Unisoc Inc.
> > + */
> > +
> > +#ifndef _SPRD_DRM_H_
> > +#define _SPRD_DRM_H_
> > +
> > +#include <drm/drm_atomic.h>
> > +#include <drm/drm_print.h>
> > +
> > +struct sprd_drm {
> > +Â Â Âstruct drm_device *drm;
> > +};
> > +
> > +#endif /* _SPRD_DRM_H_ */
> > diff --git a/drivers/gpu/drm/sprd/sprd_gem.c
> b/drivers/gpu/drm/sprd/sprd_gem.c
> > new file mode 100644
> > index 0000000..9aec78e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/sprd_gem.c
> > @@ -0,0 +1,178 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 Unisoc Inc.
> > + */
> > +
> > +#include <linux/dma-buf.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include <drm/drm_prime.h>
> > +
> > +#include "sprd_drm.h"
> > +#include "sprd_gem.h"
> > +
> > +static struct sprd_gem_obj *sprd_gem_obj_create(struct drm_device
> *drm,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long size)
> > +{
> > +Â Â Âstruct sprd_gem_obj *sprd_gem;
> > +Â Â Âint ret;
> > +
> > +Â Â Âsprd_gem = kzalloc(sizeof(*sprd_gem), GFP_KERNEL);
> > +Â Â Âif (!sprd_gem)
> > +Â Â Â Â Â Â Âreturn ERR_PTR(-ENOMEM);
> > +
> > +Â Â Âret = drm_gem_object_init(drm, &sprd_gem->base, size);
> > +Â Â Âif (ret < 0) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("failed to initialize gem object\n");
> > +Â Â Â Â Â Â Âgoto error;
> > +Â Â Â}
> > +
> > +Â Â Âret = drm_gem_create_mmap_offset(&sprd_gem->base);
> > +Â Â Âif (ret) {
> > +Â Â Â Â Â Â Âdrm_gem_object_release(&sprd_gem->base);
> > +Â Â Â Â Â Â Âgoto error;
> > +Â Â Â}
> > +
> > +Â Â Âreturn sprd_gem;
> > +
> > +error:
> > +Â Â Âkfree(sprd_gem);
> > +Â Â Âreturn ERR_PTR(ret);
> > +}
> > +
> > +void sprd_gem_free_object(struct drm_gem_object *obj)
> > +{
> > +Â Â Âstruct sprd_gem_obj *sprd_gem = to_sprd_gem_obj(obj);
> > +
> > +Â Â ÂDRM_DEBUG("gem = %p\n", obj);
> > +
> > +Â Â Âif (sprd_gem->vaddr)
> > +Â Â Â Â Â Â Âdma_free_wc(obj->dev->dev, obj->size,
> > +Â Â Â Â Â Â Â Â Â Â Âsprd_gem->vaddr, sprd_gem->dma_addr);
> > +Â Â Âelse if (sprd_gem->sgtb)
> > +Â Â Â Â Â Â Âdrm_prime_gem_destroy(obj, sprd_gem->sgtb);
> > +
> > +Â Â Âdrm_gem_object_release(obj);
> > +
> > +Â Â Âkfree(sprd_gem);
> > +}
> > +
> > +int sprd_gem_dumb_create(struct drm_file *file_priv, struct
> drm_device *drm,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Âstruct drm_mode_create_dumb *args)
> > +{
> > +Â Â Âstruct sprd_gem_obj *sprd_gem;
> > +Â Â Âint ret;
> > +
> > +Â Â Âargs->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> > +Â Â Âargs->size = round_up(args->pitch * args->height, PAGE_SIZE);
> > +
> > +Â Â Âsprd_gem = sprd_gem_obj_create(drm, args->size);
> > +Â Â Âif (IS_ERR(sprd_gem))
> > +Â Â Â Â Â Â Âreturn PTR_ERR(sprd_gem);
> > +
> > +Â Â Âsprd_gem->vaddr = dma_alloc_wc(drm->dev, args->size,
> > +Â Â Â Â Â Â Â Â Â Â Â&sprd_gem->dma_addr, GFP_KERNEL |
> __GFP_NOWARN | GFP_DMA);
>
> I might be missing something, but I still don't understand why all this
> GEM code is necessary. I quickly compared with the CMA helpers and the
> only meaningful difference is the use of the GFP_DMA flag.
>
> Rather than duplicating the CMA code, it's better to change the CMA
> helpers to provide a way of setting this flag.
>
> Best regards
> Thomas
>
> > +Â Â Âif (!sprd_gem->vaddr) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("failed to allocate buffer with size %llu\n",
> > +Â Â Â Â Â Â Â Â Â Â Â Âargs->size);
> > +Â Â Â Â Â Â Âret = -ENOMEM;
> > +Â Â Â Â Â Â Âgoto error;
> > +Â Â Â}
> > +
> > +Â Â Âret = drm_gem_handle_create(file_priv, &sprd_gem->base,
> &args->handle);
> > +Â Â Âif (ret)
> > +Â Â Â Â Â Â Âgoto error;
> > +
> > +Â Â Âdrm_gem_object_put_unlocked(&sprd_gem->base);
> > +
> > +Â Â Âreturn 0;
> > +
> > +error:
> > +Â Â Âsprd_gem_free_object(&sprd_gem->base);
> > +Â Â Âreturn ret;
> > +}
> > +
> > +static int sprd_gem_object_mmap(struct drm_gem_object *obj,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct vm_area_struct *vma)
> > +
> > +{
> > +Â Â Âint ret;
> > +Â Â Âstruct sprd_gem_obj *sprd_gem = to_sprd_gem_obj(obj);
> > +
> > +Â Â Âvma->vm_flags &= ~VM_PFNMAP;
> > +Â Â Âvma->vm_pgoff = 0;
> > +
> > +Â Â Âret = dma_mmap_wc(obj->dev->dev, vma,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âsprd_gem->vaddr, sprd_gem->dma_addr,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âvma->vm_end - vma->vm_start);
> > +Â Â Âif (ret)
> > +Â Â Â Â Â Â Âdrm_gem_vm_close(vma);
> > +
> > +Â Â Âreturn ret;
> > +}
> > +
> > +int sprd_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> > +{
> > +Â Â Âstruct drm_gem_object *obj;
> > +Â Â Âint ret;
> > +
> > +Â Â Âret = drm_gem_mmap(filp, vma);
> > +Â Â Âif (ret)
> > +Â Â Â Â Â Â Âreturn ret;
> > +
> > +Â Â Âobj = vma->vm_private_data;
> > +
> > +Â Â Âreturn sprd_gem_object_mmap(obj, vma);
> > +}
> > +
> > +int sprd_gem_prime_mmap(struct drm_gem_object *obj,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Âstruct vm_area_struct *vma)
> > +{
> > +Â Â Âint ret;
> > +
> > +Â Â Âret = drm_gem_mmap_obj(obj, obj->size, vma);
> > +Â Â Âif (ret)
> > +Â Â Â Â Â Â Âreturn ret;
> > +
> > +Â Â Âreturn sprd_gem_object_mmap(obj, vma);
> > +}
> > +
> > +struct sg_table *sprd_gem_prime_get_sg_table(struct
> drm_gem_object *obj)
> > +{
> > +Â Â Âstruct sprd_gem_obj *sprd_gem = to_sprd_gem_obj(obj);
> > +Â Â Âstruct sg_table *sgtb;
> > +Â Â Âint ret;
> > +
> > +Â Â Âsgtb = kzalloc(sizeof(*sgtb), GFP_KERNEL);
> > +Â Â Âif (!sgtb)
> > +Â Â Â Â Â Â Âreturn ERR_PTR(-ENOMEM);
> > +
> > +Â Â Âret = dma_get_sgtable(obj->dev->dev, sgtb, sprd_gem->vaddr,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Âsprd_gem->dma_addr, obj->size);
> > +Â Â Âif (ret) {
> > +Â Â Â Â Â Â ÂDRM_ERROR("failed to allocate sg_table, %d\n", ret);
> > +Â Â Â Â Â Â Âkfree(sgtb);
> > +Â Â Â Â Â Â Âreturn ERR_PTR(ret);
> > +Â Â Â}
> > +
> > +Â Â Âreturn sgtb;
> > +}
> > +
> > +struct drm_gem_object *sprd_gem_prime_import_sg_table(struct
> drm_device *drm,
> > +Â Â Â Â Â Â Âstruct dma_buf_attachment *attach, struct sg_table
> *sgtb)
> > +{
> > +Â Â Âstruct sprd_gem_obj *sprd_gem;
> > +
> > +Â Â Âsprd_gem = sprd_gem_obj_create(drm, attach->dmabuf->size);
> > +Â Â Âif (IS_ERR(sprd_gem))
> > +Â Â Â Â Â Â Âreturn ERR_CAST(sprd_gem);
> > +
> > +Â Â ÂDRM_DEBUG("gem = %p\n", &sprd_gem->base);
> > +
> > +Â Â Âif (sgtb->nents == 1)
> > +Â Â Â Â Â Â Âsprd_gem->dma_addr = sg_dma_address(sgtb->sgl);
> > +
> > +Â Â Âsprd_gem->sgtb = sgtb;
> > +
> > +Â Â Âreturn &sprd_gem->base;
> > +}
> > diff --git a/drivers/gpu/drm/sprd/sprd_gem.h
> b/drivers/gpu/drm/sprd/sprd_gem.h
> > new file mode 100644
> > index 0000000..b6740bd
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sprd/sprd_gem.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019 Unisoc Inc.
> > + */
> > +
> > +#ifndef _SPRD_GEM_H_
> > +#define _SPRD_GEM_H_
> > +
> > +#include <drm/drm_gem.h>
> > +
> > +struct sprd_gem_obj {
> > +Â Â Âstruct drm_gem_objectÂ Âbase;
> > +Â Â Âdma_addr_tÂ Â Â Â Â Â Â dma_addr;
> > +Â Â Âstruct sg_tableÂ Â Â Â Â*sgtb;
> > +Â Â ÂvoidÂ Â Â Â Â Â Â Â Â Â *vaddr;
> > +};
> > +
> > +#define to_sprd_gem_obj(x)Â Âcontainer_of(x, struct sprd_gem_obj,
> base)
> > +
> > +void sprd_gem_free_object(struct drm_gem_object *gem);
> > +int sprd_gem_dumb_create(struct drm_file *file_priv, struct
> drm_device *dev,
> > +Â Â Â Â Â Â Â Â Â Â Â Â Âstruct drm_mode_create_dumb *args);
> > +int sprd_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> > +int sprd_gem_prime_mmap(struct drm_gem_object *obj,
> > +Â Â Â Â Â Â Â Â Â Â Â struct vm_area_struct *vma);
> > +struct sg_table *sprd_gem_prime_get_sg_table(struct
> drm_gem_object *obj);
> > +struct drm_gem_object *sprd_gem_prime_import_sg_table(struct
> drm_device *dev,
> > +Â Â Â Â Â Â Âstruct dma_buf_attachment *attach, struct sg_table
> *sgtb);
> > +
> > +#endif
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 NÃrnberg, Germany
> (HRB 36809, AG NÃrnberg)
> GeschÃftsfÃhrer: Felix ImendÃrffer
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 NÃrnberg, Germany
(HRB 36809, AG NÃrnberg)
GeschÃftsfÃhrer: Felix ImendÃrffer

Attachment: signature.asc
Description: OpenPGP digital signature

Next message: Florian Weimer: "Re: [RFC PATCH glibc 09/13] glibc: Perform rseq(2) registration at C startup and thread creation (v13)"
Previous message: Thomas Richter: "[PATCH] perf test: Fix test case Merge cpu map"
In reply to: Thomas Zimmermann: "Re: [PATCH RFC v1 2/6] drm/sprd: add Unisoc's drm kms master"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]