[1/4] drm/omapdrm: omap_modeset_init: Separate crtc id and plane id indexing

Commit Message

Jyri Sarha May 20, 2016, 6:35 a.m. UTC

Separate crtc id and plane id indexing in omap_modeset_init(). The
coupling of crtc- and plane-id is hard to follow.

Signed-off-by: Jyri Sarha <[e-mail]>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

Comments

Tomi Valkeinen May 20, 2016, 12:14 p.m. UTC | #1

On 20/05/16 09:35, Jyri Sarha wrote:
> Separate crtc id and plane id indexing in omap_modeset_init(). The
> coupling of crtc- and plane-id is hard to follow.

The id stuff does need cleanup, but I don't think this patch does that
well enough. A few quick ones:

omap_crtc_init() doesn't use the id parameter, so it could just be dropped.

"crtc_id" and "plane_id" are misleading. Usually those mean the id
numbere of the DRM object. Here plane_id is really the 'enum omap_plane'
from omapdss, if I'm not mistaken.

Probably the first step with cleaning this up would be to rename that
enum so that it can be used in omapdrm.

 Tomi

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index fe79498..8d0fe45 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -264,24 +264,24 @@  cleanup:
 	return r;
 }
 
-static int omap_modeset_create_crtc(struct drm_device *dev, int id,
-				    enum omap_channel channel)
+static int omap_modeset_create_crtc(struct drm_device *dev, int crtc_id,
+				    int plane_id, enum omap_channel channel)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	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);
 	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;
@@ -315,7 +315,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;
-	int i, id = 0;
+	int i, crtc_id = 0, plane_id = 0;
 	int ret;
 
 	drm_mode_config_init(dev);
@@ -377,7 +377,7 @@  static int omap_modeset_init(struct drm_device *dev)
 		 * the possible_crtcs field for all the encoders with the final
 		 * set of crtcs we create
 		 */
-		if (id == num_crtcs)
+		if (crtc_id == num_crtcs)
 			continue;
 
 		/*
@@ -396,7 +396,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);
+			ret = omap_modeset_create_crtc(dev, crtc_id, plane_id,
+						       channel);
 			if (ret < 0) {
 				dev_err(dev->dev,
 					"could not create CRTC (channel %u)\n",
@@ -404,7 +405,7 @@  static int omap_modeset_init(struct drm_device *dev)
 				return ret;
 			}
 
-			id++;
+			crtc_id++; plane_id++;
 		}
 	}
 
@@ -412,7 +413,7 @@  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++) {
+	for (; crtc_id < num_crtcs; crtc_id++) {
 
 		/* find a free manager for this crtc */
 		for (i = 0; i < num_mgrs; i++) {
@@ -426,21 +427,22 @@  static int omap_modeset_init(struct drm_device *dev)
 			return -ENOMEM;
 		}
 
-		ret = omap_modeset_create_crtc(dev, id, i);
+		ret = omap_modeset_create_crtc(dev, crtc_id, plane_id, i);
 		if (ret < 0) {
 			dev_err(dev->dev,
 				"could not create CRTC (channel %u)\n", i);
 			return ret;
 		}
+		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);
 		if (IS_ERR(plane))
 			return PTR_ERR(plane);
 
@@ -453,6 +455,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);