Re: [PATCH v4 1/6] drm/omap: Get rid of DRM_OMAP_NUM_CRTCS config option

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jyri,

Thank you for the patch.

On Friday 24 Mar 2017 16:47:51 Jyri Sarha wrote:
> Allocate one CRTC for each connected output and get rid of
> DRM_OMAP_NUM_CRTCS config option. We still can not create more CRTCs
> than we have DSS display managers. We also reserve one overlay per
> CRTC for primary plane so we can not have more CRTCs than we have
> overlays either.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/Kconfig    |  9 ------
>  drivers/gpu/drm/omapdrm/omap_drv.c | 59 ++++++++---------------------------
>  2 files changed, 16 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/Kconfig
> b/drivers/gpu/drm/omapdrm/Kconfig index 556f81f..b3d08c5 100644
> --- a/drivers/gpu/drm/omapdrm/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/Kconfig
> @@ -10,15 +10,6 @@ config DRM_OMAP
> 
>  if DRM_OMAP
> 
> -config DRM_OMAP_NUM_CRTCS
> -	int "Number of CRTCs"
> -	range 1 10
> -	default 1  if ARCH_OMAP2 || ARCH_OMAP3
> -	default 2  if ARCH_OMAP4
> -	help
> -	  Select the number of video overlays which can be used as 
framebuffers.
> -	  The remaining overlays are reserved for video.
> -
>  source "drivers/gpu/drm/omapdrm/dss/Kconfig"
>  source "drivers/gpu/drm/omapdrm/displays/Kconfig"
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index ad8d16c..b040f5f 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -34,11 +34,6 @@
>  #define DRIVER_MINOR		0
>  #define DRIVER_PATCHLEVEL	0
> 
> -static int num_crtc = CONFIG_DRM_OMAP_NUM_CRTCS;
> -
> -MODULE_PARM_DESC(num_crtc, "Number of overlays to use as CRTCs");
> -module_param(num_crtc, int, 0600);
> -
>  /*
>   * mode config funcs
>   */
> @@ -319,7 +314,7 @@ static int omap_modeset_init(struct drm_device *dev)
>  	struct omap_dss_device *dssdev = NULL;
>  	int num_ovls = priv->dispc_ops->get_num_ovls();
>  	int num_mgrs = priv->dispc_ops->get_num_mgrs();
> -	int num_crtcs;
> +	int num_crtcs = 0;
>  	int i, id = 0;
>  	int ret;
>  	u32 possible_crtcs;
> @@ -331,12 +326,15 @@ static int omap_modeset_init(struct drm_device *dev)
>  		return ret;
> 
>  	/*
> -	 * We usually don't want to create a CRTC for each manager, at least
> -	 * not until we have a way to expose private planes to userspace.
> -	 * Otherwise there would not be enough video pipes left for drm 
planes.
> -	 * We use the num_crtc argument to limit the number of crtcs we 
create.
> +	 * Let's create one CRTC for each connected DSS device if we
> +	 * have display managers and overlays (for primary planes) for
> +	 * them.
>  	 */
> -	num_crtcs = min3(num_crtc, num_mgrs, num_ovls);
> +	for_each_dss_dev(dssdev)
> +		if (omapdss_device_is_connected(dssdev))
> +			num_crtcs++;
> +
> +	num_crtcs = min3(num_crtcs, num_mgrs, num_ovls);
>  	possible_crtcs = (1 << num_crtcs) - 1;
> 
>  	dssdev = NULL;
> @@ -376,11 +374,9 @@ static int omap_modeset_init(struct drm_device *dev)
>  		drm_mode_connector_attach_encoder(connector, encoder);
> 
>  		/*
> -		 * if we have reached the limit of the crtcs we are allowed to
> -		 * create, let's not try to look for a crtc for this
> -		 * panel/encoder and onwards, we will, of course, populate the
> -		 * the possible_crtcs field for all the encoders with the 
final
> -		 * set of crtcs we create
> +		 * if we have reached the limit of the crtcs we can
> +		 * create, let's not try to create a crtc for this
> +		 * panel/encoder and onwards.
>  		 */
>  		if (id == num_crtcs)
>  			continue;
> @@ -415,33 +411,6 @@ static int omap_modeset_init(struct drm_device *dev)
>  	}
> 
>  	/*
> -	 * we have allocated crtcs according to the need of the 
panels/encoders,
> -	 * adding more crtcs here if needed
> -	 */
> -	for (; id < num_crtcs; id++) {
> -
> -		/* find a free manager for this crtc */
> -		for (i = 0; i < num_mgrs; i++) {
> -			if (!channel_used(dev, i))
> -				break;
> -		}
> -
> -		if (i == num_mgrs) {
> -			/* this shouldn't really happen */
> -			dev_err(dev->dev, "no managers left for crtc\n");
> -			return -ENOMEM;
> -		}
> -
> -		ret = omap_modeset_create_crtc(dev, id, i,
> -			possible_crtcs);
> -		if (ret < 0) {
> -			dev_err(dev->dev,
> -				"could not create CRTC (channel %u)\n", i);
> -			return ret;
> -		}
> -	}
> -
> -	/*
>  	 * Create normal planes for the remaining overlays:
>  	 */
>  	for (; id < num_ovls; id++) {
> @@ -456,6 +425,10 @@ static int omap_modeset_init(struct drm_device *dev)
>  		priv->planes[priv->num_planes++] = plane;
>  	}
> 
> +	/*
> +	 * populate the the possible_crtcs field for all the encoders
> +	 * we created.
> +	 */
>  	for (i = 0; i < priv->num_encoders; i++) {
>  		struct drm_encoder *encoder = priv->encoders[i];
>  		struct omap_dss_device *dssdev =

-- 
Regards,

Laurent Pinchart

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