Skip to contentSkip to conversation

drm/omapdrm: Remove CONFIG_DRM_OMAP_NUM_CRTCS and cleanup

PATCH Superseded Jyri Sarha 2 participants 6 messages Mar 14, 2017 – Mar 15, 2017

Subsystems (3)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
author
#4 lore raw

PATCH 3/3 drm/omapdrm: Separate ids for planes and CRTCs in omap_modeset_init()

In reply to Jyri Sarha

Add separate local id variables for indexing planes and CRTCs in omap_modeset_init(). This is to make it more explicit what each local variable is used for.

---
 drivers/gpu/drm/omapdrm/omap_drv.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.c +15 -12
index 3d30f29..9b67906 100644
@@ -275,7 +275,8 @@ static int omap_connect_dssdevs(void)
 	return r;
 }
 
-static int omap_modeset_create_crtc(struct drm_device *dev, int id,
+static int omap_modeset_create_crtc(struct drm_device *dev,
+				    int crtc_id, int plane_id,
 				    enum omap_channel channel,
 				    u32 possible_crtcs)
 {
@@ -283,18 +284,18 @@ static int omap_modeset_create_crtc(struct drm_device *dev, int id,
 	struct drm_plane *plane;
 	struct drm_crtc *crtc;
 
-	plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_PRIMARY,
+	plane = omap_plane_init(dev, plane_id, DRM_PLANE_TYPE_PRIMARY,
 		possible_crtcs);
 	if (IS_ERR(plane))
 		return PTR_ERR(plane);
 
-	crtc = omap_crtc_init(dev, plane, channel, id);
+	crtc = omap_crtc_init(dev, plane, channel, crtc_id);
 
 	BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
-	priv->crtcs[id] = crtc;
+	priv->crtcs[crtc_id] = crtc;
 	priv->num_crtcs++;
 
-	priv->planes[id] = plane;
+	priv->planes[plane_id] = plane;
 	priv->num_planes++;
 
 	return 0;
@@ -318,7 +319,7 @@ static int omap_modeset_init(struct drm_device *dev)
 	int num_ovls = dss_feat_get_num_ovls();
 	int num_mgrs = dss_feat_get_num_mgrs();
 	int num_crtcs = 0;
-	int i, id = 0;
+	int i, crtc_id = 0, plane_id = 0;
 	int ret;
 	u32 possible_crtcs_for_planes;
 
@@ -382,7 +383,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		 * create, let's not try to create a crtc for this
 		 * panel/encoder and onwards.
 		 */
-		if (id == num_crtcs)
+		if (crtc_id == num_crtcs)
 			continue;
 
 		/*
@@ -401,8 +402,8 @@ static int omap_modeset_init(struct drm_device *dev)
 		 * allocated crtc, we create a new crtc for it
 		 */
 		if (!channel_used(dev, channel)) {
-			ret = omap_modeset_create_crtc(dev, id, channel,
-				possible_crtcs_for_planes);
+			ret = omap_modeset_create_crtc(dev, crtc_id, plane_id,
+				channel, possible_crtcs_for_planes);
 			if (ret < 0) {
 				dev_err(dev->dev,
 					"could not create CRTC (channel %u)\n",
@@ -410,17 +411,18 @@ static int omap_modeset_init(struct drm_device *dev)
 				return ret;
 			}
 
-			id++;
+			crtc_id++;
+			plane_id++;
 		}
 	}
 
 	/*
 	 * Create normal planes for the remaining overlays:
 	 */
-	for (; id < num_ovls; id++) {
+	for (; plane_id < num_ovls; plane_id++) {
 		struct drm_plane *plane;
 
-		plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_OVERLAY,
+		plane = omap_plane_init(dev, plane_id, DRM_PLANE_TYPE_OVERLAY,
 			possible_crtcs_for_planes);
 		if (IS_ERR(plane))
 			return PTR_ERR(plane);
@@ -438,6 +440,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		struct omap_dss_device *dssdev =
 					omap_encoder_get_dssdev(encoder);
 		struct omap_dss_device *output;
+		int id;
 
 		output = omapdss_find_output_from_display(dssdev);
 
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author
#0 lore raw

cover drm/omapdrm: Remove CONFIG_DRM_OMAP_NUM_CRTCS and cleanup

The first patch removes CONFIG_DRM_OMAP_NUM_CRTCS config option. The rest is just adding comments and making the code more readable.

Jyri Sarha (3):
drm/omapdrm: Get rid of DRM_OMAP_NUM_CRTCS config option drm/omapdrm: Change possible_crtcs to possible_crtcs_for_planes drm/omapdrm: Separate ids for planes and CRTCs in omap_modeset_init()

 drivers/gpu/drm/omapdrm/Kconfig    |  9 ----
 drivers/gpu/drm/omapdrm/omap_drv.c | 93 ++++++++++++++------------------------
 2 files changed, 35 insertions(+), 67 deletions(-)
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#1 lore raw

PATCH 1/3 drm/omapdrm: Get rid of DRM_OMAP_NUM_CRTCS config option

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.

---
 drivers/gpu/drm/omapdrm/Kconfig    |  9 ------
 drivers/gpu/drm/omapdrm/omap_drv.c | 59 +++++++++++---------------------------
 2 files changed, 16 insertions(+), 52 deletions(-)
drivers/gpu/drm/omapdrm/Kconfig +0 -9
index 556f81f..b3d08c5 100644
@@ -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"
 
drivers/gpu/drm/omapdrm/omap_drv.c +16 -43

96 lines not on this page · raw message

Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#2 lore raw

PATCH 2/3 drm/omapdrm: Change possible_crtcs to possible_crtcs_for_planes

In reply to Jyri Sarha

Rename possible_crtcs local variable to possible_crtcs_for_planes in omap_modeset_init() and add a comment about its initialization. This is to make it more explicit what the variable is used for.

---
 drivers/gpu/drm/omapdrm/omap_drv.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.c +5 -4
index 17c40aa..3d30f29 100644
@@ -320,7 +320,7 @@ static int omap_modeset_init(struct drm_device *dev)
 	int num_crtcs = 0;
 	int i, id = 0;
 	int ret;
-	u32 possible_crtcs;
+	u32 possible_crtcs_for_planes;
 
 	drm_mode_config_init(dev);
 
@@ -338,7 +338,8 @@ static int omap_modeset_init(struct drm_device *dev)
 			num_crtcs++;
 
 	num_crtcs = min3(num_crtcs, num_mgrs, num_ovls);
-	possible_crtcs = (1 << num_crtcs) - 1;
+	/* All planes can be put to any CRTC */
+	possible_crtcs_for_planes = (1 << num_crtcs) - 1;
 
 	dssdev = NULL;
 
@@ -401,7 +402,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		 */
 		if (!channel_used(dev, channel)) {
 			ret = omap_modeset_create_crtc(dev, id, channel,
-				possible_crtcs);
+				possible_crtcs_for_planes);
 			if (ret < 0) {
 				dev_err(dev->dev,
 					"could not create CRTC (channel %u)\n",
@@ -420,7 +421,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		struct drm_plane *plane;
 
 		plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_OVERLAY,
-			possible_crtcs);
+			possible_crtcs_for_planes);
 		if (IS_ERR(plane))
 			return PTR_ERR(plane);
 
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#5 lore raw

Re: [PATCH 3/3] drm/omapdrm: Separate ids for planes and CRTCs in omap_modeset_init()

In reply to Jyri Sarha

Hi,

Quoted 3 lines · Jyri Sarha

Add separate local id variables for indexing planes and CRTCs in omap_modeset_init(). This is to make it more explicit what each local variable is used for.

"id" should be used for DRM object ids. I think here plane_id and crtc_id are really idx.

And the code still mixes up at least plane index and DSS plane (enum omap_plane from omapdss.h). For crtcs I think we already separate the DRM crtc idx and the DSS "enum omap_channel".

But I belive omap_drv.c shouldn't even care about those omap_channels, it should all be in omap_crtc.c.

These patches are in the correct direction, but see if you can take it further and get the code to be something like this:

For each display, we create a crtc.

Each crtc will figure out the omap_channel to use, based on the display (out->dispc_channel), in omap_crtc.c.

Each crtc will get a primary plane. omap_plane.c will figure out which "enum omap_plane" to to use for a plane. I think it can be a direct mapping from plane idx to enum omap_plane, but that has to be explicit in the code.

Each crtc will also get an encoder and a connector. We can configure an encoder and a connector to be usable only with a single crtc.

The rest of the planes will be created as overlay planes.

Tomi

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#3 lore raw

Re: [PATCH 2/3] drm/omapdrm: Change possible_crtcs to possible_crtcs_for_planes

In reply to Jyri Sarha
Quoted 14 lines · Jyri Sarha

Rename possible_crtcs local variable to possible_crtcs_for_planes in omap_modeset_init() and add a comment about its initialization. This is to make it more explicit what the variable is used for.

---
 drivers/gpu/drm/omapdrm/omap_drv.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.c +0 -0
index 17c40aa..3d30f29 100644

drivers/gpu/drm/omapdrm/omap_drv.c static int omap_modeset_init(struct drm_device *dev) · line 323

Quoted 5 lines
drivers/gpu/drm/omapdrm/omap_drv.c +1 -1
@@ -320,7 +320,7 @@ static int omap_modeset_init(struct drm_device *dev)
 	int num_crtcs = 0;
 	int i, id = 0;
 	int ret;
-	u32 possible_crtcs;
+	u32 possible_crtcs_for_planes;

Maybe "crtc_mask"?

Tomi

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel