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 <laurent.pinch...@ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reic...@collabora.com>

-- 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 = &priv->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 = &priv->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",
>                       __func__);
> @@ -293,33 +262,58 @@ static int omap_modeset_init(struct drm_device *dev)
>               priv->planes[priv->num_planes++] = plane;
>       }
>  
> -     /* Create the CRTCs, encoders and connectors. */
> +     /* Create the encoders and get the pipelines aliases. */
>       for (i = 0; i < priv->num_pipes; i++) {
>               struct omap_drm_pipeline *pipe = &priv->pipes[i];
> -             struct omap_dss_device *display = pipe->display;
> -             struct drm_connector *connector;
> -             struct drm_encoder *encoder;
> -             struct drm_crtc *crtc;
> +             int id;
>  
> -             encoder = omap_encoder_init(dev, pipe->output);
> -             if (!encoder)
> +             pipe->encoder = omap_encoder_init(dev, pipe->output);
> +             if (!pipe->encoder)
>                       return -ENOMEM;
>  
> -             connector = omap_connector_init(dev, pipe->output, display,
> -                                             encoder);
> +             id = of_alias_get_id(pipe->display->dev->of_node, "display");
> +             pipe->alias_id = id >= 0 ? id : i;
> +     }
> +
> +     /* Sort the pipelines by DT aliases. */
> +     sort(priv->pipes, priv->num_pipes, sizeof(priv->pipes[0]),
> +          omap_compare_pipelines, 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 = &priv->pipes[i];
> +             enum omap_channel channel = pipe->output->dispc_channel;
> +
> +             if (WARN_ON(priv->channels[channel] != NULL))
> +                     return -EINVAL;
> +
> +             priv->channels[channel] = pipe;
> +     }
> +
> +     /* Create the connectors and CRTCs. */
> +     for (i = 0; i < priv->num_pipes; i++) {
> +             struct omap_drm_pipeline *pipe = &priv->pipes[i];
> +             struct drm_encoder *encoder = pipe->encoder;
> +             struct drm_connector *connector;
> +             struct drm_crtc *crtc;
> +
> +             connector = omap_connector_init(dev, pipe->output,
> +                                             pipe->display, encoder);
>               if (!connector)
>                       return -ENOMEM;
>  
> +             drm_connector_attach_encoder(connector, encoder);
> +             pipe->connector = connector;
> +
>               crtc = omap_crtc_init(dev, pipe, priv->planes[i]);
>               if (IS_ERR(crtc))
>                       return PTR_ERR(crtc);
>  
> -             drm_connector_attach_encoder(connector, encoder);
>               encoder->possible_crtcs = 1 << i;
> -
>               pipe->crtc = crtc;
> -             pipe->encoder = encoder;
> -             pipe->connector = connector;
>       }
>  
>       DBG("registered %u planes, %u crtcs/encoders/connectors\n",
> @@ -556,10 +550,6 @@ static int omapdrm_init(struct omap_drm_private *priv, 
> struct device *dev)
>  
>       omap_crtc_pre_init(priv);
>  
> -     ret = omap_connect_pipelines(ddev);
> -     if (ret)
> -             goto err_crtc_uninit;
> -
>       soc = soc_device_match(omapdrm_soc_devices);
>       priv->omaprev = soc ? (unsigned int)soc->data : 0;
>       priv->wq = alloc_ordered_workqueue("omapdrm", 0);
> @@ -617,7 +607,6 @@ static int omapdrm_init(struct omap_drm_private *priv, 
> struct device *dev)
>       omap_gem_deinit(ddev);
>       destroy_workqueue(priv->wq);
>       omap_disconnect_pipelines(ddev);
> -err_crtc_uninit:
>       omap_crtc_pre_uninit(priv);
>       drm_dev_put(ddev);
>       return ret;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to