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

Commit Message

Jyri Sarha March 14, 2017, 8:35 p.m. UTC

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.

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

Comments

Tomi Valkeinen March 15, 2017, 10:15 a.m. UTC | #1

Hi,

On 14/03/17 22:35, Jyri Sarha wrote:
> 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

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 3d30f29..9b67906 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -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);