diff mbox

[RFC,5/7] drm/omap: Do dss_device (display) ordering in omap_drv.c

Message ID 20170829073218.11097-6-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi Aug. 29, 2017, 7:32 a.m. UTC
Sort the dssdev array based on DT aliases.

With this change we can remove the panel ordering from dss/display.c and
have all sorting related to dssdevs in one place.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Laurent Pinchart Sept. 1, 2017, 11:32 a.m. UTC | #1
Hi Peter,

Thank  you for the patch.

On Tuesday, 29 August 2017 10:32:16 EEST Peter Ujfalusi wrote:
> Sort the dssdev array based on DT aliases.
> 
> With this change we can remove the panel ordering from dss/display.c and
> have all sorting related to dssdevs in one place.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 32dc0e88220f..0e100a359d26
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -18,6 +18,8 @@
>   */
> 
>  #include <linux/sys_soc.h>
> +#include <linux/sort.h>
> +#include <linux/of.h>

Please keep this list alphabetically sorted.

>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -162,6 +164,22 @@ static void omap_disconnect_dssdevs(struct drm_device
> *ddev) priv->num_dssdevs = 0;
>  }
> 
> +static int omap_compare_dssdevs(const void *a, const void *b)
> +{
> +	const struct omap_dss_device *dssdev1 = *(struct omap_dss_device **)a;
> +	const struct omap_dss_device *dssdev2 = *(struct omap_dss_device **)b;
> +	int  id1, id2;
> +
> +	id1 = of_alias_get_id(dssdev1->dev->of_node, "display");
> +	id2 = of_alias_get_id(dssdev2->dev->of_node, "display");

Getting the alias id is a bit costly, how about caching them ?

> +	if (id1 > id2)
> +		return 1;
> +	else if (id1 < id2)
> +		return -1;
> +	return 0;
> +}
> +
>  static void omap_collect_dssdevs(struct drm_device *ddev)
>  {
>  	struct omap_drm_private *priv = ddev->dev_private;
> @@ -176,6 +194,10 @@ static void omap_collect_dssdevs(struct drm_device
> *ddev) break;
>  		}
>  	}
> +
> +	/* Sort the list by DT aliases */
> +	sort(priv->dssdevs, priv->num_dssdevs, sizeof(priv->dssdevs[0]),
> +	     omap_compare_dssdevs, NULL);
>  }
> 
>  static int omap_connect_dssdevs(struct drm_device *ddev)
Peter Ujfalusi Sept. 4, 2017, 9:26 a.m. UTC | #2
Hi Laurent,


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

On 2017-09-01 14:32, Laurent Pinchart wrote:
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
>> b/drivers/gpu/drm/omapdrm/omap_drv.c index 32dc0e88220f..0e100a359d26
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -18,6 +18,8 @@
>>   */
>>
>>  #include <linux/sys_soc.h>
>> +#include <linux/sort.h>
>> +#include <linux/of.h>
> 
> Please keep this list alphabetically sorted.

OK.

> 
>>  #include <drm/drm_atomic.h>
>>  #include <drm/drm_atomic_helper.h>
>> @@ -162,6 +164,22 @@ static void omap_disconnect_dssdevs(struct drm_device
>> *ddev) priv->num_dssdevs = 0;
>>  }
>>
>> +static int omap_compare_dssdevs(const void *a, const void *b)
>> +{
>> +	const struct omap_dss_device *dssdev1 = *(struct omap_dss_device **)a;
>> +	const struct omap_dss_device *dssdev2 = *(struct omap_dss_device **)b;
>> +	int  id1, id2;
>> +
>> +	id1 = of_alias_get_id(dssdev1->dev->of_node, "display");
>> +	id2 = of_alias_get_id(dssdev2->dev->of_node, "display");
> 
> Getting the alias id is a bit costly, how about caching them ?

This is going to be done once and the ID will be never used after this.
We could add the ID to 'struct omap_dss_device' and initialize it in
omapdss_register_display, if that's what you are referring to.

- Péter
Laurent Pinchart Sept. 4, 2017, 9:46 a.m. UTC | #3
Hi Peter,

On Monday, 4 September 2017 12:26:08 EEST Peter Ujfalusi wrote:
> On 2017-09-01 14:32, Laurent Pinchart wrote:
> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 32dc0e88220f..0e100a359d26
> >> 100644
> >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> >> @@ -18,6 +18,8 @@
> >> 
> >>   */
> >>  
> >>  #include <linux/sys_soc.h>
> >> 
> >> +#include <linux/sort.h>
> >> +#include <linux/of.h>
> > 
> > Please keep this list alphabetically sorted.
> 
> OK.
> 
> >>  #include <drm/drm_atomic.h>
> >>  #include <drm/drm_atomic_helper.h>
> >> 
> >> @@ -162,6 +164,22 @@ static void omap_disconnect_dssdevs(struct
> >> drm_device
> >> *ddev) priv->num_dssdevs = 0;
> >> 
> >>  }
> >> 
> >> +static int omap_compare_dssdevs(const void *a, const void *b)
> >> +{
> >> +	const struct omap_dss_device *dssdev1 = *(struct omap_dss_device **)a;
> >> +	const struct omap_dss_device *dssdev2 = *(struct omap_dss_device **)b;
> >> +	int  id1, id2;
> >> +
> >> +	id1 = of_alias_get_id(dssdev1->dev->of_node, "display");
> >> +	id2 = of_alias_get_id(dssdev2->dev->of_node, "display");
> > 
> > Getting the alias id is a bit costly, how about caching them ?
> 
> This is going to be done once and the ID will be never used after this.
> We could add the ID to 'struct omap_dss_device' and initialize it in
> omapdss_register_display, if that's what you are referring to.

Yes, that's what I meant.
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 32dc0e88220f..0e100a359d26 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -18,6 +18,8 @@ 
  */
 
 #include <linux/sys_soc.h>
+#include <linux/sort.h>
+#include <linux/of.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -162,6 +164,22 @@  static void omap_disconnect_dssdevs(struct drm_device *ddev)
 	priv->num_dssdevs = 0;
 }
 
+static int omap_compare_dssdevs(const void *a, const void *b)
+{
+	const struct omap_dss_device *dssdev1 = *(struct omap_dss_device **)a;
+	const struct omap_dss_device *dssdev2 = *(struct omap_dss_device **)b;
+	int  id1, id2;
+
+	id1 = of_alias_get_id(dssdev1->dev->of_node, "display");
+	id2 = of_alias_get_id(dssdev2->dev->of_node, "display");
+
+	if (id1 > id2)
+		return 1;
+	else if (id1 < id2)
+		return -1;
+	return 0;
+}
+
 static void omap_collect_dssdevs(struct drm_device *ddev)
 {
 	struct omap_drm_private *priv = ddev->dev_private;
@@ -176,6 +194,10 @@  static void omap_collect_dssdevs(struct drm_device *ddev)
 			break;
 		}
 	}
+
+	/* Sort the list by DT aliases */
+	sort(priv->dssdevs, priv->num_dssdevs, sizeof(priv->dssdevs[0]),
+	     omap_compare_dssdevs, NULL);
 }
 
 static int omap_connect_dssdevs(struct drm_device *ddev)