From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFA48C4332F for ; Thu, 23 Sep 2021 03:32:52 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A5FE46103D for ; Thu, 23 Sep 2021 03:32:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A5FE46103D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Spy6W6aitfZVVjRfUQjQlttmslS51Zp5sGzuQerjDDo=; b=VgI90+jdanVqXY vB8HQIkaDwaldbznRxlEMa2Rq6fQh1yRNiZ6LA8wmdJbJAQOeWgtntdKTTIK8bOapuo86wONDgl1A hznmKMvBKUm5UyD4hO9Jn5yw7nAcJ8JSilD+4+4B32p/58G73mnblpVRwiO4gG3ePWtkPIPPUpHHH tb8Lc8In2AgGYbLklb3nEaGk/hxf5yeyaqrrZ4aWkmNAu7T+roiCrKJxDRHTYWYibAZ9qmfIVOuek //37XG0xs7hbq7NaSR9t/79yNTObaSUxVBFLo8eK9NILaGJ3gyx8IvQmDmAzQt7BlzRgFM3L5N3wS gLIF1SFGrQGBS9eTgZSg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mTFS7-00A9jS-IC; Thu, 23 Sep 2021 03:31:15 +0000 Received: from mail-oi1-x22a.google.com ([2607:f8b0:4864:20::22a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mTFS2-00A9iU-Ow for linux-arm-kernel@lists.infradead.org; Thu, 23 Sep 2021 03:31:12 +0000 Received: by mail-oi1-x22a.google.com with SMTP id o66so7680842oib.11 for ; Wed, 22 Sep 2021 20:31:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=WZnkbDqe35agx+YRdoGtlkEIS1vsNJiYQTbIpjWyLqg=; b=P3kClDnLsxyd8P5md1GK2izJhbgXFpqpxbnKHA/qDWSDqZ1P9e4S93LBbgYC62Yq8T R9UaBvr71Hsdw9Qj0ogvmHgPrs9nA1wIHDSmQSuCmI5BXq1lFS+SXlcuuJMu87sN//ts uoAPab1EJfLPjXzcHCtyOod6MCr36/JsjZGHXkrS08VftM1XbrMnc3oaxktFgmfo0vMJ t3zsbSBGmyNFg8GpXBD4sDxGxAGUUstl+bhic/DrGsf1M8oshxwuxGJQR4jQBkF6tgbQ 9K8Mc5CYg7yqg3huKD7lL3thxynHYPVd6unTn3tlAVKYNZOFsjTVusdqhsqsmGe5Tnii M9Vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=WZnkbDqe35agx+YRdoGtlkEIS1vsNJiYQTbIpjWyLqg=; b=2k1LhCBgvX+omMIbqrk5NrQV6utlNvyWb0ACKCltUCJhW3ZJex59wXFoWwTLOkWr+0 iNF0wcpwsmexGyf1TbnwjJHZlw3Mtv3Z6+FrYFLDuFQzNZQ79IZYPntLu79P6H8d9pEo ljvqRy0zdSmJPbzuN/wFnUzRpHTnny9QacjN0THUEdUMGFXLBcX5M9qzg0IutlxxzSb6 fQgDGf1QqGKJwmBNm33kkhcLIVglRlxRpxUS4bEZENewu+1blxpcIgB89knVx7YP+o+c 6CSxHsRcwxfzIbQ7mzoQZa+CWwxBSv1aSjWXpcGV1vI9VrvLv9Lc/Bed1/VArgod6gCm L+Yw== X-Gm-Message-State: AOAM5337VHdywwW0hnSwdNsUATaRw45dd2tsZE5UQx1FCgCBqEZ3F7gA af9Sh2xDc55A2GGHMzVIV+g1kw== X-Google-Smtp-Source: ABdhPJyEqCFgpzYG5stN1XffFzyyUS7C0XaZxoEnwmEkDrixdgCQQ6CDVf7BYB8h73LGP9V2OIFLXA== X-Received: by 2002:a05:6808:618:: with SMTP id y24mr1978169oih.179.1632367868718; Wed, 22 Sep 2021 20:31:08 -0700 (PDT) Received: from yoga (104-57-184-186.lightspeed.austtx.sbcglobal.net. [104.57.184.186]) by smtp.gmail.com with ESMTPSA id bf6sm1051367oib.0.2021.09.22.20.31.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Sep 2021 20:31:08 -0700 (PDT) Date: Wed, 22 Sep 2021 22:31:06 -0500 From: Bjorn Andersson To: Alexandre Bailon Cc: airlied@linux.ie, daniel@ffwll.ch, robh+dt@kernel.org, matthias.bgg@gmail.com, maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, ohad@wizery.com, mathieu.poirier@linaro.org, sumit.semwal@linaro.org, christian.koenig@amd.com, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, khilman@baylibre.com, gpain@baylibre.com Subject: Re: [RFC PATCH 3/4] rpmsg: Add support of AI Processor Unit (APU) Message-ID: References: <20210917125945.620097-1-abailon@baylibre.com> <20210917125945.620097-4-abailon@baylibre.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210917125945.620097-4-abailon@baylibre.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210922_203110_877224_9B0906F5 X-CRM114-Status: GOOD ( 35.89 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri 17 Sep 07:59 CDT 2021, Alexandre Bailon wrote: > Some Mediatek SoC provides hardware accelerator for AI / ML. > This driver use the DRM driver to manage the shared memory, > and use rpmsg to execute jobs on the APU. > > Signed-off-by: Alexandre Bailon > --- > drivers/rpmsg/Kconfig | 10 +++ > drivers/rpmsg/Makefile | 1 + > drivers/rpmsg/apu_rpmsg.c | 184 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 195 insertions(+) > create mode 100644 drivers/rpmsg/apu_rpmsg.c > > diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig > index 0b4407abdf138..fc1668f795004 100644 > --- a/drivers/rpmsg/Kconfig > +++ b/drivers/rpmsg/Kconfig > @@ -73,4 +73,14 @@ config RPMSG_VIRTIO > select RPMSG_NS > select VIRTIO > > +config RPMSG_APU > + tristate "APU RPMSG driver" > + select REMOTEPROC > + select RPMSG_VIRTIO > + select DRM_APU > + help > + This provides a RPMSG driver that provides some facilities to > + communicate with an accelerated processing unit (APU). > + This Uses the APU DRM driver to manage memory and job scheduling. Similar to how a driver for e.g. an I2C device doesn't live in drivers/i2c, this doesn't belong in drivers/rpmsg. Probably rather directly in the DRM driver. > + > endmenu > diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile > index 8d452656f0ee3..8b336b9a817c1 100644 > --- a/drivers/rpmsg/Makefile > +++ b/drivers/rpmsg/Makefile > @@ -9,3 +9,4 @@ obj-$(CONFIG_RPMSG_QCOM_GLINK_RPM) += qcom_glink_rpm.o > obj-$(CONFIG_RPMSG_QCOM_GLINK_SMEM) += qcom_glink_smem.o > obj-$(CONFIG_RPMSG_QCOM_SMD) += qcom_smd.o > obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o > +obj-$(CONFIG_RPMSG_APU) += apu_rpmsg.o > diff --git a/drivers/rpmsg/apu_rpmsg.c b/drivers/rpmsg/apu_rpmsg.c > new file mode 100644 > index 0000000000000..7e504bd176a4d > --- /dev/null > +++ b/drivers/rpmsg/apu_rpmsg.c > @@ -0,0 +1,184 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright 2020 BayLibre SAS > + > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include "rpmsg_internal.h" > + > +#define APU_RPMSG_SERVICE_MT8183 "rpmsg-mt8183-apu0" > + > +struct rpmsg_apu { > + struct apu_core *core; > + struct rpmsg_device *rpdev; > +}; > + > +static int apu_rpmsg_callback(struct rpmsg_device *rpdev, void *data, int count, > + void *priv, u32 addr) > +{ > + struct rpmsg_apu *apu = dev_get_drvdata(&rpdev->dev); > + struct apu_core *apu_core = apu->core; > + > + return apu_drm_callback(apu_core, data, count); > +} > + > +static int apu_rpmsg_send(struct apu_core *apu_core, void *data, int len) > +{ > + struct rpmsg_apu *apu = apu_drm_priv(apu_core); > + struct rpmsg_device *rpdev = apu->rpdev; > + > + return rpmsg_send(rpdev->ept, data, len); The rpmsg API is exposed outside drivers/rpmsg, so as I said above, just implement this directly in your driver, no need to lug around a dummy wrapper for things like this. > +} > + > +static struct apu_drm_ops apu_rpmsg_ops = { > + .send = apu_rpmsg_send, > +}; > + > +static int apu_init_iovad(struct rproc *rproc, struct rpmsg_apu *apu) > +{ > + struct resource_table *table; > + struct fw_rsc_carveout *rsc; > + int i; > + > + if (!rproc->table_ptr) { > + dev_err(&apu->rpdev->dev, > + "No resource_table: has the firmware been loaded ?\n"); > + return -ENODEV; > + } > + > + table = rproc->table_ptr; > + for (i = 0; i < table->num; i++) { > + int offset = table->offset[i]; > + struct fw_rsc_hdr *hdr = (void *)table + offset; > + > + if (hdr->type != RSC_CARVEOUT) > + continue; > + > + rsc = (void *)hdr + sizeof(*hdr); > + if (apu_drm_reserve_iova(apu->core, rsc->da, rsc->len)) { > + dev_err(&apu->rpdev->dev, > + "failed to reserve iova\n"); > + return -ENOMEM; > + } > + } > + > + return 0; > +} > + > +static struct rproc *apu_get_rproc(struct rpmsg_device *rpdev) > +{ > + /* > + * To work, the APU RPMsg driver need to get the rproc device. > + * Currently, we only use virtio so we could use that to find the > + * remoteproc parent. > + */ > + if (!rpdev->dev.parent && rpdev->dev.parent->bus) { > + dev_err(&rpdev->dev, "invalid rpmsg device\n"); > + return ERR_PTR(-EINVAL); > + } > + > + if (strcmp(rpdev->dev.parent->bus->name, "virtio")) { > + dev_err(&rpdev->dev, "unsupported bus\n"); > + return ERR_PTR(-EINVAL); > + } > + > + return vdev_to_rproc(dev_to_virtio(rpdev->dev.parent)); > +} > + > +static int apu_rpmsg_probe(struct rpmsg_device *rpdev) > +{ > + struct rpmsg_apu *apu; > + struct rproc *rproc; > + int ret; > + > + apu = devm_kzalloc(&rpdev->dev, sizeof(*apu), GFP_KERNEL); > + if (!apu) > + return -ENOMEM; > + apu->rpdev = rpdev; > + > + rproc = apu_get_rproc(rpdev); I believe that you can replace apu_get_rproc() with: rproc = rproc_get_by_child(&rpdev->dev); > + if (IS_ERR_OR_NULL(rproc)) > + return PTR_ERR(rproc); > + > + /* Make device dma capable by inheriting from parent's capabilities */ > + set_dma_ops(&rpdev->dev, get_dma_ops(rproc->dev.parent)); > + > + ret = dma_coerce_mask_and_coherent(&rpdev->dev, > + dma_get_mask(rproc->dev.parent)); > + if (ret) > + goto err_put_device; > + > + rpdev->dev.iommu_group = rproc->dev.parent->iommu_group; Would it be better or you if we have a device_node, so that you could specify the iommus property for this compute device? I'm asking because I've seen cases where multi-purpose remoteproc firmware operate using multiple different iommu streams... > + > + apu->core = apu_drm_register_core(rproc, &apu_rpmsg_ops, apu); > + if (!apu->core) { > + ret = -ENODEV; > + goto err_put_device; > + } > + > + ret = apu_init_iovad(rproc, apu); > + > + dev_set_drvdata(&rpdev->dev, apu); > + > + return ret; > + > +err_put_device: This label looks misplaced, and sure enough, if apu_init_iovad() fails you're not apu_drm_unregister_core(). But on that note, don't you want to apu_init_iovad() before you apu_drm_register_core()? > + devm_kfree(&rpdev->dev, apu); The reason for using devm_kzalloc() is that once you return unsuccessfully from probe, or from remove the memory is freed. So devm_kfree() should go in both cases. > + > + return ret; > +} > + > +static void apu_rpmsg_remove(struct rpmsg_device *rpdev) > +{ > + struct rpmsg_apu *apu = dev_get_drvdata(&rpdev->dev); > + > + apu_drm_unregister_core(apu); > + devm_kfree(&rpdev->dev, apu); No need to explicitly free devm resources. Regards, Bjorn > +} > + > +static const struct rpmsg_device_id apu_rpmsg_match[] = { > + { APU_RPMSG_SERVICE_MT8183 }, > + {} > +}; > + > +static struct rpmsg_driver apu_rpmsg_driver = { > + .probe = apu_rpmsg_probe, > + .remove = apu_rpmsg_remove, > + .callback = apu_rpmsg_callback, > + .id_table = apu_rpmsg_match, > + .drv = { > + .name = "apu_rpmsg", > + }, > +}; > + > +static int __init apu_rpmsg_init(void) > +{ > + return register_rpmsg_driver(&apu_rpmsg_driver); > +} > +arch_initcall(apu_rpmsg_init); > + > +static void __exit apu_rpmsg_exit(void) > +{ > + unregister_rpmsg_driver(&apu_rpmsg_driver); > +} > +module_exit(apu_rpmsg_exit); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("APU RPMSG driver"); > -- > 2.31.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel