[PATCH 18/29] drm/omap: Don't store display pointer in omap_connector structure

[Sebastian Reichel]

Hi,

On Wed, Dec 05, 2018 at 05:00:11PM +0200, Laurent Pinchart wrote:
> Display pipelines based on drm_bridge are handled from the bridge
> closest to the CRTC. To move to that model we thus need to transition
> away from walking pipelines in the other direction, and from accessing
> the device at the end of the pipeline when possible.
> 
> Remove most accesses to the display device from the omap_connector
> implementation, and don't store it in the omap_connector structure.
> 
> - For debug messages we can simply use the connector name instead.
> - For type checks we can use the drm_connector type.
> - For operation lookup we can start at the other end of the pipeline and
>   locate the last matching device.
> 
> The display device is still passed to the connector init function in
> order to find its type, which requires access to the end of the
> pipeline.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel at collabora.com>

-- Sebastian

>  drivers/gpu/drm/omapdrm/omap_connector.c | 31 ++++++++++--------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index 0bbbf4d0502d..31b6d6d1def3 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -30,7 +30,6 @@
>  struct omap_connector {
>  	struct drm_connector base;
>  	struct omap_dss_device *output;
> -	struct omap_dss_device *display;
>  	struct omap_dss_device *hpd;
>  	bool hdmi_mode;
>  };
> @@ -103,20 +102,20 @@ omap_connector_find_device(struct drm_connector *connector,
>  			   enum omap_dss_device_ops_flag op)
>  {
>  	struct omap_connector *omap_connector = to_omap_connector(connector);
> -	struct omap_dss_device *dssdev;
> +	struct omap_dss_device *dssdev = NULL;
> +	struct omap_dss_device *d;
>  
> -	for (dssdev = omap_connector->display; dssdev; dssdev = dssdev->src) {
> -		if (dssdev->ops_flags & op)
> -			return dssdev;
> +	for (d = omap_connector->output; d; d = d->next) {
> +		if (d->ops_flags & op)
> +			dssdev = d;
>  	}
>  
> -	return NULL;
> +	return dssdev;
>  }
>  
>  static enum drm_connector_status omap_connector_detect(
>  		struct drm_connector *connector, bool force)
>  {
> -	struct omap_connector *omap_connector = to_omap_connector(connector);
>  	struct omap_dss_device *dssdev;
>  	enum drm_connector_status status;
>  
> @@ -130,11 +129,10 @@ static enum drm_connector_status omap_connector_detect(
>  
>  		omap_connector_hpd_notify(connector, dssdev->src, status);
>  	} else {
> -		switch (omap_connector->display->type) {
> -		case OMAP_DISPLAY_TYPE_DPI:
> -		case OMAP_DISPLAY_TYPE_DBI:
> -		case OMAP_DISPLAY_TYPE_SDI:
> -		case OMAP_DISPLAY_TYPE_DSI:
> +		switch (connector->connector_type) {
> +		case DRM_MODE_CONNECTOR_DPI:
> +		case DRM_MODE_CONNECTOR_LVDS:
> +		case DRM_MODE_CONNECTOR_DSI:
>  			status = connector_status_connected;
>  			break;
>  		default:
> @@ -143,7 +141,7 @@ static enum drm_connector_status omap_connector_detect(
>  		}
>  	}
>  
> -	VERB("%s: %d (force=%d)", omap_connector->display->name, status, force);
> +	VERB("%s: %d (force=%d)", connector->name, status, force);
>  
>  	return status;
>  }
> @@ -152,7 +150,7 @@ static void omap_connector_destroy(struct drm_connector *connector)
>  {
>  	struct omap_connector *omap_connector = to_omap_connector(connector);
>  
> -	DBG("%s", omap_connector->display->name);
> +	DBG("%s", connector->name);
>  
>  	if (omap_connector->hpd) {
>  		struct omap_dss_device *hpd = omap_connector->hpd;
> @@ -166,7 +164,6 @@ static void omap_connector_destroy(struct drm_connector *connector)
>  	drm_connector_cleanup(connector);
>  
>  	omapdss_device_put(omap_connector->output);
> -	omapdss_device_put(omap_connector->display);
>  
>  	kfree(omap_connector);
>  }
> @@ -210,10 +207,9 @@ static int omap_connector_get_modes_edid(struct drm_connector *connector,
>  
>  static int omap_connector_get_modes(struct drm_connector *connector)
>  {
> -	struct omap_connector *omap_connector = to_omap_connector(connector);
>  	struct omap_dss_device *dssdev;
>  
> -	DBG("%s", omap_connector->display->name);
> +	DBG("%s", connector->name);
>  
>  	/*
>  	 * If display exposes EDID, then we parse that in the normal way to
> @@ -346,7 +342,6 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
>  		goto fail;
>  
>  	omap_connector->output = omapdss_device_get(output);
> -	omap_connector->display = omapdss_device_get(display);
>  
>  	connector = &omap_connector->base;
>  	connector->interlace_allowed = 1;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20181209/9205954f/attachment-0001.sig>