Re: [PATCH v2 33/60] drm/omap: dss: Move DSS mgr ops and private data to dss_device

From: Sebastian Reichel <sre@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 33/60] drm/omap: dss: Move DSS mgr ops and private data to dss_device
Date: Sun, 10 Jun 2018 22:43:38 +0200	[thread overview]
Message-ID: <20180610204338.xy3yavsv77covvcb@earth.universe> (raw)
In-Reply-To: <20180526172518.18710-34-laurent.pinchart@ideasonboard.com>


[-- Attachment #1.1: Type: text/plain, Size: 9022 bytes --]

Hi,

On Sat, May 26, 2018 at 08:24:51PM +0300, Laurent Pinchart wrote:
> The DSS manager ops and private data pointer are specific to a DSS
> instance. Store them in the dss_device structure instead of global
> variable.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/dss.h     |  2 ++
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |  5 +--
>  drivers/gpu/drm/omapdrm/dss/output.c  | 58 ++++++++++++++++++++---------------
>  drivers/gpu/drm/omapdrm/omap_crtc.c   |  6 ++--
>  drivers/gpu/drm/omapdrm/omap_crtc.h   |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c    |  4 +--
>  6 files changed, 44 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
> index a4514843e925..54f96241b9ea 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -266,6 +266,8 @@ struct dss_device {
>  
>  	struct dispc_device *dispc;
>  	const struct dispc_ops *dispc_ops;
> +	const struct dss_mgr_ops *mgr_ops;
> +	struct omap_drm_private *mgr_ops_priv;
>  };
>  
>  /* core */
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 4befe8aab333..4df405ae20db 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -574,9 +574,10 @@ struct dss_mgr_ops {
>  			void (*handler)(void *), void *data);
>  };
>  
> -int dss_install_mgr_ops(const struct dss_mgr_ops *mgr_ops,
> +int dss_install_mgr_ops(struct dss_device *dss,
> +			const struct dss_mgr_ops *mgr_ops,
>  			struct omap_drm_private *priv);
> -void dss_uninstall_mgr_ops(void);
> +void dss_uninstall_mgr_ops(struct dss_device *dss);
>  
>  int dss_mgr_connect(struct omap_dss_device *dssdev,
>  		    struct omap_dss_device *dst);
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
> index b5bf7a5e35d9..a5df6eed4aef 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  
> +#include "dss.h"
>  #include "omapdss.h"
>  
>  static DEFINE_MUTEX(output_lock);
> @@ -99,90 +100,97 @@ struct omap_dss_device *omapdss_find_output_from_display(struct omap_dss_device
>  }
>  EXPORT_SYMBOL(omapdss_find_output_from_display);
>  
> -static const struct dss_mgr_ops *dss_mgr_ops;
> -static struct omap_drm_private *dss_mgr_ops_priv;
> -
> -int dss_install_mgr_ops(const struct dss_mgr_ops *mgr_ops,
> +int dss_install_mgr_ops(struct dss_device *dss,
> +			const struct dss_mgr_ops *mgr_ops,
>  			struct omap_drm_private *priv)
>  {
> -	if (dss_mgr_ops)
> +	if (dss->mgr_ops)
>  		return -EBUSY;
>  
> -	dss_mgr_ops = mgr_ops;
> -	dss_mgr_ops_priv = priv;
> +	dss->mgr_ops = mgr_ops;
> +	dss->mgr_ops_priv = priv;
>  
>  	return 0;
>  }
>  EXPORT_SYMBOL(dss_install_mgr_ops);
>  
> -void dss_uninstall_mgr_ops(void)
> +void dss_uninstall_mgr_ops(struct dss_device *dss)
>  {
> -	dss_mgr_ops = NULL;
> -	dss_mgr_ops_priv = NULL;
> +	dss->mgr_ops = NULL;
> +	dss->mgr_ops_priv = NULL;
>  }
>  EXPORT_SYMBOL(dss_uninstall_mgr_ops);
>  
>  int dss_mgr_connect(struct omap_dss_device *dssdev, struct omap_dss_device *dst)
>  {
> -	return dss_mgr_ops->connect(dss_mgr_ops_priv,
> -				    dssdev->dispc_channel, dst);
> +	return dssdev->dss->mgr_ops->connect(dssdev->dss->mgr_ops_priv,
> +					     dssdev->dispc_channel, dst);
>  }
>  EXPORT_SYMBOL(dss_mgr_connect);
>  
>  void dss_mgr_disconnect(struct omap_dss_device *dssdev,
>  			struct omap_dss_device *dst)
>  {
> -	dss_mgr_ops->disconnect(dss_mgr_ops_priv, dssdev->dispc_channel, dst);
> +	dssdev->dss->mgr_ops->disconnect(dssdev->dss->mgr_ops_priv,
> +					 dssdev->dispc_channel, dst);
>  }
>  EXPORT_SYMBOL(dss_mgr_disconnect);
>  
>  void dss_mgr_set_timings(struct omap_dss_device *dssdev,
>  			 const struct videomode *vm)
>  {
> -	dss_mgr_ops->set_timings(dss_mgr_ops_priv, dssdev->dispc_channel, vm);
> +	dssdev->dss->mgr_ops->set_timings(dssdev->dss->mgr_ops_priv,
> +					  dssdev->dispc_channel, vm);
>  }
>  EXPORT_SYMBOL(dss_mgr_set_timings);
>  
>  void dss_mgr_set_lcd_config(struct omap_dss_device *dssdev,
>  		const struct dss_lcd_mgr_config *config)
>  {
> -	dss_mgr_ops->set_lcd_config(dss_mgr_ops_priv,
> -				    dssdev->dispc_channel, config);
> +	dssdev->dss->mgr_ops->set_lcd_config(dssdev->dss->mgr_ops_priv,
> +					     dssdev->dispc_channel, config);
>  }
>  EXPORT_SYMBOL(dss_mgr_set_lcd_config);
>  
>  int dss_mgr_enable(struct omap_dss_device *dssdev)
>  {
> -	return dss_mgr_ops->enable(dss_mgr_ops_priv, dssdev->dispc_channel);
> +	return dssdev->dss->mgr_ops->enable(dssdev->dss->mgr_ops_priv,
> +					    dssdev->dispc_channel);
>  }
>  EXPORT_SYMBOL(dss_mgr_enable);
>  
>  void dss_mgr_disable(struct omap_dss_device *dssdev)
>  {
> -	dss_mgr_ops->disable(dss_mgr_ops_priv, dssdev->dispc_channel);
> +	dssdev->dss->mgr_ops->disable(dssdev->dss->mgr_ops_priv,
> +				      dssdev->dispc_channel);
>  }
>  EXPORT_SYMBOL(dss_mgr_disable);
>  
>  void dss_mgr_start_update(struct omap_dss_device *dssdev)
>  {
> -	dss_mgr_ops->start_update(dss_mgr_ops_priv, dssdev->dispc_channel);
> +	dssdev->dss->mgr_ops->start_update(dssdev->dss->mgr_ops_priv,
> +					   dssdev->dispc_channel);
>  }
>  EXPORT_SYMBOL(dss_mgr_start_update);
>  
>  int dss_mgr_register_framedone_handler(struct omap_dss_device *dssdev,
>  		void (*handler)(void *), void *data)
>  {
> -	return dss_mgr_ops->register_framedone_handler(dss_mgr_ops_priv,
> -						       dssdev->dispc_channel,
> -						       handler, data);
> +	struct dss_device *dss = dssdev->dss;
> +
> +	return dss->mgr_ops->register_framedone_handler(dss->mgr_ops_priv,
> +							dssdev->dispc_channel,
> +							handler, data);
>  }
>  EXPORT_SYMBOL(dss_mgr_register_framedone_handler);
>  
>  void dss_mgr_unregister_framedone_handler(struct omap_dss_device *dssdev,
>  		void (*handler)(void *), void *data)
>  {
> -	dss_mgr_ops->unregister_framedone_handler(dss_mgr_ops_priv,
> -						  dssdev->dispc_channel,
> -						  handler, data);
> +	struct dss_device *dss = dssdev->dss;
> +
> +	dss->mgr_ops->unregister_framedone_handler(dss->mgr_ops_priv,
> +						   dssdev->dispc_channel,
> +						   handler, data);
>  }
>  EXPORT_SYMBOL(dss_mgr_unregister_framedone_handler);
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index e18ca6cdc0d6..4ddc4ed18b47 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -683,12 +683,12 @@ void omap_crtc_pre_init(struct omap_drm_private *priv)
>  {
>  	memset(omap_crtcs, 0, sizeof(omap_crtcs));
>  
> -	dss_install_mgr_ops(&mgr_ops, priv);
> +	dss_install_mgr_ops(priv->dss, &mgr_ops, priv);
>  }
>  
> -void omap_crtc_pre_uninit(void)
> +void omap_crtc_pre_uninit(struct omap_drm_private *priv)
>  {
> -	dss_uninstall_mgr_ops();
> +	dss_uninstall_mgr_ops(priv->dss);
>  }
>  
>  /* initialize crtc */
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
> index eaab2d7f0324..1c6530703855 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.h
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
> @@ -33,7 +33,7 @@ struct videomode;
>  struct videomode *omap_crtc_timings(struct drm_crtc *crtc);
>  enum omap_channel omap_crtc_channel(struct drm_crtc *crtc);
>  void omap_crtc_pre_init(struct omap_drm_private *priv);
> -void omap_crtc_pre_uninit(void);
> +void omap_crtc_pre_uninit(struct omap_drm_private *priv);
>  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  		struct drm_plane *plane, struct omap_dss_device *dssdev);
>  int omap_crtc_wait_pending(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 042a3ca526ca..9b33859e9ac6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -638,7 +638,7 @@ static int omapdrm_init(struct omap_drm_private *priv, struct device *dev)
>  	destroy_workqueue(priv->wq);
>  	omap_disconnect_dssdevs(ddev);
>  err_crtc_uninit:
> -	omap_crtc_pre_uninit();
> +	omap_crtc_pre_uninit(priv);
>  	drm_dev_unref(ddev);
>  	return ret;
>  }
> @@ -666,7 +666,7 @@ static void omapdrm_cleanup(struct omap_drm_private *priv)
>  	destroy_workqueue(priv->wq);
>  
>  	omap_disconnect_dssdevs(ddev);
> -	omap_crtc_pre_uninit();
> +	omap_crtc_pre_uninit(priv);
>  
>  	drm_dev_unref(ddev);
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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