Subject: Re: [PATCH v1 04/26] drm: get drm_bridge_panel connector via helper

From: Laurent Pinchart <[email-masked]>
Date: Tue, 3 Dec 2019 08:44:14 +0200

Hi Sam,

Thank you for the patch.

On Mon, Dec 02, 2019 at 08:32:08PM +0100, Sam Ravnborg wrote:
> The drm_connector created by drm_panel_bridge was accessed
> via drm_panel.connector.
> Avoid the detour around drm_panel by providing a simple get method.
> This avoids direct access to the connector field in drm_panel in
> the two users.

You may want to add that this is needed to prepare for removal of
drm_panel.connector.

> Update pl111 and tve200 to use the new helper.
> 
> Signed-off-by: Sam Ravnborg <[email-masked]>
> Cc: Andrzej Hajda <[email-masked]>
> Cc: Neil Armstrong <[email-masked]>
> Cc: Laurent Pinchart <[email-masked]>
> Cc: Jonas Karlman <[email-masked]>
> Cc: Jernej Skrabec <[email-masked]>
> Cc: Eric Anholt <[email-masked]>
> Cc: Linus Walleij <[email-masked]>
> Cc: Maarten Lankhorst <[email-masked]>
> Cc: Maxime Ripard <[email-masked]>
> Cc: David Airlie <[email-masked]>
> Cc: Daniel Vetter <[email-masked]>
> ---
>  drivers/gpu/drm/bridge/panel.c      | 16 ++++++++++++++++
>  drivers/gpu/drm/pl111/pl111_drv.c   |  2 +-
>  drivers/gpu/drm/tve200/tve200_drv.c |  2 +-
>  include/drm/drm_bridge.h            |  1 +
>  4 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index f4e293e7cf64..c5e27b81588a 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -289,3 +289,19 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
>  	return bridge;
>  }
>  EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
> +
> +/**
> + * drm_panel_bridge_connector - return pointer to connector

Maybe "return the connector for the panel bridge" ?

> + *
> + * drm_panel_bridge creates the connector.
> + * This function gives external access to the connector.
> + *
> + * Returns: Pointer to drm_conneector

s/drm_conneector/drm_connector/

> + */
> +struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge)
> +{
> +	struct panel_bridge * panel_bridge;

s/\* panel_bridge/*panel_bridge/

There's also a missing blank line, didn't checkpatch warn about this ?

With those small issues fixed,

Reviewed-by: Laurent Pinchart <[email-masked]>

> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> +
> +	return &panel_bridge->connector;
> +}
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index 63dfcda04147..aa8aa8d9e405 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -166,7 +166,7 @@ static int pl111_modeset_init(struct drm_device *dev)
>  	priv->bridge = bridge;
>  	if (panel) {
>  		priv->panel = panel;
> -		priv->connector = panel->connector;
> +		priv->connector = drm_panel_bridge_connector(bridge);
>  	}
>  
>  	ret = pl111_display_init(dev);
> diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c
> index 954b09c948eb..00ba9e5ce130 100644
> --- a/drivers/gpu/drm/tve200/tve200_drv.c
> +++ b/drivers/gpu/drm/tve200/tve200_drv.c
> @@ -110,7 +110,7 @@ static int tve200_modeset_init(struct drm_device *dev)
>  	}
>  
>  	priv->panel = panel;
> -	priv->connector = panel->connector;
> +	priv->connector = drm_panel_bridge_connector(bridge);
>  	priv->bridge = bridge;
>  
>  	dev_info(dev->dev, "attached to panel %s\n",
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index c0a2286a81e9..9a4902accfe5 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -438,6 +438,7 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
>  struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
>  						   struct drm_panel *panel,
>  						   u32 connector_type);
> +struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge);
>  #endif
>  
>  #endif

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel