Skip to contentSkip to conversation

drm/omapdrm: Remove CONFIG_DRM_OMAP_NUM_CRTCS and cleanup

PATCH Superseded Jyri Sarha 3 participants 5 messages Mar 20, 2017 – Mar 21, 2017

Subsystems (3)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jyri Sarha

author
#0 lore raw

v2 cover drm/omapdrm: Remove CONFIG_DRM_OMAP_NUM_CRTCS and cleanup

The first patch removes CONFIG_DRM_OMAP_NUM_CRTCS config option. The rest cleans up the unnecessary complexity from omap_modeset_init().

Changes since first version:
- drm/omapdrm: Get rid of DRM_OMAP_NUM_CRTCS config option
- drm/omapdrm: -> drm/omap:
- Drop:
- drm/omapdrm: Change possible_crtcs to possible_crtcs_for_planes
- drm/omapdrm: Separate ids for planes and CRTCs in omap_modeset_init()
- Add:
- drm/omap: Major omap_modeset_init() cleanup

Jyri Sarha (2):
drm/omap: Get rid of DRM_OMAP_NUM_CRTCS config option drm/omap: Major omap_modeset_init() cleanup

 drivers/gpu/drm/omapdrm/Kconfig      |   9 --
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  23 +++-
 drivers/gpu/drm/omapdrm/omap_drv.c   | 210 ++++++++---------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |   2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c |   3 +
 5 files changed, 73 insertions(+), 174 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 v2 1/2 drm/omap: 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 v2 2/2 drm/omap: Major omap_modeset_init() cleanup

In reply to Jyri Sarha

Cleanup overly complex omap_modeset_init(). The function is trying to support many unusual configuration, that have never been tested and are not supported by other parts of the dirver.

After cleanup the init function creates exactly one connector, encoder, crtc, and primary plane per each connected dss-device. Each connector->encoder->crtc chain is expected to be separate and each crtc is connect to a single dss-channel. If the configuration does not match the expectations or exceeds the available resources, the configuration is rejected.

---
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  23 ++++-
 drivers/gpu/drm/omapdrm/omap_drv.c   | 173 +++++++++--------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |   2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c |   3 +
 4 files changed, 68 insertions(+), 133 deletions(-)
drivers/gpu/drm/omapdrm/omap_crtc.c +20 -3
index 2fe735c..4bafc7b 100644
@@ -521,6 +521,11 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
 
 void omap_crtc_pre_init(void)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(omap_crtcs); i++)
+		omap_crtcs[i] = NULL;
+
 	dss_install_mgr_ops(&mgr_ops);
 }
 
@@ -531,17 +536,27 @@ void omap_crtc_pre_uninit(void)
 
 /* initialize crtc */
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
-		struct drm_plane *plane, enum omap_channel channel, int id)
+		struct drm_plane *plane, struct omap_dss_device *dssdev)
 {
 	struct drm_crtc *crtc = NULL;
 	struct omap_crtc *omap_crtc;
+	enum omap_channel channel;
+	struct omap_dss_device *out;
 	int ret;
 
+	out = omapdss_find_output_from_display(dssdev);
+	channel = out->dispc_channel;
+	omap_dss_put_device(out);
+
 	DBG("%s", channel_names[channel]);
 
+	/* Multiple displays on same channel is not allowed */
+	if (WARN_ON(omap_crtcs[channel] != NULL))
+		return ERR_PTR(-EINVAL);
+
 	omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL);
 	if (!omap_crtc)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	crtc = &omap_crtc->base;
 
@@ -553,8 +568,10 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
 					&omap_crtc_funcs, NULL);
 	if (ret < 0) {
+		dev_err(dev->dev, "%s(): could not init crtc for: %s\n",
+			__func__, dssdev->name);
 		kfree(omap_crtc);
-		return NULL;
+		return ERR_PTR(ret);
 	}
 
 	drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs);
drivers/gpu/drm/omapdrm/omap_drv.c +44 -129

238 lines not on this page · raw message

drivers/gpu/drm/omapdrm/omap_drv.h +1 -1
index 9098ea1..1e077a9 100644
@@ -135,7 +135,7 @@ static inline void omap_fbdev_free(struct drm_device *dev)
 void omap_crtc_pre_init(void);
 void omap_crtc_pre_uninit(void);
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
-		struct drm_plane *plane, enum omap_channel channel, int id);
+		struct drm_plane *plane, struct omap_dss_device *dssdev);
 int omap_crtc_wait_pending(struct drm_crtc *crtc);
 void omap_crtc_error_irq(struct drm_crtc *crtc, uint32_t irqstatus);
 void omap_crtc_vblank_irq(struct drm_crtc *crtc);
drivers/gpu/drm/omapdrm/omap_plane.c +3 -0
index 386d90a..8fc1f4a 100644
@@ -371,6 +371,9 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	return plane;
 
 error:
+	dev_err(dev->dev, "%s(): could not create plane: %s\n",
+		__func__, plane_names[id]);
+
 	kfree(omap_plane);
 	return NULL;
 }
Signature · 6 lines
1.9.1

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

Re: [PATCH v2 2/2] drm/omap: Major omap_modeset_init() cleanup

Quoted 24 lines · Jyri Sarha

Cleanup overly complex omap_modeset_init(). The function is trying to support many unusual configuration, that have never been tested and are not supported by other parts of the dirver.

After cleanup the init function creates exactly one connector, encoder, crtc, and primary plane per each connected dss-device. Each connector->encoder->crtc chain is expected to be separate and each crtc is connect to a single dss-channel. If the configuration does not match the expectations or exceeds the available resources, the configuration is rejected.

---
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  23 ++++-
 drivers/gpu/drm/omapdrm/omap_drv.c   | 173 +++++++++--------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |   2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c |   3 +
 4 files changed, 68 insertions(+), 133 deletions(-)
drivers/gpu/drm/omapdrm/omap_crtc.c +0 -0
index 2fe735c..4bafc7b 100644

drivers/gpu/drm/omapdrm/omap_crtc.c static int omap_crtc_atomic_get_property(struct drm_crtc *crtc, · line 528

Quoted 8 lines
drivers/gpu/drm/omapdrm/omap_crtc.c +5 -0
@@ -521,6 +521,11 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
 
 void omap_crtc_pre_init(void)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(omap_crtcs); i++)
+		omap_crtcs[i] = NULL;
+

Use memset to clear.

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

Quoted 6 lines
drivers/gpu/drm/omapdrm/omap_crtc.c +1 -1
@@ -318,9 +279,9 @@ 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 crtc_idx = 0, plane_idx = 0;

In a bit longer function it's usually better to initialize the variables near to the place where they are first used, so that you will see the initial value when looking at the code in question.

Quoted 5 lines
drivers/gpu/drm/omapdrm/omap_plane.c +0 -0
index 386d90a..8fc1f4a 100644

drivers/gpu/drm/omapdrm/omap_plane.c struct drm_plane *omap_plane_init(struct drm_device *dev, · line 379

Quoted 9 lines
drivers/gpu/drm/omapdrm/omap_plane.c +3 -0
@@ -371,6 +371,9 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	return plane;
 
 error:
+	dev_err(dev->dev, "%s(): could not create plane: %s\n",
+		__func__, plane_names[id]);
+
 	kfree(omap_plane);
 	return NULL;
 }

omap_plane_init() still uses "id" instead of "idx", and it's stored in struct omap_plane as 'id', and there's no explicit mapping of the DRM idx to the DSS plane id. This is confusing as the DRM plane idx is not strictly tied to the DSS plane id.

So, call it "idx" when it's about the DRM plane index and call it something else when it's about the dss plane id (it's "enum omap_plane" in dss, but that clashes with drm's struct omap_plane. So... 'dss_plane'?). And create a function that maps the DRM plane idx to DSS plane id (for now it can be just a direct map).

This change should probably be an additional patch.

Otherwise I think this looks good, much cleaner than the current confusion.

Tomi

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

#4 lore raw
In reply to Jyri Sarha
Quoted 12 lines · Jyri Sarha

The first patch removes CONFIG_DRM_OMAP_NUM_CRTCS config option. The rest cleans up the unnecessary complexity from omap_modeset_init().

Changes since first version:
- drm/omapdrm: Get rid of DRM_OMAP_NUM_CRTCS config option
- drm/omapdrm: -> drm/omap:
- Drop:
- drm/omapdrm: Change possible_crtcs to possible_crtcs_for_planes
- drm/omapdrm: Separate ids for planes and CRTCs in omap_modeset_init()
- Add:
- drm/omap: Major omap_modeset_init() cleanup

Quoted 3 lines

Jyri Sarha (2):
drm/omap: Get rid of DRM_OMAP_NUM_CRTCS config option drm/omap: Major omap_modeset_init() cleanup

On the topic of entirely useless connector limits, max_conn_count from the fbdev helpers is also dead ever since we started to dynamically alloc connectors because of dp mst hotplugging. In case you want to remove more cruft :-)
-Daniel

Quoted 11 lines
 drivers/gpu/drm/omapdrm/Kconfig      |   9 --
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  23 +++-
 drivers/gpu/drm/omapdrm/omap_drv.c   | 210 ++++++++---------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |   2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c |   3 +
 5 files changed, 73 insertions(+), 174 deletions(-)
Signature · 1 line
1.9.1
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel