Re: [PATCH 29/29] drm/omap: Refactor initialization sequence

2018-12-10 Thread Sebastian Reichel
Hi,

On Wed, Dec 05, 2018 at 05:00:22PM +0200, Laurent Pinchart wrote:
> The omapdrm driver initialization procedure starts by connecting all
> available pipelines, gathering related information (such as output and
> display DSS devices, and DT aliases), sorting them by alias, and finally
> creates all the DRM/KMS objects.
> 
> When using DRM bridges instead of DSS devices, we will need to attach to
> the bridges before getting the aliases. As attaching to bridges requires
> an encoder object, we have to reorganize the initialization sequence to
> create encoders before getting aliases and sorting the pipelines.
> 
> Signed-off-by: Laurent Pinchart 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/omap_drv.c | 123 +
>  1 file changed, 56 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
> b/drivers/gpu/drm/omapdrm/omap_drv.c
> index c9b578a49b24..4cd4fb47660a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -150,48 +150,27 @@ static void omap_disconnect_pipelines(struct drm_device 
> *ddev)
>   priv->num_pipes = 0;
>  }
>  
> -static int omap_compare_pipes(const void *a, const void *b)
> -{
> - const struct omap_drm_pipeline *pipe1 = a;
> - const struct omap_drm_pipeline *pipe2 = b;
> -
> - if (pipe1->alias_id > pipe2->alias_id)
> - return 1;
> - else if (pipe1->alias_id < pipe2->alias_id)
> - return -1;
> - return 0;
> -}
> -
>  static int omap_connect_pipelines(struct drm_device *ddev)
>  {
>   struct omap_drm_private *priv = ddev->dev_private;
>   struct omap_dss_device *output = NULL;
> - unsigned int i;
>   int r;
>  
> - if (!omapdss_stack_is_ready())
> - return -EPROBE_DEFER;
> -
>   for_each_dss_output(output) {
>   r = omapdss_device_connect(priv->dss, NULL, output);
>   if (r == -EPROBE_DEFER) {
>   omapdss_device_put(output);
> - goto cleanup;
> + return r;
>   } else if (r) {
>   dev_warn(output->dev, "could not connect output %s\n",
>output->name);
>   } else {
>   struct omap_drm_pipeline *pipe;
> - int id;
>  
>   pipe = >pipes[priv->num_pipes++];
>   pipe->output = omapdss_device_get(output);
>   pipe->display = omapdss_display_get(output);
>  
> - id = of_alias_get_id(pipe->display->dev->of_node,
> -  "display");
> - pipe->alias_id = id >= 0 ? id : priv->num_pipes - 1;
> -
>   if (priv->num_pipes == ARRAY_SIZE(priv->pipes)) {
>   /* To balance the 'for_each_dss_output' loop */
>   omapdss_device_put(output);
> @@ -200,36 +179,19 @@ static int omap_connect_pipelines(struct drm_device 
> *ddev)
>   }
>   }
>  
> - /* Sort the list by DT aliases */
> - sort(priv->pipes, priv->num_pipes, sizeof(priv->pipes[0]),
> -  omap_compare_pipes, NULL);
> -
> - /*
> -  * Populate the pipeline lookup table by DISPC channel. Only one display
> -  * is allowed per channel.
> -  */
> - for (i = 0; i < priv->num_pipes; ++i) {
> - struct omap_drm_pipeline *pipe = >pipes[i];
> - enum omap_channel channel = pipe->output->dispc_channel;
> -
> - if (WARN_ON(priv->channels[channel] != NULL)) {
> - r = -EINVAL;
> - goto cleanup;
> - }
> -
> - priv->channels[channel] = pipe;
> - }
> -
>   return 0;
> +}
>  
> -cleanup:
> - /*
> -  * if we are deferring probe, we disconnect the devices we previously
> -  * connected
> -  */
> - omap_disconnect_pipelines(ddev);
> +static int omap_compare_pipelines(const void *a, const void *b)
> +{
> + const struct omap_drm_pipeline *pipe1 = a;
> + const struct omap_drm_pipeline *pipe2 = b;
>  
> - return r;
> + if (pipe1->alias_id > pipe2->alias_id)
> + return 1;
> + else if (pipe1->alias_id < pipe2->alias_id)
> + return -1;
> + return 0;
>  }
>  
>  static int omap_modeset_init_properties(struct drm_device *dev)
> @@ -254,6 +216,9 @@ static int omap_modeset_init(struct drm_device *dev)
>   int ret;
>   u32 plane_crtc_mask;
>  
> + if (!omapdss_stack_is_ready())
> + return -EPROBE_DEFER;
> +
>   drm_mode_config_init(dev);
>  
>   ret = omap_modeset_init_properties(dev);
> @@ -268,6 +233,10 @@ static int omap_modeset_init(struct drm_device *dev)
>* configuration does not match the expectations or exceeds
>* the available resources, the configuration is rejected.
>*/
> + 

[PATCH 29/29] drm/omap: Refactor initialization sequence

2018-12-05 Thread Laurent Pinchart
The omapdrm driver initialization procedure starts by connecting all
available pipelines, gathering related information (such as output and
display DSS devices, and DT aliases), sorting them by alias, and finally
creates all the DRM/KMS objects.

When using DRM bridges instead of DSS devices, we will need to attach to
the bridges before getting the aliases. As attaching to bridges requires
an encoder object, we have to reorganize the initialization sequence to
create encoders before getting aliases and sorting the pipelines.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 123 +
 1 file changed, 56 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index c9b578a49b24..4cd4fb47660a 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -150,48 +150,27 @@ static void omap_disconnect_pipelines(struct drm_device 
*ddev)
priv->num_pipes = 0;
 }
 
-static int omap_compare_pipes(const void *a, const void *b)
-{
-   const struct omap_drm_pipeline *pipe1 = a;
-   const struct omap_drm_pipeline *pipe2 = b;
-
-   if (pipe1->alias_id > pipe2->alias_id)
-   return 1;
-   else if (pipe1->alias_id < pipe2->alias_id)
-   return -1;
-   return 0;
-}
-
 static int omap_connect_pipelines(struct drm_device *ddev)
 {
struct omap_drm_private *priv = ddev->dev_private;
struct omap_dss_device *output = NULL;
-   unsigned int i;
int r;
 
-   if (!omapdss_stack_is_ready())
-   return -EPROBE_DEFER;
-
for_each_dss_output(output) {
r = omapdss_device_connect(priv->dss, NULL, output);
if (r == -EPROBE_DEFER) {
omapdss_device_put(output);
-   goto cleanup;
+   return r;
} else if (r) {
dev_warn(output->dev, "could not connect output %s\n",
 output->name);
} else {
struct omap_drm_pipeline *pipe;
-   int id;
 
pipe = >pipes[priv->num_pipes++];
pipe->output = omapdss_device_get(output);
pipe->display = omapdss_display_get(output);
 
-   id = of_alias_get_id(pipe->display->dev->of_node,
-"display");
-   pipe->alias_id = id >= 0 ? id : priv->num_pipes - 1;
-
if (priv->num_pipes == ARRAY_SIZE(priv->pipes)) {
/* To balance the 'for_each_dss_output' loop */
omapdss_device_put(output);
@@ -200,36 +179,19 @@ static int omap_connect_pipelines(struct drm_device *ddev)
}
}
 
-   /* Sort the list by DT aliases */
-   sort(priv->pipes, priv->num_pipes, sizeof(priv->pipes[0]),
-omap_compare_pipes, NULL);
-
-   /*
-* Populate the pipeline lookup table by DISPC channel. Only one display
-* is allowed per channel.
-*/
-   for (i = 0; i < priv->num_pipes; ++i) {
-   struct omap_drm_pipeline *pipe = >pipes[i];
-   enum omap_channel channel = pipe->output->dispc_channel;
-
-   if (WARN_ON(priv->channels[channel] != NULL)) {
-   r = -EINVAL;
-   goto cleanup;
-   }
-
-   priv->channels[channel] = pipe;
-   }
-
return 0;
+}
 
-cleanup:
-   /*
-* if we are deferring probe, we disconnect the devices we previously
-* connected
-*/
-   omap_disconnect_pipelines(ddev);
+static int omap_compare_pipelines(const void *a, const void *b)
+{
+   const struct omap_drm_pipeline *pipe1 = a;
+   const struct omap_drm_pipeline *pipe2 = b;
 
-   return r;
+   if (pipe1->alias_id > pipe2->alias_id)
+   return 1;
+   else if (pipe1->alias_id < pipe2->alias_id)
+   return -1;
+   return 0;
 }
 
 static int omap_modeset_init_properties(struct drm_device *dev)
@@ -254,6 +216,9 @@ static int omap_modeset_init(struct drm_device *dev)
int ret;
u32 plane_crtc_mask;
 
+   if (!omapdss_stack_is_ready())
+   return -EPROBE_DEFER;
+
drm_mode_config_init(dev);
 
ret = omap_modeset_init_properties(dev);
@@ -268,6 +233,10 @@ static int omap_modeset_init(struct drm_device *dev)
 * configuration does not match the expectations or exceeds
 * the available resources, the configuration is rejected.
 */
+   ret = omap_connect_pipelines(dev);
+   if (ret < 0)
+   return ret;
+
if (priv->num_pipes > num_mgrs || priv->num_pipes > num_ovls) {
dev_err(dev->dev, "%s(): Too many connected displays\n",