[Openpvrsgx-devgroup] [PATCH] staging: pvr: Add a simplified pvr-drv.c as replacement for messy pvr_drm.c

H. Nikolaus Schaller hns at goldelico.com
Thu Nov 7 11:22:53 CET 2019


Hi Tony,

> Am 07.11.2019 um 08:43 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
> 
> Hi Tony,
> cool work!
> 
> I'll try asap and then comment.

I did and can only say: great work!

Just added the patch, rebuilt the kernel and was able to run the
gpu-demo on the omap5 Pyra.

Then I added some printk and debugging to your code to verify that
it is really running and doing the work.

And it did work as well.

So I can add it to our openpvrsgx-devgroup repo.

And I like the approach to "move" rewritten driver code to
the final place. This allows to easily separate what has already
been worked on.

BR and thanks,
Nikolaus

> 
> 
>> Am 07.11.2019 um 02:35 schrieb Tony Lindgren <tony at atomide.com>:
>> 
>> We can have just a basic Linux DRM driver that uses the functions from the
>> Imagination DDK. This makes it easier to rewrite components from the
>> Imagination DDK rather than spend time on cleaning up the existing stuff.
>> 
>> I've added basic quirk support based on the compatible, but that is not
>> yet used. It can be used to set flags to init let's say pvr-sgx530.c
>> and pvr-sgx540.c for the different register defines.
>> 
>> Eventually this driver could become a standalone driver with integrated 2D
>> acceleration capabilities along the lines we already have in the gma500
>> driver in drivers/gpu/drm/gma500/accel_2d.c.
>> 
>> At some point we should probably move most of the Makefile logic to the
>> top level Makefile, but that can wait.
>> 
>> Signed-off-by: Tony Lindgren <tony at atomide.com>
>> ---
>> 
>> Please test with the demos too.
>> 
>> I've only tested pvrsrvctl --start --no-module on beagle-x15 to make sure
>> things still load
>> 
>> ---
>> .../pvr/1.14.3699939/eurasia_km/Makefile      |  11 +-
>> drivers/staging/pvr/pvr-drv.c                 | 330 ++++++++++++++++++
>> drivers/staging/pvr/pvr-drv.h                 | 126 +++++++
>> 3 files changed, 465 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/staging/pvr/pvr-drv.c
>> create mode 100644 drivers/staging/pvr/pvr-drv.h
>> 
>> diff --git a/drivers/staging/pvr/1.14.3699939/eurasia_km/Makefile b/drivers/staging/pvr/1.14.3699939/eurasia_km/Makefile
>> --- a/drivers/staging/pvr/1.14.3699939/eurasia_km/Makefile
>> +++ b/drivers/staging/pvr/1.14.3699939/eurasia_km/Makefile
>> @@ -34,6 +34,9 @@ ccflags-y += -DPVR_BUILD_DATE="\"$(shell date "+%Y%m%d" )\""
>> # this defines the compatible string which must be present in the DTB of the board
>> ccflags-y += -DSYS_SGX_DEV_NAME="\"$(SOC_VENDOR),$(SOC)-$(SGX)-$(SGX_REV)\""
>> 
>> +# vendor_soc_sgx_rev for macro use
>> +ccflags-y += -D$(SOC_VENDOR)_$(SOC)_$(SGX)_$(SGX_REV)
>> +
>> ccflags-y += -DSGX_CORE_REV=$(SGX_REV)
>> 
>> ccflags-y += \
>> @@ -149,6 +152,11 @@ ccflags-y += \
>> 	-fno-strict-aliasing -Wno-pointer-arith -Wno-sign-conversion
>> endif
>> 
>> +# Add rewritten driver components here for now
>> +$(TARGET) += \
>> +	../../pvr-drv.o \
>> +
>> +# Keep Imagination SDK components here
>> $(TARGET) += \
>> 	services4/srvkm/bridged/bridged_pvr_bridge.o \
>> 	services4/srvkm/bridged/bridged_support.o \
>> @@ -190,8 +198,7 @@ $(TARGET) += \
>> 	services4/srvkm/env/linux/pdump.o \
>> 	services4/srvkm/env/linux/proc.o \
>> 	services4/srvkm/env/linux/pvr_bridge_k.o \
>> -	services4/srvkm/env/linux/pvr_debug.o \
>> -	services4/srvkm/env/linux/pvr_drm.o
>> +	services4/srvkm/env/linux/pvr_debug.o
>> 
>> ifneq ($(CONFIG_SGX_DRM),)
>> 
>> diff --git a/drivers/staging/pvr/pvr-drv.c b/drivers/staging/pvr/pvr-drv.c
>> new file mode 100644
>> --- /dev/null
>> +++ b/drivers/staging/pvr/pvr-drv.c
>> @@ -0,0 +1,330 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +/*
>> + * Linux DRM Driver for PoverVR SGX
>> + *
>> + * Copyright (C) 2019 Tony Lindgren <tony at atomide.com>
>> + *
>> + * Some parts of code based on earlier Imagination driver
>> + * Copyright (C) Imagination Technologies Ltd.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm.h>
>> +#include <drm/drm_modeset_helper.h>
>> +
>> +#include "pvr-drv.h"
>> +
>> +/* Currently used Imagination SDK includes */
>> +#include "services.h"
>> +#include "mmap.h"
>> +#include "linkage.h"
>> +#include "pvr_bridge.h"
>> +#include "pvrversion.h"
>> +
>> +#define PVR_DRM_NAME		PVRSRV_MODNAME
>> +#define PVR_DRM_DESC		"Imagination Technologies PVR DRM"
>> +#define PVR_DRM_DATE		"20110701"
>> +
>> +#define PVR_QUIRK_OMAP4		BIT(0)
>> +
>> +struct pvr_capabilities {
>> +	u32 quirks;
>> +	unsigned long smp:1;
>> +};
>> +
>> +struct pvr {
>> +	struct device *dev;
>> +	struct drm_device *ddev;
>> +	const struct pvr_capabilities *cap;
>> +	u32 quirks;
>> +};
>> +
>> +struct platform_device *gpsPVRLDMDev;
>> +static struct drm_device *gpsPVRDRMDev;
>> +
>> +static int pvr_drm_load(struct drm_device *dev, unsigned long flags)
>> +{
>> +	dev_dbg(dev->dev, "%s\n", __func__);
>> +	gpsPVRDRMDev = dev;
>> +	gpsPVRLDMDev = to_platform_device(dev->dev);
>> +
>> +	return PVRCore_Init();
>> +}
>> +
>> +static int pvr_drm_unload(struct drm_device *dev)
>> +{
>> +	dev_dbg(dev->dev, "%s\n", __func__);
>> +	PVRCore_Cleanup();
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_open(struct drm_device *dev, struct drm_file *file)
>> +{
>> +	dev_dbg(dev->dev, "%s\n", __func__);
>> +
>> +	return PVRSRVOpen(dev, file);
>> +}
>> +
>> +static int pvr_ioctl_command(struct drm_device *dev, void *arg, struct drm_file *filp)
>> +{
>> +	dev_dbg(dev->dev, "%s: dev: %px arg: %px filp: %px\n", __func__, dev, arg, filp);
>> +
>> +	return PVRSRV_BridgeDispatchKM(dev, arg, filp);
>> +}
>> +
>> +static int pvr_ioctl_drm_is_master(struct drm_device *dev, void *arg, struct drm_file *filp)
>> +{
>> +	dev_dbg(dev->dev, "%s: dev: %px arg: %px filp: %px\n", __func__, dev, arg, filp);
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_ioctl_unpriv(struct drm_device *dev, void *arg, struct drm_file *filp)
>> +{
>> +	dev_dbg(dev->dev, "%s: dev: %px arg: %px filp: %px\n", __func__, dev, arg, filp);
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_ioctl_dbgdrv(struct drm_device *dev, void *arg, struct drm_file *filp)
>> +{
>> +	dev_dbg(dev->dev, "%s: dev: %px arg: %px filp: %px\n", __func__, dev, arg, filp);
>> +
>> +#ifdef PDUMP
>> +	return dbgdrv_ioctl(dev, arg, filp);
>> +#endif
>> +
>> +	return 0;
>> +}
>> +
>> +static struct drm_ioctl_desc pvr_ioctls[] = {
>> +	DRM_IOCTL_DEF_DRV(PVR_SRVKM, pvr_ioctl_command, DRM_RENDER_ALLOW),
>> +        DRM_IOCTL_DEF_DRV(PVR_IS_MASTER, pvr_ioctl_drm_is_master,

above seems to be some tab vs. whitespace issue

>> +			  DRM_RENDER_ALLOW | DRM_MASTER),
>> +        DRM_IOCTL_DEF_DRV(PVR_UNPRIV, pvr_ioctl_unpriv, DRM_RENDER_ALLOW),
>> +        DRM_IOCTL_DEF_DRV(PVR_DBGDRV, pvr_ioctl_dbgdrv, DRM_RENDER_ALLOW),
>> +#ifdef PVR_DISPLAY_CONTROLLER_DRM_IOCTL
>> +        DRM_IOCTL_DEF_DRV(PVR_DISP, drm_invalid_op, DRM_MASTER)
>> +#endif
>> +};
>> +
>> +/* REVISIT: This is used by dmabuf.c */
>> +struct device *PVRLDMGetDevice(void)
>> +{
>> +	return gpsPVRDRMDev->dev;
>> +}
>> +
>> +static const struct file_operations pvr_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = drm_open,
>> +	.unlocked_ioctl = drm_ioctl,
>> +	.compat_ioctl = drm_compat_ioctl,
>> +	.mmap = PVRMMap,
>> +	.poll = drm_poll,
>> +	.read = drm_read,
>> +	.llseek = noop_llseek,
>> +};
>> +
>> +static struct drm_driver pvr_drm_driver = {
>> +	.driver_features = DRIVER_RENDER,
>> +	.dev_priv_size = 0,
>> +	.open = pvr_open,
>> +	.ioctls = pvr_ioctls,
>> +	.num_ioctls = ARRAY_SIZE(pvr_ioctls),
>> +	.fops = &pvr_fops,
>> +	.name = "pvr",
>> +	.desc = PVR_DRM_DESC,
>> +	.date = PVR_DRM_DATE,
>> +	.major = PVRVERSION_MAJ,
>> +	.minor = PVRVERSION_MIN,
>> +	.patchlevel = PVRVERSION_BUILD,
>> +};
>> +
>> +static int __maybe_unused pvr_runtime_suspend(struct device *dev)
>> +{
>> +	dev_dbg(dev, "%s\n", __func__);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused pvr_runtime_resume(struct device *dev)
>> +{
>> +	dev_dbg(dev, "%s\n", __func__);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int pvr_suspend(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct pvr *ddata = dev_get_drvdata(dev);
>> +	struct drm_device *drm_dev = ddata->ddev;
>> +	int error;
>> +
>> +	error = drm_mode_config_helper_suspend(drm_dev);
>> +	if (error)
>> +		dev_warn(dev, "%s: error: %i\n", __func__, error);
>> +
>> +	return PVRSRVDriverSuspend(pdev, PMSG_SUSPEND);
>> +}
>> +
>> +static int pvr_resume(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct pvr *ddata = dev_get_drvdata(dev);
>> +	struct drm_device *drm_dev = ddata->ddev;
>> +	int error;
>> +
>> +	error = drm_mode_config_helper_resume(drm_dev);
>> +	if (error)
>> +		dev_warn(dev, "%s: error: %i\n", __func__, error);
>> +
>> +	return PVRSRVDriverResume(pdev);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops pvr_pm_ops = {
>> +        SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pvr_suspend, pvr_resume)
>> +        SET_RUNTIME_PM_OPS(pvr_runtime_suspend,
>> +                           pvr_runtime_resume,
>> +                           NULL)
>> +};
>> +
>> +static const struct pvr_capabilities __maybe_unused pvr_omap3 = {
>> +};
>> +
>> +static const struct pvr_capabilities __maybe_unused pvr_omap4 = {
>> +	.quirks = PVR_QUIRK_OMAP4,
>> +};
>> +
>> +static const struct pvr_capabilities __maybe_unused pvr_omap4470 = {
>> +	.smp = true,
>> +};
>> +
>> +static const struct pvr_capabilities __maybe_unused pvr_omap5 = {
>> +	.smp = true,
>> +};
>> +
>> +static const struct of_device_id pvr_ids[] = {
>> +	OMAP3_SGX530_121("ti,omap3-sgx530-121", &pvr_omap3)
>> +	OMAP3630_SGX530_125("ti,omap3-sgx530-125", &pvr_omap3)
>> +	AM3517_SGX530_125("ti,am3517-sgx530-125", &pvr_omap3)
>> +	AM335X_SGX530_125("ti,am335x-sgx530-125", &pvr_omap3)
>> +	OMAP4_SGX540_120("ti,omap4-sgx540-120", &pvr_omap4)
>> +	OMAP4470_SGX544_112("ti,omap4-sgx544-112", &pvr_omap4470)
>> +	OMAP5_SGX544_116("ti,omap5-sgx544-116", &pvr_omap5)
>> +	DRA7_SGX544_116("ti,dra7-sgx544-116", &pvr_omap5)
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, pvr_ids);
>> +
>> +static int pvr_init_match(struct pvr *ddata)
>> +{
>> +	const struct pvr_capabilities *cap;
>> +
>> +	cap = of_device_get_match_data(ddata->dev);
>> +	if (!cap)
>> +		return 0;
>> +
>> +	ddata->cap = cap;
>> +
>> +	ddata->quirks = cap->quirks;
>> +	dev_info(ddata->dev, "Enabling quirks %08x\n", ddata->quirks);
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_probe(struct platform_device *pdev)
>> +{
>> +	struct drm_device *ddev;
>> +	struct pvr *ddata;
>> +	int error;
>> +
>> +	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
>> +	if (!ddata)
>> +		return -ENOMEM;
>> +
>> +	ddata->dev = &pdev->dev;
>> +	platform_set_drvdata(pdev, ddata);
>> +
>> +	error = pvr_init_match(ddata);
>> +	if (error)
>> +		return error;
>> +
>> +	pm_runtime_enable(ddata->dev);
>> +	error = pm_runtime_get_sync(ddata->dev);
>> +	if (error < 0) {
>> +		pm_runtime_put_noidle(ddata->dev);
>> +
>> +		return error;
>> +	}
>> +
>> +	ddev = drm_dev_alloc(&pvr_drm_driver, ddata->dev);
>> +	if (IS_ERR(ddev))
>> +		return PTR_ERR(ddev);
>> +
>> +	error = pvr_drm_load(ddev, 0);
>> +	if (error)
>> +		return error;
>> +
>> +	error = drm_dev_register(ddev, 0);
>> +	if (error < 0) {
>> +		pvr_drm_unload(gpsPVRDRMDev);
>> +
>> +		return error;
>> +	}
>> +
>> +	ddata->ddev = ddev;
>> +        ddev->dev_private = ddata;

here as well.

>> +
>> +	gpsPVRLDMDev = pdev;
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_remove(struct platform_device *pdev)
>> +{
>> +	drm_put_dev(gpsPVRDRMDev);
>> +	pvr_drm_unload(gpsPVRDRMDev);
>> +	gpsPVRDRMDev = NULL;
>> +
>> +	pm_runtime_put_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver pvr_driver = {
>> +	.driver = {
>> +		.name = PVR_DRM_NAME,
>> +		.of_match_table = pvr_ids,
>> +		.pm = &pvr_pm_ops,
>> +	},
>> +	.probe = pvr_probe,
>> +	.remove = pvr_remove,
>> +	.shutdown = PVRSRVDriverShutdown,
>> +};
>> +
>> +static int __init pvr_init(void)
>> +{
>> +	/* Must come before attempting to print anything via Services */
>> +	PVRDPFInit();
>> +
>> +	return platform_driver_register(&pvr_driver);
>> +}
>> +
>> +static void __exit pvr_exit(void)
>> +{
>> +	platform_driver_unregister(&pvr_driver);
>> +}
>> +
>> +module_init(pvr_init);
>> +module_exit(pvr_exit);
>> diff --git a/drivers/staging/pvr/pvr-drv.h b/drivers/staging/pvr/pvr-drv.h
>> new file mode 100644
>> --- /dev/null
>> +++ b/drivers/staging/pvr/pvr-drv.h
>> @@ -0,0 +1,126 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +
>> +struct pvr_ioctl {
>> +	u32 ui32Cmd;			/* ioctl command */
>> +	u32 ui32Size;			/* needs to be correctly set */
>> +	void *pInBuffer;		/* input data buffer */
>> +	u32 ui32InBufferSize;		/* size of input data buffer */
>> +	void *pOutBuffer;		/* output data buffer */
>> +	u32 ui32OutBufferSize;		/* size of output data buffer */
>> +};
>> +
>> +struct pvr_dummy {
>> +	char dummy[4];
>> +};
>> +
>> +struct pvr_unpriv {
>> +	u32 cmd;
>> +	u32 res;
>> +};
>> +
>> +#define DRM_PVR_SRVKM		0x0
>> +#define DRM_PVR_DISP		0x1
>> +#define DRM_PVR_BC		0x2
>> +#define DRM_PVR_IS_MASTER	0x3
>> +#define DRM_PVR_UNPRIV		0x4
>> +#define DRM_PVR_DBGDRV		0x5
>> +
>> +#define	DRM_IOCTL_PVR_SRVKM	DRM_IOWR(DRM_COMMAND_BASE + DRM_PVR_SRVKM, PVRSRV_BRIDGE_PACKAGE)
>> +#define	DRM_IOCTL_PVR_IS_MASTER	DRM_IOW(DRM_COMMAND_BASE + DRM_PVR_IS_MASTER, struct pvr_dummy)
>> +#define	DRM_IOCTL_PVR_UNPRIV	DRM_IOWR(DRM_COMMAND_BASE + DRM_PVR_UNPRIV, struct pvr_unpriv)
>> +#define	DRM_IOCTL_PVR_DBGDRV	DRM_IOWR(DRM_COMMAND_BASE + DRM_PVR_DBGDRV, struct pvr_ioctl)
>> +#define	DRM_IOCTL_PVR_DISP	DRM_IOWR(DRM_COMMAND_BASE + DRM_PVR_DISP, drm_pvr_display_cmd)
>> +
>> +/* We are currently calling these from the Imagination SDK */
>> +int PVRCore_Init(void);
>> +void PVRCore_Cleanup(void);
>> +int PVRSRVOpen(struct drm_device *dev, struct drm_file *pFile);
>> +void PVRSRVRelease(void *pvPrivData);
>> +int PVRSRV_BridgeDispatchKM(struct drm_device *dev, void *arg, struct drm_file *pFile);
>> +int PVRSRVDriverSuspend(struct platform_device *pdev, pm_message_t state);
>> +int PVRSRVDriverResume(struct platform_device *pdev);
>> +void PVRSRVDriverShutdown(struct platform_device *pdev);
>> +
>> +/*
>> + * These are currently needed to prevent the multiple instances of
>> + * the driver from trying to probe.
>> + */
>> +#ifdef ti_omap3_sgx530_121
>> +#define OMAP3_SGX530_121(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define OMAP3_SGX530_121(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_omap3630_sgx530_125
>> +#define OMAP3630_SGX530_125(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define OMAP3630_SGX530_125(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_am3517_sgx530_125
>> +#define AM3517_SGX530_125(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define AM3517_SGX530_125(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_am335x_sgx530_125
>> +#define AM335X_SGX530_125(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define AM335X_SGX530_125(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_omap4_sgx540_120
>> +#define OMAP4_SGX540_120(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define OMAP4_SGX540_120(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_omap44370_sgx544_112
>> +#define OMAP4470_SGX544_112(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define OMAP4470_SGX544_112(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_omap5_sgx544_116
>> +#define OMAP5_SGX544_116(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define OMAP5_SGX544_116(comp, dat)
>> +#endif
>> +
>> +#ifdef ti_dra7_sgx544_116
>> +#define DRA7_SGX544_116(comp, dat)	\
>> +	{				\
>> +		.compatible = comp,	\
>> +		.data = dat,		\
>> +	},
>> +#else
>> +#define DRA7_SGX544_116(comp, dat)
>> +#endif
>> -- 
>> 2.23.0
> 
> _______________________________________________
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx
> openpvrsgx-devgroup mailing list
> openpvrsgx-devgroup at letux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/openpvrsgx-devgroup



More information about the openpvrsgx-devgroup mailing list