[11/24] drm/omap: Add kernel parameter to specify the desired display order

Message ID 1518428694-18018-12-git-send-email-tomi.valkeinen@ti.com
State New
Headers show
Series
  • drm/omap: misc patches
Related show

Commit Message

Tomi Valkeinen Feb. 12, 2018, 9:44 a.m.
From: Peter Ujfalusi <peter.ujfalusi@ti.com>

omapdrm.displays (int array) can be used to reorder the displays by id if
needed. It can be also used to disable display.

If the board have two active displays:
0 - LCD
1 - HDMI
then:
omapdrm.displays=0,1 - represents the original order (LCD, HDMI)
omapdrm.displays=1,0 - represents reverse order (HDMI, LCD)
omapdrm.displays=0 - only the LCD is enabled
omapdrm.displays=1 - only the HDMI is enabled
omapdrm.displays=-1 - disable all displays

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 55 +++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Feb. 27, 2018, 1:35 p.m. | #1
Hi Tomi,

Thank you for the patch.

On Monday, 12 February 2018 11:44:41 EET Tomi Valkeinen wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> omapdrm.displays (int array) can be used to reorder the displays by id if
> needed. It can be also used to disable display.
> 
> If the board have two active displays:
> 0 - LCD
> 1 - HDMI
> then:
> omapdrm.displays=0,1 - represents the original order (LCD, HDMI)
> omapdrm.displays=1,0 - represents reverse order (HDMI, LCD)
> omapdrm.displays=0 - only the LCD is enabled
> omapdrm.displays=1 - only the HDMI is enabled
> omapdrm.displays=-1 - disable all displays

What's the use case for this ?

> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 55 ++++++++++++++++++++++++++++++++---
>  1 file changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 877c3749f69b..5a27a47b628e
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -34,6 +34,14 @@
>  #define DRIVER_MINOR		0
>  #define DRIVER_PATCHLEVEL	0
> 
> +#define MAX_NR_DISPLAYS		8
> +static int display_order[MAX_NR_DISPLAYS];
> +static int display_order_nelm;
> +module_param_array_named(displays, display_order, int, &display_order_nelm,
> +			 0444);
> +MODULE_PARM_DESC(displays,
> +		 "ID array to specify the order of the active displays");
> +
>  /*
>   * mode config funcs
>   */
> @@ -182,12 +190,21 @@ static int omap_compare_dssdevs(const void *a, const
> void *b) static void omap_collect_dssdevs(struct drm_device *ddev)
>  {
>  	struct omap_drm_private *priv = ddev->dev_private;
> +	struct omap_dss_device *dssdevs[ARRAY_SIZE(priv->dssdevs)];
>  	struct omap_dss_device *dssdev = NULL;
> +	int num_dssdevs = 0;

unsigned int.

> +	unsigned long dssdev_mask = 0;
> +	int i;

unsigned int.

> +
> +	/* No displays should be enabled */
> +	if (display_order_nelm == 1 && display_order[0] < 0)
> +		return;
> 
>  	for_each_dss_dev(dssdev) {
>  		omap_dss_get_device(dssdev);
> -		priv->dssdevs[priv->num_dssdevs++] = dssdev;
> -		if (priv->num_dssdevs == ARRAY_SIZE(priv->dssdevs)) {
> +		set_bit(num_dssdevs, &dssdev_mask);
> +		dssdevs[num_dssdevs++] = dssdev;
> +		if (num_dssdevs == ARRAY_SIZE(dssdevs)) {
>  			/* To balance the 'for_each_dss_dev' loop */
>  			omap_dss_put_device(dssdev);
>  			break;
> @@ -195,8 +212,38 @@ static void omap_collect_dssdevs(struct drm_device
> *ddev) }
> 
>  	/* Sort the list by DT aliases */
> -	sort(priv->dssdevs, priv->num_dssdevs, sizeof(priv->dssdevs[0]),
> -	     omap_compare_dssdevs, NULL);
> +	sort(dssdevs, num_dssdevs, sizeof(dssdevs[0]), omap_compare_dssdevs,
> +	     NULL);
> +
> +	/* Do ordering based on the display_order parameter array */
> +	for (i = 0; i < display_order_nelm; i++) {
> +		int old_index = display_order[i];
> +
> +		if ((old_index >= 0 && old_index < num_dssdevs) &&

No need for the inner parentheses.

> +		    (dssdev_mask & BIT(old_index))) {
> +			priv->dssdevs[priv->num_dssdevs++] = dssdevs[old_index];
> +			clear_bit(old_index, &dssdev_mask);
> +		} else {
> +			dev_err(ddev->dev,
> +				"Ignoring invalid displays module parameter\n");
> +			priv->num_dssdevs = 0;
> +			break;
> +		}
> +	}
> +
> +	/* if the target list is empty, copy the collected dssdevs, if any */
> +	if (priv->num_dssdevs == 0) {
> +		for (i = 0; i < num_dssdevs; i++)
> +			priv->dssdevs[i] = dssdevs[i];
> +
> +		priv->num_dssdevs = num_dssdevs;
> +	} else {
> +		u32 idx;
> +
> +		/* check if we have dssdev which is not carried over */
> +		for_each_set_bit(idx, &dssdev_mask, ARRAY_SIZE(dssdevs))
> +			omap_dss_put_device(dssdevs[idx]);
> +	}
>  }
> 
>  static int omap_connect_dssdevs(struct drm_device *ddev)
Tomi Valkeinen Feb. 28, 2018, 9:16 a.m. | #2
On 27/02/18 15:35, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Monday, 12 February 2018 11:44:41 EET Tomi Valkeinen wrote:
>> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>
>> omapdrm.displays (int array) can be used to reorder the displays by id if
>> needed. It can be also used to disable display.
>>
>> If the board have two active displays:
>> 0 - LCD
>> 1 - HDMI
>> then:
>> omapdrm.displays=0,1 - represents the original order (LCD, HDMI)
>> omapdrm.displays=1,0 - represents reverse order (HDMI, LCD)
>> omapdrm.displays=0 - only the LCD is enabled
>> omapdrm.displays=1 - only the HDMI is enabled
>> omapdrm.displays=-1 - disable all displays
> 
> What's the use case for this ?

fbdev uses the first display as "main" display. There are also many apps
using DRM APIs that unfortunately depend on the order, and presume the
first one is the initial or main display. It's also a nice tool for
debugging, so that you can easily fully disable a display or all displays.

 Tomi

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 877c3749f69b..5a27a47b628e 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -34,6 +34,14 @@ 
 #define DRIVER_MINOR		0
 #define DRIVER_PATCHLEVEL	0
 
+#define MAX_NR_DISPLAYS		8
+static int display_order[MAX_NR_DISPLAYS];
+static int display_order_nelm;
+module_param_array_named(displays, display_order, int, &display_order_nelm,
+			 0444);
+MODULE_PARM_DESC(displays,
+		 "ID array to specify the order of the active displays");
+
 /*
  * mode config funcs
  */
@@ -182,12 +190,21 @@  static int omap_compare_dssdevs(const void *a, const void *b)
 static void omap_collect_dssdevs(struct drm_device *ddev)
 {
 	struct omap_drm_private *priv = ddev->dev_private;
+	struct omap_dss_device *dssdevs[ARRAY_SIZE(priv->dssdevs)];
 	struct omap_dss_device *dssdev = NULL;
+	int num_dssdevs = 0;
+	unsigned long dssdev_mask = 0;
+	int i;
+
+	/* No displays should be enabled */
+	if (display_order_nelm == 1 && display_order[0] < 0)
+		return;
 
 	for_each_dss_dev(dssdev) {
 		omap_dss_get_device(dssdev);
-		priv->dssdevs[priv->num_dssdevs++] = dssdev;
-		if (priv->num_dssdevs == ARRAY_SIZE(priv->dssdevs)) {
+		set_bit(num_dssdevs, &dssdev_mask);
+		dssdevs[num_dssdevs++] = dssdev;
+		if (num_dssdevs == ARRAY_SIZE(dssdevs)) {
 			/* To balance the 'for_each_dss_dev' loop */
 			omap_dss_put_device(dssdev);
 			break;
@@ -195,8 +212,38 @@  static void omap_collect_dssdevs(struct drm_device *ddev)
 	}
 
 	/* Sort the list by DT aliases */
-	sort(priv->dssdevs, priv->num_dssdevs, sizeof(priv->dssdevs[0]),
-	     omap_compare_dssdevs, NULL);
+	sort(dssdevs, num_dssdevs, sizeof(dssdevs[0]), omap_compare_dssdevs,
+	     NULL);
+
+	/* Do ordering based on the display_order parameter array */
+	for (i = 0; i < display_order_nelm; i++) {
+		int old_index = display_order[i];
+
+		if ((old_index >= 0 && old_index < num_dssdevs) &&
+		    (dssdev_mask & BIT(old_index))) {
+			priv->dssdevs[priv->num_dssdevs++] = dssdevs[old_index];
+			clear_bit(old_index, &dssdev_mask);
+		} else {
+			dev_err(ddev->dev,
+				"Ignoring invalid displays module parameter\n");
+			priv->num_dssdevs = 0;
+			break;
+		}
+	}
+
+	/* if the target list is empty, copy the collected dssdevs, if any */
+	if (priv->num_dssdevs == 0) {
+		for (i = 0; i < num_dssdevs; i++)
+			priv->dssdevs[i] = dssdevs[i];
+
+		priv->num_dssdevs = num_dssdevs;
+	} else {
+		u32 idx;
+
+		/* check if we have dssdev which is not carried over */
+		for_each_set_bit(idx, &dssdev_mask, ARRAY_SIZE(dssdevs))
+			omap_dss_put_device(dssdevs[idx]);
+	}
 }
 
 static int omap_connect_dssdevs(struct drm_device *ddev)