2020-02-24 19:13:06

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 0/3] ti-sysc changes for probing DSS with dts data

Hi all,

Here are some changes to start probing display susbsystem (DSS) with
device tree data instead of platform data.

These changes are against v5.6-rc1, and depend on the earlier series
"[PATCH 0/7] ti-sysc driver fix for hdq1w and few improvments".

I'll be posting the related dts changes separately.

Regards,

Tony


Tony Lindgren (3):
drm/omap: Prepare DSS for probing without legacy platform data
bus: ti-sysc: Detect display subsystem related devices
bus: ti-sysc: Implement display subsystem reset quirk

drivers/bus/ti-sysc.c | 144 ++++++++++++++++++
drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++-
.../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 ++-
include/linux/platform_data/ti-sysc.h | 1 +
4 files changed, 184 insertions(+), 11 deletions(-)

--
2.25.1


2020-02-24 19:13:12

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

In order to probe display subsystem (DSS) components with ti-sysc
interconnect target module without legacy platform data and using
devicetree, we need to update dss probing a bit.

In the device tree, we will be defining the data also for the interconnect
target modules as DSS really is a private interconnect. There is some
information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
example where it mentions "32-bit interconnect (SLX)".

The changes we need to make are:

1. Parse also device tree subnodes for the compatible property fixup

2. Update the component code to consider device tree subnodes

Cc: [email protected]
Cc: Jyri Sarha <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---

This is needed for dropping DSS platform data that I'll be posting
seprately. If this looks OK, can you guys please test and ack?

---
drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++++++++++++++++---
.../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 +++++++++++++------
2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -1339,9 +1339,15 @@ static int dss_component_compare(struct device *dev, void *data)
return dev == child;
}

+struct dss_component_match_data {
+ struct device *dev;
+ struct component_match **match;
+};
+
static int dss_add_child_component(struct device *dev, void *data)
{
- struct component_match **match = data;
+ struct dss_component_match_data *cmatch = data;
+ struct component_match **match = cmatch->match;

/*
* HACK
@@ -1352,7 +1358,17 @@ static int dss_add_child_component(struct device *dev, void *data)
if (strstr(dev_name(dev), "rfbi"))
return 0;

- component_match_add(dev->parent, match, dss_component_compare, dev);
+ /*
+ * Handle possible interconnect target modules defined within the DSS.
+ * The DSS components can be children of an interconnect target module
+ * after the device tree has been updated for the module data.
+ * See also omapdss_boot_init() for compatible fixup.
+ */
+ if (strstr(dev_name(dev), "target-module"))
+ return device_for_each_child(dev, cmatch,
+ dss_add_child_component);
+
+ component_match_add(cmatch->dev, match, dss_component_compare, dev);

return 0;
}
@@ -1395,6 +1411,7 @@ static int dss_probe_hardware(struct dss_device *dss)
static int dss_probe(struct platform_device *pdev)
{
const struct soc_device_attribute *soc;
+ struct dss_component_match_data cmatch;
struct component_match *match = NULL;
struct resource *dss_mem;
struct dss_device *dss;
@@ -1472,7 +1489,9 @@ static int dss_probe(struct platform_device *pdev)

omapdss_gather_components(&pdev->dev);

- device_for_each_child(&pdev->dev, &match, dss_add_child_component);
+ cmatch.dev = &pdev->dev;
+ cmatch.match = &match;
+ device_for_each_child(&pdev->dev, &cmatch, dss_add_child_component);

r = component_master_add_with_match(&pdev->dev, &dss_component_ops, match);
if (r)
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
--- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
@@ -183,9 +183,24 @@ static const struct of_device_id omapdss_of_fixups_whitelist[] __initconst = {
{},
};

+static void __init omapdss_find_children(struct device_node *np)
+{
+ struct device_node *child;
+
+ for_each_available_child_of_node(np, child) {
+ if (!of_find_property(child, "compatible", NULL))
+ continue;
+
+ omapdss_walk_device(child, true);
+
+ if (of_device_is_compatible(child, "ti,sysc"))
+ omapdss_find_children(child);
+ }
+}
+
static int __init omapdss_boot_init(void)
{
- struct device_node *dss, *child;
+ struct device_node *dss;

INIT_LIST_HEAD(&dss_conv_list);

@@ -195,13 +210,7 @@ static int __init omapdss_boot_init(void)
return 0;

omapdss_walk_device(dss, true);
-
- for_each_available_child_of_node(dss, child) {
- if (!of_find_property(child, "compatible", NULL))
- continue;
-
- omapdss_walk_device(child, true);
- }
+ omapdss_find_children(dss);

while (!list_empty(&dss_conv_list)) {
struct dss_conv_node *n;
--
2.25.1

2020-02-24 19:13:14

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 2/3] bus: ti-sysc: Detect display subsystem related devices

In order to prepare probing display subsystem (DSS) with ti-sysc
interconnect target module driver and device tree data, let's
detect DSS related modules.

We need to also add reset quirk handling for DSS, but until that's
done, let's just enable the optional clock quirks for DSS and
omap4 HDMI. The rest is just naming of modules if CONFIG_DEBUG
is set.

Cc: Jyri Sarha <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1302,10 +1302,18 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
SYSC_MODULE_QUIRK_AESS),
SYSC_QUIRK("dcan", 0x48480000, 0x20, -ENODEV, -ENODEV, 0xa3170504, 0xffffffff,
SYSC_QUIRK_CLKDM_NOAUTO),
+ SYSC_QUIRK("dss", 0x4832a000, 0, 0x10, 0x14, 0x00000020, 0xffffffff,
+ SYSC_QUIRK_OPT_CLKS_IN_RESET),
+ SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000040, 0xffffffff,
+ SYSC_QUIRK_OPT_CLKS_IN_RESET),
+ SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000061, 0xffffffff,
+ SYSC_QUIRK_OPT_CLKS_IN_RESET),
SYSC_QUIRK("dwc3", 0x48880000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
SYSC_QUIRK_CLKDM_NOAUTO),
SYSC_QUIRK("dwc3", 0x488c0000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
SYSC_QUIRK_CLKDM_NOAUTO),
+ SYSC_QUIRK("hdmi", 0, 0, 0x10, -ENODEV, 0x50030200, 0xffffffff,
+ SYSC_QUIRK_OPT_CLKS_NEEDED),
SYSC_QUIRK("hdq1w", 0, 0, 0x14, 0x18, 0x00000006, 0xffffffff,
SYSC_MODULE_QUIRK_HDQ1W),
SYSC_QUIRK("hdq1w", 0, 0, 0x14, 0x18, 0x0000000a, 0xffffffff,
@@ -1342,13 +1350,21 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
0xffff00f0, 0),
SYSC_QUIRK("dcan", 0, 0x20, -ENODEV, -ENODEV, 0xa3170504, 0xffffffff, 0),
SYSC_QUIRK("dcan", 0, 0x20, -ENODEV, -ENODEV, 0x4edb1902, 0xffffffff, 0),
+ SYSC_QUIRK("dispc", 0x4832a400, 0, 0x10, 0x14, 0x00000030, 0xffffffff, 0),
+ SYSC_QUIRK("dispc", 0x58001000, 0, 0x10, 0x14, 0x00000040, 0xffffffff, 0),
+ SYSC_QUIRK("dispc", 0x58001000, 0, 0x10, 0x14, 0x00000051, 0xffffffff, 0),
SYSC_QUIRK("dmic", 0, 0, 0x10, -ENODEV, 0x50010000, 0xffffffff, 0),
+ SYSC_QUIRK("dsi", 0x58004000, 0, 0x10, 0x14, 0x00000030, 0xffffffff, 0),
+ SYSC_QUIRK("dsi", 0x58005000, 0, 0x10, 0x14, 0x00000030, 0xffffffff, 0),
+ SYSC_QUIRK("dsi", 0x58005000, 0, 0x10, 0x14, 0x00000040, 0xffffffff, 0),
+ SYSC_QUIRK("dsi", 0x58009000, 0, 0x10, 0x14, 0x00000040, 0xffffffff, 0),
SYSC_QUIRK("dwc3", 0, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff, 0),
SYSC_QUIRK("d2d", 0x4a0b6000, 0, 0x10, 0x14, 0x00000010, 0xffffffff, 0),
SYSC_QUIRK("d2d", 0x4a0cd000, 0, 0x10, 0x14, 0x00000010, 0xffffffff, 0),
SYSC_QUIRK("epwmss", 0, 0, 0x4, -ENODEV, 0x47400001, 0xffffffff, 0),
SYSC_QUIRK("gpu", 0, 0x1fc00, 0x1fc10, -ENODEV, 0, 0, 0),
SYSC_QUIRK("gpu", 0, 0xfe00, 0xfe10, -ENODEV, 0x40000000 , 0xffffffff, 0),
+ SYSC_QUIRK("hdmi", 0, 0, 0x10, -ENODEV, 0x50031d00, 0xffffffff, 0),
SYSC_QUIRK("hsi", 0, 0, 0x10, 0x14, 0x50043101, 0xffffffff, 0),
SYSC_QUIRK("iss", 0, 0, 0x10, -ENODEV, 0x40000101, 0xffffffff, 0),
SYSC_QUIRK("lcdc", 0, 0, 0x54, -ENODEV, 0x4f201000, 0xffffffff, 0),
@@ -1366,6 +1382,8 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
SYSC_QUIRK("prcm", 0, 0, -ENODEV, -ENODEV, 0x40000100, 0xffffffff, 0),
SYSC_QUIRK("prcm", 0, 0, -ENODEV, -ENODEV, 0x00004102, 0xffffffff, 0),
SYSC_QUIRK("prcm", 0, 0, -ENODEV, -ENODEV, 0x40000400, 0xffffffff, 0),
+ SYSC_QUIRK("rfbi", 0x4832a800, 0, 0x10, 0x14, 0x00000010, 0xffffffff, 0),
+ SYSC_QUIRK("rfbi", 0x58002000, 0, 0x10, 0x14, 0x00000010, 0xffffffff, 0),
SYSC_QUIRK("scm", 0, 0, 0x10, -ENODEV, 0x40000900, 0xffffffff, 0),
SYSC_QUIRK("scm", 0, 0, -ENODEV, -ENODEV, 0x4e8b0100, 0xffffffff, 0),
SYSC_QUIRK("scm", 0, 0, -ENODEV, -ENODEV, 0x4f000100, 0xffffffff, 0),
@@ -1383,6 +1401,7 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
SYSC_QUIRK("usbhstll", 0, 0, 0x10, 0x14, 0x00000008, 0xffffffff, 0),
SYSC_QUIRK("usb_host_hs", 0, 0, 0x10, 0x14, 0x50700100, 0xffffffff, 0),
SYSC_QUIRK("usb_host_hs", 0, 0, 0x10, -ENODEV, 0x50700101, 0xffffffff, 0),
+ SYSC_QUIRK("venc", 0x58003000, 0, -ENODEV, -ENODEV, 0x00000002, 0xffffffff, 0),
SYSC_QUIRK("vfpe", 0, 0, 0x104, -ENODEV, 0x4d001200, 0xffffffff, 0),
#endif
};
--
2.25.1

2020-02-24 19:13:15

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

The display subsystem (DSS) needs the child outputs disabled for reset.
In order to prepare to probe DSS without legacy platform data, let's
implement sysc_pre_reset_quirk_dss() similar to what we have for the
platform data with omap_dss_reset().

Note that we cannot directly use the old omap_dss_reset() without
platform data callbacks and updating omap_dss_reset() to understand
struct device. And we will be dropping omap_dss_reset() anyways when
all the SoCs are probing with device tree, so let's not mess with the
legacy code at all.

Cc: Jyri Sarha <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 131 +++++++++++++++++++++++++-
include/linux/platform_data/ti-sysc.h | 1 +
2 files changed, 129 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1303,11 +1303,11 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
SYSC_QUIRK("dcan", 0x48480000, 0x20, -ENODEV, -ENODEV, 0xa3170504, 0xffffffff,
SYSC_QUIRK_CLKDM_NOAUTO),
SYSC_QUIRK("dss", 0x4832a000, 0, 0x10, 0x14, 0x00000020, 0xffffffff,
- SYSC_QUIRK_OPT_CLKS_IN_RESET),
+ SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000040, 0xffffffff,
- SYSC_QUIRK_OPT_CLKS_IN_RESET),
+ SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000061, 0xffffffff,
- SYSC_QUIRK_OPT_CLKS_IN_RESET),
+ SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
SYSC_QUIRK("dwc3", 0x48880000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
SYSC_QUIRK_CLKDM_NOAUTO),
SYSC_QUIRK("dwc3", 0x488c0000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
@@ -1468,6 +1468,128 @@ static void sysc_init_revision_quirks(struct sysc *ddata)
}
}

+/*
+ * DSS needs dispc outputs disabled to reset modules. Returns mask of
+ * enabled DSS interrupts. Eventually we may be able to do this on
+ * dispc init rather than top-level DSS init.
+ */
+static u32 sysc_quirk_dispc(struct sysc *ddata, int dispc_offset,
+ bool disable)
+{
+ bool lcd_en, digit_en, lcd2_en = false, lcd3_en = false;
+ const int lcd_en_mask = BIT(0), digit_en_mask = BIT(1);
+ int manager_count;
+ bool framedonetv_irq;
+ u32 val, irq_mask = 0;
+
+ switch (sysc_soc->soc) {
+ case SOC_2420 ... SOC_3630:
+ manager_count = 2;
+ framedonetv_irq = false;
+ break;
+ case SOC_4430 ... SOC_4470:
+ manager_count = 3;
+ break;
+ case SOC_5430:
+ case SOC_DRA7:
+ manager_count = 4;
+ break;
+ case SOC_AM4:
+ manager_count = 1;
+ break;
+ case SOC_UNKNOWN:
+ default:
+ return 0;
+ };
+
+ /* Remap the whole module range to be able to reset dispc outputs */
+ devm_iounmap(ddata->dev, ddata->module_va);
+ ddata->module_va = devm_ioremap(ddata->dev,
+ ddata->module_pa,
+ ddata->module_size);
+ if (!ddata->module_va)
+ return -EIO;
+
+ /* DISP_CONTROL */
+ val = sysc_read(ddata, dispc_offset + 0x40);
+ lcd_en = val & lcd_en_mask;
+ digit_en = val & digit_en_mask;
+ if (lcd_en)
+ irq_mask |= BIT(0); /* FRAMEDONE */
+ if (digit_en) {
+ if (framedonetv_irq)
+ irq_mask |= BIT(24); /* FRAMEDONETV */
+ else
+ irq_mask |= BIT(2) | BIT(3); /* EVSYNC bits */
+ }
+ if (disable & (lcd_en | digit_en))
+ sysc_write(ddata, dispc_offset + 0x40,
+ val & ~(lcd_en_mask | digit_en_mask));
+
+ if (manager_count <= 2)
+ return irq_mask;
+
+ /* DISPC_CONTROL2 */
+ val = sysc_read(ddata, dispc_offset + 0x238);
+ lcd2_en = val & lcd_en_mask;
+ if (lcd2_en)
+ irq_mask |= BIT(22); /* FRAMEDONE2 */
+ if (disable && lcd2_en)
+ sysc_write(ddata, dispc_offset + 0x238,
+ val & ~lcd_en_mask);
+
+ if (manager_count <= 3)
+ return irq_mask;
+
+ /* DISPC_CONTROL3 */
+ val = sysc_read(ddata, dispc_offset + 0x848);
+ lcd3_en = val & lcd_en_mask;
+ if (lcd3_en)
+ irq_mask |= BIT(30); /* FRAMEDONE3 */
+ if (disable && lcd3_en)
+ sysc_write(ddata, dispc_offset + 0x848,
+ val & ~lcd_en_mask);
+
+ return irq_mask;
+}
+
+/* DSS needs child outputs disabled and SDI registers cleared for reset */
+static void sysc_pre_reset_quirk_dss(struct sysc *ddata)
+{
+ const int dispc_offset = 0x1000;
+ int error;
+ u32 irq_mask, val;
+
+ /* Get enabled outputs */
+ irq_mask = sysc_quirk_dispc(ddata, dispc_offset, false);
+ if (!irq_mask)
+ return;
+
+ /* Clear IRQSTATUS */
+ sysc_write(ddata, 0x1000 + 0x18, irq_mask);
+
+ /* Disable outputs */
+ val = sysc_quirk_dispc(ddata, dispc_offset, true);
+
+ /* Poll IRQSTATUS */
+ error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18,
+ val, val != irq_mask, 100, 50);
+ if (error)
+ dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n",
+ __func__, val, irq_mask);
+
+ if (sysc_soc->soc == SOC_3430) {
+ /* Clear DSS_SDI_CONTROL */
+ sysc_write(ddata, dispc_offset + 0x44, 0);
+
+ /* Clear DSS_PLL_CONTROL */
+ sysc_write(ddata, dispc_offset + 0x48, 0);
+ }
+
+ /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
+ sysc_write(ddata, dispc_offset + 0x40, 0);
+}
+
/* 1-wire needs module's internal clocks enabled for reset */
static void sysc_pre_reset_quirk_hdq1w(struct sysc *ddata)
{
@@ -1606,6 +1728,9 @@ static void sysc_init_module_quirks(struct sysc *ddata)
if (ddata->cfg.quirks & SYSC_MODULE_QUIRK_AESS)
ddata->module_enable_quirk = sysc_module_enable_quirk_aess;

+ if (ddata->cfg.quirks & SYSC_MODULE_QUIRK_DSS_RESET)
+ ddata->pre_reset_quirk = sysc_pre_reset_quirk_dss;
+
if (ddata->cfg.quirks & SYSC_MODULE_QUIRK_RTC_UNLOCK) {
ddata->module_unlock_quirk = sysc_module_unlock_quirk_rtc;
ddata->module_lock_quirk = sysc_module_lock_quirk_rtc;
diff --git a/include/linux/platform_data/ti-sysc.h b/include/linux/platform_data/ti-sysc.h
--- a/include/linux/platform_data/ti-sysc.h
+++ b/include/linux/platform_data/ti-sysc.h
@@ -49,6 +49,7 @@ struct sysc_regbits {
s8 emufree_shift;
};

+#define SYSC_MODULE_QUIRK_DSS_RESET BIT(23)
#define SYSC_MODULE_QUIRK_RTC_UNLOCK BIT(22)
#define SYSC_QUIRK_CLKDM_NOAUTO BIT(21)
#define SYSC_QUIRK_FORCE_MSTANDBY BIT(20)
--
2.25.1

2020-02-24 21:42:18

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

Hi Tony,

Thank you for the patch.

On Mon, Feb 24, 2020 at 11:12:28AM -0800, Tony Lindgren wrote:
> In order to probe display subsystem (DSS) components with ti-sysc
> interconnect target module without legacy platform data and using
> devicetree, we need to update dss probing a bit.
>
> In the device tree, we will be defining the data also for the interconnect
> target modules as DSS really is a private interconnect. There is some
> information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> example where it mentions "32-bit interconnect (SLX)".
>
> The changes we need to make are:
>
> 1. Parse also device tree subnodes for the compatible property fixup
>
> 2. Update the component code to consider device tree subnodes
>
> Cc: [email protected]
> Cc: Jyri Sarha <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

> ---
>
> This is needed for dropping DSS platform data that I'll be posting
> seprately. If this looks OK, can you guys please test and ack?
>
> ---
> drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++++++++++++++++---
> .../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 +++++++++++++------
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
> --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> @@ -1339,9 +1339,15 @@ static int dss_component_compare(struct device *dev, void *data)
> return dev == child;
> }
>
> +struct dss_component_match_data {
> + struct device *dev;
> + struct component_match **match;
> +};
> +
> static int dss_add_child_component(struct device *dev, void *data)
> {
> - struct component_match **match = data;
> + struct dss_component_match_data *cmatch = data;
> + struct component_match **match = cmatch->match;
>
> /*
> * HACK
> @@ -1352,7 +1358,17 @@ static int dss_add_child_component(struct device *dev, void *data)
> if (strstr(dev_name(dev), "rfbi"))
> return 0;
>
> - component_match_add(dev->parent, match, dss_component_compare, dev);
> + /*
> + * Handle possible interconnect target modules defined within the DSS.
> + * The DSS components can be children of an interconnect target module
> + * after the device tree has been updated for the module data.
> + * See also omapdss_boot_init() for compatible fixup.
> + */
> + if (strstr(dev_name(dev), "target-module"))
> + return device_for_each_child(dev, cmatch,
> + dss_add_child_component);
> +
> + component_match_add(cmatch->dev, match, dss_component_compare, dev);
>
> return 0;
> }
> @@ -1395,6 +1411,7 @@ static int dss_probe_hardware(struct dss_device *dss)
> static int dss_probe(struct platform_device *pdev)
> {
> const struct soc_device_attribute *soc;
> + struct dss_component_match_data cmatch;
> struct component_match *match = NULL;
> struct resource *dss_mem;
> struct dss_device *dss;
> @@ -1472,7 +1489,9 @@ static int dss_probe(struct platform_device *pdev)
>
> omapdss_gather_components(&pdev->dev);
>
> - device_for_each_child(&pdev->dev, &match, dss_add_child_component);
> + cmatch.dev = &pdev->dev;
> + cmatch.match = &match;
> + device_for_each_child(&pdev->dev, &cmatch, dss_add_child_component);
>
> r = component_master_add_with_match(&pdev->dev, &dss_component_ops, match);
> if (r)
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> @@ -183,9 +183,24 @@ static const struct of_device_id omapdss_of_fixups_whitelist[] __initconst = {
> {},
> };
>
> +static void __init omapdss_find_children(struct device_node *np)
> +{
> + struct device_node *child;
> +
> + for_each_available_child_of_node(np, child) {
> + if (!of_find_property(child, "compatible", NULL))
> + continue;
> +
> + omapdss_walk_device(child, true);
> +
> + if (of_device_is_compatible(child, "ti,sysc"))
> + omapdss_find_children(child);
> + }
> +}
> +
> static int __init omapdss_boot_init(void)
> {
> - struct device_node *dss, *child;
> + struct device_node *dss;
>
> INIT_LIST_HEAD(&dss_conv_list);
>
> @@ -195,13 +210,7 @@ static int __init omapdss_boot_init(void)
> return 0;
>
> omapdss_walk_device(dss, true);
> -
> - for_each_available_child_of_node(dss, child) {
> - if (!of_find_property(child, "compatible", NULL))
> - continue;
> -
> - omapdss_walk_device(child, true);
> - }
> + omapdss_find_children(dss);
>
> while (!list_empty(&dss_conv_list)) {
> struct dss_conv_node *n;

--
Regards,

Laurent Pinchart

2020-02-24 23:31:45

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

Hi,

On Mon, Feb 24, 2020 at 11:12:28AM -0800, Tony Lindgren wrote:
> In order to probe display subsystem (DSS) components with ti-sysc
> interconnect target module without legacy platform data and using
> devicetree, we need to update dss probing a bit.
>
> In the device tree, we will be defining the data also for the interconnect
> target modules as DSS really is a private interconnect. There is some
> information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> example where it mentions "32-bit interconnect (SLX)".
>
> The changes we need to make are:
>
> 1. Parse also device tree subnodes for the compatible property fixup
>
> 2. Update the component code to consider device tree subnodes
>
> Cc: [email protected]
> Cc: Jyri Sarha <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
>
> This is needed for dropping DSS platform data that I'll be posting
> seprately. If this looks OK, can you guys please test and ack?
>
> ---

Reviewed-by: Sebastian Reichel <[email protected]>

FWIW, I dropped omapdss-boot-init.c in my patch series updating DSI
code to use common panel infrastructure, so this will conflict.

-- Sebastian

> drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++++++++++++++++---
> .../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 +++++++++++++------
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c
> --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> @@ -1339,9 +1339,15 @@ static int dss_component_compare(struct device *dev, void *data)
> return dev == child;
> }
>
> +struct dss_component_match_data {
> + struct device *dev;
> + struct component_match **match;
> +};
> +
> static int dss_add_child_component(struct device *dev, void *data)
> {
> - struct component_match **match = data;
> + struct dss_component_match_data *cmatch = data;
> + struct component_match **match = cmatch->match;
>
> /*
> * HACK
> @@ -1352,7 +1358,17 @@ static int dss_add_child_component(struct device *dev, void *data)
> if (strstr(dev_name(dev), "rfbi"))
> return 0;
>
> - component_match_add(dev->parent, match, dss_component_compare, dev);
> + /*
> + * Handle possible interconnect target modules defined within the DSS.
> + * The DSS components can be children of an interconnect target module
> + * after the device tree has been updated for the module data.
> + * See also omapdss_boot_init() for compatible fixup.
> + */
> + if (strstr(dev_name(dev), "target-module"))
> + return device_for_each_child(dev, cmatch,
> + dss_add_child_component);
> +
> + component_match_add(cmatch->dev, match, dss_component_compare, dev);
>
> return 0;
> }
> @@ -1395,6 +1411,7 @@ static int dss_probe_hardware(struct dss_device *dss)
> static int dss_probe(struct platform_device *pdev)
> {
> const struct soc_device_attribute *soc;
> + struct dss_component_match_data cmatch;
> struct component_match *match = NULL;
> struct resource *dss_mem;
> struct dss_device *dss;
> @@ -1472,7 +1489,9 @@ static int dss_probe(struct platform_device *pdev)
>
> omapdss_gather_components(&pdev->dev);
>
> - device_for_each_child(&pdev->dev, &match, dss_add_child_component);
> + cmatch.dev = &pdev->dev;
> + cmatch.match = &match;
> + device_for_each_child(&pdev->dev, &cmatch, dss_add_child_component);
>
> r = component_master_add_with_match(&pdev->dev, &dss_component_ops, match);
> if (r)
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> @@ -183,9 +183,24 @@ static const struct of_device_id omapdss_of_fixups_whitelist[] __initconst = {
> {},
> };
>
> +static void __init omapdss_find_children(struct device_node *np)
> +{
> + struct device_node *child;
> +
> + for_each_available_child_of_node(np, child) {
> + if (!of_find_property(child, "compatible", NULL))
> + continue;
> +
> + omapdss_walk_device(child, true);
> +
> + if (of_device_is_compatible(child, "ti,sysc"))
> + omapdss_find_children(child);
> + }
> +}
> +
> static int __init omapdss_boot_init(void)
> {
> - struct device_node *dss, *child;
> + struct device_node *dss;
>
> INIT_LIST_HEAD(&dss_conv_list);
>
> @@ -195,13 +210,7 @@ static int __init omapdss_boot_init(void)
> return 0;
>
> omapdss_walk_device(dss, true);
> -
> - for_each_available_child_of_node(dss, child) {
> - if (!of_find_property(child, "compatible", NULL))
> - continue;
> -
> - omapdss_walk_device(child, true);
> - }
> + omapdss_find_children(dss);
>
> while (!list_empty(&dss_conv_list)) {
> struct dss_conv_node *n;
> --
> 2.25.1


Attachments:
(No filename) (4.98 kB)
signature.asc (849.00 B)
Download all attachments

2020-02-24 23:43:59

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

* Sebastian Reichel <[email protected]> [200224 23:32]:
> Hi,
>
> On Mon, Feb 24, 2020 at 11:12:28AM -0800, Tony Lindgren wrote:
> > In order to probe display subsystem (DSS) components with ti-sysc
> > interconnect target module without legacy platform data and using
> > devicetree, we need to update dss probing a bit.
> >
> > In the device tree, we will be defining the data also for the interconnect
> > target modules as DSS really is a private interconnect. There is some
> > information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> > example where it mentions "32-bit interconnect (SLX)".
> >
> > The changes we need to make are:
> >
> > 1. Parse also device tree subnodes for the compatible property fixup
> >
> > 2. Update the component code to consider device tree subnodes
> >
> > Cc: [email protected]
> > Cc: Jyri Sarha <[email protected]>
> > Cc: Laurent Pinchart <[email protected]>
> > Cc: Tomi Valkeinen <[email protected]>
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
> >
> > This is needed for dropping DSS platform data that I'll be posting
> > seprately. If this looks OK, can you guys please test and ack?
> >
> > ---
>
> Reviewed-by: Sebastian Reichel <[email protected]>
>
> FWIW, I dropped omapdss-boot-init.c in my patch series updating DSI
> code to use common panel infrastructure, so this will conflict.

Hey that's great :) Sounds like we can set up an immutable branch
for just this $subject patch against v5.6-rc1 to resolve the
conflict. I can set it up for Tomi or Tomi can set it up for me,
whichever Tomi prefers.

Regards,

Tony

2020-02-27 17:44:59

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

Tomi,

* Tony Lindgren <[email protected]> [200224 23:44]:
> * Sebastian Reichel <[email protected]> [200224 23:32]:
> > Hi,
> >
> > On Mon, Feb 24, 2020 at 11:12:28AM -0800, Tony Lindgren wrote:
> > > In order to probe display subsystem (DSS) components with ti-sysc
> > > interconnect target module without legacy platform data and using
> > > devicetree, we need to update dss probing a bit.
> > >
> > > In the device tree, we will be defining the data also for the interconnect
> > > target modules as DSS really is a private interconnect. There is some
> > > information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> > > example where it mentions "32-bit interconnect (SLX)".
> > >
> > > The changes we need to make are:
> > >
> > > 1. Parse also device tree subnodes for the compatible property fixup
> > >
> > > 2. Update the component code to consider device tree subnodes
> > >
> > > Cc: [email protected]
> > > Cc: Jyri Sarha <[email protected]>
> > > Cc: Laurent Pinchart <[email protected]>
> > > Cc: Tomi Valkeinen <[email protected]>
> > > Signed-off-by: Tony Lindgren <[email protected]>
> > > ---
> > >
> > > This is needed for dropping DSS platform data that I'll be posting
> > > seprately. If this looks OK, can you guys please test and ack?
> > >
> > > ---
> >
> > Reviewed-by: Sebastian Reichel <[email protected]>
> >
> > FWIW, I dropped omapdss-boot-init.c in my patch series updating DSI
> > code to use common panel infrastructure, so this will conflict.
>
> Hey that's great :) Sounds like we can set up an immutable branch
> for just this $subject patch against v5.6-rc1 to resolve the
> conflict. I can set it up for Tomi or Tomi can set it up for me,
> whichever Tomi prefers.

Do you want me to send you a pull request for just this one patch
against v5.6-rc1?

Regards,

Tony

2020-03-02 10:29:08

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

On 27/02/2020 19:44, Tony Lindgren wrote:

>>> FWIW, I dropped omapdss-boot-init.c in my patch series updating DSI
>>> code to use common panel infrastructure, so this will conflict.
>>
>> Hey that's great :) Sounds like we can set up an immutable branch
>> for just this $subject patch against v5.6-rc1 to resolve the
>> conflict. I can set it up for Tomi or Tomi can set it up for me,
>> whichever Tomi prefers.
>
> Do you want me to send you a pull request for just this one patch
> against v5.6-rc1?

It's probably easier if Sebastian drops the removal patch, and instead creates a patch that removes
the panel-dsi-cm from omapdss_of_fixups_whitelist. That change should not conflict, and effectively
makes the omapdss-boot-init.c a no-op.

We can then remove the file later.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-03-02 15:02:16

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

* Tomi Valkeinen <[email protected]> [200302 10:29]:
> On 27/02/2020 19:44, Tony Lindgren wrote:
>
> > > > FWIW, I dropped omapdss-boot-init.c in my patch series updating DSI
> > > > code to use common panel infrastructure, so this will conflict.
> > >
> > > Hey that's great :) Sounds like we can set up an immutable branch
> > > for just this $subject patch against v5.6-rc1 to resolve the
> > > conflict. I can set it up for Tomi or Tomi can set it up for me,
> > > whichever Tomi prefers.
> >
> > Do you want me to send you a pull request for just this one patch
> > against v5.6-rc1?
>
> It's probably easier if Sebastian drops the removal patch, and instead
> creates a patch that removes the panel-dsi-cm from
> omapdss_of_fixups_whitelist. That change should not conflict, and
> effectively makes the omapdss-boot-init.c a no-op.
>
> We can then remove the file later.

OK for resolving the merge commit that works too.

Tomi, so do you care to ack the $subject patch though so I can set
up an immutable branch for us for the $subject patch?

Or Tomi, do you want to set up an immutable branch for me for the
$subject patch?

Regards,

Tony

2020-03-03 06:03:27

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

On 24/02/2020 21:12, Tony Lindgren wrote:
> The display subsystem (DSS) needs the child outputs disabled for reset.
> In order to prepare to probe DSS without legacy platform data, let's
> implement sysc_pre_reset_quirk_dss() similar to what we have for the
> platform data with omap_dss_reset().
>
> Note that we cannot directly use the old omap_dss_reset() without
> platform data callbacks and updating omap_dss_reset() to understand
> struct device. And we will be dropping omap_dss_reset() anyways when
> all the SoCs are probing with device tree, so let's not mess with the
> legacy code at all.
>
> Cc: Jyri Sarha <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/bus/ti-sysc.c | 131 +++++++++++++++++++++++++-
> include/linux/platform_data/ti-sysc.h | 1 +
> 2 files changed, 129 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -1303,11 +1303,11 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
> SYSC_QUIRK("dcan", 0x48480000, 0x20, -ENODEV, -ENODEV, 0xa3170504, 0xffffffff,
> SYSC_QUIRK_CLKDM_NOAUTO),
> SYSC_QUIRK("dss", 0x4832a000, 0, 0x10, 0x14, 0x00000020, 0xffffffff,
> - SYSC_QUIRK_OPT_CLKS_IN_RESET),
> + SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
> SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000040, 0xffffffff,
> - SYSC_QUIRK_OPT_CLKS_IN_RESET),
> + SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
> SYSC_QUIRK("dss", 0x58000000, 0, -ENODEV, 0x14, 0x00000061, 0xffffffff,
> - SYSC_QUIRK_OPT_CLKS_IN_RESET),
> + SYSC_QUIRK_OPT_CLKS_IN_RESET | SYSC_MODULE_QUIRK_DSS_RESET),
> SYSC_QUIRK("dwc3", 0x48880000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
> SYSC_QUIRK_CLKDM_NOAUTO),
> SYSC_QUIRK("dwc3", 0x488c0000, 0, 0x10, -ENODEV, 0x500a0200, 0xffffffff,
> @@ -1468,6 +1468,128 @@ static void sysc_init_revision_quirks(struct sysc *ddata)
> }
> }
>
> +/*
> + * DSS needs dispc outputs disabled to reset modules. Returns mask of
> + * enabled DSS interrupts. Eventually we may be able to do this on
> + * dispc init rather than top-level DSS init.
> + */
> +static u32 sysc_quirk_dispc(struct sysc *ddata, int dispc_offset,
> + bool disable)
> +{
> + bool lcd_en, digit_en, lcd2_en = false, lcd3_en = false;
> + const int lcd_en_mask = BIT(0), digit_en_mask = BIT(1);
> + int manager_count;
> + bool framedonetv_irq;
> + u32 val, irq_mask = 0;
> +
> + switch (sysc_soc->soc) {
> + case SOC_2420 ... SOC_3630:
> + manager_count = 2;
> + framedonetv_irq = false;
> + break;
> + case SOC_4430 ... SOC_4470:
> + manager_count = 3;
> + break;
> + case SOC_5430:
> + case SOC_DRA7:
> + manager_count = 4;
> + break;
> + case SOC_AM4:
> + manager_count = 1;
> + break;
> + case SOC_UNKNOWN:
> + default:
> + return 0;
> + };
> +
> + /* Remap the whole module range to be able to reset dispc outputs */
> + devm_iounmap(ddata->dev, ddata->module_va);
> + ddata->module_va = devm_ioremap(ddata->dev,
> + ddata->module_pa,
> + ddata->module_size);

Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss() is called? This will
unmap and remap twice, as this function is called twice. And then left mapped.

> + if (!ddata->module_va)
> + return -EIO;
> +
> + /* DISP_CONTROL */
> + val = sysc_read(ddata, dispc_offset + 0x40);

Defines for dss/dispc register offsets could have been copied from the platform display.c and used
in this file.

> + lcd_en = val & lcd_en_mask;
> + digit_en = val & digit_en_mask;
> + if (lcd_en)
> + irq_mask |= BIT(0); /* FRAMEDONE */
> + if (digit_en) {
> + if (framedonetv_irq)
> + irq_mask |= BIT(24); /* FRAMEDONETV */
> + else
> + irq_mask |= BIT(2) | BIT(3); /* EVSYNC bits */
> + }
> + if (disable & (lcd_en | digit_en))
> + sysc_write(ddata, dispc_offset + 0x40,
> + val & ~(lcd_en_mask | digit_en_mask));
> +
> + if (manager_count <= 2)
> + return irq_mask;
> +
> + /* DISPC_CONTROL2 */
> + val = sysc_read(ddata, dispc_offset + 0x238);
> + lcd2_en = val & lcd_en_mask;
> + if (lcd2_en)
> + irq_mask |= BIT(22); /* FRAMEDONE2 */
> + if (disable && lcd2_en)
> + sysc_write(ddata, dispc_offset + 0x238,
> + val & ~lcd_en_mask);
> +
> + if (manager_count <= 3)
> + return irq_mask;
> +
> + /* DISPC_CONTROL3 */
> + val = sysc_read(ddata, dispc_offset + 0x848);
> + lcd3_en = val & lcd_en_mask;
> + if (lcd3_en)
> + irq_mask |= BIT(30); /* FRAMEDONE3 */
> + if (disable && lcd3_en)
> + sysc_write(ddata, dispc_offset + 0x848,
> + val & ~lcd_en_mask);
> +
> + return irq_mask;
> +}
> +
> +/* DSS needs child outputs disabled and SDI registers cleared for reset */
> +static void sysc_pre_reset_quirk_dss(struct sysc *ddata)
> +{
> + const int dispc_offset = 0x1000;
> + int error;
> + u32 irq_mask, val;
> +
> + /* Get enabled outputs */
> + irq_mask = sysc_quirk_dispc(ddata, dispc_offset, false);
> + if (!irq_mask)
> + return;
> +
> + /* Clear IRQSTATUS */
> + sysc_write(ddata, 0x1000 + 0x18, irq_mask);

dispc_offset instead of 0x1000.

> +
> + /* Disable outputs */
> + val = sysc_quirk_dispc(ddata, dispc_offset, true);
> +
> + /* Poll IRQSTATUS */
> + error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18,
> + val, val != irq_mask, 100, 50);
> + if (error)
> + dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n",
> + __func__, val, irq_mask);
> +
> + if (sysc_soc->soc == SOC_3430) {
> + /* Clear DSS_SDI_CONTROL */
> + sysc_write(ddata, dispc_offset + 0x44, 0);
> +
> + /* Clear DSS_PLL_CONTROL */
> + sysc_write(ddata, dispc_offset + 0x48, 0);

These are not dispc registers, but dss registers.

> + }
> +
> + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
> + sysc_write(ddata, dispc_offset + 0x40, 0);

Same here.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-03-03 09:19:39

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

On 24/02/2020 21:12, Tony Lindgren wrote:
> In order to probe display subsystem (DSS) components with ti-sysc
> interconnect target module without legacy platform data and using
> devicetree, we need to update dss probing a bit.
>
> In the device tree, we will be defining the data also for the interconnect
> target modules as DSS really is a private interconnect. There is some
> information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> example where it mentions "32-bit interconnect (SLX)".
>
> The changes we need to make are:
>
> 1. Parse also device tree subnodes for the compatible property fixup
>
> 2. Update the component code to consider device tree subnodes
>
> Cc: [email protected]
> Cc: Jyri Sarha <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
>
> This is needed for dropping DSS platform data that I'll be posting
> seprately. If this looks OK, can you guys please test and ack?
>
> ---
> drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++++++++++++++++---
> .../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 +++++++++++++------
> 2 files changed, 39 insertions(+), 11 deletions(-)

Reviewed-by: Tomi Valkeinen <[email protected]>

This doesn't conflict with drm-next (with Laurent's recent patches), so it should be fine for you to
have this in your branch.

And not a biggie, but I wonder if the changes to these two files should be in separate patches, due
to omapdss-boot-init going away. Well, probably doesn't matter.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-03-03 15:48:07

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

Hi,

* Tomi Valkeinen <[email protected]> [200303 06:03]:
> On 24/02/2020 21:12, Tony Lindgren wrote:
> > + /* Remap the whole module range to be able to reset dispc outputs */
> > + devm_iounmap(ddata->dev, ddata->module_va);
> > + ddata->module_va = devm_ioremap(ddata->dev,
> > + ddata->module_pa,
> > + ddata->module_size);
>
> Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss()
> is called? This will unmap and remap twice, as this function is called
> twice. And then left mapped.

That's because by default we only ioremap the module revision, sysconfig
and sysstatus register are and provide the rest as a range for the child
nodes.

In the dss quirk case we need to tinker with registers also in the dispc
range, and at the parent dss probe time dispc has not probed yet.

We may be able to eventually move the reset quirk to dispc, but then
it won't happen in the current setup until after dss top level driver
has loaded.

We leave the module range ioremapped as we still need to access
sysconfig related registers for PM runtime.

> > + if (!ddata->module_va)
> > + return -EIO;
> > +
> > + /* DISP_CONTROL */
> > + val = sysc_read(ddata, dispc_offset + 0x40);
>
> Defines for dss/dispc register offsets could have been copied from the
> platform display.c and used in this file.

Yeah I though about that, but decided to keep everything local to
the quirk handling. We could have them defined in some dss header
though.

> > + /* Clear IRQSTATUS */
> > + sysc_write(ddata, 0x1000 + 0x18, irq_mask);
>
> dispc_offset instead of 0x1000.

OK

> > +
> > + /* Disable outputs */
> > + val = sysc_quirk_dispc(ddata, dispc_offset, true);
> > +
> > + /* Poll IRQSTATUS */
> > + error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18,
> > + val, val != irq_mask, 100, 50);
> > + if (error)
> > + dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n",
> > + __func__, val, irq_mask);
> > +
> > + if (sysc_soc->soc == SOC_3430) {
> > + /* Clear DSS_SDI_CONTROL */
> > + sysc_write(ddata, dispc_offset + 0x44, 0);
> > +
> > + /* Clear DSS_PLL_CONTROL */
> > + sysc_write(ddata, dispc_offset + 0x48, 0);
>
> These are not dispc registers, but dss registers.

Ouch. Thanks for catching this, will include in the fix.

> > + }
> > +
> > + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
> > + sysc_write(ddata, dispc_offset + 0x40, 0);
>
> Same here.

OK

Regards,

Tony

2020-03-03 15:54:45

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/3] drm/omap: Prepare DSS for probing without legacy platform data

* Tomi Valkeinen <[email protected]> [200303 09:19]:
> On 24/02/2020 21:12, Tony Lindgren wrote:
> > In order to probe display subsystem (DSS) components with ti-sysc
> > interconnect target module without legacy platform data and using
> > devicetree, we need to update dss probing a bit.
> >
> > In the device tree, we will be defining the data also for the interconnect
> > target modules as DSS really is a private interconnect. There is some
> > information about that in 4460 TRM in "Figure 10-3. DSS Integration" for
> > example where it mentions "32-bit interconnect (SLX)".
> >
> > The changes we need to make are:
> >
> > 1. Parse also device tree subnodes for the compatible property fixup
> >
> > 2. Update the component code to consider device tree subnodes
> >
> > Cc: [email protected]
> > Cc: Jyri Sarha <[email protected]>
> > Cc: Laurent Pinchart <[email protected]>
> > Cc: Tomi Valkeinen <[email protected]>
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
> >
> > This is needed for dropping DSS platform data that I'll be posting
> > seprately. If this looks OK, can you guys please test and ack?
> >
> > ---
> > drivers/gpu/drm/omapdrm/dss/dss.c | 25 ++++++++++++++++---
> > .../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 25 +++++++++++++------
> > 2 files changed, 39 insertions(+), 11 deletions(-)
>
> Reviewed-by: Tomi Valkeinen <[email protected]>
>
> This doesn't conflict with drm-next (with Laurent's recent patches), so it
> should be fine for you to have this in your branch.

OK thank you. I've pushed out omap-for-v5.7/dss-probe which has just
this commit against v5.6-rc1 [0][1]. Let's consider commit cef766300353
("drm/omap: Prepare DSS for probing without legacy platform data")
immutable so we can both merge it in as needed.

I have not added any tag yet as it seems that we could add also
apply Sebastian's few preparatory dts changes to this branch when
ready.

> And not a biggie, but I wonder if the changes to these two files should be
> in separate patches, due to omapdss-boot-init going away. Well, probably
> doesn't matter.

Hmm yeah good reason to put every change into a seprate patch
for future. I really did not expect this to conflict with anything
after years of no changes :)

Regards,

Tony

[0] git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git omap-for-v5.7/dss-probe
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/commit/?h=omap-for-v5.7/dss-probe&id=cef766300353613aa273791f70b3125d1f0420ae

2020-03-03 15:54:58

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

On 03/03/2020 17:13, Tony Lindgren wrote:
> Hi,
>
> * Tomi Valkeinen <[email protected]> [200303 06:03]:
>> On 24/02/2020 21:12, Tony Lindgren wrote:
>>> + /* Remap the whole module range to be able to reset dispc outputs */
>>> + devm_iounmap(ddata->dev, ddata->module_va);
>>> + ddata->module_va = devm_ioremap(ddata->dev,
>>> + ddata->module_pa,
>>> + ddata->module_size);
>>
>> Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss()
>> is called? This will unmap and remap twice, as this function is called
>> twice. And then left mapped.
>
> That's because by default we only ioremap the module revision, sysconfig
> and sysstatus register are and provide the rest as a range for the child
> nodes.
>
> In the dss quirk case we need to tinker with registers also in the dispc
> range, and at the parent dss probe time dispc has not probed yet.
>
> We may be able to eventually move the reset quirk to dispc, but then
> it won't happen in the current setup until after dss top level driver
> has loaded.
>
> We leave the module range ioremapped as we still need to access
> sysconfig related registers for PM runtime.

Ok, makes sense. I guess a minor improvement would be to unmap & remap once in
sysc_pre_reset_quirk_dss before calling sysc_quirk_dispc.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-03-03 15:56:38

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

* Tomi Valkeinen <[email protected]> [200303 15:36]:
> On 03/03/2020 17:13, Tony Lindgren wrote:
> > Hi,
> >
> > * Tomi Valkeinen <[email protected]> [200303 06:03]:
> > > On 24/02/2020 21:12, Tony Lindgren wrote:
> > > > + /* Remap the whole module range to be able to reset dispc outputs */
> > > > + devm_iounmap(ddata->dev, ddata->module_va);
> > > > + ddata->module_va = devm_ioremap(ddata->dev,
> > > > + ddata->module_pa,
> > > > + ddata->module_size);
> > >
> > > Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss()
> > > is called? This will unmap and remap twice, as this function is called
> > > twice. And then left mapped.
> >
> > That's because by default we only ioremap the module revision, sysconfig
> > and sysstatus register are and provide the rest as a range for the child
> > nodes.
> >
> > In the dss quirk case we need to tinker with registers also in the dispc
> > range, and at the parent dss probe time dispc has not probed yet.
> >
> > We may be able to eventually move the reset quirk to dispc, but then
> > it won't happen in the current setup until after dss top level driver
> > has loaded.
> >
> > We leave the module range ioremapped as we still need to access
> > sysconfig related registers for PM runtime.
>
> Ok, makes sense. I guess a minor improvement would be to unmap & remap once
> in sysc_pre_reset_quirk_dss before calling sysc_quirk_dispc.

Yeah well we'd have to sprawl the module specific quirk checks
there too then.

I thought about using the whole module range for modules with a large
IO range, but so far DSS is the only one needing a quirk hadling
covering also child modules like this.

Regards,

Tony

2020-03-03 15:58:29

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

* Tony Lindgren <[email protected]> [200303 15:14]:
> * Tomi Valkeinen <[email protected]> [200303 06:03]:
> > On 24/02/2020 21:12, Tony Lindgren wrote:
> > > + if (sysc_soc->soc == SOC_3430) {
> > > + /* Clear DSS_SDI_CONTROL */
> > > + sysc_write(ddata, dispc_offset + 0x44, 0);
> > > +
> > > + /* Clear DSS_PLL_CONTROL */
> > > + sysc_write(ddata, dispc_offset + 0x48, 0);
> >
> > These are not dispc registers, but dss registers.
>
> Ouch. Thanks for catching this, will include in the fix.
>
> > > + }
> > > +
> > > + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
> > > + sysc_write(ddata, dispc_offset + 0x40, 0);
> >
> > Same here.

Below is a fix using dispc offset for dss registers.

Regards,

Tony

8< ----------------------
From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <[email protected]>
Date: Tue, 3 Mar 2020 07:17:43 -0800
Subject: [PATCH] bus: ti-sysc: Fix wrong offset for display subsystem
reset quirk

Commit 7324a7a0d5e2 ("bus: ti-sysc: Implement display subsystem reset
quirk") added support for DSS reset, but is using dispc offset also for
DSS also registers as reported by Tomi Valkeinen <[email protected]>.
Also, we're not using dispc_offset for dispc IRQSTATUS register so let's
fix that too.

Fixes: 7324a7a0d5e2 ("bus: ti-sysc: Implement display subsystem reset quirk")
Reported-by: Tomi Valkeinen <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1566,7 +1566,7 @@ static void sysc_pre_reset_quirk_dss(struct sysc *ddata)
return;

/* Clear IRQSTATUS */
- sysc_write(ddata, 0x1000 + 0x18, irq_mask);
+ sysc_write(ddata, dispc_offset + 0x18, irq_mask);

/* Disable outputs */
val = sysc_quirk_dispc(ddata, dispc_offset, true);
@@ -1580,14 +1580,14 @@ static void sysc_pre_reset_quirk_dss(struct sysc *ddata)

if (sysc_soc->soc == SOC_3430) {
/* Clear DSS_SDI_CONTROL */
- sysc_write(ddata, dispc_offset + 0x44, 0);
+ sysc_write(ddata, 0x44, 0);

/* Clear DSS_PLL_CONTROL */
- sysc_write(ddata, dispc_offset + 0x48, 0);
+ sysc_write(ddata, 0x48, 0);
}

/* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
- sysc_write(ddata, dispc_offset + 0x40, 0);
+ sysc_write(ddata, 0x40, 0);
}

/* 1-wire needs module's internal clocks enabled for reset */
--
2.25.1

2020-03-04 07:04:07

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk

On 03/03/2020 17:49, Tony Lindgren wrote:
> * Tony Lindgren <[email protected]> [200303 15:14]:
>> * Tomi Valkeinen <[email protected]> [200303 06:03]:
>>> On 24/02/2020 21:12, Tony Lindgren wrote:
>>>> + if (sysc_soc->soc == SOC_3430) {
>>>> + /* Clear DSS_SDI_CONTROL */
>>>> + sysc_write(ddata, dispc_offset + 0x44, 0);
>>>> +
>>>> + /* Clear DSS_PLL_CONTROL */
>>>> + sysc_write(ddata, dispc_offset + 0x48, 0);
>>>
>>> These are not dispc registers, but dss registers.
>>
>> Ouch. Thanks for catching this, will include in the fix.
>>
>>>> + }
>>>> +
>>>> + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
>>>> + sysc_write(ddata, dispc_offset + 0x40, 0);
>>>
>>> Same here.
>
> Below is a fix using dispc offset for dss registers.
>
> Regards,
>
> Tony
>
> 8< ----------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <[email protected]>
> Date: Tue, 3 Mar 2020 07:17:43 -0800
> Subject: [PATCH] bus: ti-sysc: Fix wrong offset for display subsystem
> reset quirk
>
> Commit 7324a7a0d5e2 ("bus: ti-sysc: Implement display subsystem reset
> quirk") added support for DSS reset, but is using dispc offset also for
> DSS also registers as reported by Tomi Valkeinen <[email protected]>.
> Also, we're not using dispc_offset for dispc IRQSTATUS register so let's
> fix that too.
>
> Fixes: 7324a7a0d5e2 ("bus: ti-sysc: Implement display subsystem reset quirk")
> Reported-by: Tomi Valkeinen <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/bus/ti-sysc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -1566,7 +1566,7 @@ static void sysc_pre_reset_quirk_dss(struct sysc *ddata)
> return;
>
> /* Clear IRQSTATUS */
> - sysc_write(ddata, 0x1000 + 0x18, irq_mask);
> + sysc_write(ddata, dispc_offset + 0x18, irq_mask);
>
> /* Disable outputs */
> val = sysc_quirk_dispc(ddata, dispc_offset, true);
> @@ -1580,14 +1580,14 @@ static void sysc_pre_reset_quirk_dss(struct sysc *ddata)
>
> if (sysc_soc->soc == SOC_3430) {
> /* Clear DSS_SDI_CONTROL */
> - sysc_write(ddata, dispc_offset + 0x44, 0);
> + sysc_write(ddata, 0x44, 0);
>
> /* Clear DSS_PLL_CONTROL */
> - sysc_write(ddata, dispc_offset + 0x48, 0);
> + sysc_write(ddata, 0x48, 0);
> }
>
> /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */
> - sysc_write(ddata, dispc_offset + 0x40, 0);
> + sysc_write(ddata, 0x40, 0);
> }
>
> /* 1-wire needs module's internal clocks enabled for reset */
>

Reviewed-by: Tomi Valkeinen <[email protected]>

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki