On 20/03/17 18:06, Jyri Sarha wrote:
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.
Signed-off-by: Jyri Sarha jsarha@ti.com
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(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2fe735c..4bafc7b 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -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.
@@ -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.
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 386d90a..8fc1f4a 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -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