diff mbox

drm/exynos: separate dpi from fimd

Message ID 1396535160-11037-1-git-send-email-a.hajda@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrzej Hajda April 3, 2014, 2:26 p.m. UTC
The patch separates dpi related routines from fimd.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi Inki,

This is my attempt to separate DPI from FIMD,
it requires putting real probe back into fimd_probe, but I
guess it should not be a problem, as it is done already for dsi.
It is based on v4 of your patch.

The patch was written quickly without proper review, I can
do it tomorrow if you are interested.
If it is OK for you, please merge it with your patch.
Anyway, I have made few tests - it works.

Regards
Andrzej
---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c  |  40 ++++++------
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  15 ++---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 109 +++++++++++++------------------
 3 files changed, 69 insertions(+), 95 deletions(-)

Comments

Inki Dae April 3, 2014, 4:29 p.m. UTC | #1
Hi Andrzej,

2014-04-03 23:26 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> The patch separates dpi related routines from fimd.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Inki,
>
> This is my attempt to separate DPI from FIMD,

Ah, I understood now. Right, if we can separate DPI from FIMD, we can
also move some codes for getting resources from fimd_bind to
fimd_probe.

Picked it up.

Thanks,
Inki Dae

> it requires putting real probe back into fimd_probe, but I
> guess it should not be a problem, as it is done already for dsi.
> It is based on v4 of your patch.
>
> The patch was written quickly without proper review, I can
> do it tomorrow if you are interested.
> If it is OK for you, please merge it with your patch.
> Anyway, I have made few tests - it works.
>
> Regards
> Andrzej
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |  40 ++++++------
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  15 ++---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 109 +++++++++++++------------------
>  3 files changed, 69 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index ac206e7..03cb126 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -40,20 +40,10 @@ exynos_dpi_detect(struct drm_connector *connector, bool force)
>  {
>         struct exynos_dpi *ctx = connector_to_dpi(connector);
>
> -       /* panels supported only by boot-loader are always connected */
> -       if (!ctx->panel_node)
> -               return connector_status_connected;
> -
> -       if (!ctx->panel) {
> -               ctx->panel = of_drm_find_panel(ctx->panel_node);
> -               if (ctx->panel)
> -                       drm_panel_attach(ctx->panel, &ctx->connector);
> -       }
> +       if (!ctx->panel->connector)
> +               drm_panel_attach(ctx->panel, &ctx->connector);
>
> -       if (ctx->panel)
> -               return connector_status_connected;
> -
> -       return connector_status_disconnected;
> +       return connector_status_connected;
>  }
>
>  static void exynos_dpi_connector_destroy(struct drm_connector *connector)
> @@ -291,8 +281,10 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>                         return -ENOMEM;
>
>                 ret = of_get_videomode(dn, vm, 0);
> -               if (ret < 0)
> +               if (ret < 0) {
> +                       devm_kfree(dev, vm);
>                         return ret;
> +               }
>
>                 ctx->vm = vm;
>
> @@ -305,27 +297,35 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>         return 0;
>  }
>
> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev)
> +struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>  {
>         struct exynos_dpi *ctx;
>         int ret;
>
>         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>         if (!ctx)
> -               return -ENOMEM;
> +               return NULL;
>
>         ctx->dev = dev;
>         exynos_dpi_display.ctx = ctx;
>         ctx->dpms_mode = DRM_MODE_DPMS_OFF;
>
>         ret = exynos_dpi_parse_dt(ctx);
> -       if (ret < 0)
> -               return ret;
> +       if (ret < 0) {
> +               devm_kfree(dev, ctx);
> +               return NULL;
> +       }
> +
> +       if (ctx->panel_node) {
> +               ctx->panel = of_drm_find_panel(ctx->panel_node);
> +               if (!ctx->panel)
> +                       return ERR_PTR(-EPROBE_DEFER);
> +       }
>
> -       return exynos_drm_create_enc_conn(drm_dev, &exynos_dpi_display);
> +       return &exynos_dpi_display;
>  }
>
> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev)
> +int exynos_dpi_remove(struct device *dev)
>  {
>         struct drm_encoder *encoder = exynos_dpi_display.encoder;
>         struct exynos_dpi *ctx = exynos_dpi_display.ctx;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 2b87eb7..583a0bd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -334,17 +334,12 @@ int exynos_platform_device_ipp_register(void);
>  void exynos_platform_device_ipp_unregister(void);
>
>  #ifdef CONFIG_DRM_EXYNOS_DPI
> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev);
> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev);
> -struct device_node *exynos_dpi_of_find_panel_node(struct device *dev);
> +struct exynos_drm_display * exynos_dpi_probe(struct device *dev);
> +int exynos_dpi_remove(struct device *dev);
>  #else
> -static inline int exynos_dpi_probe(struct drm_device *drm_dev,
> -                                       struct device *dev) { return 0; }
> -static inline int exynos_dpi_remove(struct drm_device *drm_dev,
> -                                       struct device *dev) { return 0; }
> -static inline struct device_node
> -                       *exynos_dpi_of_find_panel_node(struct device *dev)
> -{ return NULL; }
> +static inline struct exynos_drm_display *
> +exynos_dpi_probe(struct device *dev) { return 0; }
> +static inline int exynos_dpi_remove(struct device *dev) { return 0; }
>  #endif
>
>  /*
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 76282b3..11cce7b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -24,7 +24,6 @@
>  #include <video/of_display_timing.h>
>  #include <video/of_videomode.h>
>  #include <video/samsung_fimd.h>
> -#include <drm/drm_panel.h>
>  #include <drm/exynos_drm.h>
>
>  #include "exynos_drm_drv.h"
> @@ -124,6 +123,7 @@ struct fimd_context {
>
>         struct exynos_drm_panel_info panel;
>         struct fimd_driver_data *driver_data;
> +       struct exynos_drm_display *dpi;
>  };
>
>  static const struct of_device_id fimd_driver_dt_match[] = {
> @@ -853,12 +853,49 @@ out:
>
>  static int fimd_bind(struct device *dev, struct device *master, void *data)
>  {
> -       struct platform_device *pdev = to_platform_device(dev);
> +       struct fimd_context *ctx = fimd_manager.ctx;
>         struct drm_device *drm_dev = data;
> +       int win;
> +
> +       fimd_mgr_initialize(&fimd_manager, drm_dev);
> +       exynos_drm_crtc_create(&fimd_manager);
> +       if (ctx->dpi)
> +               exynos_drm_create_enc_conn(drm_dev, ctx->dpi);
> +
> +       for (win = 0; win < WINDOWS_NR; win++)
> +               fimd_clear_win(ctx, win);
> +
> +       return 0;
> +
> +}
> +
> +static void fimd_unbind(struct device *dev, struct device *master,
> +                       void *data)
> +{
> +       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
> +       struct fimd_context *ctx = fimd_manager.ctx;
> +       struct drm_crtc *crtc = mgr->crtc;
> +
> +       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
> +
> +       if (ctx->dpi)
> +               exynos_dpi_remove(dev);
> +
> +       fimd_mgr_remove(mgr);
> +
> +       crtc->funcs->destroy(crtc);
> +}
> +
> +static const struct component_ops fimd_component_ops = {
> +       .bind   = fimd_bind,
> +       .unbind = fimd_unbind,
> +};
> +
> +static int fimd_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
>         struct fimd_context *ctx;
> -       struct device_node *dn;
>         struct resource *res;
> -       int win;
>         int ret = -EINVAL;
>
>         if (!dev->of_node)
> @@ -914,68 +951,10 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>         platform_set_drvdata(pdev, &fimd_manager);
>
>         fimd_manager.ctx = ctx;
> -       fimd_mgr_initialize(&fimd_manager, drm_dev);
> -
> -       exynos_drm_crtc_create(&fimd_manager);
> -
> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
> -       if (dn) {
> -               /*
> -                * It should be called after exynos_drm_crtc_create call
> -                * because exynos_dpi_probe call will try to find same lcd
> -                * type of manager to setup possible_crtcs.
> -                */
> -               exynos_dpi_probe(drm_dev, dev);
> -       }
> -
> -       for (win = 0; win < WINDOWS_NR; win++)
> -               fimd_clear_win(ctx, win);
> -
> -       return 0;
> -}
> -
> -static void fimd_unbind(struct device *dev, struct device *master,
> -                       void *data)
> -{
> -       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
> -       struct drm_crtc *crtc = mgr->crtc;
> -       struct device_node *dn;
>
> -       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
> -
> -       dn = exynos_dpi_of_find_panel_node(dev);
> -       if (dn)
> -               exynos_dpi_remove(mgr->drm_dev, dev);
> -
> -       fimd_mgr_remove(mgr);
> -
> -       crtc->funcs->destroy(crtc);
> -}
> -
> -static const struct component_ops fimd_component_ops = {
> -       .bind   = fimd_bind,
> -       .unbind = fimd_unbind,
> -};
> -
> -static int fimd_probe(struct platform_device *pdev)
> -{
> -       struct device_node *dn;
> -
> -       /* Check if fimd node has port node. */
> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
> -       if (dn) {
> -               struct drm_panel *panel;
> -
> -               /*
> -                * Do not bind if there is the port node but a drm_panel
> -                * isn't added to panel_list yet.
> -                * In this case, fimd_probe will be called by defered probe
> -                * again after the drm_panel is added to panel_list.
> -                */
> -               panel = of_drm_find_panel(dn);
> -               if (!panel)
> -                       return -EPROBE_DEFER;
> -       }
> +       ctx->dpi = exynos_dpi_probe(dev);
> +       if (IS_ERR(ctx->dpi))
> +               return PTR_ERR(ctx->dpi);
>
>         pm_runtime_enable(&pdev->dev);
>
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Inki Dae April 3, 2014, 4:35 p.m. UTC | #2
2014-04-03 23:26 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
> The patch separates dpi related routines from fimd.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Inki,
>
> This is my attempt to separate DPI from FIMD,
> it requires putting real probe back into fimd_probe, but I
> guess it should not be a problem, as it is done already for dsi.
> It is based on v4 of your patch.
>
> The patch was written quickly without proper review, I can
> do it tomorrow if you are interested.
> If it is OK for you, please merge it with your patch.
> Anyway, I have made few tests - it works.
>
> Regards
> Andrzej
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |  40 ++++++------
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  15 ++---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 109 +++++++++++++------------------
>  3 files changed, 69 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index ac206e7..03cb126 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -40,20 +40,10 @@ exynos_dpi_detect(struct drm_connector *connector, bool force)
>  {
>         struct exynos_dpi *ctx = connector_to_dpi(connector);
>
> -       /* panels supported only by boot-loader are always connected */
> -       if (!ctx->panel_node)
> -               return connector_status_connected;
> -
> -       if (!ctx->panel) {
> -               ctx->panel = of_drm_find_panel(ctx->panel_node);
> -               if (ctx->panel)
> -                       drm_panel_attach(ctx->panel, &ctx->connector);
> -       }
> +       if (!ctx->panel->connector)
> +               drm_panel_attach(ctx->panel, &ctx->connector);
>
> -       if (ctx->panel)
> -               return connector_status_connected;
> -
> -       return connector_status_disconnected;
> +       return connector_status_connected;
>  }
>
>  static void exynos_dpi_connector_destroy(struct drm_connector *connector)
> @@ -291,8 +281,10 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>                         return -ENOMEM;
>
>                 ret = of_get_videomode(dn, vm, 0);
> -               if (ret < 0)
> +               if (ret < 0) {
> +                       devm_kfree(dev, vm);
>                         return ret;
> +               }
>
>                 ctx->vm = vm;
>
> @@ -305,27 +297,35 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>         return 0;
>  }
>
> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev)
> +struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>  {
>         struct exynos_dpi *ctx;
>         int ret;
>
>         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>         if (!ctx)
> -               return -ENOMEM;
> +               return NULL;
>
>         ctx->dev = dev;
>         exynos_dpi_display.ctx = ctx;
>         ctx->dpms_mode = DRM_MODE_DPMS_OFF;
>
>         ret = exynos_dpi_parse_dt(ctx);
> -       if (ret < 0)
> -               return ret;
> +       if (ret < 0) {
> +               devm_kfree(dev, ctx);
> +               return NULL;
> +       }
> +
> +       if (ctx->panel_node) {
> +               ctx->panel = of_drm_find_panel(ctx->panel_node);
> +               if (!ctx->panel)
> +                       return ERR_PTR(-EPROBE_DEFER);
> +       }
>
> -       return exynos_drm_create_enc_conn(drm_dev, &exynos_dpi_display);
> +       return &exynos_dpi_display;
>  }
>
> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev)
> +int exynos_dpi_remove(struct device *dev)
>  {
>         struct drm_encoder *encoder = exynos_dpi_display.encoder;
>         struct exynos_dpi *ctx = exynos_dpi_display.ctx;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 2b87eb7..583a0bd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -334,17 +334,12 @@ int exynos_platform_device_ipp_register(void);
>  void exynos_platform_device_ipp_unregister(void);
>
>  #ifdef CONFIG_DRM_EXYNOS_DPI
> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev);
> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev);
> -struct device_node *exynos_dpi_of_find_panel_node(struct device *dev);
> +struct exynos_drm_display * exynos_dpi_probe(struct device *dev);
> +int exynos_dpi_remove(struct device *dev);
>  #else
> -static inline int exynos_dpi_probe(struct drm_device *drm_dev,
> -                                       struct device *dev) { return 0; }
> -static inline int exynos_dpi_remove(struct drm_device *drm_dev,
> -                                       struct device *dev) { return 0; }
> -static inline struct device_node
> -                       *exynos_dpi_of_find_panel_node(struct device *dev)
> -{ return NULL; }
> +static inline struct exynos_drm_display *
> +exynos_dpi_probe(struct device *dev) { return 0; }
> +static inline int exynos_dpi_remove(struct device *dev) { return 0; }
>  #endif
>
>  /*
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 76282b3..11cce7b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -24,7 +24,6 @@
>  #include <video/of_display_timing.h>
>  #include <video/of_videomode.h>
>  #include <video/samsung_fimd.h>
> -#include <drm/drm_panel.h>
>  #include <drm/exynos_drm.h>
>
>  #include "exynos_drm_drv.h"
> @@ -124,6 +123,7 @@ struct fimd_context {
>
>         struct exynos_drm_panel_info panel;
>         struct fimd_driver_data *driver_data;
> +       struct exynos_drm_display *dpi;
>  };
>
>  static const struct of_device_id fimd_driver_dt_match[] = {
> @@ -853,12 +853,49 @@ out:
>
>  static int fimd_bind(struct device *dev, struct device *master, void *data)
>  {
> -       struct platform_device *pdev = to_platform_device(dev);
> +       struct fimd_context *ctx = fimd_manager.ctx;
>         struct drm_device *drm_dev = data;
> +       int win;
> +
> +       fimd_mgr_initialize(&fimd_manager, drm_dev);
> +       exynos_drm_crtc_create(&fimd_manager);
> +       if (ctx->dpi)
> +               exynos_drm_create_enc_conn(drm_dev, ctx->dpi);
> +
> +       for (win = 0; win < WINDOWS_NR; win++)
> +               fimd_clear_win(ctx, win);
> +
> +       return 0;
> +
> +}
> +
> +static void fimd_unbind(struct device *dev, struct device *master,
> +                       void *data)
> +{
> +       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
> +       struct fimd_context *ctx = fimd_manager.ctx;
> +       struct drm_crtc *crtc = mgr->crtc;
> +
> +       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
> +
> +       if (ctx->dpi)
> +               exynos_dpi_remove(dev);

Above codes would better to move fimd_remove() because
exynos_dpi_remove is pair with exynos_dpi_probe, and exynos_dpi_probe
will be called at fimd_probe. So I just modified it.

Thanks,
Inki Dae

> +
> +       fimd_mgr_remove(mgr);
> +
> +       crtc->funcs->destroy(crtc);
> +}
> +
> +static const struct component_ops fimd_component_ops = {
> +       .bind   = fimd_bind,
> +       .unbind = fimd_unbind,
> +};
> +
> +static int fimd_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
>         struct fimd_context *ctx;
> -       struct device_node *dn;
>         struct resource *res;
> -       int win;
>         int ret = -EINVAL;
>
>         if (!dev->of_node)
> @@ -914,68 +951,10 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>         platform_set_drvdata(pdev, &fimd_manager);
>
>         fimd_manager.ctx = ctx;
> -       fimd_mgr_initialize(&fimd_manager, drm_dev);
> -
> -       exynos_drm_crtc_create(&fimd_manager);
> -
> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
> -       if (dn) {
> -               /*
> -                * It should be called after exynos_drm_crtc_create call
> -                * because exynos_dpi_probe call will try to find same lcd
> -                * type of manager to setup possible_crtcs.
> -                */
> -               exynos_dpi_probe(drm_dev, dev);
> -       }
> -
> -       for (win = 0; win < WINDOWS_NR; win++)
> -               fimd_clear_win(ctx, win);
> -
> -       return 0;
> -}
> -
> -static void fimd_unbind(struct device *dev, struct device *master,
> -                       void *data)
> -{
> -       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
> -       struct drm_crtc *crtc = mgr->crtc;
> -       struct device_node *dn;
>
> -       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
> -
> -       dn = exynos_dpi_of_find_panel_node(dev);
> -       if (dn)
> -               exynos_dpi_remove(mgr->drm_dev, dev);
> -
> -       fimd_mgr_remove(mgr);
> -
> -       crtc->funcs->destroy(crtc);
> -}
> -
> -static const struct component_ops fimd_component_ops = {
> -       .bind   = fimd_bind,
> -       .unbind = fimd_unbind,
> -};
> -
> -static int fimd_probe(struct platform_device *pdev)
> -{
> -       struct device_node *dn;
> -
> -       /* Check if fimd node has port node. */
> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
> -       if (dn) {
> -               struct drm_panel *panel;
> -
> -               /*
> -                * Do not bind if there is the port node but a drm_panel
> -                * isn't added to panel_list yet.
> -                * In this case, fimd_probe will be called by defered probe
> -                * again after the drm_panel is added to panel_list.
> -                */
> -               panel = of_drm_find_panel(dn);
> -               if (!panel)
> -                       return -EPROBE_DEFER;
> -       }
> +       ctx->dpi = exynos_dpi_probe(dev);
> +       if (IS_ERR(ctx->dpi))
> +               return PTR_ERR(ctx->dpi);
>
>         pm_runtime_enable(&pdev->dev);
>
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Inki Dae April 3, 2014, 5:01 p.m. UTC | #3
2014-04-04 1:35 GMT+09:00 Inki Dae <inki.dae@samsung.com>:
> 2014-04-03 23:26 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
>> The patch separates dpi related routines from fimd.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> Hi Inki,
>>
>> This is my attempt to separate DPI from FIMD,
>> it requires putting real probe back into fimd_probe, but I
>> guess it should not be a problem, as it is done already for dsi.
>> It is based on v4 of your patch.
>>
>> The patch was written quickly without proper review, I can
>> do it tomorrow if you are interested.
>> If it is OK for you, please merge it with your patch.
>> Anyway, I have made few tests - it works.
>>
>> Regards
>> Andrzej
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |  40 ++++++------
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  15 ++---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 109 +++++++++++++------------------
>>  3 files changed, 69 insertions(+), 95 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> index ac206e7..03cb126 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> @@ -40,20 +40,10 @@ exynos_dpi_detect(struct drm_connector *connector, bool force)
>>  {
>>         struct exynos_dpi *ctx = connector_to_dpi(connector);
>>
>> -       /* panels supported only by boot-loader are always connected */
>> -       if (!ctx->panel_node)
>> -               return connector_status_connected;
>> -
>> -       if (!ctx->panel) {
>> -               ctx->panel = of_drm_find_panel(ctx->panel_node);
>> -               if (ctx->panel)
>> -                       drm_panel_attach(ctx->panel, &ctx->connector);
>> -       }
>> +       if (!ctx->panel->connector)
>> +               drm_panel_attach(ctx->panel, &ctx->connector);
>>
>> -       if (ctx->panel)
>> -               return connector_status_connected;
>> -
>> -       return connector_status_disconnected;
>> +       return connector_status_connected;
>>  }
>>
>>  static void exynos_dpi_connector_destroy(struct drm_connector *connector)
>> @@ -291,8 +281,10 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>>                         return -ENOMEM;
>>
>>                 ret = of_get_videomode(dn, vm, 0);
>> -               if (ret < 0)
>> +               if (ret < 0) {
>> +                       devm_kfree(dev, vm);
>>                         return ret;
>> +               }
>>
>>                 ctx->vm = vm;
>>
>> @@ -305,27 +297,35 @@ static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
>>         return 0;
>>  }
>>
>> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev)
>> +struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>>  {
>>         struct exynos_dpi *ctx;
>>         int ret;
>>
>>         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>         if (!ctx)
>> -               return -ENOMEM;
>> +               return NULL;
>>
>>         ctx->dev = dev;
>>         exynos_dpi_display.ctx = ctx;
>>         ctx->dpms_mode = DRM_MODE_DPMS_OFF;
>>
>>         ret = exynos_dpi_parse_dt(ctx);
>> -       if (ret < 0)
>> -               return ret;
>> +       if (ret < 0) {
>> +               devm_kfree(dev, ctx);
>> +               return NULL;
>> +       }
>> +
>> +       if (ctx->panel_node) {
>> +               ctx->panel = of_drm_find_panel(ctx->panel_node);
>> +               if (!ctx->panel)
>> +                       return ERR_PTR(-EPROBE_DEFER);
>> +       }
>>
>> -       return exynos_drm_create_enc_conn(drm_dev, &exynos_dpi_display);
>> +       return &exynos_dpi_display;
>>  }
>>
>> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev)
>> +int exynos_dpi_remove(struct device *dev)
>>  {
>>         struct drm_encoder *encoder = exynos_dpi_display.encoder;
>>         struct exynos_dpi *ctx = exynos_dpi_display.ctx;
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 2b87eb7..583a0bd 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -334,17 +334,12 @@ int exynos_platform_device_ipp_register(void);
>>  void exynos_platform_device_ipp_unregister(void);
>>
>>  #ifdef CONFIG_DRM_EXYNOS_DPI
>> -int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev);
>> -int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev);
>> -struct device_node *exynos_dpi_of_find_panel_node(struct device *dev);
>> +struct exynos_drm_display * exynos_dpi_probe(struct device *dev);
>> +int exynos_dpi_remove(struct device *dev);
>>  #else
>> -static inline int exynos_dpi_probe(struct drm_device *drm_dev,
>> -                                       struct device *dev) { return 0; }
>> -static inline int exynos_dpi_remove(struct drm_device *drm_dev,
>> -                                       struct device *dev) { return 0; }
>> -static inline struct device_node
>> -                       *exynos_dpi_of_find_panel_node(struct device *dev)
>> -{ return NULL; }
>> +static inline struct exynos_drm_display *
>> +exynos_dpi_probe(struct device *dev) { return 0; }
>> +static inline int exynos_dpi_remove(struct device *dev) { return 0; }
>>  #endif
>>
>>  /*
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 76282b3..11cce7b 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -24,7 +24,6 @@
>>  #include <video/of_display_timing.h>
>>  #include <video/of_videomode.h>
>>  #include <video/samsung_fimd.h>
>> -#include <drm/drm_panel.h>
>>  #include <drm/exynos_drm.h>
>>
>>  #include "exynos_drm_drv.h"
>> @@ -124,6 +123,7 @@ struct fimd_context {
>>
>>         struct exynos_drm_panel_info panel;
>>         struct fimd_driver_data *driver_data;
>> +       struct exynos_drm_display *dpi;

Changed dpi -> display.

>>  };
>>
>>  static const struct of_device_id fimd_driver_dt_match[] = {
>> @@ -853,12 +853,49 @@ out:
>>
>>  static int fimd_bind(struct device *dev, struct device *master, void *data)
>>  {
>> -       struct platform_device *pdev = to_platform_device(dev);
>> +       struct fimd_context *ctx = fimd_manager.ctx;
>>         struct drm_device *drm_dev = data;
>> +       int win;
>> +
>> +       fimd_mgr_initialize(&fimd_manager, drm_dev);
>> +       exynos_drm_crtc_create(&fimd_manager);
>> +       if (ctx->dpi)
>> +               exynos_drm_create_enc_conn(drm_dev, ctx->dpi);
>> +
>> +       for (win = 0; win < WINDOWS_NR; win++)
>> +               fimd_clear_win(ctx, win);
>> +
>> +       return 0;
>> +
>> +}
>> +
>> +static void fimd_unbind(struct device *dev, struct device *master,
>> +                       void *data)
>> +{
>> +       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
>> +       struct fimd_context *ctx = fimd_manager.ctx;
>> +       struct drm_crtc *crtc = mgr->crtc;
>> +
>> +       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
>> +
>> +       if (ctx->dpi)
>> +               exynos_dpi_remove(dev);
>
> Above codes would better to move fimd_remove() because
> exynos_dpi_remove is pair with exynos_dpi_probe, and exynos_dpi_probe
> will be called at fimd_probe. So I just modified it.
>

Hm.. there seems something unpaired. exynos_drm_create_enc_conn() is
called in fimd_bind but cleanup for it is done in exynos_dpi_remove so
it cannot move exynos_dpi_remove call to fimd_remove().

Thanks,
Inki Dae

> Thanks,
> Inki Dae
>
>> +
>> +       fimd_mgr_remove(mgr);
>> +
>> +       crtc->funcs->destroy(crtc);
>> +}
>> +
>> +static const struct component_ops fimd_component_ops = {
>> +       .bind   = fimd_bind,
>> +       .unbind = fimd_unbind,
>> +};
>> +
>> +static int fimd_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx;
>> -       struct device_node *dn;
>>         struct resource *res;
>> -       int win;
>>         int ret = -EINVAL;
>>
>>         if (!dev->of_node)
>> @@ -914,68 +951,10 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>>         platform_set_drvdata(pdev, &fimd_manager);
>>
>>         fimd_manager.ctx = ctx;
>> -       fimd_mgr_initialize(&fimd_manager, drm_dev);
>> -
>> -       exynos_drm_crtc_create(&fimd_manager);
>> -
>> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
>> -       if (dn) {
>> -               /*
>> -                * It should be called after exynos_drm_crtc_create call
>> -                * because exynos_dpi_probe call will try to find same lcd
>> -                * type of manager to setup possible_crtcs.
>> -                */
>> -               exynos_dpi_probe(drm_dev, dev);
>> -       }
>> -
>> -       for (win = 0; win < WINDOWS_NR; win++)
>> -               fimd_clear_win(ctx, win);
>> -
>> -       return 0;
>> -}
>> -
>> -static void fimd_unbind(struct device *dev, struct device *master,
>> -                       void *data)
>> -{
>> -       struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
>> -       struct drm_crtc *crtc = mgr->crtc;
>> -       struct device_node *dn;
>>
>> -       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
>> -
>> -       dn = exynos_dpi_of_find_panel_node(dev);
>> -       if (dn)
>> -               exynos_dpi_remove(mgr->drm_dev, dev);
>> -
>> -       fimd_mgr_remove(mgr);
>> -
>> -       crtc->funcs->destroy(crtc);
>> -}
>> -
>> -static const struct component_ops fimd_component_ops = {
>> -       .bind   = fimd_bind,
>> -       .unbind = fimd_unbind,
>> -};
>> -
>> -static int fimd_probe(struct platform_device *pdev)
>> -{
>> -       struct device_node *dn;
>> -
>> -       /* Check if fimd node has port node. */
>> -       dn = exynos_dpi_of_find_panel_node(&pdev->dev);
>> -       if (dn) {
>> -               struct drm_panel *panel;
>> -
>> -               /*
>> -                * Do not bind if there is the port node but a drm_panel
>> -                * isn't added to panel_list yet.
>> -                * In this case, fimd_probe will be called by defered probe
>> -                * again after the drm_panel is added to panel_list.
>> -                */
>> -               panel = of_drm_find_panel(dn);
>> -               if (!panel)
>> -                       return -EPROBE_DEFER;
>> -       }
>> +       ctx->dpi = exynos_dpi_probe(dev);
>> +       if (IS_ERR(ctx->dpi))
>> +               return PTR_ERR(ctx->dpi);
>>
>>         pm_runtime_enable(&pdev->dev);
>>
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index ac206e7..03cb126 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -40,20 +40,10 @@  exynos_dpi_detect(struct drm_connector *connector, bool force)
 {
 	struct exynos_dpi *ctx = connector_to_dpi(connector);
 
-	/* panels supported only by boot-loader are always connected */
-	if (!ctx->panel_node)
-		return connector_status_connected;
-
-	if (!ctx->panel) {
-		ctx->panel = of_drm_find_panel(ctx->panel_node);
-		if (ctx->panel)
-			drm_panel_attach(ctx->panel, &ctx->connector);
-	}
+	if (!ctx->panel->connector)
+		drm_panel_attach(ctx->panel, &ctx->connector);
 
-	if (ctx->panel)
-		return connector_status_connected;
-
-	return connector_status_disconnected;
+	return connector_status_connected;
 }
 
 static void exynos_dpi_connector_destroy(struct drm_connector *connector)
@@ -291,8 +281,10 @@  static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
 			return -ENOMEM;
 
 		ret = of_get_videomode(dn, vm, 0);
-		if (ret < 0)
+		if (ret < 0) {
+			devm_kfree(dev, vm);
 			return ret;
+		}
 
 		ctx->vm = vm;
 
@@ -305,27 +297,35 @@  static int exynos_dpi_parse_dt(struct exynos_dpi *ctx)
 	return 0;
 }
 
-int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev)
+struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
 {
 	struct exynos_dpi *ctx;
 	int ret;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
-		return -ENOMEM;
+		return NULL;
 
 	ctx->dev = dev;
 	exynos_dpi_display.ctx = ctx;
 	ctx->dpms_mode = DRM_MODE_DPMS_OFF;
 
 	ret = exynos_dpi_parse_dt(ctx);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		devm_kfree(dev, ctx);
+		return NULL;
+	}
+
+	if (ctx->panel_node) {
+		ctx->panel = of_drm_find_panel(ctx->panel_node);
+		if (!ctx->panel)
+			return ERR_PTR(-EPROBE_DEFER);
+	}
 
-	return exynos_drm_create_enc_conn(drm_dev, &exynos_dpi_display);
+	return &exynos_dpi_display;
 }
 
-int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev)
+int exynos_dpi_remove(struct device *dev)
 {
 	struct drm_encoder *encoder = exynos_dpi_display.encoder;
 	struct exynos_dpi *ctx = exynos_dpi_display.ctx;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 2b87eb7..583a0bd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -334,17 +334,12 @@  int exynos_platform_device_ipp_register(void);
 void exynos_platform_device_ipp_unregister(void);
 
 #ifdef CONFIG_DRM_EXYNOS_DPI
-int exynos_dpi_probe(struct drm_device *drm_dev, struct device *dev);
-int exynos_dpi_remove(struct drm_device *drm_dev, struct device *dev);
-struct device_node *exynos_dpi_of_find_panel_node(struct device *dev);
+struct exynos_drm_display * exynos_dpi_probe(struct device *dev);
+int exynos_dpi_remove(struct device *dev);
 #else
-static inline int exynos_dpi_probe(struct drm_device *drm_dev,
-					struct device *dev) { return 0; }
-static inline int exynos_dpi_remove(struct drm_device *drm_dev,
-					struct device *dev) { return 0; }
-static inline struct device_node
-			*exynos_dpi_of_find_panel_node(struct device *dev)
-{ return NULL; }
+static inline struct exynos_drm_display *
+exynos_dpi_probe(struct device *dev) { return 0; }
+static inline int exynos_dpi_remove(struct device *dev) { return 0; }
 #endif
 
 /*
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 76282b3..11cce7b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -24,7 +24,6 @@ 
 #include <video/of_display_timing.h>
 #include <video/of_videomode.h>
 #include <video/samsung_fimd.h>
-#include <drm/drm_panel.h>
 #include <drm/exynos_drm.h>
 
 #include "exynos_drm_drv.h"
@@ -124,6 +123,7 @@  struct fimd_context {
 
 	struct exynos_drm_panel_info panel;
 	struct fimd_driver_data *driver_data;
+	struct exynos_drm_display *dpi;
 };
 
 static const struct of_device_id fimd_driver_dt_match[] = {
@@ -853,12 +853,49 @@  out:
 
 static int fimd_bind(struct device *dev, struct device *master, void *data)
 {
-	struct platform_device *pdev = to_platform_device(dev);
+	struct fimd_context *ctx = fimd_manager.ctx;
 	struct drm_device *drm_dev = data;
+	int win;
+
+	fimd_mgr_initialize(&fimd_manager, drm_dev);
+	exynos_drm_crtc_create(&fimd_manager);
+	if (ctx->dpi)
+		exynos_drm_create_enc_conn(drm_dev, ctx->dpi);
+
+	for (win = 0; win < WINDOWS_NR; win++)
+		fimd_clear_win(ctx, win);
+
+	return 0;
+
+}
+
+static void fimd_unbind(struct device *dev, struct device *master,
+			void *data)
+{
+	struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
+	struct fimd_context *ctx = fimd_manager.ctx;
+	struct drm_crtc *crtc = mgr->crtc;
+
+	fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
+
+	if (ctx->dpi)
+		exynos_dpi_remove(dev);
+
+	fimd_mgr_remove(mgr);
+
+	crtc->funcs->destroy(crtc);
+}
+
+static const struct component_ops fimd_component_ops = {
+	.bind	= fimd_bind,
+	.unbind = fimd_unbind,
+};
+
+static int fimd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx;
-	struct device_node *dn;
 	struct resource *res;
-	int win;
 	int ret = -EINVAL;
 
 	if (!dev->of_node)
@@ -914,68 +951,10 @@  static int fimd_bind(struct device *dev, struct device *master, void *data)
 	platform_set_drvdata(pdev, &fimd_manager);
 
 	fimd_manager.ctx = ctx;
-	fimd_mgr_initialize(&fimd_manager, drm_dev);
-
-	exynos_drm_crtc_create(&fimd_manager);
-
-	dn = exynos_dpi_of_find_panel_node(&pdev->dev);
-	if (dn) {
-		/*
-		 * It should be called after exynos_drm_crtc_create call
-		 * because exynos_dpi_probe call will try to find same lcd
-		 * type of manager to setup possible_crtcs.
-		 */
-		exynos_dpi_probe(drm_dev, dev);
-	}
-
-	for (win = 0; win < WINDOWS_NR; win++)
-		fimd_clear_win(ctx, win);
-
-	return 0;
-}
-
-static void fimd_unbind(struct device *dev, struct device *master,
-			void *data)
-{
-	struct exynos_drm_manager *mgr = dev_get_drvdata(dev);
-	struct drm_crtc *crtc = mgr->crtc;
-	struct device_node *dn;
 
-	fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
-
-	dn = exynos_dpi_of_find_panel_node(dev);
-	if (dn)
-		exynos_dpi_remove(mgr->drm_dev, dev);
-
-	fimd_mgr_remove(mgr);
-
-	crtc->funcs->destroy(crtc);
-}
-
-static const struct component_ops fimd_component_ops = {
-	.bind	= fimd_bind,
-	.unbind = fimd_unbind,
-};
-
-static int fimd_probe(struct platform_device *pdev)
-{
-	struct device_node *dn;
-
-	/* Check if fimd node has port node. */
-	dn = exynos_dpi_of_find_panel_node(&pdev->dev);
-	if (dn) {
-		struct drm_panel *panel;
-
-		/*
-		 * Do not bind if there is the port node but a drm_panel
-		 * isn't added to panel_list yet.
-		 * In this case, fimd_probe will be called by defered probe
-		 * again after the drm_panel is added to panel_list.
-		 */
-		panel = of_drm_find_panel(dn);
-		if (!panel)
-			return -EPROBE_DEFER;
-	}
+	ctx->dpi = exynos_dpi_probe(dev);
+	if (IS_ERR(ctx->dpi))
+		return PTR_ERR(ctx->dpi);
 
 	pm_runtime_enable(&pdev->dev);