On Fri, 23 Aug 2019 22:32:45 +0300
Laurent Pinchart <laurent.pinch...@ideasonboard.com> wrote:

> The drm panel bridge creates a connector using a connector type explicit

                                                                  ^explicitly

> passed by the display controller or bridge driver that instantiates the
> panel bridge. Now that drm_panel reports its connector type, we can use
> it to avoid passing an explicit (and often incorrect) connector type to
> drm_panel_bridge_add() and devm_drm_panel_bridge_add().
> 
> Several drivers report incorrect or unknown connector types to
> userspace. Reporting a different type may result in a breakage. For that
> reason, rename (devm_)drm_panel_bridge_add() to
> (devm_)drm_panel_bridge_add_typed(), and add new
> (devm_)drm_panel_bridge_add() functions that use the panel connector
> type. Update all callers of (devm_)drm_panel_bridge_add() to the _typed
> function, they will be converted one by one after testing.
> 
> The panel drivers have been updated with the following Coccinelle
> semantic patch, with manual inspection and fixes to indentation.
> 
> @@
> expression bridge;
> expression dev;
> expression panel;
> identifier type;
> @@
> (
> -bridge = drm_panel_bridge_add(panel, type);
> +bridge = drm_panel_bridge_add_typed(panel, type);
> |
> -bridge = devm_drm_panel_bridge_add(dev, panel, type);
> +bridge = devm_drm_panel_bridge_add_typed(dev, panel, type);
> )
> 
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Reviewed-by: Boris Brezillon <boris.brezil...@collabora.com>

> ---
> Changes since v1:
> 
> - Add and use _typed variants
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c  |  3 +-
>  drivers/gpu/drm/bridge/cdns-dsi.c             |  3 +-
>  drivers/gpu/drm/bridge/lvds-encoder.c         |  3 +-
>  drivers/gpu/drm/bridge/panel.c                | 69 ++++++++++++++++---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  3 +-
>  drivers/gpu/drm/ingenic/ingenic-drm.c         |  4 +-
>  drivers/gpu/drm/mcde/mcde_dsi.c               |  4 +-
>  drivers/gpu/drm/pl111/pl111_drv.c             |  4 +-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c     |  4 +-
>  drivers/gpu/drm/rockchip/rockchip_rgb.c       |  3 +-
>  drivers/gpu/drm/stm/ltdc.c                    |  4 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c      |  4 +-
>  drivers/gpu/drm/tve200/tve200_drv.c           |  4 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                 |  3 +-
>  drivers/gpu/drm/vc4/vc4_dsi.c                 |  4 +-
>  include/drm/drm_bridge.h                      | 11 +--
>  16 files changed, 93 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 375fa84c548b..121b62682d80 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -107,7 +107,8 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
> *dev, int endpoint)
>       output->encoder.possible_crtcs = 0x1;
>  
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel, 
> DRM_MODE_CONNECTOR_Unknown);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_Unknown);
>               if (IS_ERR(bridge))
>                       return PTR_ERR(bridge);
>       }
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c 
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index 6166dca6be81..3a5bd4e7fd1e 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -956,7 +956,8 @@ static int cdns_dsi_attach(struct mipi_dsi_host *host,
>  
>       panel = of_drm_find_panel(np);
>       if (!IS_ERR(panel)) {
> -             bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_DSI);
>       } else {
>               bridge = of_drm_find_bridge(dev->dev.of_node);
>               if (!bridge)
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
> b/drivers/gpu/drm/bridge/lvds-encoder.c
> index 2ab2c234f26c..e2132a8d5106 100644
> --- a/drivers/gpu/drm/bridge/lvds-encoder.c
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -106,7 +106,8 @@ static int lvds_encoder_probe(struct platform_device 
> *pdev)
>       }
>  
>       lvds_encoder->panel_bridge =
> -             devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_LVDS);
> +             devm_drm_panel_bridge_add_typed(dev, panel,
> +                                             DRM_MODE_CONNECTOR_LVDS);
>       if (IS_ERR(lvds_encoder->panel_bridge))
>               return PTR_ERR(lvds_encoder->panel_bridge);
>  
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index b12ae3a4c5f1..9d20d4a297ee 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -133,8 +133,6 @@ static const struct drm_bridge_funcs 
> panel_bridge_bridge_funcs = {
>   * just calls the appropriate functions from &drm_panel.
>   *
>   * @panel: The drm_panel being wrapped.  Must be non-NULL.
> - * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
> - * created.
>   *
>   * For drivers converting from directly using drm_panel: The expected
>   * usage pattern is that during either encoder module probe or DSI
> @@ -148,11 +146,37 @@ static const struct drm_bridge_funcs 
> panel_bridge_bridge_funcs = {
>   * drm_mode_config_cleanup() if the bridge has already been attached), then
>   * drm_panel_bridge_remove() to free it.
>   *
> + * The connector type is set to @panel->connector_type, which must be set to 
> a
> + * known type. Calling this function with a panel whose connector type is
> + * DRM_MODE_CONNECTOR_Unknown will return NULL.
> + *
>   * See devm_drm_panel_bridge_add() for an automatically manged version of 
> this
>   * function.
>   */
> -struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> -                                     u32 connector_type)
> +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel)
> +{
> +     if (WARN_ON(panel->connector_type == DRM_MODE_CONNECTOR_Unknown))
> +             return NULL;
> +
> +     return drm_panel_bridge_add_typed(panel, panel->connector_type);
> +}
> +EXPORT_SYMBOL(drm_panel_bridge_add);
> +
> +/**
> + * drm_panel_bridge_add_typed - Creates a &drm_bridge and &drm_connector with
> + * an explicit connector type.
> + * @panel: The drm_panel being wrapped.  Must be non-NULL.
> + * @connector_type: The connector type (DRM_MODE_CONNECTOR_*)
> + *
> + * This is just like drm_panel_bridge_add(), but forces the connector type to
> + * @connector_type instead of infering it from the panel.
> + *
> + * This function is deprecated and should not be used in new drivers. Use
> + * drm_panel_bridge_add() instead, and fix panel drivers as necessary if they
> + * don't report a connector type.
> + */
> +struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
> +                                           u32 connector_type)
>  {
>       struct panel_bridge *panel_bridge;
>  
> @@ -176,7 +200,7 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel 
> *panel,
>  
>       return &panel_bridge->bridge;
>  }
> -EXPORT_SYMBOL(drm_panel_bridge_add);
> +EXPORT_SYMBOL(drm_panel_bridge_add_typed);
>  
>  /**
>   * drm_panel_bridge_remove - Unregisters and frees a drm_bridge
> @@ -213,15 +237,38 @@ static void devm_drm_panel_bridge_release(struct device 
> *dev, void *res)
>   * that just calls the appropriate functions from &drm_panel.
>   * @dev: device to tie the bridge lifetime to
>   * @panel: The drm_panel being wrapped.  Must be non-NULL.
> - * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
> - * created.
>   *
>   * This is the managed version of drm_panel_bridge_add() which automatically
>   * calls drm_panel_bridge_remove() when @dev is unbound.
>   */
>  struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> -                                          struct drm_panel *panel,
> -                                          u32 connector_type)
> +                                          struct drm_panel *panel)
> +{
> +     if (WARN_ON(panel->connector_type == DRM_MODE_CONNECTOR_Unknown))
> +             return NULL;
> +
> +     return devm_drm_panel_bridge_add_typed(dev, panel,
> +                                            panel->connector_type);
> +}
> +EXPORT_SYMBOL(devm_drm_panel_bridge_add);
> +
> +/**
> + * devm_drm_panel_bridge_add_typed - Creates a managed &drm_bridge and
> + * &drm_connector with an explicit connector type.
> + * @dev: device to tie the bridge lifetime to
> + * @panel: The drm_panel being wrapped.  Must be non-NULL.
> + * @connector_type: The connector type (DRM_MODE_CONNECTOR_*)
> + *
> + * This is just like devm_drm_panel_bridge_add(), but forces the connector 
> type
> + * to @connector_type instead of infering it from the panel.
> + *
> + * This function is deprecated and should not be used in new drivers. Use
> + * devm_drm_panel_bridge_add() instead, and fix panel drivers as necessary if
> + * they don't report a connector type.
> + */
> +struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
> +                                                struct drm_panel *panel,
> +                                                u32 connector_type)
>  {
>       struct drm_bridge **ptr, *bridge;
>  
> @@ -230,7 +277,7 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct 
> device *dev,
>       if (!ptr)
>               return ERR_PTR(-ENOMEM);
>  
> -     bridge = drm_panel_bridge_add(panel, connector_type);
> +     bridge = drm_panel_bridge_add_typed(panel, connector_type);
>       if (!IS_ERR(bridge)) {
>               *ptr = bridge;
>               devres_add(dev, ptr);
> @@ -240,4 +287,4 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct 
> device *dev,
>  
>       return bridge;
>  }
> -EXPORT_SYMBOL(devm_drm_panel_bridge_add);
> +EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 675442bfc1bd..b8b1c288f663 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -316,7 +316,8 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host 
> *host,
>               return ret;
>  
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_DSI);
>               if (IS_ERR(bridge))
>                       return PTR_ERR(bridge);
>       }
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index ce1fae3a78a9..73fd33d2595c 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -676,8 +676,8 @@ static int ingenic_drm_probe(struct platform_device *pdev)
>       }
>  
>       if (panel) {
> -             bridge = devm_drm_panel_bridge_add(dev, panel,
> -                                                DRM_MODE_CONNECTOR_Unknown);
> +             bridge = devm_drm_panel_bridge_add_typed(dev, panel,
> +                                                      
> DRM_MODE_CONNECTOR_Unknown);
>       }
>  
>       priv->dma_hwdesc = dma_alloc_coherent(dev, sizeof(*priv->dma_hwdesc),
> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index 07f7090d08b3..ffb55d3b662c 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c
> @@ -922,8 +922,8 @@ static int mcde_dsi_bind(struct device *dev, struct 
> device *master,
>               }
>       }
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel,
> -                                           DRM_MODE_CONNECTOR_DSI);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_DSI);
>               if (IS_ERR(bridge)) {
>                       dev_err(dev, "error adding panel bridge\n");
>                       return PTR_ERR(bridge);
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c 
> b/drivers/gpu/drm/pl111/pl111_drv.c
> index 276b53473a84..63dfcda04147 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -150,8 +150,8 @@ static int pl111_modeset_init(struct drm_device *dev)
>               return -EPROBE_DEFER;
>  
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel,
> -                                           DRM_MODE_CONNECTOR_Unknown);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_Unknown);
>               if (IS_ERR(bridge)) {
>                       ret = PTR_ERR(bridge);
>                       goto out_config;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index 0f00bdfe2366..f940d0c6aeb9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -84,8 +84,8 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>                       goto done;
>               }
>  
> -             bridge = devm_drm_panel_bridge_add(rcdu->dev, panel,
> -                                                DRM_MODE_CONNECTOR_DPI);
> +             bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
> +                                                      
> DRM_MODE_CONNECTOR_DPI);
>               if (IS_ERR(bridge)) {
>                       ret = PTR_ERR(bridge);
>                       goto done;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c 
> b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> index 89e0bb0fe0ab..a3ba0d5914e8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> @@ -135,7 +135,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
>       drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
>  
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_LVDS);
>               if (IS_ERR(bridge))
>                       return ERR_CAST(bridge);
>       }
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 3ab4fbf8eb0d..7cdab685b527 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -1236,8 +1236,8 @@ int ltdc_load(struct drm_device *ddev)
>       /* Add endpoints panels or bridges if any */
>       for (i = 0; i < MAX_ENDPOINTS; i++) {
>               if (panel[i]) {
> -                     bridge[i] = drm_panel_bridge_add(panel[i],
> -                                                     DRM_MODE_CONNECTOR_DPI);
> +                     bridge[i] = drm_panel_bridge_add_typed(panel[i],
> +                                                            
> DRM_MODE_CONNECTOR_DPI);
>                       if (IS_ERR(bridge[i])) {
>                               DRM_ERROR("panel-bridge endpoint %d\n", i);
>                               ret = PTR_ERR(bridge[i]);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index 43d756b7810e..39e9a97610db 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -139,8 +139,8 @@ int tilcdc_attach_external_device(struct drm_device *ddev)
>       }
>  
>       if (panel) {
> -             bridge = devm_drm_panel_bridge_add(ddev->dev, panel,
> -                                                DRM_MODE_CONNECTOR_DPI);
> +             bridge = devm_drm_panel_bridge_add_typed(ddev->dev, panel,
> +                                                      
> DRM_MODE_CONNECTOR_DPI);
>               if (IS_ERR(bridge)) {
>                       ret = PTR_ERR(bridge);
>                       goto err_encoder_cleanup;
> diff --git a/drivers/gpu/drm/tve200/tve200_drv.c 
> b/drivers/gpu/drm/tve200/tve200_drv.c
> index 416f24823c0a..954b09c948eb 100644
> --- a/drivers/gpu/drm/tve200/tve200_drv.c
> +++ b/drivers/gpu/drm/tve200/tve200_drv.c
> @@ -80,8 +80,8 @@ static int tve200_modeset_init(struct drm_device *dev)
>       if (ret && ret != -ENODEV)
>               return ret;
>       if (panel) {
> -             bridge = drm_panel_bridge_add(panel,
> -                                           DRM_MODE_CONNECTOR_Unknown);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_Unknown);
>               if (IS_ERR(bridge)) {
>                       ret = PTR_ERR(bridge);
>                       goto out_bridge;
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 8a27a6acee61..c586325de2a5 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -249,7 +249,8 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
>       }
>  
>       if (panel)
> -             bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI);
> +             bridge = drm_panel_bridge_add_typed(panel,
> +                                                 DRM_MODE_CONNECTOR_DPI);
>  
>       return drm_bridge_attach(dpi->encoder, bridge, NULL);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index c78fa8144776..bf4d2a1a22dc 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -1575,8 +1575,8 @@ static int vc4_dsi_bind(struct device *dev, struct 
> device *master, void *data)
>       }
>  
>       if (panel) {
> -             dsi->bridge = devm_drm_panel_bridge_add(dev, panel,
> -                                                     DRM_MODE_CONNECTOR_DSI);
> +             dsi->bridge = devm_drm_panel_bridge_add_typed(dev, panel,
> +                                                           
> DRM_MODE_CONNECTOR_DSI);
>               if (IS_ERR(dsi->bridge))
>                       return PTR_ERR(dsi->bridge);
>       }
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 7616f6562fe4..1a4b8ecf5bdb 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -429,12 +429,15 @@ void drm_atomic_bridge_enable(struct drm_bridge *bridge,
>                             struct drm_atomic_state *state);
>  
>  #ifdef CONFIG_DRM_PANEL_BRIDGE
> -struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> -                                     u32 connector_type);
> +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
> +struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
> +                                           u32 connector_type);
>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
>  struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> -                                          struct drm_panel *panel,
> -                                          u32 connector_type);
> +                                          struct drm_panel *panel);
> +struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
> +                                                struct drm_panel *panel,
> +                                                u32 connector_type);
>  #endif
>  
>  #endif

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

Reply via email to