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