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

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

[Tony Lindgren]

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

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

[Tony Lindgren]

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: dri-devel@lists.freedesktop.org
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

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

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

[Laurent Pinchart]

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: dri-devel@lists.freedesktop.org
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
> 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

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5103 bytes --]

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: dri-devel@lists.freedesktop.org
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> 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 <sebastian.reichel@collabora.com>

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [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: dri-devel@lists.freedesktop.org
> > Cc: Jyri Sarha <jsarha@ti.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> > 
> > 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 <sebastian.reichel@collabora.com>
> 
> 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

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

[Tony Lindgren]

Tomi,

* Tony Lindgren <tony@atomide.com> [200224 23:44]:
> * Sebastian Reichel <sre@kernel.org> [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: dri-devel@lists.freedesktop.org
> > > Cc: Jyri Sarha <jsarha@ti.com>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > ---
> > > 
> > > 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 <sebastian.reichel@collabora.com>
> > 
> > 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

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

[Tomi Valkeinen]

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

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

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [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

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

[Tomi Valkeinen]

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: dri-devel@lists.freedesktop.org
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> 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 <tomi.valkeinen@ti.com>

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

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

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [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: dri-devel@lists.freedesktop.org
> > Cc: Jyri Sarha <jsarha@ti.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> > 
> > 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 <tomi.valkeinen@ti.com>
> 
> 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

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

[Tony Lindgren]

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 <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 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

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

[Tony Lindgren]

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 <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 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

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

[Tomi Valkeinen]

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 <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   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

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

[Tony Lindgren]

Hi,

* Tomi Valkeinen <tomi.valkeinen@ti.com> [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

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

[Tomi Valkeinen]

On 03/03/2020 17:13, Tony Lindgren wrote:
> Hi,
> 
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [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

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

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [200303 15:36]:
> On 03/03/2020 17:13, Tony Lindgren wrote:
> > Hi,
> > 
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [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

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

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [200303 15:14]:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [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 <tony@atomide.com>
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 <tomi.valkeinen@ti.com>.
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 <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 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

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

[Tomi Valkeinen]

On 03/03/2020 17:49, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [200303 15:14]:
>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [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 <tony@atomide.com>
> 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 <tomi.valkeinen@ti.com>.
> 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 <tomi.valkeinen@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   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 <tomi.valkeinen@ti.com>

  Tomi

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