* [PATCH 0/4] soc: amlogic: add meson-canvas driver @ 2018-08-01 18:51 Maxime Jourdan 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-01 18:51 UTC (permalink / raw) To: linux-amlogic Cc: Maxime Jourdan, Kevin Hilman, Neil Armstrong, linux-arm-kernel, linux-kernel, devicetree, dri-devel Amlogic SoCs feature a set of 256 canvas that act as pixel buffer descriptors. Some IPs like the display and video decoders access those pixels by using canvas IDs rather than direct phy addresses. As such, allocating/manipulating canvases can be done concurrently and there is a need for a standalone, lock-aware canvas provider module. Currently, canvas code lies in the drm/meson module as it is the sole user. This patchset adds such canvas provider module and converts drm/meson to using it, stripping/moving the current canvas code. Maxime Jourdan (4): soc: amlogic: add meson-canvas driver dt-bindings: soc: amlogic: add meson-canvas documentation ARM64: dts: meson-gx: add dmcbus and canvas nodes. drm/meson: convert to the new canvas module .../bindings/display/amlogic,meson-vpu.txt | 9 +- .../soc/amlogic/amlogic,meson-canvas.txt | 36 ++++ arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 24 ++- drivers/gpu/drm/meson/Kconfig | 1 + drivers/gpu/drm/meson/Makefile | 2 +- drivers/gpu/drm/meson/meson_canvas.c | 70 ------- drivers/gpu/drm/meson/meson_canvas.h | 42 ---- drivers/gpu/drm/meson/meson_crtc.c | 5 +- drivers/gpu/drm/meson/meson_drv.c | 35 ++-- drivers/gpu/drm/meson/meson_drv.h | 5 +- drivers/gpu/drm/meson/meson_plane.c | 3 +- drivers/gpu/drm/meson/meson_viu.c | 1 - drivers/soc/amlogic/Kconfig | 7 + drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson-canvas.c | 182 ++++++++++++++++++ include/linux/soc/amlogic/meson-canvas.h | 37 ++++ 16 files changed, 319 insertions(+), 141 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-canvas.txt delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h create mode 100644 drivers/soc/amlogic/meson-canvas.c create mode 100644 include/linux/soc/amlogic/meson-canvas.h -- 2.17.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-01 18:51 [PATCH 0/4] soc: amlogic: add meson-canvas driver Maxime Jourdan @ 2018-08-01 18:51 ` Maxime Jourdan 2018-08-02 8:38 ` Neil Armstrong ` (2 more replies) 2018-08-01 18:51 ` [PATCH 2/4] dt-bindings: soc: amlogic: add meson-canvas documentation Maxime Jourdan ` (2 subsequent siblings) 3 siblings, 3 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-01 18:51 UTC (permalink / raw) To: Kevin Hilman Cc: Maxime Jourdan, Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel Amlogic SoCs have a repository of 256 canvas which they use to describe pixel buffers. They contain metadata like width, height, block mode, endianness [..] Many IPs within those SoCs like vdec/vpu rely on those canvas to read/write pixels. Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> --- drivers/soc/amlogic/Kconfig | 7 + drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson-canvas.c | 182 +++++++++++++++++++++++ include/linux/soc/amlogic/meson-canvas.h | 37 +++++ 4 files changed, 227 insertions(+) create mode 100644 drivers/soc/amlogic/meson-canvas.c create mode 100644 include/linux/soc/amlogic/meson-canvas.h diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig index b04f6e4aedbc..5bd049899d88 100644 --- a/drivers/soc/amlogic/Kconfig +++ b/drivers/soc/amlogic/Kconfig @@ -1,5 +1,12 @@ menu "Amlogic SoC drivers" +config MESON_CANVAS + bool "Amlogic Meson Canvas driver" + depends on ARCH_MESON || COMPILE_TEST + default ARCH_MESON + help + Say yes to support the canvas IP within Amlogic Meson Soc family. + config MESON_GX_SOCINFO bool "Amlogic Meson GX SoC Information driver" depends on ARCH_MESON || COMPILE_TEST diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile index 8fa321893928..0ab16d35ac36 100644 --- a/drivers/soc/amlogic/Makefile +++ b/drivers/soc/amlogic/Makefile @@ -1,3 +1,4 @@ +obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o diff --git a/drivers/soc/amlogic/meson-canvas.c b/drivers/soc/amlogic/meson-canvas.c new file mode 100644 index 000000000000..671eb89c8904 --- /dev/null +++ b/drivers/soc/amlogic/meson-canvas.c @@ -0,0 +1,182 @@ +/* + * Copyright (C) 2018 Maxime Jourdan + * Copyright (C) 2016 BayLibre, SAS + * Author: Neil Armstrong <narmstrong@baylibre.com> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. + * Copyright (C) 2014 Endless Mobile + * + * 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. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> +#include <linux/soc/amlogic/meson-canvas.h> +#include <asm/io.h> + +#define NUM_CANVAS 256 + +/* DMC Registers */ +#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ + #define CANVAS_WIDTH_LBIT 29 + #define CANVAS_WIDTH_LWID 3 +#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ + #define CANVAS_WIDTH_HBIT 0 + #define CANVAS_HEIGHT_BIT 9 + #define CANVAS_BLKMODE_BIT 24 +#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ + #define CANVAS_LUT_WR_EN (0x2 << 8) + #define CANVAS_LUT_RD_EN (0x1 << 8) + +struct meson_canvas { + struct device *dev; + struct regmap *regmap_dmc; + struct mutex lock; + u8 used[NUM_CANVAS]; +}; + +static struct meson_canvas canvas = { 0 }; + +static int meson_canvas_setup(uint8_t canvas_index, uint32_t addr, + uint32_t stride, uint32_t height, + unsigned int wrap, + unsigned int blkmode, + unsigned int endian) +{ + struct regmap *regmap = canvas.regmap_dmc; + u32 val; + + mutex_lock(&canvas.lock); + + if (!canvas.used[canvas_index]) { + dev_err(canvas.dev, + "Trying to setup non allocated canvas %u\n", + canvas_index); + mutex_unlock(&canvas.lock); + return -EINVAL; + } + + regmap_write(regmap, DMC_CAV_LUT_DATAL, + ((addr + 7) >> 3) | + (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); + + regmap_write(regmap, DMC_CAV_LUT_DATAH, + ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << + CANVAS_WIDTH_HBIT) | + (height << CANVAS_HEIGHT_BIT) | + (wrap << 22) | + (blkmode << CANVAS_BLKMODE_BIT) | + (endian << 26)); + + regmap_write(regmap, DMC_CAV_LUT_ADDR, + CANVAS_LUT_WR_EN | canvas_index); + + /* Force a read-back to make sure everything is flushed. */ + regmap_read(regmap, DMC_CAV_LUT_DATAH, &val); + mutex_unlock(&canvas.lock); + + return 0; +} + +static int meson_canvas_alloc(uint8_t *canvas_index) +{ + int i; + + mutex_lock(&canvas.lock); + for (i = 0; i < NUM_CANVAS; ++i) { + if (!canvas.used[i]) { + canvas.used[i] = 1; + mutex_unlock(&canvas.lock); + *canvas_index = i; + return 0; + } + } + mutex_unlock(&canvas.lock); + dev_err(canvas.dev, "No more canvas available\n"); + + return -ENODEV; +} + +static int meson_canvas_free(uint8_t canvas_index) +{ + mutex_lock(&canvas.lock); + if (!canvas.used[canvas_index]) { + dev_err(canvas.dev, + "Trying to free unused canvas %u\n", canvas_index); + mutex_unlock(&canvas.lock); + return -EINVAL; + } + canvas.used[canvas_index] = 0; + mutex_unlock(&canvas.lock); + + return 0; +} + +static struct meson_canvas_platform_data canvas_platform_data = { + .alloc = meson_canvas_alloc, + .free = meson_canvas_free, + .setup = meson_canvas_setup, +}; + +static int meson_canvas_probe(struct platform_device *pdev) +{ + struct regmap *regmap_dmc; + struct device *dev; + + dev = &pdev->dev; + + regmap_dmc = syscon_node_to_regmap(of_get_parent(dev->of_node)); + if (IS_ERR(regmap_dmc)) { + dev_err(&pdev->dev, "failed to get DMC regmap\n"); + return PTR_ERR(regmap_dmc); + } + + canvas.dev = dev; + canvas.regmap_dmc = regmap_dmc; + mutex_init(&canvas.lock); + + dev->platform_data = &canvas_platform_data; + + return 0; +} + +static int meson_canvas_remove(struct platform_device *pdev) +{ + mutex_destroy(&canvas.lock); + return 0; +} + +static const struct of_device_id canvas_dt_match[] = { + { .compatible = "amlogic,meson-canvas" }, + {} +}; +MODULE_DEVICE_TABLE(of, canvas_dt_match); + +static struct platform_driver meson_canvas_driver = { + .probe = meson_canvas_probe, + .remove = meson_canvas_remove, + .driver = { + .name = "meson-canvas", + .of_match_table = canvas_dt_match, + }, +}; +module_platform_driver(meson_canvas_driver); + +MODULE_ALIAS("platform:meson-canvas"); +MODULE_DESCRIPTION("AMLogic Meson Canvas driver"); +MODULE_AUTHOR("Maxime Jourdan <maxi.jourdan@wanadoo.fr>"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/soc/amlogic/meson-canvas.h b/include/linux/soc/amlogic/meson-canvas.h new file mode 100644 index 000000000000..af9e2415056a --- /dev/null +++ b/include/linux/soc/amlogic/meson-canvas.h @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2018 Maxime Jourdan + * Author: Maxime Jourdan <maxi.jourdan@wanadoo.fr> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#ifndef MESON_CANVAS_H +#define MESON_CANVAS_H + +#include <linux/kernel.h> + +#define MESON_CANVAS_WRAP_NONE 0x00 +#define MESON_CANVAS_WRAP_X 0x01 +#define MESON_CANVAS_WRAP_Y 0x02 + +#define MESON_CANVAS_BLKMODE_LINEAR 0x00 +#define MESON_CANVAS_BLKMODE_32x32 0x01 +#define MESON_CANVAS_BLKMODE_64x64 0x02 + +struct meson_canvas_platform_data { + int (*alloc)(uint8_t *canvas_index); + int (*free) (uint8_t canvas_index); + int (*setup)(uint8_t canvas_index, uint32_t addr, + uint32_t stride, uint32_t height, + unsigned int wrap, + unsigned int blkmode, + unsigned int endian); +}; + +#endif -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan @ 2018-08-02 8:38 ` Neil Armstrong 2018-08-02 8:52 ` Neil Armstrong 2018-08-03 14:14 ` Yixun Lan 2 siblings, 0 replies; 19+ messages in thread From: Neil Armstrong @ 2018-08-02 8:38 UTC (permalink / raw) To: Maxime Jourdan, Kevin Hilman Cc: linux-amlogic, linux-arm-kernel, linux-kernel Hi Maxime, On 01/08/2018 20:51, Maxime Jourdan wrote: > Amlogic SoCs have a repository of 256 canvas which they use to > describe pixel buffers. > > They contain metadata like width, height, block mode, endianness [..] > > Many IPs within those SoCs like vdec/vpu rely on those canvas to read/write > pixels. > > Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > --- > drivers/soc/amlogic/Kconfig | 7 + > drivers/soc/amlogic/Makefile | 1 + > drivers/soc/amlogic/meson-canvas.c | 182 +++++++++++++++++++++++ > include/linux/soc/amlogic/meson-canvas.h | 37 +++++ > 4 files changed, 227 insertions(+) > create mode 100644 drivers/soc/amlogic/meson-canvas.c > create mode 100644 include/linux/soc/amlogic/meson-canvas.h > > diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig > index b04f6e4aedbc..5bd049899d88 100644 > --- a/drivers/soc/amlogic/Kconfig > +++ b/drivers/soc/amlogic/Kconfig > @@ -1,5 +1,12 @@ > menu "Amlogic SoC drivers" > > +config MESON_CANVAS > + bool "Amlogic Meson Canvas driver" > + depends on ARCH_MESON || COMPILE_TEST > + default ARCH_MESON > + help > + Say yes to support the canvas IP within Amlogic Meson Soc family. > + > config MESON_GX_SOCINFO > bool "Amlogic Meson GX SoC Information driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile > index 8fa321893928..0ab16d35ac36 100644 > --- a/drivers/soc/amlogic/Makefile > +++ b/drivers/soc/amlogic/Makefile > @@ -1,3 +1,4 @@ > +obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o > obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o > obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o > obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o > diff --git a/drivers/soc/amlogic/meson-canvas.c b/drivers/soc/amlogic/meson-canvas.c > new file mode 100644 > index 000000000000..671eb89c8904 > --- /dev/null > +++ b/drivers/soc/amlogic/meson-canvas.c > @@ -0,0 +1,182 @@ > +/* > + * Copyright (C) 2018 Maxime Jourdan > + * Copyright (C) 2016 BayLibre, SAS > + * Author: Neil Armstrong <narmstrong@baylibre.com> > + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. > + * Copyright (C) 2014 Endless Mobile > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + */ Please switch to the spdx header format here and in the .h. > + > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/mfd/syscon.h> > +#include <linux/soc/amlogic/meson-canvas.h> > +#include <asm/io.h> > + > +#define NUM_CANVAS 256 > + > +/* DMC Registers */ > +#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ > + #define CANVAS_WIDTH_LBIT 29 > + #define CANVAS_WIDTH_LWID 3 > +#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ > + #define CANVAS_WIDTH_HBIT 0 > + #define CANVAS_HEIGHT_BIT 9 > + #define CANVAS_BLKMODE_BIT 24 > +#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ > + #define CANVAS_LUT_WR_EN (0x2 << 8) > + #define CANVAS_LUT_RD_EN (0x1 << 8) > + > +struct meson_canvas { > + struct device *dev; > + struct regmap *regmap_dmc; > + struct mutex lock; > + u8 used[NUM_CANVAS]; > +}; > + > +static struct meson_canvas canvas = { 0 }; > + > +static int meson_canvas_setup(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian) > +{ > + struct regmap *regmap = canvas.regmap_dmc; > + u32 val; > + > + mutex_lock(&canvas.lock); In the DRM driver these are updated in IRQ context, we should make sure we don't sleep in interrupt context if IRQ occurs when the VDEC updates it's canvases. Could you switch to spin_lock_irqsave() instead ? > + > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to setup non allocated canvas %u\n", > + canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + > + regmap_write(regmap, DMC_CAV_LUT_DATAL, > + ((addr + 7) >> 3) | > + (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); > + > + regmap_write(regmap, DMC_CAV_LUT_DATAH, > + ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << > + CANVAS_WIDTH_HBIT) | > + (height << CANVAS_HEIGHT_BIT) | > + (wrap << 22) | > + (blkmode << CANVAS_BLKMODE_BIT) | > + (endian << 26)); > + > + regmap_write(regmap, DMC_CAV_LUT_ADDR, > + CANVAS_LUT_WR_EN | canvas_index); > + > + /* Force a read-back to make sure everything is flushed. */ > + regmap_read(regmap, DMC_CAV_LUT_DATAH, &val); > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static int meson_canvas_alloc(uint8_t *canvas_index) > +{ > + int i; > + > + mutex_lock(&canvas.lock); > + for (i = 0; i < NUM_CANVAS; ++i) { > + if (!canvas.used[i]) { > + canvas.used[i] = 1; > + mutex_unlock(&canvas.lock); > + *canvas_index = i; > + return 0; > + } > + } > + mutex_unlock(&canvas.lock); > + dev_err(canvas.dev, "No more canvas available\n"); > + > + return -ENODEV; > +} > + > +static int meson_canvas_free(uint8_t canvas_index) > +{ > + mutex_lock(&canvas.lock); > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to free unused canvas %u\n", canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + canvas.used[canvas_index] = 0; > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static struct meson_canvas_platform_data canvas_platform_data = { > + .alloc = meson_canvas_alloc, > + .free = meson_canvas_free, > + .setup = meson_canvas_setup, > +}; > + > +static int meson_canvas_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap_dmc; > + struct device *dev; > + > + dev = &pdev->dev; > + > + regmap_dmc = syscon_node_to_regmap(of_get_parent(dev->of_node)); > + if (IS_ERR(regmap_dmc)) { > + dev_err(&pdev->dev, "failed to get DMC regmap\n"); > + return PTR_ERR(regmap_dmc); > + } > + > + canvas.dev = dev; > + canvas.regmap_dmc = regmap_dmc; > + mutex_init(&canvas.lock); > + > + dev->platform_data = &canvas_platform_data; > + > + return 0; > +} > + > +static int meson_canvas_remove(struct platform_device *pdev) > +{ > + mutex_destroy(&canvas.lock); > + return 0; > +} > + > +static const struct of_device_id canvas_dt_match[] = { > + { .compatible = "amlogic,meson-canvas" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, canvas_dt_match); > + > +static struct platform_driver meson_canvas_driver = { > + .probe = meson_canvas_probe, > + .remove = meson_canvas_remove, > + .driver = { > + .name = "meson-canvas", > + .of_match_table = canvas_dt_match, > + }, > +}; > +module_platform_driver(meson_canvas_driver); > + > +MODULE_ALIAS("platform:meson-canvas"); > +MODULE_DESCRIPTION("AMLogic Meson Canvas driver"); > +MODULE_AUTHOR("Maxime Jourdan <maxi.jourdan@wanadoo.fr>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/soc/amlogic/meson-canvas.h b/include/linux/soc/amlogic/meson-canvas.h > new file mode 100644 > index 000000000000..af9e2415056a > --- /dev/null > +++ b/include/linux/soc/amlogic/meson-canvas.h > @@ -0,0 +1,37 @@ > +/* > + * Copyright (c) 2018 Maxime Jourdan > + * Author: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#ifndef MESON_CANVAS_H > +#define MESON_CANVAS_H > + > +#include <linux/kernel.h> > + > +#define MESON_CANVAS_WRAP_NONE 0x00 > +#define MESON_CANVAS_WRAP_X 0x01 > +#define MESON_CANVAS_WRAP_Y 0x02 > + > +#define MESON_CANVAS_BLKMODE_LINEAR 0x00 > +#define MESON_CANVAS_BLKMODE_32x32 0x01 > +#define MESON_CANVAS_BLKMODE_64x64 0x02 Can you add the endian defines ? #define MESON_CANVAS_ENDIAN_SWAP16 0x1 #define MESON_CANVAS_ENDIAN_SWAP32 0x3 #define MESON_CANVAS_ENDIAN_SWAP64 0x7 #define MESON_CANVAS_ENDIAN_SWAP128 0xf the SWAP64 is the one used in the VDEC and DRM Overlays. > + > +struct meson_canvas_platform_data { > + int (*alloc)(uint8_t *canvas_index); > + int (*free) (uint8_t canvas_index); > + int (*setup)(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian); > +}; > + > +#endif > Thanks, Neil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan 2018-08-02 8:38 ` Neil Armstrong @ 2018-08-02 8:52 ` Neil Armstrong 2018-08-02 13:14 ` Maxime Jourdan 2018-08-03 14:14 ` Yixun Lan 2 siblings, 1 reply; 19+ messages in thread From: Neil Armstrong @ 2018-08-02 8:52 UTC (permalink / raw) To: Maxime Jourdan, Kevin Hilman Cc: linux-amlogic, linux-arm-kernel, linux-kernel Hi Maxime, On 01/08/2018 20:51, Maxime Jourdan wrote: > Amlogic SoCs have a repository of 256 canvas which they use to > describe pixel buffers. > > They contain metadata like width, height, block mode, endianness [..] > > Many IPs within those SoCs like vdec/vpu rely on those canvas to read/write > pixels. I rebased my DRM Overlay patch on top of this and : Tested-by: Neil Armstrong <narmstrong@baylibre.com> You can keep this tag on future versions. The tree is available at : https://github.com/superna9999/linux/commits/amlogic/v4.19/drm-overlay-canvas Neil > > Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > --- > drivers/soc/amlogic/Kconfig | 7 + > drivers/soc/amlogic/Makefile | 1 + > drivers/soc/amlogic/meson-canvas.c | 182 +++++++++++++++++++++++ > include/linux/soc/amlogic/meson-canvas.h | 37 +++++ > 4 files changed, 227 insertions(+) > create mode 100644 drivers/soc/amlogic/meson-canvas.c > create mode 100644 include/linux/soc/amlogic/meson-canvas.h > > diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig > index b04f6e4aedbc..5bd049899d88 100644 > --- a/drivers/soc/amlogic/Kconfig > +++ b/drivers/soc/amlogic/Kconfig > @@ -1,5 +1,12 @@ > menu "Amlogic SoC drivers" > > +config MESON_CANVAS > + bool "Amlogic Meson Canvas driver" > + depends on ARCH_MESON || COMPILE_TEST > + default ARCH_MESON > + help > + Say yes to support the canvas IP within Amlogic Meson Soc family. > + > config MESON_GX_SOCINFO > bool "Amlogic Meson GX SoC Information driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile > index 8fa321893928..0ab16d35ac36 100644 > --- a/drivers/soc/amlogic/Makefile > +++ b/drivers/soc/amlogic/Makefile > @@ -1,3 +1,4 @@ > +obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o > obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o > obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o > obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o > diff --git a/drivers/soc/amlogic/meson-canvas.c b/drivers/soc/amlogic/meson-canvas.c > new file mode 100644 > index 000000000000..671eb89c8904 > --- /dev/null > +++ b/drivers/soc/amlogic/meson-canvas.c > @@ -0,0 +1,182 @@ > +/* > + * Copyright (C) 2018 Maxime Jourdan > + * Copyright (C) 2016 BayLibre, SAS > + * Author: Neil Armstrong <narmstrong@baylibre.com> > + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. > + * Copyright (C) 2014 Endless Mobile > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/mfd/syscon.h> > +#include <linux/soc/amlogic/meson-canvas.h> > +#include <asm/io.h> > + > +#define NUM_CANVAS 256 > + > +/* DMC Registers */ > +#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ > + #define CANVAS_WIDTH_LBIT 29 > + #define CANVAS_WIDTH_LWID 3 > +#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ > + #define CANVAS_WIDTH_HBIT 0 > + #define CANVAS_HEIGHT_BIT 9 > + #define CANVAS_BLKMODE_BIT 24 > +#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ > + #define CANVAS_LUT_WR_EN (0x2 << 8) > + #define CANVAS_LUT_RD_EN (0x1 << 8) > + > +struct meson_canvas { > + struct device *dev; > + struct regmap *regmap_dmc; > + struct mutex lock; > + u8 used[NUM_CANVAS]; > +}; > + > +static struct meson_canvas canvas = { 0 }; > + > +static int meson_canvas_setup(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian) > +{ > + struct regmap *regmap = canvas.regmap_dmc; > + u32 val; > + > + mutex_lock(&canvas.lock); > + > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to setup non allocated canvas %u\n", > + canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + > + regmap_write(regmap, DMC_CAV_LUT_DATAL, > + ((addr + 7) >> 3) | > + (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); > + > + regmap_write(regmap, DMC_CAV_LUT_DATAH, > + ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << > + CANVAS_WIDTH_HBIT) | > + (height << CANVAS_HEIGHT_BIT) | > + (wrap << 22) | > + (blkmode << CANVAS_BLKMODE_BIT) | > + (endian << 26)); > + > + regmap_write(regmap, DMC_CAV_LUT_ADDR, > + CANVAS_LUT_WR_EN | canvas_index); > + > + /* Force a read-back to make sure everything is flushed. */ > + regmap_read(regmap, DMC_CAV_LUT_DATAH, &val); > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static int meson_canvas_alloc(uint8_t *canvas_index) > +{ > + int i; > + > + mutex_lock(&canvas.lock); > + for (i = 0; i < NUM_CANVAS; ++i) { > + if (!canvas.used[i]) { > + canvas.used[i] = 1; > + mutex_unlock(&canvas.lock); > + *canvas_index = i; > + return 0; > + } > + } > + mutex_unlock(&canvas.lock); > + dev_err(canvas.dev, "No more canvas available\n"); > + > + return -ENODEV; > +} > + > +static int meson_canvas_free(uint8_t canvas_index) > +{ > + mutex_lock(&canvas.lock); > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to free unused canvas %u\n", canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + canvas.used[canvas_index] = 0; > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static struct meson_canvas_platform_data canvas_platform_data = { > + .alloc = meson_canvas_alloc, > + .free = meson_canvas_free, > + .setup = meson_canvas_setup, > +}; > + > +static int meson_canvas_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap_dmc; > + struct device *dev; > + > + dev = &pdev->dev; > + > + regmap_dmc = syscon_node_to_regmap(of_get_parent(dev->of_node)); > + if (IS_ERR(regmap_dmc)) { > + dev_err(&pdev->dev, "failed to get DMC regmap\n"); > + return PTR_ERR(regmap_dmc); > + } > + > + canvas.dev = dev; > + canvas.regmap_dmc = regmap_dmc; > + mutex_init(&canvas.lock); > + > + dev->platform_data = &canvas_platform_data; > + > + return 0; > +} > + > +static int meson_canvas_remove(struct platform_device *pdev) > +{ > + mutex_destroy(&canvas.lock); > + return 0; > +} > + > +static const struct of_device_id canvas_dt_match[] = { > + { .compatible = "amlogic,meson-canvas" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, canvas_dt_match); > + > +static struct platform_driver meson_canvas_driver = { > + .probe = meson_canvas_probe, > + .remove = meson_canvas_remove, > + .driver = { > + .name = "meson-canvas", > + .of_match_table = canvas_dt_match, > + }, > +}; > +module_platform_driver(meson_canvas_driver); > + > +MODULE_ALIAS("platform:meson-canvas"); > +MODULE_DESCRIPTION("AMLogic Meson Canvas driver"); > +MODULE_AUTHOR("Maxime Jourdan <maxi.jourdan@wanadoo.fr>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/soc/amlogic/meson-canvas.h b/include/linux/soc/amlogic/meson-canvas.h > new file mode 100644 > index 000000000000..af9e2415056a > --- /dev/null > +++ b/include/linux/soc/amlogic/meson-canvas.h > @@ -0,0 +1,37 @@ > +/* > + * Copyright (c) 2018 Maxime Jourdan > + * Author: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#ifndef MESON_CANVAS_H > +#define MESON_CANVAS_H > + > +#include <linux/kernel.h> > + > +#define MESON_CANVAS_WRAP_NONE 0x00 > +#define MESON_CANVAS_WRAP_X 0x01 > +#define MESON_CANVAS_WRAP_Y 0x02 > + > +#define MESON_CANVAS_BLKMODE_LINEAR 0x00 > +#define MESON_CANVAS_BLKMODE_32x32 0x01 > +#define MESON_CANVAS_BLKMODE_64x64 0x02 > + > +struct meson_canvas_platform_data { > + int (*alloc)(uint8_t *canvas_index); > + int (*free) (uint8_t canvas_index); > + int (*setup)(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian); > +}; > + > +#endif > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-02 8:52 ` Neil Armstrong @ 2018-08-02 13:14 ` Maxime Jourdan 0 siblings, 0 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-02 13:14 UTC (permalink / raw) To: Neil Armstrong Cc: Maxime Jourdan, Kevin Hilman, linux-amlogic, linux-arm-kernel, linux-kernel Hi Neil, 2018-08-02 10:38 GMT+02:00 Neil Armstrong <narmstrong@baylibre.com>: > Please switch to the spdx header format here and in the .h. > In the DRM driver these are updated in IRQ context, we should make sure we don't sleep > in interrupt context if IRQ occurs when the VDEC updates it's canvases. > > Could you switch to spin_lock_irqsave() instead ? Will do. > Can you add the endian defines ? > > #define MESON_CANVAS_ENDIAN_SWAP16 0x1 > #define MESON_CANVAS_ENDIAN_SWAP32 0x3 > #define MESON_CANVAS_ENDIAN_SWAP64 0x7 > #define MESON_CANVAS_ENDIAN_SWAP128 0xf > > the SWAP64 is the one used in the VDEC and DRM Overlays. Sure. Maybe I should push the flags (0x1, 0x2, 0x4, 0x8) rather than the combinations? Or both, since the combinations are more likely to be used in practice more than the single flags. > I rebased my DRM Overlay patch on top of this and : > Tested-by: Neil Armstrong <narmstrong@baylibre.com> > > You can keep this tag on future versions. Thanks! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan 2018-08-02 8:38 ` Neil Armstrong 2018-08-02 8:52 ` Neil Armstrong @ 2018-08-03 14:14 ` Yixun Lan 2018-08-03 21:47 ` Maxime Jourdan 2 siblings, 1 reply; 19+ messages in thread From: Yixun Lan @ 2018-08-03 14:14 UTC (permalink / raw) To: Maxime Jourdan Cc: Kevin Hilman, linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong HI Maxime thanks for contributing the patches ;-) On Thu, Aug 2, 2018 at 2:51 AM, Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote: > Amlogic SoCs have a repository of 256 canvas which they use to > describe pixel buffers. > > They contain metadata like width, height, block mode, endianness [..] > > Many IPs within those SoCs like vdec/vpu rely on those canvas to read/write > pixels. > > Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > --- > drivers/soc/amlogic/Kconfig | 7 + > drivers/soc/amlogic/Makefile | 1 + > drivers/soc/amlogic/meson-canvas.c | 182 +++++++++++++++++++++++ > include/linux/soc/amlogic/meson-canvas.h | 37 +++++ > 4 files changed, 227 insertions(+) > create mode 100644 drivers/soc/amlogic/meson-canvas.c > create mode 100644 include/linux/soc/amlogic/meson-canvas.h > > diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig > index b04f6e4aedbc..5bd049899d88 100644 > --- a/drivers/soc/amlogic/Kconfig > +++ b/drivers/soc/amlogic/Kconfig > @@ -1,5 +1,12 @@ > menu "Amlogic SoC drivers" > > +config MESON_CANVAS > + bool "Amlogic Meson Canvas driver" shouldn't this a 'tristate'? since you'd make the driver a kernel module.. > + depends on ARCH_MESON || COMPILE_TEST > + default ARCH_MESON > + help > + Say yes to support the canvas IP within Amlogic Meson Soc family. > + > config MESON_GX_SOCINFO > bool "Amlogic Meson GX SoC Information driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile > index 8fa321893928..0ab16d35ac36 100644 > --- a/drivers/soc/amlogic/Makefile > +++ b/drivers/soc/amlogic/Makefile > @@ -1,3 +1,4 @@ > +obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o > obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o > obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o > obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o > diff --git a/drivers/soc/amlogic/meson-canvas.c b/drivers/soc/amlogic/meson-canvas.c > new file mode 100644 > index 000000000000..671eb89c8904 > --- /dev/null > +++ b/drivers/soc/amlogic/meson-canvas.c > @@ -0,0 +1,182 @@ > +/* > + * Copyright (C) 2018 Maxime Jourdan > + * Copyright (C) 2016 BayLibre, SAS > + * Author: Neil Armstrong <narmstrong@baylibre.com> > + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. > + * Copyright (C) 2014 Endless Mobile > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + */ use SPDX license header as Neil already mentioned check doc: Documentation/process/license-rules.rst > + > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/mfd/syscon.h> > +#include <linux/soc/amlogic/meson-canvas.h> > +#include <asm/io.h> > + > +#define NUM_CANVAS 256 > + > +/* DMC Registers */ > +#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ > + #define CANVAS_WIDTH_LBIT 29 > + #define CANVAS_WIDTH_LWID 3 > +#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ > + #define CANVAS_WIDTH_HBIT 0 > + #define CANVAS_HEIGHT_BIT 9 > + #define CANVAS_BLKMODE_BIT 24 > +#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ > + #define CANVAS_LUT_WR_EN (0x2 << 8) > + #define CANVAS_LUT_RD_EN (0x1 << 8) > + > +struct meson_canvas { > + struct device *dev; > + struct regmap *regmap_dmc; > + struct mutex lock; > + u8 used[NUM_CANVAS]; > +}; > + > +static struct meson_canvas canvas = { 0 }; > + > +static int meson_canvas_setup(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian) use "./scripts/checkpatch.pl --strict" to check you will get a few complaints.. > +{ > + struct regmap *regmap = canvas.regmap_dmc; > + u32 val; > + > + mutex_lock(&canvas.lock); > + > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to setup non allocated canvas %u\n", > + canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + > + regmap_write(regmap, DMC_CAV_LUT_DATAL, > + ((addr + 7) >> 3) | > + (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); > + > + regmap_write(regmap, DMC_CAV_LUT_DATAH, > + ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << > + CANVAS_WIDTH_HBIT) | > + (height << CANVAS_HEIGHT_BIT) | > + (wrap << 22) | > + (blkmode << CANVAS_BLKMODE_BIT) | > + (endian << 26)); > + > + regmap_write(regmap, DMC_CAV_LUT_ADDR, > + CANVAS_LUT_WR_EN | canvas_index); > + > + /* Force a read-back to make sure everything is flushed. */ > + regmap_read(regmap, DMC_CAV_LUT_DATAH, &val); > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static int meson_canvas_alloc(uint8_t *canvas_index) > +{ > + int i; > + > + mutex_lock(&canvas.lock); > + for (i = 0; i < NUM_CANVAS; ++i) { > + if (!canvas.used[i]) { > + canvas.used[i] = 1; > + mutex_unlock(&canvas.lock); > + *canvas_index = i; > + return 0; > + } > + } > + mutex_unlock(&canvas.lock); > + dev_err(canvas.dev, "No more canvas available\n"); > + > + return -ENODEV; > +} > + > +static int meson_canvas_free(uint8_t canvas_index) > +{ > + mutex_lock(&canvas.lock); > + if (!canvas.used[canvas_index]) { > + dev_err(canvas.dev, > + "Trying to free unused canvas %u\n", canvas_index); > + mutex_unlock(&canvas.lock); > + return -EINVAL; > + } > + canvas.used[canvas_index] = 0; > + mutex_unlock(&canvas.lock); > + > + return 0; > +} > + > +static struct meson_canvas_platform_data canvas_platform_data = { > + .alloc = meson_canvas_alloc, > + .free = meson_canvas_free, > + .setup = meson_canvas_setup, > +}; > + > +static int meson_canvas_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap_dmc; > + struct device *dev; > + > + dev = &pdev->dev; you could fold into one if you like(may save a few lines), so something like: struct device *dev = &pdev->dev; > + > + regmap_dmc = syscon_node_to_regmap(of_get_parent(dev->of_node)); > + if (IS_ERR(regmap_dmc)) { > + dev_err(&pdev->dev, "failed to get DMC regmap\n"); > + return PTR_ERR(regmap_dmc); > + } > + > + canvas.dev = dev; > + canvas.regmap_dmc = regmap_dmc; > + mutex_init(&canvas.lock); > + > + dev->platform_data = &canvas_platform_data; > + > + return 0; > +} > + > +static int meson_canvas_remove(struct platform_device *pdev) > +{ > + mutex_destroy(&canvas.lock); > + return 0; > +} > + > +static const struct of_device_id canvas_dt_match[] = { > + { .compatible = "amlogic,meson-canvas" }, drop "meson-" prefix, already comment in another thread > + {} > +}; > +MODULE_DEVICE_TABLE(of, canvas_dt_match); > + > +static struct platform_driver meson_canvas_driver = { > + .probe = meson_canvas_probe, > + .remove = meson_canvas_remove, > + .driver = { > + .name = "meson-canvas", > + .of_match_table = canvas_dt_match, > + }, > +}; > +module_platform_driver(meson_canvas_driver); > + > +MODULE_ALIAS("platform:meson-canvas"); > +MODULE_DESCRIPTION("AMLogic Meson Canvas driver"); Jerome complained several times about the camel expression, so just use "Amlogic" ;-) > +MODULE_AUTHOR("Maxime Jourdan <maxi.jourdan@wanadoo.fr>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/soc/amlogic/meson-canvas.h b/include/linux/soc/amlogic/meson-canvas.h > new file mode 100644 > index 000000000000..af9e2415056a > --- /dev/null > +++ b/include/linux/soc/amlogic/meson-canvas.h > @@ -0,0 +1,37 @@ > +/* > + * Copyright (c) 2018 Maxime Jourdan > + * Author: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#ifndef MESON_CANVAS_H > +#define MESON_CANVAS_H > + > +#include <linux/kernel.h> > + > +#define MESON_CANVAS_WRAP_NONE 0x00 > +#define MESON_CANVAS_WRAP_X 0x01 > +#define MESON_CANVAS_WRAP_Y 0x02 > + > +#define MESON_CANVAS_BLKMODE_LINEAR 0x00 > +#define MESON_CANVAS_BLKMODE_32x32 0x01 > +#define MESON_CANVAS_BLKMODE_64x64 0x02 > + > +struct meson_canvas_platform_data { > + int (*alloc)(uint8_t *canvas_index); > + int (*free) (uint8_t canvas_index); > + int (*setup)(uint8_t canvas_index, uint32_t addr, > + uint32_t stride, uint32_t height, > + unsigned int wrap, > + unsigned int blkmode, > + unsigned int endian); > +}; > + > +#endif > -- > 2.17.1 > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver 2018-08-03 14:14 ` Yixun Lan @ 2018-08-03 21:47 ` Maxime Jourdan 0 siblings, 0 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-03 21:47 UTC (permalink / raw) To: Yixun Lan Cc: Maxime Jourdan, Kevin Hilman, Neil Armstrong, linux-kernel, linux-arm-kernel, linux-amlogic Hi Yixun, thanks for the review 2018-08-03 16:14 GMT+02:00 Yixun Lan <yixun.lan@gmail.com>: >> +config MESON_CANVAS >> + bool "Amlogic Meson Canvas driver" > shouldn't this a 'tristate'? since you'd make the driver a kernel module.. Yep it should! I well noted your other feedback and v2 will have fixes for them :) . ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] dt-bindings: soc: amlogic: add meson-canvas documentation 2018-08-01 18:51 [PATCH 0/4] soc: amlogic: add meson-canvas driver Maxime Jourdan 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan @ 2018-08-01 18:51 ` Maxime Jourdan 2018-08-01 18:51 ` [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes Maxime Jourdan 2018-08-01 18:51 ` [PATCH 4/4] drm/meson: convert to the new canvas module Maxime Jourdan 3 siblings, 0 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-01 18:51 UTC (permalink / raw) To: Kevin Hilman Cc: Maxime Jourdan, Neil Armstrong, linux-arm-kernel, linux-amlogic, linux-kernel, devicetree DT bindings doc for amlogic,meson-canvas Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> --- .../soc/amlogic/amlogic,meson-canvas.txt | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-canvas.txt diff --git a/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-canvas.txt b/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-canvas.txt new file mode 100644 index 000000000000..96e14374b9dd --- /dev/null +++ b/Documentation/devicetree/bindings/soc/amlogic/amlogic,meson-canvas.txt @@ -0,0 +1,36 @@ +Amlogic Meson Canvas +================================ + +A canvas is a collection of metadata that describes a pixel buffer. +Those metadata include: width, height, phyaddr, wrapping, block mode +and endianness. + +Many IPs within Amlogic SoCs rely on canvas indexes to read/write pixel data +rather than use the phy addresses directly. For instance, this is the case for +the video decoders and the display. + +Amlogic SoCs have 256 canvas. + +Device Tree Bindings: +--------------------- + +Canvas Provider +-------------------------- + +Required properties: +- compatible: "amlogic,meson-canvas" + +Parent node should have the following properties : +- compatible: "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd" +- reg: base address and size of the DMC system control register space. + +Example: + +sysctrl_DMC: system-controller@0 { + compatible = "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd"; + reg = <0x0 0x0 0x0 0x1000>; + + canvas: canvas-provider@0 { + compatible = "amlogic,meson-canvas"; + }; +}; -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes. 2018-08-01 18:51 [PATCH 0/4] soc: amlogic: add meson-canvas driver Maxime Jourdan 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan 2018-08-01 18:51 ` [PATCH 2/4] dt-bindings: soc: amlogic: add meson-canvas documentation Maxime Jourdan @ 2018-08-01 18:51 ` Maxime Jourdan 2018-08-03 13:50 ` Yixun Lan 2018-08-01 18:51 ` [PATCH 4/4] drm/meson: convert to the new canvas module Maxime Jourdan 3 siblings, 1 reply; 19+ messages in thread From: Maxime Jourdan @ 2018-08-01 18:51 UTC (permalink / raw) To: Kevin Hilman Cc: Maxime Jourdan, Neil Armstrong, linux-arm-kernel, linux-amlogic, linux-kernel, devicetree Wrap the canvas node in a syscon node. Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> --- arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index b8dc4dbb391b..d104b9e111fb 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi @@ -423,6 +423,23 @@ }; }; + dmcbus: bus@c8838000 { + compatible = "simple-bus"; + reg = <0x0 0xc8838000 0x0 0x1000>; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0x0 0x0 0x0 0xc8838000 0x0 0x1000>; + + sysctrl_DMC: system-controller@0 { + compatible = "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd"; + reg = <0x0 0x0 0x0 0x1000>; + + canvas: canvas-provider@0 { + compatible = "amlogic,meson-canvas"; + }; + }; + }; + hiubus: bus@c883c000 { compatible = "simple-bus"; reg = <0x0 0xc883c000 0x0 0x2000>; -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes. 2018-08-01 18:51 ` [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes Maxime Jourdan @ 2018-08-03 13:50 ` Yixun Lan 2018-08-04 20:02 ` Maxime Jourdan 0 siblings, 1 reply; 19+ messages in thread From: Yixun Lan @ 2018-08-03 13:50 UTC (permalink / raw) To: Maxime Jourdan Cc: Kevin Hilman, devicetree, Neil Armstrong, linux-kernel, linux-amlogic, linux-arm-kernel Hi Maxime great job! thanks for contributing the patches.. On Thu, Aug 2, 2018 at 2:51 AM, Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote: > Wrap the canvas node in a syscon node. > > Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > --- > arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > index b8dc4dbb391b..d104b9e111fb 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > @@ -423,6 +423,23 @@ > }; > }; > > + dmcbus: bus@c8838000 { > + compatible = "simple-bus"; > + reg = <0x0 0xc8838000 0x0 0x1000>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges = <0x0 0x0 0x0 0xc8838000 0x0 0x1000>; > + > + sysctrl_DMC: system-controller@0 { > + compatible = "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd"; we'd like to drop 'meson-' prefix, so better using "amlogic,gx-dmc-sysctrl", please take a look at the discussion here [1] [1] https://lkml.kernel.org/r/7hk1prmg4w.fsf@baylibre.com > + reg = <0x0 0x0 0x0 0x1000>; > + > + canvas: canvas-provider@0 { > + compatible = "amlogic,meson-canvas"; ditto > + }; > + }; > + }; > + > hiubus: bus@c883c000 { > compatible = "simple-bus"; > reg = <0x0 0xc883c000 0x0 0x2000>; > -- > 2.17.1 > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes. 2018-08-03 13:50 ` Yixun Lan @ 2018-08-04 20:02 ` Maxime Jourdan 2018-08-07 1:29 ` Yixun Lan 0 siblings, 1 reply; 19+ messages in thread From: Maxime Jourdan @ 2018-08-04 20:02 UTC (permalink / raw) To: Yixun Lan Cc: Maxime Jourdan, Kevin Hilman, devicetree, Neil Armstrong, linux-kernel, linux-amlogic, linux-arm-kernel >> + sysctrl_DMC: system-controller@0 { >> + compatible = "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd"; > > we'd like to drop 'meson-' prefix, so better using "amlogic,gx-dmc-sysctrl", > please take a look at the discussion here [1] > > [1] https://lkml.kernel.org/r/7hk1prmg4w.fsf@baylibre.com > On that subject, should I remove the meson keyword from dts only, or from everything ? e.g use amlogic_canvas_* symbols instead of meson_canvas_*, name the source file "amlogic-canvas.c", etc. ? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes. 2018-08-04 20:02 ` Maxime Jourdan @ 2018-08-07 1:29 ` Yixun Lan 0 siblings, 0 replies; 19+ messages in thread From: Yixun Lan @ 2018-08-07 1:29 UTC (permalink / raw) To: Maxime Jourdan Cc: devicetree, Neil Armstrong, Kevin Hilman, linux-kernel, linux-amlogic, linux-arm-kernel hi Maxime: On Sun, Aug 5, 2018 at 4:02 AM, Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote: >>> + sysctrl_DMC: system-controller@0 { >>> + compatible = "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd"; >> >> we'd like to drop 'meson-' prefix, so better using "amlogic,gx-dmc-sysctrl", >> please take a look at the discussion here [1] >> >> [1] https://lkml.kernel.org/r/7hk1prmg4w.fsf@baylibre.com >> > > On that subject, should I remove the meson keyword from dts only, or > from everything ? > remove the 'meson-' from dts is enough.. > e.g use amlogic_canvas_* symbols instead of meson_canvas_*, name the > source file "amlogic-canvas.c", etc. ? > Actually, I'd suggest to keep using meson_canvas_* in the code for the consistency, unless Kevin or Jerome/Neil has something to say? Yixun ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-01 18:51 [PATCH 0/4] soc: amlogic: add meson-canvas driver Maxime Jourdan ` (2 preceding siblings ...) 2018-08-01 18:51 ` [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes Maxime Jourdan @ 2018-08-01 18:51 ` Maxime Jourdan 2018-08-02 8:39 ` Jerome Brunet [not found] ` <5b6cc316.1c69fb81.682d3.1216@mx.google.com> 3 siblings, 2 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-01 18:51 UTC (permalink / raw) To: Neil Armstrong Cc: Maxime Jourdan, Kevin Hilman, linux-arm-kernel, linux-amlogic, linux-kernel, dri-devel, devicetree This removes the meson_canvas files within the meson/drm layer and makes use of the new canvas module that is referenced in the dts. Canvases can be used by different IPs and modules, and it is as such preferable to rely on a module that can safely dispatch canvases on demand. Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> --- .../bindings/display/amlogic,meson-vpu.txt | 9 +-- arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 7 +- drivers/gpu/drm/meson/Kconfig | 1 + drivers/gpu/drm/meson/Makefile | 2 +- drivers/gpu/drm/meson/meson_canvas.c | 70 ------------------- drivers/gpu/drm/meson/meson_canvas.h | 42 ----------- drivers/gpu/drm/meson/meson_crtc.c | 5 +- drivers/gpu/drm/meson/meson_drv.c | 35 ++++++---- drivers/gpu/drm/meson/meson_drv.h | 5 +- drivers/gpu/drm/meson/meson_plane.c | 3 +- drivers/gpu/drm/meson/meson_viu.c | 1 - 11 files changed, 39 insertions(+), 141 deletions(-) delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt index 057b81335775..60b6e1398636 100644 --- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt +++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt @@ -60,9 +60,9 @@ Required properties: - reg: base address and size of he following memory-mapped regions : - vpu - hhi - - dmc - reg-names: should contain the names of the previous memory regions - interrupts: should contain the VENC Vsync interrupt number +- amlogic,canvas: should point to a meson canvas provider node Optional properties: - power-domains: Optional phandle to associated power domain as described in @@ -98,13 +98,14 @@ tv-connector { vpu: vpu@d0100000 { compatible = "amlogic,meson-gxbb-vpu"; reg = <0x0 0xd0100000 0x0 0x100000>, - <0x0 0xc883c000 0x0 0x1000>, - <0x0 0xc8838000 0x0 0x1000>; - reg-names = "vpu", "hhi", "dmc"; + <0x0 0xc883c000 0x0 0x1000>; + reg-names = "vpu", "hhi"; interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>; #address-cells = <1>; #size-cells = <0>; + amlogic,canvas = <&canvas>; + /* CVBS VDAC output port */ port@0 { reg = <0>; diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index d104b9e111fb..7c4d971ecd80 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi @@ -503,13 +503,14 @@ vpu: vpu@d0100000 { compatible = "amlogic,meson-gx-vpu"; reg = <0x0 0xd0100000 0x0 0x100000>, - <0x0 0xc883c000 0x0 0x1000>, - <0x0 0xc8838000 0x0 0x1000>; - reg-names = "vpu", "hhi", "dmc"; + <0x0 0xc883c000 0x0 0x1000>; + reg-names = "vpu", "hhi"; interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>; #address-cells = <1>; #size-cells = <0>; + amlogic,canvas = <&canvas>; + /* CVBS VDAC output port */ cvbs_vdac_port: port@0 { reg = <0>; diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig index 3ce51d8dfe1c..c28b69f48555 100644 --- a/drivers/gpu/drm/meson/Kconfig +++ b/drivers/gpu/drm/meson/Kconfig @@ -7,6 +7,7 @@ config DRM_MESON select DRM_GEM_CMA_HELPER select VIDEOMODE_HELPERS select REGMAP_MMIO + select MESON_CANVAS config DRM_MESON_DW_HDMI tristate "HDMI Synopsys Controller support for Amlogic Meson Display" diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile index c5c4cc362f02..bd67429185ff 100644 --- a/drivers/gpu/drm/meson/Makefile +++ b/drivers/gpu/drm/meson/Makefile @@ -1,5 +1,5 @@ meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o -meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_canvas.o +meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o obj-$(CONFIG_DRM_MESON) += meson-drm.o obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o diff --git a/drivers/gpu/drm/meson/meson_canvas.c b/drivers/gpu/drm/meson/meson_canvas.c deleted file mode 100644 index 08f6073d967e..000000000000 --- a/drivers/gpu/drm/meson/meson_canvas.c +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (C) 2016 BayLibre, SAS - * Author: Neil Armstrong <narmstrong@baylibre.com> - * Copyright (C) 2015 Amlogic, Inc. All rights reserved. - * Copyright (C) 2014 Endless Mobile - * - * 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. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, see <http://www.gnu.org/licenses/>. - */ - -#include <linux/kernel.h> -#include <linux/module.h> -#include "meson_drv.h" -#include "meson_canvas.h" -#include "meson_registers.h" - -/** - * DOC: Canvas - * - * CANVAS is a memory zone where physical memory frames information - * are stored for the VIU to scanout. - */ - -/* DMC Registers */ -#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ -#define CANVAS_WIDTH_LBIT 29 -#define CANVAS_WIDTH_LWID 3 -#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ -#define CANVAS_WIDTH_HBIT 0 -#define CANVAS_HEIGHT_BIT 9 -#define CANVAS_BLKMODE_BIT 24 -#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ -#define CANVAS_LUT_WR_EN (0x2 << 8) -#define CANVAS_LUT_RD_EN (0x1 << 8) - -void meson_canvas_setup(struct meson_drm *priv, - uint32_t canvas_index, uint32_t addr, - uint32_t stride, uint32_t height, - unsigned int wrap, - unsigned int blkmode) -{ - unsigned int val; - - regmap_write(priv->dmc, DMC_CAV_LUT_DATAL, - (((addr + 7) >> 3)) | - (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); - - regmap_write(priv->dmc, DMC_CAV_LUT_DATAH, - ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << - CANVAS_WIDTH_HBIT) | - (height << CANVAS_HEIGHT_BIT) | - (wrap << 22) | - (blkmode << CANVAS_BLKMODE_BIT)); - - regmap_write(priv->dmc, DMC_CAV_LUT_ADDR, - CANVAS_LUT_WR_EN | canvas_index); - - /* Force a read-back to make sure everything is flushed. */ - regmap_read(priv->dmc, DMC_CAV_LUT_DATAH, &val); -} diff --git a/drivers/gpu/drm/meson/meson_canvas.h b/drivers/gpu/drm/meson/meson_canvas.h deleted file mode 100644 index af1759da4b27..000000000000 --- a/drivers/gpu/drm/meson/meson_canvas.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (C) 2016 BayLibre, SAS - * Author: Neil Armstrong <narmstrong@baylibre.com> - * Copyright (C) 2014 Endless Mobile - * - * 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. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, see <http://www.gnu.org/licenses/>. - */ - -/* Canvas LUT Memory */ - -#ifndef __MESON_CANVAS_H -#define __MESON_CANVAS_H - -#define MESON_CANVAS_ID_OSD1 0x4e - -/* Canvas configuration. */ -#define MESON_CANVAS_WRAP_NONE 0x00 -#define MESON_CANVAS_WRAP_X 0x01 -#define MESON_CANVAS_WRAP_Y 0x02 - -#define MESON_CANVAS_BLKMODE_LINEAR 0x00 -#define MESON_CANVAS_BLKMODE_32x32 0x01 -#define MESON_CANVAS_BLKMODE_64x64 0x02 - -void meson_canvas_setup(struct meson_drm *priv, - uint32_t canvas_index, uint32_t addr, - uint32_t stride, uint32_t height, - unsigned int wrap, - unsigned int blkmode); - -#endif /* __MESON_CANVAS_H */ diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c index 05520202c967..506b3619c983 100644 --- a/drivers/gpu/drm/meson/meson_crtc.c +++ b/drivers/gpu/drm/meson/meson_crtc.c @@ -36,7 +36,6 @@ #include "meson_venc.h" #include "meson_vpp.h" #include "meson_viu.h" -#include "meson_canvas.h" #include "meson_registers.h" /* CRTC definition */ @@ -193,10 +192,10 @@ void meson_crtc_irq(struct meson_drm *priv) } else meson_vpp_disable_interlace_vscaler_osd1(priv); - meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1, + priv->canvas_ops->setup(priv->canvas_id_osd1, priv->viu.osd1_addr, priv->viu.osd1_stride, priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE, - MESON_CANVAS_BLKMODE_LINEAR); + MESON_CANVAS_BLKMODE_LINEAR, 0); /* Enable OSD1 */ writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND, diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index d3443125e661..de468339d75a 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -26,6 +26,7 @@ #include <linux/platform_device.h> #include <linux/component.h> #include <linux/of_graph.h> +#include <linux/of_platform.h> #include <drm/drmP.h> #include <drm/drm_atomic.h> @@ -47,7 +48,6 @@ #include "meson_vpp.h" #include "meson_viu.h" #include "meson_venc.h" -#include "meson_canvas.h" #include "meson_registers.h" #define DRIVER_NAME "meson" @@ -165,6 +165,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) struct meson_drm *priv; struct drm_device *drm; struct resource *res; + struct device_node *canvas; + struct platform_device *canvas_pdev; void __iomem *regs; int ret; @@ -211,31 +213,35 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) priv->hhi = devm_regmap_init_mmio(dev, regs, &meson_regmap_config); if (IS_ERR(priv->hhi)) { - dev_err(&pdev->dev, "Couldn't create the HHI regmap\n"); + dev_err(dev, "Couldn't create the HHI regmap\n"); ret = PTR_ERR(priv->hhi); goto free_drm; } - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc"); - if (!res) { - ret = -EINVAL; + canvas = of_parse_phandle(dev->of_node, "amlogic,canvas", 0); + if (!canvas) { + ret = -ENODEV; goto free_drm; } - /* Simply ioremap since it may be a shared register zone */ - regs = devm_ioremap(dev, res->start, resource_size(res)); - if (!regs) { - ret = -EADDRNOTAVAIL; + + canvas_pdev = of_find_device_by_node(canvas); + if (!canvas_pdev) { + dev_err(dev, "Unable to find canvas pdev\n"); + ret = -ENODEV; goto free_drm; } - priv->dmc = devm_regmap_init_mmio(dev, regs, - &meson_regmap_config); - if (IS_ERR(priv->dmc)) { - dev_err(&pdev->dev, "Couldn't create the DMC regmap\n"); - ret = PTR_ERR(priv->dmc); + priv->canvas_ops = dev_get_platdata(&canvas_pdev->dev); + if (!priv->canvas_ops) { + dev_err(dev, "canvas pdata structure NULL\n"); + ret = -EINVAL; goto free_drm; } + ret = priv->canvas_ops->alloc(&priv->canvas_id_osd1); + if (ret) + goto free_drm; + priv->vsync_irq = platform_get_irq(pdev, 0); ret = drm_vblank_init(drm, 1); @@ -315,6 +321,7 @@ static void meson_drv_unbind(struct device *dev) struct drm_device *drm = dev_get_drvdata(dev); struct meson_drm *priv = drm->dev_private; + priv->canvas_ops->free(priv->canvas_id_osd1); drm_dev_unregister(drm); drm_kms_helper_poll_fini(drm); drm_fbdev_cma_fini(priv->fbdev); diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h index 8450d6ac8c9b..dfea959baaa3 100644 --- a/drivers/gpu/drm/meson/meson_drv.h +++ b/drivers/gpu/drm/meson/meson_drv.h @@ -22,15 +22,18 @@ #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/of.h> +#include <linux/soc/amlogic/meson-canvas.h> #include <drm/drmP.h> struct meson_drm { struct device *dev; void __iomem *io_base; struct regmap *hhi; - struct regmap *dmc; int vsync_irq; + struct meson_canvas_platform_data *canvas_ops; + uint8_t canvas_id_osd1; + struct drm_device *drm; struct drm_crtc *crtc; struct drm_fbdev_cma *fbdev; diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c index 12c80dfcff59..8745f9209625 100644 --- a/drivers/gpu/drm/meson/meson_plane.c +++ b/drivers/gpu/drm/meson/meson_plane.c @@ -36,7 +36,6 @@ #include "meson_plane.h" #include "meson_vpp.h" #include "meson_viu.h" -#include "meson_canvas.h" #include "meson_registers.h" struct meson_plane { @@ -105,7 +104,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane, OSD_BLK0_ENABLE; /* Set up BLK0 to point to the right canvas */ - priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) | + priv->viu.osd1_blk0_cfg[0] = ((priv->canvas_id_osd1 << OSD_CANVAS_SEL) | OSD_ENDIANNESS_LE); /* On GXBB, Use the old non-HDR RGB2YUV converter */ diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c index 6bcfa527c180..5b48c4c0985b 100644 --- a/drivers/gpu/drm/meson/meson_viu.c +++ b/drivers/gpu/drm/meson/meson_viu.c @@ -25,7 +25,6 @@ #include "meson_viu.h" #include "meson_vpp.h" #include "meson_venc.h" -#include "meson_canvas.h" #include "meson_registers.h" /** -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-01 18:51 ` [PATCH 4/4] drm/meson: convert to the new canvas module Maxime Jourdan @ 2018-08-02 8:39 ` Jerome Brunet 2018-08-02 12:34 ` Maxime Jourdan [not found] ` <5b6cc316.1c69fb81.682d3.1216@mx.google.com> 1 sibling, 1 reply; 19+ messages in thread From: Jerome Brunet @ 2018-08-02 8:39 UTC (permalink / raw) To: Maxime Jourdan, Neil Armstrong Cc: Kevin Hilman, linux-arm-kernel, linux-amlogic, linux-kernel, dri-devel, devicetree On Wed, 2018-08-01 at 20:51 +0200, Maxime Jourdan wrote: > This removes the meson_canvas files within the meson/drm layer > and makes use of the new canvas module that is referenced in the dts. > > Canvases can be used by different IPs and modules, and it is as such > preferable to rely on a module that can safely dispatch canvases on > demand. > > Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > --- > .../bindings/display/amlogic,meson-vpu.txt | 9 +-- > arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 7 +- > drivers/gpu/drm/meson/Kconfig | 1 + > drivers/gpu/drm/meson/Makefile | 2 +- > drivers/gpu/drm/meson/meson_canvas.c | 70 ------------------- > drivers/gpu/drm/meson/meson_canvas.h | 42 ----------- > drivers/gpu/drm/meson/meson_crtc.c | 5 +- > drivers/gpu/drm/meson/meson_drv.c | 35 ++++++---- > drivers/gpu/drm/meson/meson_drv.h | 5 +- > drivers/gpu/drm/meson/meson_plane.c | 3 +- > drivers/gpu/drm/meson/meson_viu.c | 1 - > 11 files changed, 39 insertions(+), 141 deletions(-) > delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c > delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h > > diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt > index 057b81335775..60b6e1398636 100644 > --- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt > +++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt > @@ -60,9 +60,9 @@ Required properties: > - reg: base address and size of he following memory-mapped regions : > - vpu > - hhi > - - dmc > - reg-names: should contain the names of the previous memory regions > - interrupts: should contain the VENC Vsync interrupt number > +- amlogic,canvas: should point to a meson canvas provider node > > Optional properties: > - power-domains: Optional phandle to associated power domain as described in > @@ -98,13 +98,14 @@ tv-connector { > vpu: vpu@d0100000 { > compatible = "amlogic,meson-gxbb-vpu"; > reg = <0x0 0xd0100000 0x0 0x100000>, > - <0x0 0xc883c000 0x0 0x1000>, > - <0x0 0xc8838000 0x0 0x1000>; > - reg-names = "vpu", "hhi", "dmc"; > + <0x0 0xc883c000 0x0 0x1000>; > + reg-names = "vpu", "hhi"; > interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>; > #address-cells = <1>; > #size-cells = <0>; > > + amlogic,canvas = <&canvas>; > + > /* CVBS VDAC output port */ > port@0 { > reg = <0>; > diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > index d104b9e111fb..7c4d971ecd80 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > @@ -503,13 +503,14 @@ > vpu: vpu@d0100000 { > compatible = "amlogic,meson-gx-vpu"; > reg = <0x0 0xd0100000 0x0 0x100000>, > - <0x0 0xc883c000 0x0 0x1000>, > - <0x0 0xc8838000 0x0 0x1000>; > - reg-names = "vpu", "hhi", "dmc"; > + <0x0 0xc883c000 0x0 0x1000>; > + reg-names = "vpu", "hhi"; > interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>; > #address-cells = <1>; > #size-cells = <0>; > > + amlogic,canvas = <&canvas>; > + > /* CVBS VDAC output port */ > cvbs_vdac_port: port@0 { > reg = <0>; > diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig > index 3ce51d8dfe1c..c28b69f48555 100644 > --- a/drivers/gpu/drm/meson/Kconfig > +++ b/drivers/gpu/drm/meson/Kconfig > @@ -7,6 +7,7 @@ config DRM_MESON > select DRM_GEM_CMA_HELPER > select VIDEOMODE_HELPERS > select REGMAP_MMIO > + select MESON_CANVAS > > config DRM_MESON_DW_HDMI > tristate "HDMI Synopsys Controller support for Amlogic Meson Display" > diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile > index c5c4cc362f02..bd67429185ff 100644 > --- a/drivers/gpu/drm/meson/Makefile > +++ b/drivers/gpu/drm/meson/Makefile > @@ -1,5 +1,5 @@ > meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o > -meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_canvas.o > +meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o > > obj-$(CONFIG_DRM_MESON) += meson-drm.o > obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o > diff --git a/drivers/gpu/drm/meson/meson_canvas.c b/drivers/gpu/drm/meson/meson_canvas.c > deleted file mode 100644 > index 08f6073d967e..000000000000 > --- a/drivers/gpu/drm/meson/meson_canvas.c > +++ /dev/null > @@ -1,70 +0,0 @@ > -/* > - * Copyright (C) 2016 BayLibre, SAS > - * Author: Neil Armstrong <narmstrong@baylibre.com> > - * Copyright (C) 2015 Amlogic, Inc. All rights reserved. > - * Copyright (C) 2014 Endless Mobile > - * > - * 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. > - * > - * This program is distributed in the hope that it will be useful, but > - * WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - * General Public License for more details. > - * > - * You should have received a copy of the GNU General Public License > - * along with this program; if not, see <http://www.gnu.org/licenses/>. > - */ > - > -#include <linux/kernel.h> > -#include <linux/module.h> > -#include "meson_drv.h" > -#include "meson_canvas.h" > -#include "meson_registers.h" > - > -/** > - * DOC: Canvas > - * > - * CANVAS is a memory zone where physical memory frames information > - * are stored for the VIU to scanout. > - */ > - > -/* DMC Registers */ > -#define DMC_CAV_LUT_DATAL 0x48 /* 0x12 offset in data sheet */ > -#define CANVAS_WIDTH_LBIT 29 > -#define CANVAS_WIDTH_LWID 3 > -#define DMC_CAV_LUT_DATAH 0x4c /* 0x13 offset in data sheet */ > -#define CANVAS_WIDTH_HBIT 0 > -#define CANVAS_HEIGHT_BIT 9 > -#define CANVAS_BLKMODE_BIT 24 > -#define DMC_CAV_LUT_ADDR 0x50 /* 0x14 offset in data sheet */ > -#define CANVAS_LUT_WR_EN (0x2 << 8) > -#define CANVAS_LUT_RD_EN (0x1 << 8) > - > -void meson_canvas_setup(struct meson_drm *priv, > - uint32_t canvas_index, uint32_t addr, > - uint32_t stride, uint32_t height, > - unsigned int wrap, > - unsigned int blkmode) > -{ > - unsigned int val; > - > - regmap_write(priv->dmc, DMC_CAV_LUT_DATAL, > - (((addr + 7) >> 3)) | > - (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT)); > - > - regmap_write(priv->dmc, DMC_CAV_LUT_DATAH, > - ((((stride + 7) >> 3) >> CANVAS_WIDTH_LWID) << > - CANVAS_WIDTH_HBIT) | > - (height << CANVAS_HEIGHT_BIT) | > - (wrap << 22) | > - (blkmode << CANVAS_BLKMODE_BIT)); > - > - regmap_write(priv->dmc, DMC_CAV_LUT_ADDR, > - CANVAS_LUT_WR_EN | canvas_index); > - > - /* Force a read-back to make sure everything is flushed. */ > - regmap_read(priv->dmc, DMC_CAV_LUT_DATAH, &val); > -} > diff --git a/drivers/gpu/drm/meson/meson_canvas.h b/drivers/gpu/drm/meson/meson_canvas.h > deleted file mode 100644 > index af1759da4b27..000000000000 > --- a/drivers/gpu/drm/meson/meson_canvas.h > +++ /dev/null > @@ -1,42 +0,0 @@ > -/* > - * Copyright (C) 2016 BayLibre, SAS > - * Author: Neil Armstrong <narmstrong@baylibre.com> > - * Copyright (C) 2014 Endless Mobile > - * > - * 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. > - * > - * This program is distributed in the hope that it will be useful, but > - * WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - * General Public License for more details. > - * > - * You should have received a copy of the GNU General Public License > - * along with this program; if not, see <http://www.gnu.org/licenses/>. > - */ > - > -/* Canvas LUT Memory */ > - > -#ifndef __MESON_CANVAS_H > -#define __MESON_CANVAS_H > - > -#define MESON_CANVAS_ID_OSD1 0x4e > - > -/* Canvas configuration. */ > -#define MESON_CANVAS_WRAP_NONE 0x00 > -#define MESON_CANVAS_WRAP_X 0x01 > -#define MESON_CANVAS_WRAP_Y 0x02 > - > -#define MESON_CANVAS_BLKMODE_LINEAR 0x00 > -#define MESON_CANVAS_BLKMODE_32x32 0x01 > -#define MESON_CANVAS_BLKMODE_64x64 0x02 > - > -void meson_canvas_setup(struct meson_drm *priv, > - uint32_t canvas_index, uint32_t addr, > - uint32_t stride, uint32_t height, > - unsigned int wrap, > - unsigned int blkmode); > - > -#endif /* __MESON_CANVAS_H */ > diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c > index 05520202c967..506b3619c983 100644 > --- a/drivers/gpu/drm/meson/meson_crtc.c > +++ b/drivers/gpu/drm/meson/meson_crtc.c > @@ -36,7 +36,6 @@ > #include "meson_venc.h" > #include "meson_vpp.h" > #include "meson_viu.h" > -#include "meson_canvas.h" > #include "meson_registers.h" > > /* CRTC definition */ > @@ -193,10 +192,10 @@ void meson_crtc_irq(struct meson_drm *priv) > } else > meson_vpp_disable_interlace_vscaler_osd1(priv); > > - meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1, > + priv->canvas_ops->setup(priv->canvas_id_osd1, > priv->viu.osd1_addr, priv->viu.osd1_stride, > priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE, > - MESON_CANVAS_BLKMODE_LINEAR); > + MESON_CANVAS_BLKMODE_LINEAR, 0); > > /* Enable OSD1 */ > writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND, > diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c > index d3443125e661..de468339d75a 100644 > --- a/drivers/gpu/drm/meson/meson_drv.c > +++ b/drivers/gpu/drm/meson/meson_drv.c > @@ -26,6 +26,7 @@ > #include <linux/platform_device.h> > #include <linux/component.h> > #include <linux/of_graph.h> > +#include <linux/of_platform.h> > > #include <drm/drmP.h> > #include <drm/drm_atomic.h> > @@ -47,7 +48,6 @@ > #include "meson_vpp.h" > #include "meson_viu.h" > #include "meson_venc.h" > -#include "meson_canvas.h" > #include "meson_registers.h" > > #define DRIVER_NAME "meson" > @@ -165,6 +165,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) > struct meson_drm *priv; > struct drm_device *drm; > struct resource *res; > + struct device_node *canvas; > + struct platform_device *canvas_pdev; > void __iomem *regs; > int ret; > > @@ -211,31 +213,35 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) > priv->hhi = devm_regmap_init_mmio(dev, regs, > &meson_regmap_config); > if (IS_ERR(priv->hhi)) { > - dev_err(&pdev->dev, "Couldn't create the HHI regmap\n"); > + dev_err(dev, "Couldn't create the HHI regmap\n"); > ret = PTR_ERR(priv->hhi); > goto free_drm; > } > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc"); > - if (!res) { > - ret = -EINVAL; > + canvas = of_parse_phandle(dev->of_node, "amlogic,canvas", 0); > + if (!canvas) { > + ret = -ENODEV; > goto free_drm; > } > - /* Simply ioremap since it may be a shared register zone */ > - regs = devm_ioremap(dev, res->start, resource_size(res)); > - if (!regs) { > - ret = -EADDRNOTAVAIL; > + > + canvas_pdev = of_find_device_by_node(canvas); > + if (!canvas_pdev) { > + dev_err(dev, "Unable to find canvas pdev\n"); > + ret = -ENODEV; > goto free_drm; > } > > - priv->dmc = devm_regmap_init_mmio(dev, regs, > - &meson_regmap_config); > - if (IS_ERR(priv->dmc)) { > - dev_err(&pdev->dev, "Couldn't create the DMC regmap\n"); > - ret = PTR_ERR(priv->dmc); > + priv->canvas_ops = dev_get_platdata(&canvas_pdev->dev); I looks like the consumer of your 'canvas' devices must know how the canvas device is organized internally. Maybe something better can be done ? Your canvas driver could provide a consumer API, for example: meson_canvas_get(): to translate for struct device_node to whatever abstract pointer you would need. meson_canvas_alloc(), setup(), etc ... ... This is just adding a bit of indirection but it would help hide the plumbing of your canvas driver from the consumers (and repeat this code in each). This might be usefull if you ever to make this canvas driver evolve. > + if (!priv->canvas_ops) { > + dev_err(dev, "canvas pdata structure NULL\n"); > + ret = -EINVAL; > goto free_drm; > } > > + ret = priv->canvas_ops->alloc(&priv->canvas_id_osd1); > + if (ret) > + goto free_drm; > + > priv->vsync_irq = platform_get_irq(pdev, 0); > > ret = drm_vblank_init(drm, 1); > @@ -315,6 +321,7 @@ static void meson_drv_unbind(struct device *dev) > struct drm_device *drm = dev_get_drvdata(dev); > struct meson_drm *priv = drm->dev_private; > > + priv->canvas_ops->free(priv->canvas_id_osd1); > drm_dev_unregister(drm); > drm_kms_helper_poll_fini(drm); > drm_fbdev_cma_fini(priv->fbdev); > diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h > index 8450d6ac8c9b..dfea959baaa3 100644 > --- a/drivers/gpu/drm/meson/meson_drv.h > +++ b/drivers/gpu/drm/meson/meson_drv.h > @@ -22,15 +22,18 @@ > #include <linux/platform_device.h> > #include <linux/regmap.h> > #include <linux/of.h> > +#include <linux/soc/amlogic/meson-canvas.h> > #include <drm/drmP.h> > > struct meson_drm { > struct device *dev; > void __iomem *io_base; > struct regmap *hhi; > - struct regmap *dmc; > int vsync_irq; > > + struct meson_canvas_platform_data *canvas_ops; > + uint8_t canvas_id_osd1; > + > struct drm_device *drm; > struct drm_crtc *crtc; > struct drm_fbdev_cma *fbdev; > diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c > index 12c80dfcff59..8745f9209625 100644 > --- a/drivers/gpu/drm/meson/meson_plane.c > +++ b/drivers/gpu/drm/meson/meson_plane.c > @@ -36,7 +36,6 @@ > #include "meson_plane.h" > #include "meson_vpp.h" > #include "meson_viu.h" > -#include "meson_canvas.h" > #include "meson_registers.h" > > struct meson_plane { > @@ -105,7 +104,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane, > OSD_BLK0_ENABLE; > > /* Set up BLK0 to point to the right canvas */ > - priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) | > + priv->viu.osd1_blk0_cfg[0] = ((priv->canvas_id_osd1 << OSD_CANVAS_SEL) | > OSD_ENDIANNESS_LE); > > /* On GXBB, Use the old non-HDR RGB2YUV converter */ > diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c > index 6bcfa527c180..5b48c4c0985b 100644 > --- a/drivers/gpu/drm/meson/meson_viu.c > +++ b/drivers/gpu/drm/meson/meson_viu.c > @@ -25,7 +25,6 @@ > #include "meson_viu.h" > #include "meson_vpp.h" > #include "meson_venc.h" > -#include "meson_canvas.h" > #include "meson_registers.h" > > /** ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-02 8:39 ` Jerome Brunet @ 2018-08-02 12:34 ` Maxime Jourdan 2018-08-02 13:01 ` Jerome Brunet 0 siblings, 1 reply; 19+ messages in thread From: Maxime Jourdan @ 2018-08-02 12:34 UTC (permalink / raw) To: Jerome Brunet Cc: Maxime Jourdan, Neil Armstrong, Kevin Hilman, linux-arm-kernel, linux-amlogic, linux-kernel, dri-devel, devicetree Hi Jerome, 2018-08-02 10:39 GMT+02:00 Jerome Brunet <jbrunet@baylibre.com>: > I looks like the consumer of your 'canvas' devices must know how the canvas > device is organized internally. Maybe something better can be done ? > > Your canvas driver could provide a consumer API, for example: > meson_canvas_get(): to translate for struct device_node to whatever abstract > pointer you would need. > meson_canvas_alloc(), setup(), etc ... > > ... This is just adding a bit of indirection but it would help hide the plumbing > of your canvas driver from the consumers (and repeat this code in each). This > might be usefull if you ever to make this canvas driver evolve. Overall the inner workings are hidden as there is an ops struct instead of public functions. I agree that the "fetch the node" boilerplate code could be put behind a helper, but at the same time this code helps remind the developer that there needs to be a canvas node in the dts, and that it has to be linked in your own device node. I would like to keep it that way if that is okay with you. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-02 12:34 ` Maxime Jourdan @ 2018-08-02 13:01 ` Jerome Brunet 2018-08-02 13:09 ` Maxime Jourdan 0 siblings, 1 reply; 19+ messages in thread From: Jerome Brunet @ 2018-08-02 13:01 UTC (permalink / raw) To: Maxime Jourdan Cc: Neil Armstrong, Kevin Hilman, linux-arm-kernel, linux-amlogic, linux-kernel, dri-devel, devicetree On Thu, 2018-08-02 at 14:34 +0200, Maxime Jourdan wrote: > Hi Jerome, > > 2018-08-02 10:39 GMT+02:00 Jerome Brunet <jbrunet@baylibre.com>: > > I looks like the consumer of your 'canvas' devices must know how the canvas > > device is organized internally. Maybe something better can be done ? > > > > Your canvas driver could provide a consumer API, for example: > > meson_canvas_get(): to translate for struct device_node to whatever abstract > > pointer you would need. > > meson_canvas_alloc(), setup(), etc ... > > > > ... This is just adding a bit of indirection but it would help hide the plumbing > > of your canvas driver from the consumers (and repeat this code in each). This > > might be usefull if you ever to make this canvas driver evolve. > > Overall the inner workings are hidden as there is an ops struct > instead of public functions. What I don't like is precisely that you need to expose this 'struct ops' to the consumer. I would prefer an API for this (it can be a 1:1 mapping). The canvas should remain some abstract object you get from DT. IMO, this is the same as a reset, a syscon or whatever other phandle you get from DT. The consumer should not have to 'care' how it works (how data are organized), it should just use it. Whatever you need to do to deal with your canvas phandle should (IMO) reside in your soc/amlogic/meson-canvas.c, and not be spread in the consumers. > > I agree that the "fetch the node" boilerplate code could be put behind > a helper, but at the same time this code helps remind the developer > that there needs to be a canvas node in the dts, and that it has to be > linked in your own device node. This is why we have a DT documentation. And, as far as I understand amlogic's display and decoder stuff, you won't get very far w/o a canvas, so 'the developer' will be reminded fairly quickly ;) > > I would like to keep it that way if that is okay with you. It's just my opinion, I'm not the one merging it ... :P ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-02 13:01 ` Jerome Brunet @ 2018-08-02 13:09 ` Maxime Jourdan 0 siblings, 0 replies; 19+ messages in thread From: Maxime Jourdan @ 2018-08-02 13:09 UTC (permalink / raw) To: Jerome Brunet Cc: Maxime Jourdan, Neil Armstrong, Kevin Hilman, linux-arm-kernel, linux-amlogic, linux-kernel, dri-devel, devicetree 2018-08-02 15:01 GMT+02:00 Jerome Brunet <jbrunet@baylibre.com>: > What I don't like is precisely that you need to expose this 'struct ops' to the > consumer. I would prefer an API for this (it can be a 1:1 mapping). The canvas > should remain some abstract object you get from DT. > > IMO, this is the same as a reset, a syscon or whatever other phandle you get > from DT. The consumer should not have to 'care' how it works (how data are > organized), it should just use it. > > Whatever you need to do to deal with your canvas phandle should (IMO) reside in > your soc/amlogic/meson-canvas.c, and not be spread in the consumers. > >> >> I agree that the "fetch the node" boilerplate code could be put behind >> a helper, but at the same time this code helps remind the developer >> that there needs to be a canvas node in the dts, and that it has to be >> linked in your own device node. > > This is why we have a DT documentation. > > And, as far as I understand amlogic's display and decoder stuff, you won't get > very far w/o a canvas, so 'the developer' will be reminded fairly quickly ;) > >> >> I would like to keep it that way if that is okay with you. > > It's just my opinion, I'm not the one merging it ... :P > > Fair enough, I'll see to API-ize the module in v2 :). ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <5b6cc316.1c69fb81.682d3.1216@mx.google.com>]
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module [not found] ` <5b6cc316.1c69fb81.682d3.1216@mx.google.com> @ 2018-08-10 6:35 ` Maxime Jourdan 2018-08-10 7:49 ` Neil Armstrong 0 siblings, 1 reply; 19+ messages in thread From: Maxime Jourdan @ 2018-08-10 6:35 UTC (permalink / raw) To: Rob Herring Cc: Maxime Jourdan, Neil Armstrong, Kevin Hilman, linux-arm-kernel, linux-amlogic, Linux Kernel Mailing List, dri-devel, devicetree 2018-08-10 0:41 GMT+02:00 Rob Herring <robh@kernel.org>: > Hi, this is an automated email from Rob's (experimental) review bot. I > found a couple of common problems with your patch. Please see below. > > On Wed, 1 Aug 2018 20:51:28 +0200, Maxime Jourdan wrote: >> This removes the meson_canvas files within the meson/drm layer >> and makes use of the new canvas module that is referenced in the dts. >> >> Canvases can be used by different IPs and modules, and it is as such >> preferable to rely on a module that can safely dispatch canvases on >> demand. >> >> Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> > > The preferred subject prefix is "dt-bindings: <binding dir>: ...". > >> --- >> .../bindings/display/amlogic,meson-vpu.txt | 9 +-- >> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 7 +- >> drivers/gpu/drm/meson/Kconfig | 1 + >> drivers/gpu/drm/meson/Makefile | 2 +- >> drivers/gpu/drm/meson/meson_canvas.c | 70 ------------------- >> drivers/gpu/drm/meson/meson_canvas.h | 42 ----------- >> drivers/gpu/drm/meson/meson_crtc.c | 5 +- >> drivers/gpu/drm/meson/meson_drv.c | 35 ++++++---- >> drivers/gpu/drm/meson/meson_drv.h | 5 +- >> drivers/gpu/drm/meson/meson_plane.c | 3 +- >> drivers/gpu/drm/meson/meson_viu.c | 1 - >> 11 files changed, 39 insertions(+), 141 deletions(-) >> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c >> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h >> > > DT bindings (including binding headers) should be a separate patch. See > Documentation/devicetree/bindings/submitting-patches.txt. > Hi, What's the standard procedure here ? The reason I kept devicetree+drm changes together is because I didn't want to have floating commits that would break the drm module. Should I split the changes anyway ? Maxime ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/meson: convert to the new canvas module 2018-08-10 6:35 ` Maxime Jourdan @ 2018-08-10 7:49 ` Neil Armstrong 0 siblings, 0 replies; 19+ messages in thread From: Neil Armstrong @ 2018-08-10 7:49 UTC (permalink / raw) To: Maxime Jourdan, Rob Herring Cc: Kevin Hilman, linux-arm-kernel, linux-amlogic, Linux Kernel Mailing List, dri-devel, devicetree On 10/08/2018 08:35, Maxime Jourdan wrote: > 2018-08-10 0:41 GMT+02:00 Rob Herring <robh@kernel.org>: >> Hi, this is an automated email from Rob's (experimental) review bot. I >> found a couple of common problems with your patch. Please see below. >> >> On Wed, 1 Aug 2018 20:51:28 +0200, Maxime Jourdan wrote: >>> This removes the meson_canvas files within the meson/drm layer >>> and makes use of the new canvas module that is referenced in the dts. >>> >>> Canvases can be used by different IPs and modules, and it is as such >>> preferable to rely on a module that can safely dispatch canvases on >>> demand. >>> >>> Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr> >> >> The preferred subject prefix is "dt-bindings: <binding dir>: ...". >> >>> --- >>> .../bindings/display/amlogic,meson-vpu.txt | 9 +-- >>> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 7 +- >>> drivers/gpu/drm/meson/Kconfig | 1 + >>> drivers/gpu/drm/meson/Makefile | 2 +- >>> drivers/gpu/drm/meson/meson_canvas.c | 70 ------------------- >>> drivers/gpu/drm/meson/meson_canvas.h | 42 ----------- >>> drivers/gpu/drm/meson/meson_crtc.c | 5 +- >>> drivers/gpu/drm/meson/meson_drv.c | 35 ++++++---- >>> drivers/gpu/drm/meson/meson_drv.h | 5 +- >>> drivers/gpu/drm/meson/meson_plane.c | 3 +- >>> drivers/gpu/drm/meson/meson_viu.c | 1 - >>> 11 files changed, 39 insertions(+), 141 deletions(-) >>> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c >>> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h >>> >> >> DT bindings (including binding headers) should be a separate patch. See >> Documentation/devicetree/bindings/submitting-patches.txt. >> > > Hi, What's the standard procedure here ? The reason I kept > devicetree+drm changes together is because I didn't want to have > floating commits that would break the drm module. > > Should I split the changes anyway ? > > Maxime > Yep, split the dt-bindings and the code change. In a non-perfect work, it could be great to keep the "old" canvas until everything is merged, except if Kevin let me merge everything (including DT) in drm-misc. Neil ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-08-10 7:49 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-01 18:51 [PATCH 0/4] soc: amlogic: add meson-canvas driver Maxime Jourdan 2018-08-01 18:51 ` [PATCH 1/4] " Maxime Jourdan 2018-08-02 8:38 ` Neil Armstrong 2018-08-02 8:52 ` Neil Armstrong 2018-08-02 13:14 ` Maxime Jourdan 2018-08-03 14:14 ` Yixun Lan 2018-08-03 21:47 ` Maxime Jourdan 2018-08-01 18:51 ` [PATCH 2/4] dt-bindings: soc: amlogic: add meson-canvas documentation Maxime Jourdan 2018-08-01 18:51 ` [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes Maxime Jourdan 2018-08-03 13:50 ` Yixun Lan 2018-08-04 20:02 ` Maxime Jourdan 2018-08-07 1:29 ` Yixun Lan 2018-08-01 18:51 ` [PATCH 4/4] drm/meson: convert to the new canvas module Maxime Jourdan 2018-08-02 8:39 ` Jerome Brunet 2018-08-02 12:34 ` Maxime Jourdan 2018-08-02 13:01 ` Jerome Brunet 2018-08-02 13:09 ` Maxime Jourdan [not found] ` <5b6cc316.1c69fb81.682d3.1216@mx.google.com> 2018-08-10 6:35 ` Maxime Jourdan 2018-08-10 7:49 ` Neil Armstrong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).