Re: [PATCH 2/2] drm/meson: exclusively use the canvas provider module

From: Neil Armstrong
Date: Wed Mar 13 2019 - 08:49:49 EST


On 11/03/2019 11:51, Maxime Jourdan wrote:
> Now that the DMC register range is no longer in the bindings, remove any
> mention towards it and exclusively use the meson-canvas module.
>
> Signed-off-by: Maxime Jourdan <mjourdan@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/meson/Makefile | 2 +-
> drivers/gpu/drm/meson/meson_canvas.c | 73 -----------------------
> drivers/gpu/drm/meson/meson_canvas.h | 51 ----------------
> drivers/gpu/drm/meson/meson_crtc.c | 84 ++++++++-------------------
> drivers/gpu/drm/meson/meson_drv.c | 68 ++++++++--------------
> drivers/gpu/drm/meson/meson_drv.h | 1 -
> drivers/gpu/drm/meson/meson_overlay.c | 8 ---
> drivers/gpu/drm/meson/meson_plane.c | 6 +-
> drivers/gpu/drm/meson/meson_viu.c | 1 -
> 9 files changed, 51 insertions(+), 243 deletions(-)
> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.c
> delete mode 100644 drivers/gpu/drm/meson/meson_canvas.h
>
> diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
> index 7709f2fbb9f7..d4ea82fc493b 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_overlay.o
> +meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_overlay.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 5de11aa7c775..000000000000
> --- a/drivers/gpu/drm/meson/meson_canvas.c
> +++ /dev/null
> @@ -1,73 +0,0 @@
> -/*
> - * Copyright (C) 2016 BayLibre, SAS
> - * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> - * 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 CANVAS_ENDIAN_BIT 26
> -#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 endian)
> -{
> - 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) |
> - (endian << CANVAS_ENDIAN_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 85dbf26e2826..000000000000
> --- a/drivers/gpu/drm/meson/meson_canvas.h
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -/*
> - * Copyright (C) 2016 BayLibre, SAS
> - * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> - * 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
> -#define MESON_CANVAS_ID_VD1_0 0x60
> -#define MESON_CANVAS_ID_VD1_1 0x61
> -#define MESON_CANVAS_ID_VD1_2 0x62
> -
> -/* 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
> -
> -#define MESON_CANVAS_ENDIAN_SWAP16 0x1
> -#define MESON_CANVAS_ENDIAN_SWAP32 0x3
> -#define MESON_CANVAS_ENDIAN_SWAP64 0x7
> -#define MESON_CANVAS_ENDIAN_SWAP128 0xf
> -
> -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 endian);
> -
> -#endif /* __MESON_CANVAS_H */
> diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
> index 4f5c67f70c4d..37c70641bcdf 100644
> --- a/drivers/gpu/drm/meson/meson_crtc.c
> +++ b/drivers/gpu/drm/meson/meson_crtc.c
> @@ -37,7 +37,6 @@
> #include "meson_venc.h"
> #include "meson_vpp.h"
> #include "meson_viu.h"
> -#include "meson_canvas.h"
> #include "meson_registers.h"
>
> /* CRTC definition */
> @@ -214,13 +213,7 @@ void meson_crtc_irq(struct meson_drm *priv)
> writel_relaxed(priv->viu.osd_sc_v_ctrl0,
> priv->io_base + _REG(VPP_OSD_VSC_CTRL0));
>
> - if (priv->canvas)
> - meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
> - priv->viu.osd1_addr, priv->viu.osd1_stride,
> - priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR, 0);
> - else
> - meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
> + meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
> priv->viu.osd1_addr, priv->viu.osd1_stride,
> priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> MESON_CANVAS_BLKMODE_LINEAR, 0);
> @@ -237,61 +230,34 @@ void meson_crtc_irq(struct meson_drm *priv)
>
> switch (priv->viu.vd1_planes) {
> case 3:
> - if (priv->canvas)
> - meson_canvas_config(priv->canvas,
> - priv->canvas_id_vd1_2,
> - priv->viu.vd1_addr2,
> - priv->viu.vd1_stride2,
> - priv->viu.vd1_height2,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> - else
> - meson_canvas_setup(priv, MESON_CANVAS_ID_VD1_2,
> - priv->viu.vd1_addr2,
> - priv->viu.vd1_stride2,
> - priv->viu.vd1_height2,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> + meson_canvas_config(priv->canvas,
> + priv->canvas_id_vd1_2,
> + priv->viu.vd1_addr2,
> + priv->viu.vd1_stride2,
> + priv->viu.vd1_height2,
> + MESON_CANVAS_WRAP_NONE,
> + MESON_CANVAS_BLKMODE_LINEAR,
> + MESON_CANVAS_ENDIAN_SWAP64);
> /* fallthrough */
> case 2:
> - if (priv->canvas)
> - meson_canvas_config(priv->canvas,
> - priv->canvas_id_vd1_1,
> - priv->viu.vd1_addr1,
> - priv->viu.vd1_stride1,
> - priv->viu.vd1_height1,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> - else
> - meson_canvas_setup(priv, MESON_CANVAS_ID_VD1_1,
> - priv->viu.vd1_addr2,
> - priv->viu.vd1_stride2,
> - priv->viu.vd1_height2,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> + meson_canvas_config(priv->canvas,
> + priv->canvas_id_vd1_1,
> + priv->viu.vd1_addr1,
> + priv->viu.vd1_stride1,
> + priv->viu.vd1_height1,
> + MESON_CANVAS_WRAP_NONE,
> + MESON_CANVAS_BLKMODE_LINEAR,
> + MESON_CANVAS_ENDIAN_SWAP64);
> /* fallthrough */
> case 1:
> - if (priv->canvas)
> - meson_canvas_config(priv->canvas,
> - priv->canvas_id_vd1_0,
> - priv->viu.vd1_addr0,
> - priv->viu.vd1_stride0,
> - priv->viu.vd1_height0,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> - else
> - meson_canvas_setup(priv, MESON_CANVAS_ID_VD1_0,
> - priv->viu.vd1_addr2,
> - priv->viu.vd1_stride2,
> - priv->viu.vd1_height2,
> - MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR,
> - MESON_CANVAS_ENDIAN_SWAP64);
> + meson_canvas_config(priv->canvas,
> + priv->canvas_id_vd1_0,
> + priv->viu.vd1_addr0,
> + priv->viu.vd1_stride0,
> + priv->viu.vd1_height0,
> + MESON_CANVAS_WRAP_NONE,
> + MESON_CANVAS_BLKMODE_LINEAR,
> + MESON_CANVAS_ENDIAN_SWAP64);
> };
>
> writel_relaxed(priv->viu.vd1_if0_gen_reg,
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 12ff47b13668..7913983f2981 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -48,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"
> @@ -214,50 +213,31 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> }
>
> priv->canvas = meson_canvas_get(dev);
> - if (!IS_ERR(priv->canvas)) {
> - ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
> - if (ret)
> - goto free_drm;
> - ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_0);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - goto free_drm;
> - }
> - ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_1);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> - goto free_drm;
> - }
> - ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_2);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_1);
> - goto free_drm;
> - }
> - } else {
> - priv->canvas = NULL;
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
> - if (!res) {
> - ret = -EINVAL;
> - 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;
> - goto free_drm;
> - }
> + if (IS_ERR(priv->canvas)) {
> + ret = PTR_ERR(priv->canvas);
> + 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);
> - goto free_drm;
> - }
> + ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
> + if (ret)
> + goto free_drm;
> + ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_0);
> + if (ret) {
> + meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> + goto free_drm;
> + }
> + ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_1);
> + if (ret) {
> + meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> + goto free_drm;
> + }
> + ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_2);
> + if (ret) {
> + meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_1);
> + goto free_drm;
> }
>
> priv->vsync_irq = platform_get_irq(pdev, 0);
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 4dccf4cd042a..214a7cb18ce2 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -29,7 +29,6 @@ struct meson_drm {
> struct device *dev;
> void __iomem *io_base;
> struct regmap *hhi;
> - struct regmap *dmc;
> int vsync_irq;
>
> struct meson_canvas *canvas;
> diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c
> index 691a9fd16b36..b54a22e483b9 100644
> --- a/drivers/gpu/drm/meson/meson_overlay.c
> +++ b/drivers/gpu/drm/meson/meson_overlay.c
> @@ -22,7 +22,6 @@
> #include "meson_overlay.h"
> #include "meson_vpp.h"
> #include "meson_viu.h"
> -#include "meson_canvas.h"
> #include "meson_registers.h"
>
> /* VD1_IF0_GEN_REG */
> @@ -350,13 +349,6 @@ static void meson_overlay_atomic_update(struct drm_plane *plane,
>
> DRM_DEBUG_DRIVER("\n");
>
> - /* Fallback is canvas provider is not available */
> - if (!priv->canvas) {
> - priv->canvas_id_vd1_0 = MESON_CANVAS_ID_VD1_0;
> - priv->canvas_id_vd1_1 = MESON_CANVAS_ID_VD1_1;
> - priv->canvas_id_vd1_2 = MESON_CANVAS_ID_VD1_2;
> - }
> -
> interlace_mode = state->crtc->mode.flags & DRM_MODE_FLAG_INTERLACE;
>
> spin_lock_irqsave(&priv->drm->event_lock, flags);
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 6119a0224278..b7786218cb10 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -38,7 +38,6 @@
> #include "meson_plane.h"
> #include "meson_vpp.h"
> #include "meson_viu.h"
> -#include "meson_canvas.h"
> #include "meson_registers.h"
>
> /* OSD_SCI_WH_M1 */
> @@ -148,10 +147,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
> (0xFF << OSD_GLOBAL_ALPHA_SHIFT) |
> OSD_BLK0_ENABLE;
>
> - if (priv->canvas)
> - canvas_id_osd1 = priv->canvas_id_osd1;
> - else
> - canvas_id_osd1 = MESON_CANVAS_ID_OSD1;
> + canvas_id_osd1 = priv->canvas_id_osd1;
>
> /* Set up BLK0 to point to the right canvas */
> priv->viu.osd1_blk0_cfg[0] = ((canvas_id_osd1 << OSD_CANVAS_SEL) |
> diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c
> index e46e05f50bad..ac0f3687e09a 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"
>
> /**
>

Finally !

Acked-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>