2021-07-14 10:13:08

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 0/7] Add support to the mmsys driver to be a reset controller

Dear all,

The following patchset is a reimplementation of the patch sent by Jitao
Shi [1] some time ago. As suggested by Chun-Kuang Hu, this time the
reset is done using the reset API, where the mmsys driver is the reset
controller and the mtk_dsi driver is the reset consumer.

Note that the first patch is kind of unrelated change, it's just a
cleanup but is needed if you want to apply all the following patches
cleanly.

This patchset is important in order to have the DSI panel working on some
kukui MT8183 Chromebooks (i.e Lenovo IdeaPad Duet). Without it, you just
get a black screen.

Best regards,
Enric

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/


Changes in v2:
- Fix build test ERROR Reported-by: kernel test robot <[email protected]>
- Added a new patch to describe the dsi reset optional property.

Enric Balletbo i Serra (7):
arm64: dts: mediatek: Move reset controller constants into common
location
dt-bindings: mediatek: Add #reset-cells to mmsys system controller
dt-bindings: display: mediatek: add dsi reset optional property
arm64: dts: mt8173: Add the mmsys reset bit to reset the dsi0
arm64: dts: mt8183: Add the mmsys reset bit to reset the dsi0
soc: mediatek: mmsys: Add reset controller support
drm/mediatek: mtk_dsi: Reset the dsi0 hardware

.../bindings/arm/mediatek/mediatek,mmsys.txt | 2 +
.../display/mediatek/mediatek,dsi.txt | 6 ++
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 2 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 5 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +-
drivers/soc/mediatek/mtk-mmsys.c | 69 +++++++++++++++++++
drivers/soc/mediatek/mtk-mmsys.h | 2 +
drivers/watchdog/mtk_wdt.c | 6 +-
.../mt2712-resets.h | 0
include/dt-bindings/reset/mt8173-resets.h | 2 +
.../mt8183-resets.h | 3 +
.../mt8192-resets.h | 0
12 files changed, 96 insertions(+), 6 deletions(-)
rename include/dt-bindings/{reset-controller => reset}/mt2712-resets.h (100%)
rename include/dt-bindings/{reset-controller => reset}/mt8183-resets.h (98%)
rename include/dt-bindings/{reset-controller => reset}/mt8192-resets.h (100%)

--
2.30.2


2021-07-14 10:13:41

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 4/7] arm64: dts: mt8173: Add the mmsys reset bit to reset the dsi0

Reset the DSI hardware is needed to prevent different settings between
the bootloader and the kernel.

Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

(no changes since v1)

arch/arm64/boot/dts/mediatek/mt8173.dtsi | 2 ++
include/dt-bindings/reset/mt8173-resets.h | 2 ++
2 files changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index a3de6374d495..2ac77d47bf81 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -1036,6 +1036,7 @@ mmsys: syscon@14000000 {
assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
assigned-clock-rates = <400000000>;
#clock-cells = <1>;
+ #reset-cells = <1>;
mboxes = <&gce 0 CMDQ_THR_PRIO_HIGHEST>,
<&gce 1 CMDQ_THR_PRIO_HIGHEST>;
mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0 0x1000>;
@@ -1261,6 +1262,7 @@ dsi0: dsi@1401b000 {
<&mmsys CLK_MM_DSI0_DIGITAL>,
<&mipi_tx0>;
clock-names = "engine", "digital", "hs";
+ resets = <&mmsys MT8173_MMSYS_SW0_RST_B_DISP_DSI0>;
phys = <&mipi_tx0>;
phy-names = "dphy";
status = "disabled";
diff --git a/include/dt-bindings/reset/mt8173-resets.h b/include/dt-bindings/reset/mt8173-resets.h
index ba8636eda5ae..6a60c7cecc4c 100644
--- a/include/dt-bindings/reset/mt8173-resets.h
+++ b/include/dt-bindings/reset/mt8173-resets.h
@@ -27,6 +27,8 @@
#define MT8173_INFRA_GCE_FAXI_RST 40
#define MT8173_INFRA_MMIOMMURST 47

+/* MMSYS resets */
+#define MT8173_MMSYS_SW0_RST_B_DISP_DSI0 25

/* PERICFG resets */
#define MT8173_PERI_UART0_SW_RST 0
--
2.30.2

2021-07-14 10:14:29

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support

Among other features the mmsys driver should implement a reset
controller to be able to reset different bits from their space.

Cc: Jitao Shi <[email protected]>
Suggested-by: Chun-Kuang Hu <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

(no changes since v1)

drivers/soc/mediatek/mtk-mmsys.c | 69 ++++++++++++++++++++++++++++++++
drivers/soc/mediatek/mtk-mmsys.h | 2 +
2 files changed, 71 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index e681029fe804..6ac4deff0164 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -4,10 +4,12 @@
* Author: James Liao <[email protected]>
*/

+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
#include <linux/soc/mediatek/mtk-mmsys.h>

#include "mtk-mmsys.h"
@@ -55,6 +57,8 @@ static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
struct mtk_mmsys {
void __iomem *regs;
const struct mtk_mmsys_driver_data *data;
+ spinlock_t lock; /* protects mmsys_sw_rst_b reg */
+ struct reset_controller_dev rcdev;
};

void mtk_mmsys_ddp_connect(struct device *dev,
@@ -91,6 +95,59 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
}
EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);

+static int mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned long id,
+ bool assert)
+{
+ struct mtk_mmsys *mmsys = container_of(rcdev, struct mtk_mmsys, rcdev);
+ unsigned long flags;
+ u32 reg;
+ int i;
+
+ spin_lock_irqsave(&mmsys->lock, flags);
+
+ reg = readl_relaxed(mmsys->regs + MMSYS_SW0_RST_B);
+
+ if (assert)
+ reg &= ~BIT(id);
+ else
+ reg |= BIT(id);
+
+ writel_relaxed(reg, mmsys->regs + MMSYS_SW0_RST_B);
+
+ spin_unlock_irqrestore(&mmsys->lock, flags);
+
+ return 0;
+}
+
+static int mtk_mmsys_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ return mtk_mmsys_reset_update(rcdev, id, true);
+}
+
+static int mtk_mmsys_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ return mtk_mmsys_reset_update(rcdev, id, false);
+}
+
+static int mtk_mmsys_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ int ret;
+
+ ret = mtk_mmsys_reset_assert(rcdev, id);
+ if (ret)
+ return ret;
+
+ usleep_range(1000, 1100);
+
+ return mtk_mmsys_reset_deassert(rcdev, id);
+}
+
+static const struct reset_control_ops mtk_mmsys_reset_ops = {
+ .assert = mtk_mmsys_reset_assert,
+ .deassert = mtk_mmsys_reset_deassert,
+ .reset = mtk_mmsys_reset,
+};
+
static int mtk_mmsys_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -111,6 +168,18 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
return ret;
}

+ spin_lock_init(&mmsys->lock);
+
+ mmsys->rcdev.owner = THIS_MODULE;
+ mmsys->rcdev.nr_resets = 32;
+ mmsys->rcdev.ops = &mtk_mmsys_reset_ops;
+ mmsys->rcdev.of_node = pdev->dev.of_node;
+ ret = devm_reset_controller_register(&pdev->dev, &mmsys->rcdev);
+ if (ret) {
+ dev_err(&pdev->dev, "Couldn't register mmsys reset controller: %d\n", ret);
+ return ret;
+ }
+
mmsys->data = of_device_get_match_data(&pdev->dev);
platform_set_drvdata(pdev, mmsys);

diff --git a/drivers/soc/mediatek/mtk-mmsys.h b/drivers/soc/mediatek/mtk-mmsys.h
index 11388961dded..f9f9e12ceab8 100644
--- a/drivers/soc/mediatek/mtk-mmsys.h
+++ b/drivers/soc/mediatek/mtk-mmsys.h
@@ -66,6 +66,8 @@
#define DPI_SEL_IN_BLS 0x0
#define DSI_SEL_IN_RDMA 0x1

+#define MMSYS_SW0_RST_B 0x140
+
struct mtk_mmsys_routes {
u32 from_comp;
u32 to_comp;
--
2.30.2

2021-07-14 10:14:53

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Reset the dsi0 hardware

Reset dsi0 HW to default when power on. This prevents to have different
settingbetween the bootloader and the kernel.

As not all Mediatek boards have the reset consumer configured in their
board description, also is not needed on all of them, the reset is optional,
so the change is compatible with all boards.

Cc: Jitao Shi <[email protected]>
Suggested-by: Chun-Kuang Hu <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

(no changes since v1)

drivers/gpu/drm/mediatek/mtk_dsi.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index ae403c67cbd9..d8b81e2ab841 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -11,6 +11,7 @@
#include <linux/of_platform.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>

#include <video/mipi_display.h>
#include <video/videomode.h>
@@ -980,8 +981,10 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
struct mtk_dsi *dsi = dev_get_drvdata(dev);

ret = mtk_dsi_encoder_init(drm, dsi);
+ if (ret)
+ return ret;

- return ret;
+ return device_reset_optional(dev);
}

static void mtk_dsi_unbind(struct device *dev, struct device *master,
--
2.30.2

2021-07-14 10:15:21

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 3/7] dt-bindings: display: mediatek: add dsi reset optional property

Update device tree binding documentation for the dsi to add the optional
property to reset the dsi controller.

Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

Changes in v2:
- Added a new patch to describe the dsi reset optional property.

.../devicetree/bindings/display/mediatek/mediatek,dsi.txt | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
index 8238a86686be..3209b700ded6 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
@@ -19,6 +19,11 @@ Required properties:
Documentation/devicetree/bindings/graph.txt. This port should be connected
to the input port of an attached DSI panel or DSI-to-eDP encoder chip.

+Optional properties:
+- resets: list of phandle + reset specifier pair, as described in [1].
+
+[1] Documentation/devicetree/bindings/reset/reset.txt
+
MIPI TX Configuration Module
============================

@@ -45,6 +50,7 @@ dsi0: dsi@1401b000 {
clocks = <&mmsys MM_DSI0_ENGINE>, <&mmsys MM_DSI0_DIGITAL>,
<&mipi_tx0>;
clock-names = "engine", "digital", "hs";
+ resets = <&mmsys MT8173_MMSYS_SW0_RST_B_DISP_DSI0>;
phys = <&mipi_tx0>;
phy-names = "dphy";

--
2.30.2

2021-07-14 10:16:15

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v2 5/7] arm64: dts: mt8183: Add the mmsys reset bit to reset the dsi0

Reset the DSI hardware is needed to prevent different settings between
the bootloader and the kernel.

While here, also remove the undocumented and also not used
'mediatek,syscon-dsi' property.

Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

(no changes since v1)

arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 ++-
include/dt-bindings/reset/mt8183-resets.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 4ef0b5b23047..7ae108f8ba89 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1251,6 +1251,7 @@ mmsys: syscon@14000000 {
compatible = "mediatek,mt8183-mmsys", "syscon";
reg = <0 0x14000000 0 0x1000>;
#clock-cells = <1>;
+ #reset-cells = <1>;
mboxes = <&gce 0 CMDQ_THR_PRIO_HIGHEST>,
<&gce 1 CMDQ_THR_PRIO_HIGHEST>;
mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0 0x1000>;
@@ -1365,11 +1366,11 @@ dsi0: dsi@14014000 {
reg = <0 0x14014000 0 0x1000>;
interrupts = <GIC_SPI 236 IRQ_TYPE_LEVEL_LOW>;
power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
- mediatek,syscon-dsi = <&mmsys 0x140>;
clocks = <&mmsys CLK_MM_DSI0_MM>,
<&mmsys CLK_MM_DSI0_IF>,
<&mipi_tx0>;
clock-names = "engine", "digital", "hs";
+ resets = <&mmsys MT8183_MMSYS_SW0_RST_B_DISP_DSI0>;
phys = <&mipi_tx0>;
phy-names = "dphy";
};
diff --git a/include/dt-bindings/reset/mt8183-resets.h b/include/dt-bindings/reset/mt8183-resets.h
index a1bbd41e0d12..48c5d2de0a38 100644
--- a/include/dt-bindings/reset/mt8183-resets.h
+++ b/include/dt-bindings/reset/mt8183-resets.h
@@ -80,6 +80,9 @@

#define MT8183_INFRACFG_SW_RST_NUM 128

+/* MMSYS resets */
+#define MT8183_MMSYS_SW0_RST_B_DISP_DSI0 25
+
#define MT8183_TOPRGU_MM_SW_RST 1
#define MT8183_TOPRGU_MFG_SW_RST 2
#define MT8183_TOPRGU_VENC_SW_RST 3
--
2.30.2

2021-07-16 09:00:05

by Chen, Rong A

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support


Hi Enric,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on pza/reset/next linux/master linus/master
v5.14-rc1 next-20210714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/Add-support-to-the-mmsys-driver-to-be-a-reset-controller/20210714-181318
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
for-next
:::::: branch date: 14 hours ago
:::::: commit date: 14 hours ago
compiler: riscv64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> drivers/soc/mediatek/mtk-mmsys.c:104:6: warning: Unused variable: i [unusedVariable]
int i;
^

vim +104 drivers/soc/mediatek/mtk-mmsys.c

2c758e301ed95a Enric Balletbo i Serra 2020-03-25 97 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 98 static int
mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned long id,
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 99 bool assert)
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 100 {
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 101 struct mtk_mmsys
*mmsys = container_of(rcdev, struct mtk_mmsys, rcdev);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 102 unsigned long flags;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 103 u32 reg;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 @104 int i;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 105 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 106 spin_lock_irqsave(&mmsys->lock,
flags);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 107 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 108 reg = readl_relaxed(mmsys->regs
+ MMSYS_SW0_RST_B);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 109 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 110 if (assert)
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 111 reg &= ~BIT(id);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 112 else
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 113 reg |= BIT(id);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 114 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 115 writel_relaxed(reg, mmsys->regs
+ MMSYS_SW0_RST_B);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 116 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 117
spin_unlock_irqrestore(&mmsys->lock, flags);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 118 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 119 return 0;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 120 }
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 121
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2021-07-16 09:05:33

by Chen, Rong A

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support


Hi Enric,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on pza/reset/next linux/master linus/master
v5.14-rc1 next-20210714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/Add-support-to-the-mmsys-driver-to-be-a-reset-controller/20210714-181318
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
for-next
compiler: riscv64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> drivers/soc/mediatek/mtk-mmsys.c:104:6: warning: Unused variable: i [unusedVariable]
int i;
^

vim +104 drivers/soc/mediatek/mtk-mmsys.c

2c758e301ed95a Enric Balletbo i Serra 2020-03-25 97 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 98 static int
mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned long id,
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 99 bool assert)
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 100 {
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 101 struct mtk_mmsys
*mmsys = container_of(rcdev, struct mtk_mmsys, rcdev);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 102 unsigned long flags;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 103 u32 reg;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 @104 int i;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 105 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 106 spin_lock_irqsave(&mmsys->lock,
flags);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 107 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 108 reg = readl_relaxed(mmsys->regs
+ MMSYS_SW0_RST_B);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 109 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 110 if (assert)
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 111 reg &= ~BIT(id);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 112 else
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 113 reg |= BIT(id);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 114 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 115 writel_relaxed(reg, mmsys->regs
+ MMSYS_SW0_RST_B);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 116 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 117
spin_unlock_irqrestore(&mmsys->lock, flags);
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 118 08a3068f9490f0
Enric Balletbo i Serra 2021-07-14 119 return 0;
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 120 }
08a3068f9490f0 Enric Balletbo i Serra 2021-07-14 121
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2021-07-16 17:56:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] dt-bindings: display: mediatek: add dsi reset optional property

On Wed, 14 Jul 2021 12:11:37 +0200, Enric Balletbo i Serra wrote:
> Update device tree binding documentation for the dsi to add the optional
> property to reset the dsi controller.
>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> Changes in v2:
> - Added a new patch to describe the dsi reset optional property.
>
> .../devicetree/bindings/display/mediatek/mediatek,dsi.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2021-07-16 17:57:21

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] arm64: dts: mt8173: Add the mmsys reset bit to reset the dsi0

On Wed, 14 Jul 2021 12:11:38 +0200, Enric Balletbo i Serra wrote:
> Reset the DSI hardware is needed to prevent different settings between
> the bootloader and the kernel.
>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 2 ++
> include/dt-bindings/reset/mt8173-resets.h | 2 ++
> 2 files changed, 4 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2021-07-16 17:59:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 5/7] arm64: dts: mt8183: Add the mmsys reset bit to reset the dsi0

On Wed, 14 Jul 2021 12:11:39 +0200, Enric Balletbo i Serra wrote:
> Reset the DSI hardware is needed to prevent different settings between
> the bootloader and the kernel.
>
> While here, also remove the undocumented and also not used
> 'mediatek,syscon-dsi' property.
>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 ++-
> include/dt-bindings/reset/mt8183-resets.h | 3 +++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>

Acked-by: Rob Herring <[email protected]>

2021-07-20 11:02:40

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support

Hi Enric,

On Wed, 2021-07-14 at 12:11 +0200, Enric Balletbo i Serra wrote:
> Among other features the mmsys driver should implement a reset
> controller to be able to reset different bits from their space.
>
> Cc: Jitao Shi <[email protected]>
> Suggested-by: Chun-Kuang Hu <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>

The reset controller driver looks fine, just two questions below.

> ---
>
> (no changes since v1)
>
> drivers/soc/mediatek/mtk-mmsys.c | 69 ++++++++++++++++++++++++++++++++
> drivers/soc/mediatek/mtk-mmsys.h | 2 +
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index e681029fe804..6ac4deff0164 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
[...]
> @@ -91,6 +95,59 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
[...]
> +static int mtk_mmsys_reset(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + int ret;
> +
> + ret = mtk_mmsys_reset_assert(rcdev, id);
> + if (ret)
> + return ret;
> +
> + usleep_range(1000, 1100);

Is this known to be enough for all IP cores that can be reset by this
controller?

> + return mtk_mmsys_reset_deassert(rcdev, id);
> +}
> +
> +static const struct reset_control_ops mtk_mmsys_reset_ops = {
> + .assert = mtk_mmsys_reset_assert,
> + .deassert = mtk_mmsys_reset_deassert,
> + .reset = mtk_mmsys_reset,
> +};
> +
> static int mtk_mmsys_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -111,6 +168,18 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
> return ret;
> }
>
> + spin_lock_init(&mmsys->lock);
> +
> + mmsys->rcdev.owner = THIS_MODULE;
> + mmsys->rcdev.nr_resets = 32;

Are all bits in the MMSYS_SW0_RST_B register individual reset controls?

regards
Philipp

2021-07-20 17:13:53

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support

Hi Philipp,

Thank you to take a look

On 20/7/21 12:52, Philipp Zabel wrote:
> Hi Enric,
>
> On Wed, 2021-07-14 at 12:11 +0200, Enric Balletbo i Serra wrote:
>> Among other features the mmsys driver should implement a reset
>> controller to be able to reset different bits from their space.
>>
>> Cc: Jitao Shi <[email protected]>
>> Suggested-by: Chun-Kuang Hu <[email protected]>
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>
> The reset controller driver looks fine, just two questions below.
>
>> ---
>>
>> (no changes since v1)
>>
>> drivers/soc/mediatek/mtk-mmsys.c | 69 ++++++++++++++++++++++++++++++++
>> drivers/soc/mediatek/mtk-mmsys.h | 2 +
>> 2 files changed, 71 insertions(+)
>>
>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
>> index e681029fe804..6ac4deff0164 100644
>> --- a/drivers/soc/mediatek/mtk-mmsys.c
>> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> [...]
>> @@ -91,6 +95,59 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
> [...]
>> +static int mtk_mmsys_reset(struct reset_controller_dev *rcdev, unsigned long id)
>> +{
>> + int ret;
>> +
>> + ret = mtk_mmsys_reset_assert(rcdev, id);
>> + if (ret)
>> + return ret;
>> +
>> + usleep_range(1000, 1100);
>
> Is this known to be enough for all IP cores that can be reset by this
> controller?
>

This time is copied from the downstream kernel, so, tbh, I am not totally sure
is enough or needed. Let me try to reach the Mediatek people for if they can
answer this.


>> + return mtk_mmsys_reset_deassert(rcdev, id);
>> +}
>> +
>> +static const struct reset_control_ops mtk_mmsys_reset_ops = {
>> + .assert = mtk_mmsys_reset_assert,
>> + .deassert = mtk_mmsys_reset_deassert,
>> + .reset = mtk_mmsys_reset,
>> +};
>> +
>> static int mtk_mmsys_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -111,6 +168,18 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> + spin_lock_init(&mmsys->lock);
>> +
>> + mmsys->rcdev.owner = THIS_MODULE;
>> + mmsys->rcdev.nr_resets = 32;
>
> Are all bits in the MMSYS_SW0_RST_B register individual reset controls?

Yes, all are individual reset controls, mostly related to display but not all
(i.e dsi, dpi ...)

Thanks,
Enric

>
> regards
> Philipp
>

2021-07-21 10:13:00

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support

On Tue, 2021-07-20 at 19:07 +0200, Enric Balletbo i Serra wrote:
> Hi Philipp,
>
> Thank you to take a look
>
> On 20/7/21 12:52, Philipp Zabel wrote:
> > Hi Enric,
> >
> > On Wed, 2021-07-14 at 12:11 +0200, Enric Balletbo i Serra wrote:
> > > Among other features the mmsys driver should implement a reset
> > > controller to be able to reset different bits from their space.
> > >
> > > Cc: Jitao Shi <[email protected]>
> > > Suggested-by: Chun-Kuang Hu <[email protected]>
> > > Signed-off-by: Enric Balletbo i Serra <[email protected]>
> >
> > The reset controller driver looks fine, just two questions below.
> >
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > drivers/soc/mediatek/mtk-mmsys.c | 69 ++++++++++++++++++++++++++++++++
> > > drivers/soc/mediatek/mtk-mmsys.h | 2 +
> > > 2 files changed, 71 insertions(+)
> > >
> > > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> > > index e681029fe804..6ac4deff0164 100644
> > > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > [...]
> > > @@ -91,6 +95,59 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
> > [...]
> > > +static int mtk_mmsys_reset(struct reset_controller_dev *rcdev, unsigned long id)
> > > +{
> > > + int ret;
> > > +
> > > + ret = mtk_mmsys_reset_assert(rcdev, id);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + usleep_range(1000, 1100);
> >
> > Is this known to be enough for all IP cores that can be reset by this
> > controller?
> >
>
> This time is copied from the downstream kernel, so, tbh, I am not totally sure
> is enough or needed. Let me try to reach the Mediatek people for if they can
> answer this.

That would be great. When this is resolved either way, feel free to add

Reviewed-by: Philipp Zabel <[email protected]>

regards
Philipp

2021-08-02 23:24:15

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Reset the dsi0 hardware

Hi, Enric:

Enric Balletbo i Serra <[email protected]> 於 2021年7月14日 週三 下午6:12寫道:
>
> Reset dsi0 HW to default when power on. This prevents to have different
> settingbetween the bootloader and the kernel.
>
> As not all Mediatek boards have the reset consumer configured in their
> board description, also is not needed on all of them, the reset is optional,
> so the change is compatible with all boards.

Acked-by: Chun-Kuang Hu <[email protected]>

>
> Cc: Jitao Shi <[email protected]>
> Suggested-by: Chun-Kuang Hu <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index ae403c67cbd9..d8b81e2ab841 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -11,6 +11,7 @@
> #include <linux/of_platform.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/reset.h>
>
> #include <video/mipi_display.h>
> #include <video/videomode.h>
> @@ -980,8 +981,10 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> struct mtk_dsi *dsi = dev_get_drvdata(dev);
>
> ret = mtk_dsi_encoder_init(drm, dsi);
> + if (ret)
> + return ret;
>
> - return ret;
> + return device_reset_optional(dev);
> }
>
> static void mtk_dsi_unbind(struct device *dev, struct device *master,
> --
> 2.30.2
>

2021-08-06 23:57:51

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] soc: mediatek: mmsys: Add reset controller support



On 14/07/2021 12:11, Enric Balletbo i Serra wrote:
> Among other features the mmsys driver should implement a reset
> controller to be able to reset different bits from their space.
>
> Cc: Jitao Shi <[email protected]>
> Suggested-by: Chun-Kuang Hu <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>

I'm happy to take this, as soon as we have the updated binding description.

Regards,
Matthias

> ---
>
> (no changes since v1)
>
> drivers/soc/mediatek/mtk-mmsys.c | 69 ++++++++++++++++++++++++++++++++
> drivers/soc/mediatek/mtk-mmsys.h | 2 +
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index e681029fe804..6ac4deff0164 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -4,10 +4,12 @@
> * Author: James Liao <[email protected]>
> */
>
> +#include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> #include <linux/soc/mediatek/mtk-mmsys.h>
>
> #include "mtk-mmsys.h"
> @@ -55,6 +57,8 @@ static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
> struct mtk_mmsys {
> void __iomem *regs;
> const struct mtk_mmsys_driver_data *data;
> + spinlock_t lock; /* protects mmsys_sw_rst_b reg */
> + struct reset_controller_dev rcdev;
> };
>
> void mtk_mmsys_ddp_connect(struct device *dev,
> @@ -91,6 +95,59 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
>
> +static int mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned long id,
> + bool assert)
> +{
> + struct mtk_mmsys *mmsys = container_of(rcdev, struct mtk_mmsys, rcdev);
> + unsigned long flags;
> + u32 reg;
> + int i;
> +
> + spin_lock_irqsave(&mmsys->lock, flags);
> +
> + reg = readl_relaxed(mmsys->regs + MMSYS_SW0_RST_B);
> +
> + if (assert)
> + reg &= ~BIT(id);
> + else
> + reg |= BIT(id);
> +
> + writel_relaxed(reg, mmsys->regs + MMSYS_SW0_RST_B);
> +
> + spin_unlock_irqrestore(&mmsys->lock, flags);
> +
> + return 0;
> +}
> +
> +static int mtk_mmsys_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + return mtk_mmsys_reset_update(rcdev, id, true);
> +}
> +
> +static int mtk_mmsys_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + return mtk_mmsys_reset_update(rcdev, id, false);
> +}
> +
> +static int mtk_mmsys_reset(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + int ret;
> +
> + ret = mtk_mmsys_reset_assert(rcdev, id);
> + if (ret)
> + return ret;
> +
> + usleep_range(1000, 1100);
> +
> + return mtk_mmsys_reset_deassert(rcdev, id);
> +}
> +
> +static const struct reset_control_ops mtk_mmsys_reset_ops = {
> + .assert = mtk_mmsys_reset_assert,
> + .deassert = mtk_mmsys_reset_deassert,
> + .reset = mtk_mmsys_reset,
> +};
> +
> static int mtk_mmsys_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -111,6 +168,18 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
> return ret;
> }
>
> + spin_lock_init(&mmsys->lock);
> +
> + mmsys->rcdev.owner = THIS_MODULE;
> + mmsys->rcdev.nr_resets = 32;
> + mmsys->rcdev.ops = &mtk_mmsys_reset_ops;
> + mmsys->rcdev.of_node = pdev->dev.of_node;
> + ret = devm_reset_controller_register(&pdev->dev, &mmsys->rcdev);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't register mmsys reset controller: %d\n", ret);
> + return ret;
> + }
> +
> mmsys->data = of_device_get_match_data(&pdev->dev);
> platform_set_drvdata(pdev, mmsys);
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.h b/drivers/soc/mediatek/mtk-mmsys.h
> index 11388961dded..f9f9e12ceab8 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.h
> +++ b/drivers/soc/mediatek/mtk-mmsys.h
> @@ -66,6 +66,8 @@
> #define DPI_SEL_IN_BLS 0x0
> #define DSI_SEL_IN_RDMA 0x1
>
> +#define MMSYS_SW0_RST_B 0x140
> +
> struct mtk_mmsys_routes {
> u32 from_comp;
> u32 to_comp;
>

2021-08-06 23:59:58

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Reset the dsi0 hardware



On 14/07/2021 12:11, Enric Balletbo i Serra wrote:
> Reset dsi0 HW to default when power on. This prevents to have different
> settingbetween the bootloader and the kernel.

settings between the...

>
> As not all Mediatek boards have the reset consumer configured in their
> board description, also is not needed on all of them, the reset is optional,
> so the change is compatible with all boards.
>
> Cc: Jitao Shi <[email protected]>
> Suggested-by: Chun-Kuang Hu <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>

Reviewed-by: Matthias Brugger <[email protected]>

> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index ae403c67cbd9..d8b81e2ab841 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -11,6 +11,7 @@
> #include <linux/of_platform.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/reset.h>
>
> #include <video/mipi_display.h>
> #include <video/videomode.h>
> @@ -980,8 +981,10 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> struct mtk_dsi *dsi = dev_get_drvdata(dev);
>
> ret = mtk_dsi_encoder_init(drm, dsi);
> + if (ret)
> + return ret;
>
> - return ret;
> + return device_reset_optional(dev);
> }
>
> static void mtk_dsi_unbind(struct device *dev, struct device *master,
>