[PATCH 0/4] drm/omap: Misc fixes and improvements

[PATCH 0/4] drm/omap: Misc fixes and improvements

[Archit Taneja]

These are misc fixes and improvements within omapdrm. There is one minor
omapdss fix here too, we get problems in omapdrm without it. The fixes do the
following:

- Make omapdrm smarter to choose the right overlay managers as it's crtcs. This
  makes sure that omapdrm is functional for OMAP platforms with different
  combinations of panels connected to it.

- Fix certain areas in omapdrm which allow fixed resolution panels to work.

Archit Taneja (4):
  drm/omap: Don't return from modeset_init if a panel doesn't satisfy
    omapdrm requirements
  drm/omap: Fix and improve crtc and overlay manager correlation
  drm/omap: Make fixed resolution panels work
  omapdss: features: fixed supported outputs for OMAP4

 drivers/gpu/drm/omapdrm/omap_connector.c |   10 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c      |   21 ++--
 drivers/gpu/drm/omapdrm/omap_drv.c       |  196 +++++++++++++++++++++++++-----
 drivers/gpu/drm/omapdrm/omap_drv.h       |   38 +-----
 drivers/gpu/drm/omapdrm/omap_encoder.c   |   13 +-
 drivers/gpu/drm/omapdrm/omap_irq.c       |   17 ++-
 drivers/video/omap2/dss/dss_features.c   |    6 +-
 7 files changed, 218 insertions(+), 83 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] drm/omap: Don't return from modeset_init if a panel doesn't satisfy omapdrm requirements

[Archit Taneja]

modeset_init iterates through all the registered omapdss devices and has some
initial checks to see if the panel has a driver and the required driver ops for
it to be usable by omapdrm.

The function bails out from modeset_init if a panel doesn't meet the
requirements, and stops the registration of the future panels and encoders which
come after it, that isn't the correct thing to do, we should go through the rest
of the panels. Replace the 'return's with 'continue's.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 079c54c..77b7225 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -121,7 +121,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		if (!dssdev->driver) {
 			dev_warn(dev->dev, "%s has no driver.. skipping it\n",
 					dssdev->name);
-			return 0;
+			continue;
 		}
 
 		if (!(dssdev->driver->get_timings ||
@@ -129,7 +129,7 @@ static int omap_modeset_init(struct drm_device *dev)
 			dev_warn(dev->dev, "%s driver does not support "
 				"get_timings or read_edid.. skipping it!\n",
 				dssdev->name);
-			return 0;
+			continue;
 		}
 
 		encoder = omap_encoder_init(dev, dssdev);
-- 
1.7.10.4

Re: [PATCH 1/4] drm/omap: Don't return from modeset_init if a panel doesn't satisfy omapdrm requirements

[Rob Clark]

On Tue, Mar 5, 2013 at 9:17 AM, Archit Taneja <archit@ti.com> wrote:
> modeset_init iterates through all the registered omapdss devices and has some
> initial checks to see if the panel has a driver and the required driver ops for
> it to be usable by omapdrm.
>
> The function bails out from modeset_init if a panel doesn't meet the
> requirements, and stops the registration of the future panels and encoders which
> come after it, that isn't the correct thing to do, we should go through the rest
> of the panels. Replace the 'return's with 'continue's.
>
> Signed-off-by: Archit Taneja <archit@ti.com>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 079c54c..77b7225 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -121,7 +121,7 @@ static int omap_modeset_init(struct drm_device *dev)
>                 if (!dssdev->driver) {
>                         dev_warn(dev->dev, "%s has no driver.. skipping it\n",
>                                         dssdev->name);
> -                       return 0;
> +                       continue;
>                 }
>
>                 if (!(dssdev->driver->get_timings ||
> @@ -129,7 +129,7 @@ static int omap_modeset_init(struct drm_device *dev)
>                         dev_warn(dev->dev, "%s driver does not support "
>                                 "get_timings or read_edid.. skipping it!\n",
>                                 dssdev->name);
> -                       return 0;
> +                       continue;
>                 }
>
>                 encoder = omap_encoder_init(dev, dssdev);
> --
> 1.7.10.4
>

[PATCH 2/4] drm/omap: Fix and improve crtc and overlay manager correlation

[Archit Taneja]

The omapdrm driver currently takes a config/module arg to figure out the number
of crtcs it needs to create. We could create as many crtcs as there are overlay
managers in the DSS hardware, but we don't do that because each crtc eats up
one DSS overlay, and that reduces the number of planes we can attach to a single
crtc.

Since the number of crtcs may be lesser than the number of hardware overlay
managers, we need to figure out which overlay managers to use for our crtcs. The
current approach is to use pipe2chan(), which returns a higher numbered manager
for the crtc.

The problem with this approach is that it assumes that the overlay managers we
choose will connect to the encoders the platform's panels are going to use,
this isn't true, an overlay manager connects only to a few outputs/encoders, and
choosing any overlay manager for our crtc might lead to a situation where the
encoder cannot connect to any of the crtcs we have chosen. For example, an
omap5-panda board has just one hdmi output. If num_crtc is set to 1, with the
current approach, pipe2chan will pick up the LCD2 overlay manager, which cannot
connect to the hdmi encoder at all. The only manager that could have connected
to hdmi was the TV overlay manager.

Therefore, there is a need to choose our overlay managers keeping in mind the
panels we have on that platform. The new approach iterates through all the
available panels, and creates encoders and connectors for them, and then tries
to figure out what overlay manager we could use for our crtc.

We split the overlay managers into 2 sets: the ones which are already tied to a
crtc, and the ones which are free at the moment. For every panel, we first make
a decision about whether we should try to share a crtc with an existing panel,
or try to create a new one. If we want to share an existing crtc, we iterate
through the list of used managers, if we find one which can connect to it's
encoder, we don't create a crtc. If we don't find a used overlay manager, we
try to find an unused one to which it's encoder can connect to and create a
new crtc.

This approach isn't the most optimal one, it can do either good or bad depending
on what sequence the panel drivers were registered. The optimal way would be
some sort of back tracking approach, where we improve the set of managers we use
as we iterate through the list of managers. I haven't tried that out yet though.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c    |   21 ++--
 drivers/gpu/drm/omapdrm/omap_drv.c     |  192 +++++++++++++++++++++++++++-----
 drivers/gpu/drm/omapdrm/omap_drv.h     |   38 +------
 drivers/gpu/drm/omapdrm/omap_encoder.c |    7 ++
 drivers/gpu/drm/omapdrm/omap_irq.c     |   17 ++-
 5 files changed, 202 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index bec66a4..79b200a 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -74,6 +74,13 @@ struct omap_crtc {
 	struct work_struct page_flip_work;
 };
 
+uint32_t pipe2vbl(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	return dispc_mgr_get_vsync_irq(omap_crtc->channel);
+}
+
 /*
  * Manager-ops, callbacks from output when they need to configure
  * the upstream part of the video pipe.
@@ -613,7 +620,13 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->apply.pre_apply  = omap_crtc_pre_apply;
 	omap_crtc->apply.post_apply = omap_crtc_post_apply;
 
-	omap_crtc->apply_irq.irqmask = pipe2vbl(id);
+	omap_crtc->channel = channel;
+	omap_crtc->plane = plane;
+	omap_crtc->plane->crtc = crtc;
+	omap_crtc->name = channel_names[channel];
+	omap_crtc->pipe = id;
+
+	omap_crtc->apply_irq.irqmask = pipe2vbl(crtc);
 	omap_crtc->apply_irq.irq = omap_crtc_apply_irq;
 
 	omap_crtc->error_irq.irqmask =
@@ -621,12 +634,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->error_irq.irq = omap_crtc_error_irq;
 	omap_irq_register(dev, &omap_crtc->error_irq);
 
-	omap_crtc->channel = channel;
-	omap_crtc->plane = plane;
-	omap_crtc->plane->crtc = crtc;
-	omap_crtc->name = channel_names[channel];
-	omap_crtc->pipe = id;
-
 	/* temporary: */
 	omap_crtc->mgr.id = channel;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 77b7225..275c373 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -74,53 +74,57 @@ static int get_connector_type(struct omap_dss_device *dssdev)
 	}
 }
 
+static bool channel_used(struct drm_device *dev, enum omap_channel channel)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	int i;
+
+	for (i = 0; i < priv->num_crtcs; i++) {
+		struct drm_crtc *crtc = priv->crtcs[i];
+
+		if (omap_crtc_channel(crtc) == channel)
+			return true;
+	}
+
+	return false;
+}
+
 static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_dss_device *dssdev = NULL;
 	int num_ovls = dss_feat_get_num_ovls();
-	int id;
+	int num_mgrs = dss_feat_get_num_mgrs();
+	int num_crtcs, num_panels_left = 0;
+	int i, id = 0;
 
 	drm_mode_config_init(dev);
 
 	omap_drm_irq_install(dev);
 
 	/*
-	 * Create private planes and CRTCs for the last NUM_CRTCs overlay
-	 * plus manager:
+	 * We usually don't want to create a CRTC for each manager, at least
+	 * not until we have a way to expose private planes to userspace.
+	 * Otherwise there would not be enough video pipes left for drm planes.
+	 * We use the num_crtc argument to limit the number of crtcs we create.
 	 */
-	for (id = 0; id < min(num_crtc, num_ovls); id++) {
-		struct drm_plane *plane;
-		struct drm_crtc *crtc;
+	num_crtcs = min3(num_crtc, num_mgrs, num_ovls);
 
-		plane = omap_plane_init(dev, id, true);
-		crtc = omap_crtc_init(dev, plane, pipe2chan(id), id);
-
-		BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
-		priv->crtcs[id] = crtc;
-		priv->num_crtcs++;
-
-		priv->planes[id] = plane;
-		priv->num_planes++;
-	}
-
-	/*
-	 * Create normal planes for the remaining overlays:
-	 */
-	for (; id < num_ovls; id++) {
-		struct drm_plane *plane = omap_plane_init(dev, id, false);
+	/* keep a track of the total number of panels */
+	for_each_dss_dev(dssdev) { num_panels_left++; }
 
-		BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
-		priv->planes[priv->num_planes++] = plane;
-	}
+	dssdev = NULL;
 
 	for_each_dss_dev(dssdev) {
 		struct drm_connector *connector;
 		struct drm_encoder *encoder;
+		bool mgr_found = false;
 
 		if (!dssdev->driver) {
 			dev_warn(dev->dev, "%s has no driver.. skipping it\n",
 					dssdev->name);
+			num_panels_left--;
+
 			continue;
 		}
 
@@ -129,6 +133,8 @@ static int omap_modeset_init(struct drm_device *dev)
 			dev_warn(dev->dev, "%s driver does not support "
 				"get_timings or read_edid.. skipping it!\n",
 				dssdev->name);
+			num_panels_left--;
+
 			continue;
 		}
 
@@ -137,6 +143,8 @@ static int omap_modeset_init(struct drm_device *dev)
 		if (!encoder) {
 			dev_err(dev->dev, "could not create encoder: %s\n",
 					dssdev->name);
+			num_panels_left--;
+
 			return -ENOMEM;
 		}
 
@@ -146,6 +154,8 @@ static int omap_modeset_init(struct drm_device *dev)
 		if (!connector) {
 			dev_err(dev->dev, "could not create connector: %s\n",
 					dssdev->name);
+			num_panels_left--;
+
 			return -ENOMEM;
 		}
 
@@ -157,16 +167,146 @@ static int omap_modeset_init(struct drm_device *dev)
 
 		drm_mode_connector_attach_encoder(connector, encoder);
 
+		/*
+		 * if we have reached the limit of the crtcs we are allowed to
+		 * create, let's not try to look for a crtc for this
+		 * panel/encoder and onwards, we will of course, populate the
+		 * the possible_crtcs field for all the encoders with the final
+		 * set of crtcs we create
+		 */
+		if (id == num_crtcs)
+			continue;
+
+		/*
+		 * if we have more panels/encoders left than the number of crtcs
+		 * we are allowed to create, we want to try to find a crtc which
+		 * is already created, and can be shared between these encoders,
+		 * both encoders won't be able to use the crtc at the same time
+		 * though
+		 */
+		if (num_panels_left > (num_crtcs - id)) {
+			for (i = 0; i < num_mgrs; i++) {
+				bool output_supported =
+					dss_feat_get_supported_outputs(i) &
+					dssdev->output->id;
+
+				if (channel_used(dev, i) && output_supported) {
+					mgr_found = true;
+					break;
+				} else {
+					continue;
+				}
+			}
+		}
+
+		if (mgr_found) {
+			num_panels_left--;
+			continue;
+		}
+
+		/*
+		 * if we have too few panels/encoders to worry about, or if we
+		 * couldn't find an already created crtc for this encoder, we'll
+		 * try to allocate a new crtc and find a free overlay manager
+		 * for it
+		 */
+		for (i = 0; i < num_mgrs; i++) {
+			bool output_supported =
+				dss_feat_get_supported_outputs(i) &
+				dssdev->output->id;
+
+			if (!channel_used(dev, i) && output_supported) {
+				struct drm_plane *plane;
+				struct drm_crtc *crtc;
+
+				plane = omap_plane_init(dev, id, true);
+				crtc = omap_crtc_init(dev, plane, i, id);
+
+				BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
+				priv->crtcs[id] = crtc;
+				priv->num_crtcs++;
+
+				priv->planes[id] = plane;
+				priv->num_planes++;
+
+				id++;
+				break;
+			}
+
+			if (i == num_mgrs)
+				DBG("%s doesn't have any valid manager to "
+					"connect to\n", dssdev->name);
+		}
+
+		num_panels_left--;
+	}
+
+	/*
+	 * we have allocated crtcs according to the need of the panels/encoders,
+	 * adding more crtcs here if needed
+	 */
+	for (; id < num_crtcs; id++) {
+
+		/* find a free manager for this crtc */
+		for (i = 0; i < num_mgrs; i++) {
+			if (!channel_used(dev, i)) {
+				struct drm_plane *plane;
+				struct drm_crtc *crtc;
+
+				plane = omap_plane_init(dev, id, true);
+				crtc = omap_crtc_init(dev, plane, i, id);
+
+				BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
+				priv->crtcs[id] = crtc;
+				priv->num_crtcs++;
+
+				priv->planes[id] = plane;
+				priv->num_planes++;
+
+				break;
+			} else {
+				continue;
+			}
+		}
+
+		if (i == num_mgrs) {
+			/* this shouldn't really happen */
+			dev_err(dev->dev, "no managers left for crtc\n");
+			return -ENOMEM;
+		}
+	}
+
+	/*
+	 * Create normal planes for the remaining overlays:
+	 */
+	for (; id < num_ovls; id++) {
+		struct drm_plane *plane = omap_plane_init(dev, id, false);
+
+		BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
+		priv->planes[priv->num_planes++] = plane;
+	}
+
+	for (i = 0; i < priv->num_encoders; i++) {
+		struct drm_encoder *encoder = priv->encoders[i];
+		struct omap_dss_device *dssdev = omap_encoder_get_dssdev(encoder);
+
 		/* figure out which crtc's we can connect the encoder to: */
 		encoder->possible_crtcs = 0;
 		for (id = 0; id < priv->num_crtcs; id++) {
+			struct drm_crtc *crtc = priv->crtcs[id];
+			enum omap_channel crtc_channel = omap_crtc_channel(crtc);
 			enum omap_dss_output_id supported_outputs =
-					dss_feat_get_supported_outputs(pipe2chan(id));
+					dss_feat_get_supported_outputs(crtc_channel);
+
 			if (supported_outputs & dssdev->output->id)
 				encoder->possible_crtcs |= (1 << id);
 		}
 	}
 
+	DBG("registered %d planes, %d crtcs, %d encoders and %d connectors\n",
+		priv->num_planes, priv->num_crtcs, priv->num_encoders,
+		priv->num_connectors);
+
 	dev->mode_config.min_width = 32;
 	dev->mode_config.min_height = 32;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index d4f997b..215a20d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -139,8 +139,8 @@ void omap_gem_describe_objects(struct list_head *list, struct seq_file *m);
 int omap_gem_resume(struct device *dev);
 #endif
 
-int omap_irq_enable_vblank(struct drm_device *dev, int crtc);
-void omap_irq_disable_vblank(struct drm_device *dev, int crtc);
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id);
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id);
 irqreturn_t omap_irq_handler(DRM_IRQ_ARGS);
 void omap_irq_preinstall(struct drm_device *dev);
 int omap_irq_postinstall(struct drm_device *dev);
@@ -271,39 +271,9 @@ static inline int align_pitch(int pitch, int width, int bpp)
 	return ALIGN(pitch, 8 * bytespp);
 }
 
-static inline enum omap_channel pipe2chan(int pipe)
-{
-	int num_mgrs = dss_feat_get_num_mgrs();
-
-	/*
-	 * We usually don't want to create a CRTC for each manager,
-	 * at least not until we have a way to expose private planes
-	 * to userspace.  Otherwise there would not be enough video
-	 * pipes left for drm planes.  The higher #'d managers tend
-	 * to have more features so start in reverse order.
-	 */
-	return num_mgrs - pipe - 1;
-}
-
 /* map crtc to vblank mask */
-static inline uint32_t pipe2vbl(int crtc)
-{
-	enum omap_channel channel = pipe2chan(crtc);
-	return dispc_mgr_get_vsync_irq(channel);
-}
-
-static inline int crtc2pipe(struct drm_device *dev, struct drm_crtc *crtc)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(priv->crtcs); i++)
-		if (priv->crtcs[i] == crtc)
-			return i;
-
-	BUG();  /* bogus CRTC ptr */
-	return -1;
-}
+uint32_t pipe2vbl(struct drm_crtc *crtc);
+struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder *encoder);
 
 /* should these be made into common util helpers?
  */
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 21d126d..d48df71 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -41,6 +41,13 @@ struct omap_encoder {
 	struct omap_dss_device *dssdev;
 };
 
+struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder *encoder)
+{
+	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
+
+	return omap_encoder->dssdev;
+}
+
 static void omap_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index e01303e..9263db1 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -130,12 +130,13 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
  * Zero on success, appropriate errno if the given @crtc's vblank
  * interrupt cannot be enabled.
  */
-int omap_irq_enable_vblank(struct drm_device *dev, int crtc)
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id)
 {
 	struct omap_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc = priv->crtcs[crtc_id];
 	unsigned long flags;
 
-	DBG("dev=%p, crtc=%d", dev, crtc);
+	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
 	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
@@ -156,12 +157,13 @@ int omap_irq_enable_vblank(struct drm_device *dev, int crtc)
  * a hardware vblank counter, this routine should be a no-op, since
  * interrupts will have to stay on to keep the count accurate.
  */
-void omap_irq_disable_vblank(struct drm_device *dev, int crtc)
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id)
 {
 	struct omap_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc = priv->crtcs[crtc_id];
 	unsigned long flags;
 
-	DBG("dev=%p, crtc=%d", dev, crtc);
+	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
 	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
@@ -186,9 +188,12 @@ irqreturn_t omap_irq_handler(DRM_IRQ_ARGS)
 
 	VERB("irqs: %08x", irqstatus);
 
-	for (id = 0; id < priv->num_crtcs; id++)
-		if (irqstatus & pipe2vbl(id))
+	for (id = 0; id < priv->num_crtcs; id++) {
+		struct drm_crtc *crtc = priv->crtcs[id];
+
+		if (irqstatus & pipe2vbl(crtc))
 			drm_handle_vblank(dev, id);
+	}
 
 	spin_lock_irqsave(&list_lock, flags);
 	list_for_each_entry_safe(handler, n, &priv->irq_list, node) {
-- 
1.7.10.4

[PATCH 3/4] drm/omap: Make fixed resolution panels work

[Archit Taneja]

The omapdrm driver requires omapdss panel drivers to expose ops like detect,
set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
and SDI drivers. At some places, there are no checks to see if the panel driver
has these ops or not, and that leads to a crash.

The following things are done to make fixed panels work:

- The omap_connector's detect function is modified such that it considers panel
  types which are generally fixed panels as always connected(provided the panel
  driver doesn't have a detect op). Hence, the connector corresponding to these
  panels is always in a 'connected' state.

- If a panel driver doesn't have a check_timings op, assume that it supports the
  mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)

- The function omap_encoder_update shouldn't really do anything for fixed
  resolution panels, make sure that it calls set_timings only if the panel
  driver has one.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c |   10 ++++++++--
 drivers/gpu/drm/omapdrm/omap_encoder.c   |    6 ++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index c451c41..c952324 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
 			ret = connector_status_connected;
 		else
 			ret = connector_status_disconnected;
+	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
+		ret = connector_status_connected;
 	} else {
 		ret = connector_status_unknown;
 	}
@@ -189,12 +194,13 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 	struct omap_video_timings timings = {0};
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *new_mode;
-	int ret = MODE_BAD;
+	int r, ret = MODE_BAD;
 
 	copy_timings_drm_to_omap(&timings, mode);
 	mode->vrefresh = drm_mode_vrefresh(mode);
 
-	if (!dssdrv->check_timings(dssdev, &timings)) {
+	r = dssdrv->check_timings ? dssdrv->check_timings(dssdev, &timings) : 0;
+	if (!r) {
 		/* check if vrefresh is still valid */
 		new_mode = drm_mode_duplicate(dev, mode);
 		new_mode->clock = timings.pixel_clock;
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index d48df71..9aed178 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -135,13 +135,15 @@ int omap_encoder_update(struct drm_encoder *encoder,
 
 	dssdev->output->manager = mgr;
 
-	ret = dssdrv->check_timings(dssdev, timings);
+	ret = dssdrv->check_timings ?
+		dssdrv->check_timings(dssdev, timings) : 0;
 	if (ret) {
 		dev_err(dev->dev, "could not set timings: %d\n", ret);
 		return ret;
 	}
 
-	dssdrv->set_timings(dssdev, timings);
+	if (dssdrv->set_timings)
+		dssdrv->set_timings(dssdev, timings);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH 3/4] drm/omap: Make fixed resolution panels work

[Rob Clark]

On Tue, Mar 5, 2013 at 9:17 AM, Archit Taneja <archit@ti.com> wrote:
> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
> and SDI drivers. At some places, there are no checks to see if the panel driver
> has these ops or not, and that leads to a crash.
>
> The following things are done to make fixed panels work:
>
> - The omap_connector's detect function is modified such that it considers panel
>   types which are generally fixed panels as always connected(provided the panel
>   driver doesn't have a detect op). Hence, the connector corresponding to these
>   panels is always in a 'connected' state.
>
> - If a panel driver doesn't have a check_timings op, assume that it supports the
>   mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
>
> - The function omap_encoder_update shouldn't really do anything for fixed
>   resolution panels, make sure that it calls set_timings only if the panel
>   driver has one.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_connector.c |   10 ++++++++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c   |    6 ++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index c451c41..c952324 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>                         ret = connector_status_connected;
>                 else
>                         ret = connector_status_disconnected;
> +       } else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
> +                       dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
> +                       dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
> +                       dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
> +               ret = connector_status_connected;
>         } else {
>                 ret = connector_status_unknown;
>         }
> @@ -189,12 +194,13 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
>         struct omap_video_timings timings = {0};
>         struct drm_device *dev = connector->dev;
>         struct drm_display_mode *new_mode;
> -       int ret = MODE_BAD;
> +       int r, ret = MODE_BAD;
>
>         copy_timings_drm_to_omap(&timings, mode);
>         mode->vrefresh = drm_mode_vrefresh(mode);
>
> -       if (!dssdrv->check_timings(dssdev, &timings)) {
> +       r = dssdrv->check_timings ? dssdrv->check_timings(dssdev, &timings) : 0;
> +       if (!r) {
>                 /* check if vrefresh is still valid */
>                 new_mode = drm_mode_duplicate(dev, mode);
>                 new_mode->clock = timings.pixel_clock;
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index d48df71..9aed178 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -135,13 +135,15 @@ int omap_encoder_update(struct drm_encoder *encoder,
>
>         dssdev->output->manager = mgr;
>
> -       ret = dssdrv->check_timings(dssdev, timings);
> +       ret = dssdrv->check_timings ?
> +               dssdrv->check_timings(dssdev, timings) : 0;
>         if (ret) {
>                 dev_err(dev->dev, "could not set timings: %d\n", ret);
>                 return ret;
>         }
>
> -       dssdrv->set_timings(dssdev, timings);
> +       if (dssdrv->set_timings)
> +               dssdrv->set_timings(dssdev, timings);

maybe either here or _mode_valid(), for panels that don't have
set_timings(), we should double check that the new timings match what
we get from the panel's get_timings().  Other than that, it looks
fine.

BR,
-R

>         return 0;
>  }
> --
> 1.7.10.4
>

Re: [PATCH 3/4] drm/omap: Make fixed resolution panels work

[Archit Taneja]

On Wednesday 06 March 2013 06:15 AM, Rob Clark wrote:
> On Tue, Mar 5, 2013 at 9:17 AM, Archit Taneja <archit@ti.com> wrote:
>> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
>> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
>> and SDI drivers. At some places, there are no checks to see if the panel driver
>> has these ops or not, and that leads to a crash.
>>
>> The following things are done to make fixed panels work:
>>
>> - The omap_connector's detect function is modified such that it considers panel
>>    types which are generally fixed panels as always connected(provided the panel
>>    driver doesn't have a detect op). Hence, the connector corresponding to these
>>    panels is always in a 'connected' state.
>>
>> - If a panel driver doesn't have a check_timings op, assume that it supports the
>>    mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
>>
>> - The function omap_encoder_update shouldn't really do anything for fixed
>>    resolution panels, make sure that it calls set_timings only if the panel
>>    driver has one.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/gpu/drm/omapdrm/omap_connector.c |   10 ++++++++--
>>   drivers/gpu/drm/omapdrm/omap_encoder.c   |    6 ++++--
>>   2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
>> index c451c41..c952324 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
>> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>>                          ret = connector_status_connected;
>>                  else
>>                          ret = connector_status_disconnected;
>> +       } else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
>> +                       dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
>> +                       dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
>> +                       dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
>> +               ret = connector_status_connected;
>>          } else {
>>                  ret = connector_status_unknown;
>>          }
>> @@ -189,12 +194,13 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
>>          struct omap_video_timings timings = {0};
>>          struct drm_device *dev = connector->dev;
>>          struct drm_display_mode *new_mode;
>> -       int ret = MODE_BAD;
>> +       int r, ret = MODE_BAD;
>>
>>          copy_timings_drm_to_omap(&timings, mode);
>>          mode->vrefresh = drm_mode_vrefresh(mode);
>>
>> -       if (!dssdrv->check_timings(dssdev, &timings)) {
>> +       r = dssdrv->check_timings ? dssdrv->check_timings(dssdev, &timings) : 0;
>> +       if (!r) {
>>                  /* check if vrefresh is still valid */
>>                  new_mode = drm_mode_duplicate(dev, mode);
>>                  new_mode->clock = timings.pixel_clock;
>> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
>> index d48df71..9aed178 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
>> @@ -135,13 +135,15 @@ int omap_encoder_update(struct drm_encoder *encoder,
>>
>>          dssdev->output->manager = mgr;
>>
>> -       ret = dssdrv->check_timings(dssdev, timings);
>> +       ret = dssdrv->check_timings ?
>> +               dssdrv->check_timings(dssdev, timings) : 0;
>>          if (ret) {
>>                  dev_err(dev->dev, "could not set timings: %d\n", ret);
>>                  return ret;
>>          }
>>
>> -       dssdrv->set_timings(dssdev, timings);
>> +       if (dssdrv->set_timings)
>> +               dssdrv->set_timings(dssdev, timings);
>
> maybe either here or _mode_valid(), for panels that don't have
> set_timings(), we should double check that the new timings match what
> we get from the panel's get_timings().  Other than that, it looks
> fine.

Yeah, we should do that. I guess it makes more sense to do it earlier, 
i.e, in mode_vaild.

Archit

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

The support outputs struct for overlay managers is incorrect for OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
  output.
- the TV manager can suppport DPI, but the omapdss driver doesn't support that
  yet, we require some muxing at the DSS level, and we also need to configure
  the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
  don't support that yet.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dss_features.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id omap3630_dss_supported_outputs[] = {
 
 static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
 	/* OMAP_DSS_CHANNEL_LCD */
-	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-	OMAP_DSS_OUTPUT_DSI1,
+	OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
 
 	/* OMAP_DSS_CHANNEL_DIGIT */
-	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-	OMAP_DSS_OUTPUT_DPI,
+	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
 
 	/* OMAP_DSS_CHANNEL_LCD2 */
 	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-- 
1.7.10.4

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1567 bytes --]

On 2013-03-05 16:17, Archit Taneja wrote:
> The support outputs struct for overlay managers is incorrect for OMAP4. Make
> these changes:
> 
> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
>   output.
> - the TV manager can suppport DPI, but the omapdss driver doesn't support that
>   yet, we require some muxing at the DSS level, and we also need to configure
>   the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
>   don't support that yet.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dss_features.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index d7d66ef..7f791ae 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id omap3630_dss_supported_outputs[] = {
>  
>  static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>  	/* OMAP_DSS_CHANNEL_LCD */
> -	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
> -	OMAP_DSS_OUTPUT_DSI1,
> +	OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>  
>  	/* OMAP_DSS_CHANNEL_DIGIT */
> -	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
> -	OMAP_DSS_OUTPUT_DPI,
> +	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>  
>  	/* OMAP_DSS_CHANNEL_LCD2 */
>  	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
> 

Thanks, I'll apply this to omapdss fixes branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
> On 2013-03-05 16:17, Archit Taneja wrote:
>> The support outputs struct for overlay managers is incorrect for OMAP4. Make
>> these changes:
>>
>> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
>>    output.
>> - the TV manager can suppport DPI, but the omapdss driver doesn't support that
>>    yet, we require some muxing at the DSS level, and we also need to configure
>>    the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
>>    don't support that yet.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dss_features.c |    6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
>> index d7d66ef..7f791ae 100644
>> --- a/drivers/video/omap2/dss/dss_features.c
>> +++ b/drivers/video/omap2/dss/dss_features.c
>> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id omap3630_dss_supported_outputs[] = {
>>
>>   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>>   	/* OMAP_DSS_CHANNEL_LCD */
>> -	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>> -	OMAP_DSS_OUTPUT_DSI1,
>> +	OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>>
>>   	/* OMAP_DSS_CHANNEL_DIGIT */
>> -	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
>> -	OMAP_DSS_OUTPUT_DPI,
>> +	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>>
>>   	/* OMAP_DSS_CHANNEL_LCD2 */
>>   	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>
>
> Thanks, I'll apply this to omapdss fixes branch.

Hi, just one point here, this patch is a prerequisite for the patch 2/4 
in this series. So we need to make sure that the 2/4 patch is not 
without this one in a kernel.

Tomi,

About patch '2/4', could you have a look at it too? It basically tries 
to do a dynamic assignment of channels to outputs. I worked on this 
before you posted the misc series with recommended_channel for outputs. 
This patch tries to figure out managers with supported_outputs. It isn't 
the most optimal way, as it can't back track and chose a better manager, 
but it still seems to do a reasonable job.

We could also use the recommended channel way for omapdrm, I can't 
figure out what's the better approach at the moment.

Archit

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3107 bytes --]

On 2013-03-12 08:07, Archit Taneja wrote:
> On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
>> On 2013-03-05 16:17, Archit Taneja wrote:
>>> The support outputs struct for overlay managers is incorrect for
>>> OMAP4. Make
>>> these changes:
>>>
>>> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a
>>> supported
>>>    output.
>>> - the TV manager can suppport DPI, but the omapdss driver doesn't
>>> support that
>>>    yet, we require some muxing at the DSS level, and we also need to
>>> configure
>>>    the hdmi pll in the DPI driver so that the TV manager has a pixel
>>> clock. We
>>>    don't support that yet.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>> ---
>>>   drivers/video/omap2/dss/dss_features.c |    6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dss_features.c
>>> b/drivers/video/omap2/dss/dss_features.c
>>> index d7d66ef..7f791ae 100644
>>> --- a/drivers/video/omap2/dss/dss_features.c
>>> +++ b/drivers/video/omap2/dss/dss_features.c
>>> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id
>>> omap3630_dss_supported_outputs[] = {
>>>
>>>   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>>>       /* OMAP_DSS_CHANNEL_LCD */
>>> -    OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>> -    OMAP_DSS_OUTPUT_DSI1,
>>> +    OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>>>
>>>       /* OMAP_DSS_CHANNEL_DIGIT */
>>> -    OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
>>> -    OMAP_DSS_OUTPUT_DPI,
>>> +    OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>>>
>>>       /* OMAP_DSS_CHANNEL_LCD2 */
>>>       OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>>
>>
>> Thanks, I'll apply this to omapdss fixes branch.
> 
> Hi, just one point here, this patch is a prerequisite for the patch 2/4
> in this series. So we need to make sure that the 2/4 patch is not
> without this one in a kernel.
> 
> Tomi,
> 
> About patch '2/4', could you have a look at it too? It basically tries
> to do a dynamic assignment of channels to outputs. I worked on this
> before you posted the misc series with recommended_channel for outputs.
> This patch tries to figure out managers with supported_outputs. It isn't
> the most optimal way, as it can't back track and chose a better manager,
> but it still seems to do a reasonable job.
> 
> We could also use the recommended channel way for omapdrm, I can't
> figure out what's the better approach at the moment.

Hmm, I think it'd be safer to use the recommended channel from omapdss
for now. The current omapdss code doesn't really let you use any other
channel than the recommended one (which was thus renamed to
dispc_channel in my later version).

Or does your patch do a better job at selecting the outputs (I'm mostly
thinking about OMAP5 here, which has a bit more conflicts with the mgrs
and outputs than earlier omaps).

But at some point I think we should fix those issues, and let omapdrm
decide how to connect the managers and outputs.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 04:08 PM, Tomi Valkeinen wrote:
> On 2013-03-12 08:07, Archit Taneja wrote:
>> On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
>>> On 2013-03-05 16:17, Archit Taneja wrote:
>>>> The support outputs struct for overlay managers is incorrect for
>>>> OMAP4. Make
>>>> these changes:
>>>>
>>>> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a
>>>> supported
>>>>     output.
>>>> - the TV manager can suppport DPI, but the omapdss driver doesn't
>>>> support that
>>>>     yet, we require some muxing at the DSS level, and we also need to
>>>> configure
>>>>     the hdmi pll in the DPI driver so that the TV manager has a pixel
>>>> clock. We
>>>>     don't support that yet.
>>>>
>>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>>> ---
>>>>    drivers/video/omap2/dss/dss_features.c |    6 ++----
>>>>    1 file changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/video/omap2/dss/dss_features.c
>>>> b/drivers/video/omap2/dss/dss_features.c
>>>> index d7d66ef..7f791ae 100644
>>>> --- a/drivers/video/omap2/dss/dss_features.c
>>>> +++ b/drivers/video/omap2/dss/dss_features.c
>>>> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id
>>>> omap3630_dss_supported_outputs[] = {
>>>>
>>>>    static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>>>>        /* OMAP_DSS_CHANNEL_LCD */
>>>> -    OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>>> -    OMAP_DSS_OUTPUT_DSI1,
>>>> +    OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>>>>
>>>>        /* OMAP_DSS_CHANNEL_DIGIT */
>>>> -    OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
>>>> -    OMAP_DSS_OUTPUT_DPI,
>>>> +    OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>>>>
>>>>        /* OMAP_DSS_CHANNEL_LCD2 */
>>>>        OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>>>
>>>
>>> Thanks, I'll apply this to omapdss fixes branch.
>>
>> Hi, just one point here, this patch is a prerequisite for the patch 2/4
>> in this series. So we need to make sure that the 2/4 patch is not
>> without this one in a kernel.
>>
>> Tomi,
>>
>> About patch '2/4', could you have a look at it too? It basically tries
>> to do a dynamic assignment of channels to outputs. I worked on this
>> before you posted the misc series with recommended_channel for outputs.
>> This patch tries to figure out managers with supported_outputs. It isn't
>> the most optimal way, as it can't back track and chose a better manager,
>> but it still seems to do a reasonable job.
>>
>> We could also use the recommended channel way for omapdrm, I can't
>> figure out what's the better approach at the moment.
>
> Hmm, I think it'd be safer to use the recommended channel from omapdss
> for now. The current omapdss code doesn't really let you use any other
> channel than the recommended one (which was thus renamed to
> dispc_channel in my later version).

I think omapdss needs to give the freedom to set a different dispc 
manager for an output. This is because in omapdrm crtcs are sort of 
floating, and when a connector(and it's encoder) needs to be enabled, it 
tries to look search for a crtc it can connect to. If omapdss sort of 
fixes the output->manager link, which is not how it should work.

I think it is fine for us to use this approach for omapfb when creating 
the links, but not within dss.

For example, the code here:

-	/*
-	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
-	 * source for DPI is SoC integration detail, not something that should
-	 * be configured in the dssdev
-	 */
-	dsidev = dpi_get_dsidev(dssdev->channel);
+	dsidev = dpi_get_dsidev(dpi.output.dispc_channel);

We are sort of using the recommended channel to figure out which DSI pll 
to use. We don't have much of an option here because dpi_init_display() 
happens much earlier. But if it were to connect to a different manager 
later on, we would get into trouble.

>
> Or does your patch do a better job at selecting the outputs (I'm mostly
> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
> and outputs than earlier omaps).

My approach is very drm oriented, the problem with omapdrm right now is 
that we want to limit the number of crtcs(which map to managers). The 
lesser the number of crtcs, the more free planes we have for overlaying.

If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up 
only 2 overlay managers. My patch just tries to figure out which are the 
best 2 managers to choose out of the total number of managers, in such a 
way that most encoders/panels on the platform are supported.

The drm framework(I think) keeps the connections between crtc and 
encoders flexible through the possible_crtcs flag maintained by 
encoders. It figures out which crtc is free, and tries to use that one 
at that moment. Once that encoder is done using it, the crtc can be used 
by another encoder later. We can also change the crtc of an 
encoder(after it's off).

So omapdss fixing the dispc channel for an output doesn't seem suitable 
for drm.

Archit

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3553 bytes --]

On 2013-03-12 14:57, Archit Taneja wrote:

>>> We could also use the recommended channel way for omapdrm, I can't
>>> figure out what's the better approach at the moment.
>>
>> Hmm, I think it'd be safer to use the recommended channel from omapdss
>> for now. The current omapdss code doesn't really let you use any other
>> channel than the recommended one (which was thus renamed to
>> dispc_channel in my later version).
> 
> I think omapdss needs to give the freedom to set a different dispc
> manager for an output. This is because in omapdrm crtcs are sort of
> floating, and when a connector(and it's encoder) needs to be enabled, it
> tries to look search for a crtc it can connect to. If omapdss sort of
> fixes the output->manager link, which is not how it should work.
> 
> I think it is fine for us to use this approach for omapfb when creating
> the links, but not within dss.
> 
> For example, the code here:
> 
> -    /*
> -     * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
> -     * source for DPI is SoC integration detail, not something that should
> -     * be configured in the dssdev
> -     */
> -    dsidev = dpi_get_dsidev(dssdev->channel);
> +    dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
> 
> We are sort of using the recommended channel to figure out which DSI pll
> to use. We don't have much of an option here because dpi_init_display()
> happens much earlier. But if it were to connect to a different manager
> later on, we would get into trouble.

Right. It should be changed to allow dynamic dispc channel changes (as I
said in my mail). But that requires changing how the output drivers and
the output.c work. Perhaps nothing too difficult, but just something
that has not been done =).

>> Or does your patch do a better job at selecting the outputs (I'm mostly
>> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
>> and outputs than earlier omaps).
> 
> My approach is very drm oriented, the problem with omapdrm right now is
> that we want to limit the number of crtcs(which map to managers). The
> lesser the number of crtcs, the more free planes we have for overlaying.
> 
> If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
> only 2 overlay managers. My patch just tries to figure out which are the
> best 2 managers to choose out of the total number of managers, in such a
> way that most encoders/panels on the platform are supported.
> 
> The drm framework(I think) keeps the connections between crtc and
> encoders flexible through the possible_crtcs flag maintained by
> encoders. It figures out which crtc is free, and tries to use that one
> at that moment. Once that encoder is done using it, the crtc can be used
> by another encoder later. We can also change the crtc of an
> encoder(after it's off).
> 
> So omapdss fixing the dispc channel for an output doesn't seem suitable
> for drm.

So, I don't disagree with you. But I don't quite understand why we could
not use the fixed channels for now? They should work in all the boards
we have, right? Or is there something with DRM that forces the driver to
select the channel dynamically?

As long as omapdss doesn't handle switching the clock sources (and
perhaps some other settings also, like OMAP5's DPI video source
selection. we've never had support for changing the channel later.),
having omapdrm select the channels could lead to problems if it happens
to select the wrong channel.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:
> On 2013-03-12 14:57, Archit Taneja wrote:
>
>>>> We could also use the recommended channel way for omapdrm, I can't
>>>> figure out what's the better approach at the moment.
>>>
>>> Hmm, I think it'd be safer to use the recommended channel from omapdss
>>> for now. The current omapdss code doesn't really let you use any other
>>> channel than the recommended one (which was thus renamed to
>>> dispc_channel in my later version).
>>
>> I think omapdss needs to give the freedom to set a different dispc
>> manager for an output. This is because in omapdrm crtcs are sort of
>> floating, and when a connector(and it's encoder) needs to be enabled, it
>> tries to look search for a crtc it can connect to. If omapdss sort of
>> fixes the output->manager link, which is not how it should work.
>>
>> I think it is fine for us to use this approach for omapfb when creating
>> the links, but not within dss.
>>
>> For example, the code here:
>>
>> -    /*
>> -     * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
>> -     * source for DPI is SoC integration detail, not something that should
>> -     * be configured in the dssdev
>> -     */
>> -    dsidev = dpi_get_dsidev(dssdev->channel);
>> +    dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
>>
>> We are sort of using the recommended channel to figure out which DSI pll
>> to use. We don't have much of an option here because dpi_init_display()
>> happens much earlier. But if it were to connect to a different manager
>> later on, we would get into trouble.
>
> Right. It should be changed to allow dynamic dispc channel changes (as I
> said in my mail). But that requires changing how the output drivers and
> the output.c work. Perhaps nothing too difficult, but just something
> that has not been done =).
>
>>> Or does your patch do a better job at selecting the outputs (I'm mostly
>>> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
>>> and outputs than earlier omaps).
>>
>> My approach is very drm oriented, the problem with omapdrm right now is
>> that we want to limit the number of crtcs(which map to managers). The
>> lesser the number of crtcs, the more free planes we have for overlaying.
>>
>> If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
>> only 2 overlay managers. My patch just tries to figure out which are the
>> best 2 managers to choose out of the total number of managers, in such a
>> way that most encoders/panels on the platform are supported.
>>
>> The drm framework(I think) keeps the connections between crtc and
>> encoders flexible through the possible_crtcs flag maintained by
>> encoders. It figures out which crtc is free, and tries to use that one
>> at that moment. Once that encoder is done using it, the crtc can be used
>> by another encoder later. We can also change the crtc of an
>> encoder(after it's off).
>>
>> So omapdss fixing the dispc channel for an output doesn't seem suitable
>> for drm.
>
> So, I don't disagree with you. But I don't quite understand why we could
> not use the fixed channels for now? They should work in all the boards
> we have, right? Or is there something with DRM that forces the driver to
> select the channel dynamically?

I think we can use fixed channels, but if the number of different fixed 
channels crosses the number of crtcs the kernel wants, then we would 
need to atleast change the channels of some of the outputs.

For example, suppose omapdrm is asked to use only 2 crtcs, and it picks 
up LCD2 and TV managers. Now if there is some panel which says it's 
recommended channel is LCD, then things won't work.

At the moment, omapdrm maps a crtc with a manger using a function called 
pipe2chan() which just selects a manager with the biggest channel no. So 
if the kernel is configured to have num_crtcs as 1. The single crtc will 
be mapped to LCD2. This method is wrong, as it doesn't even look at the 
type of panels at all. For an omap5 panda, the most suitable manager to 
map to the crtc would be TV(for hdmi).

I think what we probably need to do is to combine both the methods. I.e, 
make each output connectible to only one channel, and also iterate 
through the panels in omapdrm to find the most suitable channels. So in 
my patch, instead of looking at all the supported managers for an 
output(checking with dss_feat_get_supported_outputs() on each manager), 
I just look at the recommended channel, and try to map that manager.

This way, we have a better approach than the pipe2chan() thing, but we 
restrict each encoder to connect to only one crtc, which we could extend 
later if the need arises.

Should we go with this approach then?

>
> As long as omapdss doesn't handle switching the clock sources (and
> perhaps some other settings also, like OMAP5's DPI video source
> selection. we've never had support for changing the channel later.),
> having omapdrm select the channels could lead to problems if it happens
> to select the wrong channel.

Yes, most of the other combinations wont work because of missing things 
like clock source selection in DSS_CTRL and using the other video port 
of DSI.

Archit

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

On 2013-03-12 16:01, Archit Taneja wrote:
> On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:

>> So, I don't disagree with you. But I don't quite understand why we could
>> not use the fixed channels for now? They should work in all the boards
>> we have, right? Or is there something with DRM that forces the driver to
>> select the channel dynamically?
> 
> I think we can use fixed channels, but if the number of different fixed
> channels crosses the number of crtcs the kernel wants, then we would
> need to atleast change the channels of some of the outputs.
> 
> For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
> up LCD2 and TV managers. Now if there is some panel which says it's
> recommended channel is LCD, then things won't work.

Are you saying omapdrm picks the managers for the crtcs before knowing
what panels there are? That can't work right... We need to know what
outputs are to be used before we can select the managers. Or, we always
need crtcs for all the managers.

If we do know the panels, and thus outputs, then the managers to be used
are found easily from output->dispc_channel.

But, of course, the crtc to manager mapping could be changed (if omapdrm
supports this). If omapdrm is asked to use only 1 crtc, but there are
two panels, then only one panel can be used at a time, and the manager
for the crtc needs to be changed when the panel to be used is changed.
But even in this case used manager is clear, it comes from
output->dispc_channel.

> At the moment, omapdrm maps a crtc with a manger using a function called
> pipe2chan() which just selects a manager with the biggest channel no. So
> if the kernel is configured to have num_crtcs as 1. The single crtc will
> be mapped to LCD2. This method is wrong, as it doesn't even look at the
> type of panels at all. For an omap5 panda, the most suitable manager to
> map to the crtc would be TV(for hdmi).
> 
> I think what we probably need to do is to combine both the methods. I.e,
> make each output connectible to only one channel, and also iterate
> through the panels in omapdrm to find the most suitable channels. So in
> my patch, instead of looking at all the supported managers for an
> output(checking with dss_feat_get_supported_outputs() on each manager),
> I just look at the recommended channel, and try to map that manager.

I don't know, I feel like I'm not understanding something here =).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:59 PM, Tomi Valkeinen wrote:
> On 2013-03-12 16:01, Archit Taneja wrote:
>> On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:
>
>>> So, I don't disagree with you. But I don't quite understand why we could
>>> not use the fixed channels for now? They should work in all the boards
>>> we have, right? Or is there something with DRM that forces the driver to
>>> select the channel dynamically?
>>
>> I think we can use fixed channels, but if the number of different fixed
>> channels crosses the number of crtcs the kernel wants, then we would
>> need to atleast change the channels of some of the outputs.
>>
>> For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
>> up LCD2 and TV managers. Now if there is some panel which says it's
>> recommended channel is LCD, then things won't work.
>
> Are you saying omapdrm picks the managers for the crtcs before knowing
> what panels there are? That can't work right... We need to know what
> outputs are to be used before we can select the managers. Or, we always
> need crtcs for all the managers.

That's how it is right now.

>
> If we do know the panels, and thus outputs, then the managers to be used
> are found easily from output->dispc_channel.

Yes, my patch tries to do the same, but it could assign a manger which 
isn't the recommended channel. It can pick one from the list of 
supported channels. I've explained it a bit more below.

>
> But, of course, the crtc to manager mapping could be changed (if omapdrm
> supports this). If omapdrm is asked to use only 1 crtc, but there are
> two panels, then only one panel can be used at a time, and the manager
> for the crtc needs to be changed when the panel to be used is changed.
> But even in this case used manager is clear, it comes from
> output->dispc_channel.

This is something I don't know that can be done or not, or if it can be 
done easily. A crtc isn't purely an overlay manager. It also needs to 
have one plane associated to it. So, if we want to change the overlay 
manager tied to a crtc on the fly, we should make sure that it's still 
connected to a plane pointing to the same buffer. This needs a better 
understanding of drm internals. I guess Rob could answer this better.

>
>> At the moment, omapdrm maps a crtc with a manger using a function called
>> pipe2chan() which just selects a manager with the biggest channel no. So
>> if the kernel is configured to have num_crtcs as 1. The single crtc will
>> be mapped to LCD2. This method is wrong, as it doesn't even look at the
>> type of panels at all. For an omap5 panda, the most suitable manager to
>> map to the crtc would be TV(for hdmi).
>>
>> I think what we probably need to do is to combine both the methods. I.e,
>> make each output connectible to only one channel, and also iterate
>> through the panels in omapdrm to find the most suitable channels. So in
>> my patch, instead of looking at all the supported managers for an
>> output(checking with dss_feat_get_supported_outputs() on each manager),
>> I just look at the recommended channel, and try to map that manager.
>
> I don't know, I feel like I'm not understanding something here =).

:)

I think what I said above is equivalent to what you said here:

"If we do know the panels, and thus outputs, then the managers to be 
used are found easily from output->dispc_channel."

Instead of using output->dispc_channel, the patch I made tried to get 
the first supported channel(in increasing order of channel index) for an 
output. If that channel was already taken/reserved by a previous output, 
it tries to take the next supported channel if there are enough crtcs left.

My patch would break things if it chooses a manager for an output for 
which we haven't written the necessary code yet(switching clock sources 
etc).

So, what I'm saying is that we should stick to output->dispc_channel. We 
iterate through all the panels, and by using output->dispc_channel, we 
get the manager for an output, and map that manager to a crtc, and make 
sure the number of unique managers we finally use is equal to NUM_CRTC.

Does that sound good?

Archit

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On 2013-03-12 17:01, Archit Taneja wrote:

> So, what I'm saying is that we should stick to output->dispc_channel. We
> iterate through all the panels, and by using output->dispc_channel, we
> get the manager for an output, and map that manager to a crtc, and make
> sure the number of unique managers we finally use is equal to NUM_CRTC.
> 
> Does that sound good?

Yes, I think that sounds good.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

[PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Archit Taneja]

The omapdrm driver requires omapdss panel drivers to expose ops like detect,
set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
and SDI drivers. At some places, there are no checks to see if the panel driver
has these ops or not, and that leads to a crash.

The following things are done to make fixed panels work:

- The omap_connector's detect function is modified such that it considers panel
  types which are generally fixed panels as always connected(provided the panel
  driver doesn't have a detect op). Hence, the connector corresponding to these
  panels is always in a 'connected' state.

- If a panel driver doesn't have a check_timings op, assume that it supports the
  mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)

- The function omap_encoder_update shouldn't really do anything for fixed
  resolution panels, make sure that it calls set_timings only if the panel
  driver has one.

Signed-off-by: Archit Taneja <archit@ti.com>
---
Note: In v2, we make sure that the mode passed on to omapdrm matches the timings
of the fixed resolution panel.

 drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index c451c41..a72fedd 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
 			ret = connector_status_connected;
 		else
 			ret = connector_status_disconnected;
+	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
+		ret = connector_status_connected;
 	} else {
 		ret = connector_status_unknown;
 	}
@@ -189,12 +194,30 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 	struct omap_video_timings timings = {0};
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *new_mode;
-	int ret = MODE_BAD;
+	int r, ret = MODE_BAD;
 
 	copy_timings_drm_to_omap(&timings, mode);
 	mode->vrefresh = drm_mode_vrefresh(mode);
 
-	if (!dssdrv->check_timings(dssdev, &timings)) {
+	/*
+	 * if the panel driver doesn't have a check_timings, it's most likely
+	 * a fixed resolution panel, check if the timings match with the
+	 * panel's timings
+	 */
+	if (dssdrv->check_timings) {
+		r = dssdrv->check_timings(dssdev, &timings);
+	} else {
+		struct omap_video_timings t;
+
+		dssdrv->get_timings(dssdev, &t);
+
+		if (memcmp(&timings, &t, sizeof(struct omap_video_timings)))
+			r = -EINVAL;
+		else
+			r = 0;
+	}
+
+	if (!r) {
 		/* check if vrefresh is still valid */
 		new_mode = drm_mode_duplicate(dev, mode);
 		new_mode->clock = timings.pixel_clock;
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index d48df71..47971c2 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -135,13 +135,26 @@ int omap_encoder_update(struct drm_encoder *encoder,
 
 	dssdev->output->manager = mgr;
 
-	ret = dssdrv->check_timings(dssdev, timings);
+	if (dssdrv->check_timings) {
+		ret = dssdrv->check_timings(dssdev, timings);
+	} else {
+		struct omap_video_timings t;
+
+		dssdrv->get_timings(dssdev, &t);
+
+		if (memcmp(timings, &t, sizeof(struct omap_video_timings)))
+			ret = -EINVAL;
+		else
+			ret = 0;
+	}
+
 	if (ret) {
 		dev_err(dev->dev, "could not set timings: %d\n", ret);
 		return ret;
 	}
 
-	dssdrv->set_timings(dssdev, timings);
+	if (dssdrv->set_timings)
+		dssdrv->set_timings(dssdev, timings);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3934 bytes --]

On 2013-03-12 15:06, Archit Taneja wrote:
> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
> and SDI drivers. At some places, there are no checks to see if the panel driver
> has these ops or not, and that leads to a crash.
> 
> The following things are done to make fixed panels work:
> 
> - The omap_connector's detect function is modified such that it considers panel
>   types which are generally fixed panels as always connected(provided the panel
>   driver doesn't have a detect op). Hence, the connector corresponding to these
>   panels is always in a 'connected' state.
> 
> - If a panel driver doesn't have a check_timings op, assume that it supports the
>   mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
> 
> - The function omap_encoder_update shouldn't really do anything for fixed
>   resolution panels, make sure that it calls set_timings only if the panel
>   driver has one.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> Note: In v2, we make sure that the mode passed on to omapdrm matches the timings
> of the fixed resolution panel.
> 
>  drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
>  2 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index c451c41..a72fedd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>  			ret = connector_status_connected;
>  		else
>  			ret = connector_status_disconnected;
> +	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
> +		ret = connector_status_connected;

I have to say I don't like this. We shouldn't care about the type here.
I think it's better just to default to connected if there's no detect
function (or unknown? I'm not sure what is the practical difference).

If it works fine without using dssdev->type, we have one less place to
worry when doing dss dev model changes =).

>  	} else {
>  		ret = connector_status_unknown;
>  	}
> @@ -189,12 +194,30 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
>  	struct omap_video_timings timings = {0};
>  	struct drm_device *dev = connector->dev;
>  	struct drm_display_mode *new_mode;
> -	int ret = MODE_BAD;
> +	int r, ret = MODE_BAD;
>  
>  	copy_timings_drm_to_omap(&timings, mode);
>  	mode->vrefresh = drm_mode_vrefresh(mode);
>  
> -	if (!dssdrv->check_timings(dssdev, &timings)) {
> +	/*
> +	 * if the panel driver doesn't have a check_timings, it's most likely
> +	 * a fixed resolution panel, check if the timings match with the
> +	 * panel's timings
> +	 */
> +	if (dssdrv->check_timings) {
> +		r = dssdrv->check_timings(dssdev, &timings);
> +	} else {
> +		struct omap_video_timings t;
> +
> +		dssdrv->get_timings(dssdev, &t);
> +
> +		if (memcmp(&timings, &t, sizeof(struct omap_video_timings)))
> +			r = -EINVAL;
> +		else
> +			r = 0;

memcmp on two structs is often not a good idea. There could be padding
bytes there, with uninitialized data. I'm not sure if that's the case
here, though, but it could well change any time (perhaps even depending
on compiler version).

I'm still pondering whether it'd just be simpler to require all the
dssdrv ops to be filled, probably using a helper macro in the panel
drivers... Did you consider that approach? I'm not saying to go for it,
I'm saying I can't make my mind which would be better =).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Archit Taneja]

On Tuesday 12 March 2013 07:36 PM, Tomi Valkeinen wrote:
> On 2013-03-12 15:06, Archit Taneja wrote:
>> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
>> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
>> and SDI drivers. At some places, there are no checks to see if the panel driver
>> has these ops or not, and that leads to a crash.
>>
>> The following things are done to make fixed panels work:
>>
>> - The omap_connector's detect function is modified such that it considers panel
>>    types which are generally fixed panels as always connected(provided the panel
>>    driver doesn't have a detect op). Hence, the connector corresponding to these
>>    panels is always in a 'connected' state.
>>
>> - If a panel driver doesn't have a check_timings op, assume that it supports the
>>    mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
>>
>> - The function omap_encoder_update shouldn't really do anything for fixed
>>    resolution panels, make sure that it calls set_timings only if the panel
>>    driver has one.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> Note: In v2, we make sure that the mode passed on to omapdrm matches the timings
>> of the fixed resolution panel.
>>
>>   drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
>>   drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
>>   2 files changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
>> index c451c41..a72fedd 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
>> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>>   			ret = connector_status_connected;
>>   		else
>>   			ret = connector_status_disconnected;
>> +	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
>> +		ret = connector_status_connected;
>
> I have to say I don't like this. We shouldn't care about the type here.
> I think it's better just to default to connected if there's no detect
> function (or unknown? I'm not sure what is the practical difference).
>
> If it works fine without using dssdev->type, we have one less place to
> worry when doing dss dev model changes =).
>
>>   	} else {
>>   		ret = connector_status_unknown;
>>   	}
>> @@ -189,12 +194,30 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
>>   	struct omap_video_timings timings = {0};
>>   	struct drm_device *dev = connector->dev;
>>   	struct drm_display_mode *new_mode;
>> -	int ret = MODE_BAD;
>> +	int r, ret = MODE_BAD;
>>
>>   	copy_timings_drm_to_omap(&timings, mode);
>>   	mode->vrefresh = drm_mode_vrefresh(mode);
>>
>> -	if (!dssdrv->check_timings(dssdev, &timings)) {
>> +	/*
>> +	 * if the panel driver doesn't have a check_timings, it's most likely
>> +	 * a fixed resolution panel, check if the timings match with the
>> +	 * panel's timings
>> +	 */
>> +	if (dssdrv->check_timings) {
>> +		r = dssdrv->check_timings(dssdev, &timings);
>> +	} else {
>> +		struct omap_video_timings t;
>> +
>> +		dssdrv->get_timings(dssdev, &t);
>> +
>> +		if (memcmp(&timings, &t, sizeof(struct omap_video_timings)))
>> +			r = -EINVAL;
>> +		else
>> +			r = 0;
>
> memcmp on two structs is often not a good idea. There could be padding
> bytes there, with uninitialized data. I'm not sure if that's the case
> here, though, but it could well change any time (perhaps even depending
> on compiler version).

I saw usage of memcmp on structs in the kernel, but then most of them 
were in drivers and not core, and could be mistakes :)

adding '__attribute__((packed))' to omap_video_timings might be helpful 
I suppose. But I don't know if it's safe to do a memcmp even with a 
packed struct.

>
> I'm still pondering whether it'd just be simpler to require all the
> dssdrv ops to be filled, probably using a helper macro in the panel
> drivers... Did you consider that approach? I'm not saying to go for it,
> I'm saying I can't make my mind which would be better =).

I didn't consider it mainly because I thought we were going to get rid 
of our private omapdss panel drivers with CDF panels, and efforts in 
fixing it wouldn't be much useful. But if there isn't any other good 
alternative, then I can try to see what macros look like.

Of course, simpler options for this patch would be to do a manual 
compare of the fields instead of a memcmp, or adding default ops in 
omap_dss_register_driver.

One thing about common panel driver functions in general is that they 
won't use the panel driver's locking. So if a panel driver doesn't 
create a get_timings() op assuming that omapdss will make a default func 
for it, we would miss out on the panel lock. I don't know if that's 
really bad, and doing a memcmp or anything else in omapdrm also doesn't 
help with this case.

Archit

Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3072 bytes --]

On 2013-03-12 16:38, Archit Taneja wrote:

>> memcmp on two structs is often not a good idea. There could be padding
>> bytes there, with uninitialized data. I'm not sure if that's the case
>> here, though, but it could well change any time (perhaps even depending
>> on compiler version).
> 
> I saw usage of memcmp on structs in the kernel, but then most of them
> were in drivers and not core, and could be mistakes :)
> 
> adding '__attribute__((packed))' to omap_video_timings might be helpful
> I suppose. But I don't know if it's safe to do a memcmp even with a
> packed struct.

I think it's safe to use memcmp if you know that both structs have been
initialized to zero with memset.

>> I'm still pondering whether it'd just be simpler to require all the
>> dssdrv ops to be filled, probably using a helper macro in the panel
>> drivers... Did you consider that approach? I'm not saying to go for it,
>> I'm saying I can't make my mind which would be better =).
> 
> I didn't consider it mainly because I thought we were going to get rid
> of our private omapdss panel drivers with CDF panels, and efforts in
> fixing it wouldn't be much useful. But if there isn't any other good
> alternative, then I can try to see what macros look like.

Well, even if I was slightly optimistic earlier, I now have a feeling
CDF may take a while. I think we should just go for omapdss dev model
change first.

One thing to note about the ops is that NULL is currently used to convey
information, something like "this ops is not possible or valid". So
adding all the ops may not quite work. For example, adding update op for
auto-update panels could tell that the panel supports manual update.
(Well, for that particular case we have a flag, but you get the idea).

But if we can add only some of the ops to the drivers, and there is no
information lost when we won't have NULLs, I guess that could be the
simplest option. Then we don't need to add extra code in each place we
use the ops.

> Of course, simpler options for this patch would be to do a manual
> compare of the fields instead of a memcmp, or adding default ops in
> omap_dss_register_driver.

Adding default ops in omap_dss_register_driver() is not good. It
prevents us from having the ops as const in the future, and is also not
possible when we either move to CDF or change the omapdss dev model.

So I think either we need to handle the NULLs as you do in this patch,
or add the ops to the panels. But the ops could still be the default
versions offered by the omapdss.

> One thing about common panel driver functions in general is that they
> won't use the panel driver's locking. So if a panel driver doesn't
> create a get_timings() op assuming that omapdss will make a default func
> for it, we would miss out on the panel lock. I don't know if that's
> really bad, and doing a memcmp or anything else in omapdrm also doesn't
> help with this case.

That's true. The locking is a bit of a mess (read: broken =) anyway
currently.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Archit Taneja]

On Tuesday 12 March 2013 08:23 PM, Tomi Valkeinen wrote:
> On 2013-03-12 16:38, Archit Taneja wrote:
>
>>> memcmp on two structs is often not a good idea. There could be padding
>>> bytes there, with uninitialized data. I'm not sure if that's the case
>>> here, though, but it could well change any time (perhaps even depending
>>> on compiler version).
>>
>> I saw usage of memcmp on structs in the kernel, but then most of them
>> were in drivers and not core, and could be mistakes :)
>>
>> adding '__attribute__((packed))' to omap_video_timings might be helpful
>> I suppose. But I don't know if it's safe to do a memcmp even with a
>> packed struct.
>
> I think it's safe to use memcmp if you know that both structs have been
> initialized to zero with memset.
>
>>> I'm still pondering whether it'd just be simpler to require all the
>>> dssdrv ops to be filled, probably using a helper macro in the panel
>>> drivers... Did you consider that approach? I'm not saying to go for it,
>>> I'm saying I can't make my mind which would be better =).
>>
>> I didn't consider it mainly because I thought we were going to get rid
>> of our private omapdss panel drivers with CDF panels, and efforts in
>> fixing it wouldn't be much useful. But if there isn't any other good
>> alternative, then I can try to see what macros look like.
>
> Well, even if I was slightly optimistic earlier, I now have a feeling
> CDF may take a while. I think we should just go for omapdss dev model
> change first.
>
> One thing to note about the ops is that NULL is currently used to convey
> information, something like "this ops is not possible or valid". So
> adding all the ops may not quite work. For example, adding update op for
> auto-update panels could tell that the panel supports manual update.
> (Well, for that particular case we have a flag, but you get the idea).
>
> But if we can add only some of the ops to the drivers, and there is no
> information lost when we won't have NULLs, I guess that could be the
> simplest option. Then we don't need to add extra code in each place we
> use the ops.
>
>> Of course, simpler options for this patch would be to do a manual
>> compare of the fields instead of a memcmp, or adding default ops in
>> omap_dss_register_driver.
>
> Adding default ops in omap_dss_register_driver() is not good. It
> prevents us from having the ops as const in the future, and is also not
> possible when we either move to CDF or change the omapdss dev model.
>
> So I think either we need to handle the NULLs as you do in this patch,
> or add the ops to the panels. But the ops could still be the default
> versions offered by the omapdss.

I was trying to come up with a macro which could add default ops to the 
omap_dss_driver. It isn't straight forward as I thought, because you 
need to choose either the default op, or the panel driver's op if it 
exists. For example, I can't create a macro which adds an op for 
get_timings() to all panel drivers, the panel drivers which already this 
op defined will have 2 instances of get_timings() in the omap_dss_driver 
struct.

I have been looking around in the kernel to see how undeclared ops in a 
struct are generally dealt with. Till now, I've noticed that the code 
which uses this ops takes the responsibility to check whether they are 
NULL or not.

The easiest way would be to have these default funcs inlined in a 
header, and every panel driver's omap_dss_driver struct fills in the 
default op. This way we can make the driver ops const. Do you have any 
idea of a macro which could do the same as above, and leads to less 
addition of code?

Archit

>
>> One thing about common panel driver functions in general is that they
>> won't use the panel driver's locking. So if a panel driver doesn't
>> create a get_timings() op assuming that omapdss will make a default func
>> for it, we would miss out on the panel lock. I don't know if that's
>> really bad, and doing a memcmp or anything else in omapdrm also doesn't
>> help with this case.
>
> That's true. The locking is a bit of a mess (read: broken =) anyway
> currently.
>
>   Tomi
>
>

Re: [PATCH v2 3/4] drm/omap: Make fixed resolution panels work

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

On 2013-03-19 08:45, Archit Taneja wrote:

> I was trying to come up with a macro which could add default ops to the
> omap_dss_driver. It isn't straight forward as I thought, because you
> need to choose either the default op, or the panel driver's op if it
> exists. For example, I can't create a macro which adds an op for
> get_timings() to all panel drivers, the panel drivers which already this
> op defined will have 2 instances of get_timings() in the omap_dss_driver
> struct.

Yep, I noticed the same a few days ago.

> I have been looking around in the kernel to see how undeclared ops in a
> struct are generally dealt with. Till now, I've noticed that the code
> which uses this ops takes the responsibility to check whether they are
> NULL or not.
> 
> The easiest way would be to have these default funcs inlined in a
> header, and every panel driver's omap_dss_driver struct fills in the
> default op. This way we can make the driver ops const. Do you have any
> idea of a macro which could do the same as above, and leads to less
> addition of code?

Why would they need to be inlined?

Another option would be to create global funcs that are used to call the
ops. So instead of:

dssdev->dssdrv->foo(dssdev)

the user would call this function:

int dss_foo(struct omap_dss_device *dssdev)
{
	if (dssdev->dssdrv->foo == NULL)
		return 0; /* or error, depending on case */
	return dssdev->dssdrv->foo(dssdev);
}

But that'd require adding a bunch of functions, and changing all the
callers.

I think the safest way to fix this for now is to add the checks into
omapdrm as you do in your original patch. If we go for some other route,
I fear that omapfb/omap_vout could be affected, as it could presume that
an op being NULL or non-NULL means something. If we change the ops to be
always non-NULL, we should go over the uses of those ops to verify they
work correctly.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

[PATCH v2 0/8] omapdss/omapdrm: Misc fixes and improvements

[Archit Taneja]

These are misc fixes and improvements within omapdrm and omapdss. The fixes do the
following:

- Make omapdrm smarter to choose the right overlay managers as it's crtcs. This
  makes sure that omapdrm is functional for OMAP platforms with different
  combinations of panels connected to it.

- Fix certain areas in omapdrm which allow fixed resolution panels to work.

- Fix functional and pixel clock limits for DISPC, this ensures we don't try
  to try a mode that the hardware can't support.

- Some changes that came in OMAP5 ES2 silicon.

Note: The branch is based on the DSS misc series and dsi video mode calc series
posted by Tomi. Reference branch:

git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git for-3.10/misc_drm_dss

Archit Taneja (8):
  drm/omap: Don't return from modeset_init if a panel doesn't satisfy
    omapdrm requirements
  drm/omap: Fix and improve crtc and overlay manager correlation
  drm/omap: Make fixed resolution panels work
  omapdss: features: fixed supported outputs for OMAP4
  omapdss: DISPC: add max pixel clock limits for LCD and TV managers
  omapdss: Features: Fix some parameter ranges
  OMAPDSS: DISPC: Configure doublestride for NV12 when using 2D Tiler
    buffers
  OMAPDSS: DISPC: Revert to older DISPC Smart Standby mechanism for
    OMAP5

 drivers/gpu/drm/omapdrm/omap_connector.c |   27 ++++-
 drivers/gpu/drm/omapdrm/omap_crtc.c      |   21 ++--
 drivers/gpu/drm/omapdrm/omap_drv.c       |  161 ++++++++++++++++++++++++------
 drivers/gpu/drm/omapdrm/omap_drv.h       |   38 +------
 drivers/gpu/drm/omapdrm/omap_encoder.c   |   24 ++++-
 drivers/gpu/drm/omapdrm/omap_irq.c       |   17 ++--
 drivers/video/omap2/dss/dispc.c          |   52 ++++++++--
 drivers/video/omap2/dss/dispc.h          |    1 +
 drivers/video/omap2/dss/dss_features.c   |   14 ++-
 9 files changed, 260 insertions(+), 95 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/8] drm/omap: Don't return from modeset_init if a panel doesn't satisfy omapdrm requirements

[Archit Taneja]

modeset_init iterates through all the registered omapdss devices and has some
initial checks to see if the panel has a driver and the required driver ops for
it to be usable by omapdrm.

The function bails out from modeset_init if a panel doesn't meet the
requirements, and stops the registration of the future panels and encoders which
come after it, that isn't the correct thing to do, we should go through the rest
of the panels. Replace the 'return's with 'continue's.

Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 079c54c..77b7225 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -121,7 +121,7 @@ static int omap_modeset_init(struct drm_device *dev)
 		if (!dssdev->driver) {
 			dev_warn(dev->dev, "%s has no driver.. skipping it\n",
 					dssdev->name);
-			return 0;
+			continue;
 		}
 
 		if (!(dssdev->driver->get_timings ||
@@ -129,7 +129,7 @@ static int omap_modeset_init(struct drm_device *dev)
 			dev_warn(dev->dev, "%s driver does not support "
 				"get_timings or read_edid.. skipping it!\n",
 				dssdev->name);
-			return 0;
+			continue;
 		}
 
 		encoder = omap_encoder_init(dev, dssdev);
-- 
1.7.10.4

[PATCH v2 2/8] drm/omap: Fix and improve crtc and overlay manager correlation

[Archit Taneja]

The omapdrm driver currently takes a config/module arg to figure out the number
of crtcs it needs to create. We could create as many crtcs as there are overlay
managers in the DSS hardware, but we don't do that because each crtc eats up
one DSS overlay, and that reduces the number of planes we can attach to a single
crtc.

Since the number of crtcs may be lesser than the number of hardware overlay
managers, we need to figure out which overlay managers to use for our crtcs. The
current approach is to use pipe2chan(), which returns a higher numbered manager
for the crtc.

The problem with this approach is that it assumes that the overlay managers we
choose will connect to the encoders the platform's panels are going to use,
this isn't true, an overlay manager connects only to a few outputs/encoders, and
choosing any overlay manager for our crtc might lead to a situation where the
encoder cannot connect to any of the crtcs we have chosen. For example, an
omap5-panda board has just one hdmi output. If num_crtc is set to 1, with the
current approach, pipe2chan will pick up the LCD2 overlay manager, which cannot
connect to the hdmi encoder at all. The only manager that could have connected
to hdmi was the TV overlay manager.

Therefore, there is a need to choose our overlay managers keeping in mind the
panels we have on that platform. The new approach iterates through all the
available panels, creates encoders and connectors for them, and then tries to
get a suitable overlay manager to create a crtc which can connect to the
encoders.

We use the dispc_channel field in omap_dss_output to retrieve the desired
overlay manager's channel number, we then check whether the manager had already
been assigned to a crtc or not. If it was already assigned to a crtc, we assume
that out of all the encoders which intend use this crtc, only one will run at a
time. If the overlay manager wan't assigned to a crtc till then, we create a
new crtc and link it with the overlay manager.

This approach just looks for the best dispc_channel for each encoder. On DSS HW,
some encoders can connect to multiple overlay managers. Since we don't try
looking for alternate overlay managers, there is a greater possibility that 2
or more encoders end up asking for the same crtc, causing only one encoder to
run at a time.

Also, this approach isn't the most optimal one, it can do either good or bad
depending on the sequence in which the panels/outputs are parsed. The optimal
way would be some sort of back tracking approach, where we improve the set of
managers we use as we iterate through the list of panels/encoders. That's
something left for later.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c    |   21 +++--
 drivers/gpu/drm/omapdrm/omap_drv.c     |  157 ++++++++++++++++++++++++++------
 drivers/gpu/drm/omapdrm/omap_drv.h     |   38 +-------
 drivers/gpu/drm/omapdrm/omap_encoder.c |    7 ++
 drivers/gpu/drm/omapdrm/omap_irq.c     |   17 ++--
 5 files changed, 165 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index bec66a4..79b200a 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -74,6 +74,13 @@ struct omap_crtc {
 	struct work_struct page_flip_work;
 };
 
+uint32_t pipe2vbl(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	return dispc_mgr_get_vsync_irq(omap_crtc->channel);
+}
+
 /*
  * Manager-ops, callbacks from output when they need to configure
  * the upstream part of the video pipe.
@@ -613,7 +620,13 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->apply.pre_apply  = omap_crtc_pre_apply;
 	omap_crtc->apply.post_apply = omap_crtc_post_apply;
 
-	omap_crtc->apply_irq.irqmask = pipe2vbl(id);
+	omap_crtc->channel = channel;
+	omap_crtc->plane = plane;
+	omap_crtc->plane->crtc = crtc;
+	omap_crtc->name = channel_names[channel];
+	omap_crtc->pipe = id;
+
+	omap_crtc->apply_irq.irqmask = pipe2vbl(crtc);
 	omap_crtc->apply_irq.irq = omap_crtc_apply_irq;
 
 	omap_crtc->error_irq.irqmask =
@@ -621,12 +634,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->error_irq.irq = omap_crtc_error_irq;
 	omap_irq_register(dev, &omap_crtc->error_irq);
 
-	omap_crtc->channel = channel;
-	omap_crtc->plane = plane;
-	omap_crtc->plane->crtc = crtc;
-	omap_crtc->name = channel_names[channel];
-	omap_crtc->pipe = id;
-
 	/* temporary: */
 	omap_crtc->mgr.id = channel;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 77b7225..cbaa003 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -74,49 +74,48 @@ static int get_connector_type(struct omap_dss_device *dssdev)
 	}
 }
 
+static bool channel_used(struct drm_device *dev, enum omap_channel channel)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	int i;
+
+	for (i = 0; i < priv->num_crtcs; i++) {
+		struct drm_crtc *crtc = priv->crtcs[i];
+
+		if (omap_crtc_channel(crtc) == channel)
+			return true;
+	}
+
+	return false;
+}
+
 static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_dss_device *dssdev = NULL;
 	int num_ovls = dss_feat_get_num_ovls();
-	int id;
+	int num_mgrs = dss_feat_get_num_mgrs();
+	int num_crtcs;
+	int i, id = 0;
 
 	drm_mode_config_init(dev);
 
 	omap_drm_irq_install(dev);
 
 	/*
-	 * Create private planes and CRTCs for the last NUM_CRTCs overlay
-	 * plus manager:
-	 */
-	for (id = 0; id < min(num_crtc, num_ovls); id++) {
-		struct drm_plane *plane;
-		struct drm_crtc *crtc;
-
-		plane = omap_plane_init(dev, id, true);
-		crtc = omap_crtc_init(dev, plane, pipe2chan(id), id);
-
-		BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
-		priv->crtcs[id] = crtc;
-		priv->num_crtcs++;
-
-		priv->planes[id] = plane;
-		priv->num_planes++;
-	}
-
-	/*
-	 * Create normal planes for the remaining overlays:
+	 * We usually don't want to create a CRTC for each manager, at least
+	 * not until we have a way to expose private planes to userspace.
+	 * Otherwise there would not be enough video pipes left for drm planes.
+	 * We use the num_crtc argument to limit the number of crtcs we create.
 	 */
-	for (; id < num_ovls; id++) {
-		struct drm_plane *plane = omap_plane_init(dev, id, false);
+	num_crtcs = min3(num_crtc, num_mgrs, num_ovls);
 
-		BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
-		priv->planes[priv->num_planes++] = plane;
-	}
+	dssdev = NULL;
 
 	for_each_dss_dev(dssdev) {
 		struct drm_connector *connector;
 		struct drm_encoder *encoder;
+		enum omap_channel channel;
 
 		if (!dssdev->driver) {
 			dev_warn(dev->dev, "%s has no driver.. skipping it\n",
@@ -157,16 +156,118 @@ static int omap_modeset_init(struct drm_device *dev)
 
 		drm_mode_connector_attach_encoder(connector, encoder);
 
+		/*
+		 * if we have reached the limit of the crtcs we are allowed to
+		 * create, let's not try to look for a crtc for this
+		 * panel/encoder and onwards, we will, of course, populate the
+		 * the possible_crtcs field for all the encoders with the final
+		 * set of crtcs we create
+		 */
+		if (id == num_crtcs)
+			continue;
+
+		/*
+		 * get the recommended DISPC channel for this encoder. For now,
+		 * we only try to get create a crtc out of the recommended, the
+		 * other possible channels to which the encoder can connect are
+		 * not considered.
+		 */
+		channel = dssdev->output->dispc_channel;
+
+		/*
+		 * if this channel hasn't already been taken by a previously
+		 * allocated crtc, we create a new crtc for it
+		 */
+		if (!channel_used(dev, channel)) {
+			struct drm_plane *plane;
+			struct drm_crtc *crtc;
+
+			plane = omap_plane_init(dev, id, true);
+			crtc = omap_crtc_init(dev, plane, channel, id);
+
+			BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
+			priv->crtcs[id] = crtc;
+			priv->num_crtcs++;
+
+			priv->planes[id] = plane;
+			priv->num_planes++;
+
+			id++;
+		}
+	}
+
+	/*
+	 * we have allocated crtcs according to the need of the panels/encoders,
+	 * adding more crtcs here if needed
+	 */
+	for (; id < num_crtcs; id++) {
+
+		/* find a free manager for this crtc */
+		for (i = 0; i < num_mgrs; i++) {
+			if (!channel_used(dev, i)) {
+				struct drm_plane *plane;
+				struct drm_crtc *crtc;
+
+				plane = omap_plane_init(dev, id, true);
+				crtc = omap_crtc_init(dev, plane, i, id);
+
+				BUG_ON(priv->num_crtcs >=
+					ARRAY_SIZE(priv->crtcs));
+
+				priv->crtcs[id] = crtc;
+				priv->num_crtcs++;
+
+				priv->planes[id] = plane;
+				priv->num_planes++;
+
+				break;
+			} else {
+				continue;
+			}
+		}
+
+		if (i == num_mgrs) {
+			/* this shouldn't really happen */
+			dev_err(dev->dev, "no managers left for crtc\n");
+			return -ENOMEM;
+		}
+	}
+
+	/*
+	 * Create normal planes for the remaining overlays:
+	 */
+	for (; id < num_ovls; id++) {
+		struct drm_plane *plane = omap_plane_init(dev, id, false);
+
+		BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
+		priv->planes[priv->num_planes++] = plane;
+	}
+
+	for (i = 0; i < priv->num_encoders; i++) {
+		struct drm_encoder *encoder = priv->encoders[i];
+		struct omap_dss_device *dssdev =
+					omap_encoder_get_dssdev(encoder);
+
 		/* figure out which crtc's we can connect the encoder to: */
 		encoder->possible_crtcs = 0;
 		for (id = 0; id < priv->num_crtcs; id++) {
-			enum omap_dss_output_id supported_outputs =
-					dss_feat_get_supported_outputs(pipe2chan(id));
+			struct drm_crtc *crtc = priv->crtcs[id];
+			enum omap_channel crtc_channel;
+			enum omap_dss_output_id supported_outputs;
+
+			crtc_channel = omap_crtc_channel(crtc);
+			supported_outputs =
+				dss_feat_get_supported_outputs(crtc_channel);
+
 			if (supported_outputs & dssdev->output->id)
 				encoder->possible_crtcs |= (1 << id);
 		}
 	}
 
+	DBG("registered %d planes, %d crtcs, %d encoders and %d connectors\n",
+		priv->num_planes, priv->num_crtcs, priv->num_encoders,
+		priv->num_connectors);
+
 	dev->mode_config.min_width = 32;
 	dev->mode_config.min_height = 32;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index d4f997b..215a20d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -139,8 +139,8 @@ void omap_gem_describe_objects(struct list_head *list, struct seq_file *m);
 int omap_gem_resume(struct device *dev);
 #endif
 
-int omap_irq_enable_vblank(struct drm_device *dev, int crtc);
-void omap_irq_disable_vblank(struct drm_device *dev, int crtc);
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id);
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id);
 irqreturn_t omap_irq_handler(DRM_IRQ_ARGS);
 void omap_irq_preinstall(struct drm_device *dev);
 int omap_irq_postinstall(struct drm_device *dev);
@@ -271,39 +271,9 @@ static inline int align_pitch(int pitch, int width, int bpp)
 	return ALIGN(pitch, 8 * bytespp);
 }
 
-static inline enum omap_channel pipe2chan(int pipe)
-{
-	int num_mgrs = dss_feat_get_num_mgrs();
-
-	/*
-	 * We usually don't want to create a CRTC for each manager,
-	 * at least not until we have a way to expose private planes
-	 * to userspace.  Otherwise there would not be enough video
-	 * pipes left for drm planes.  The higher #'d managers tend
-	 * to have more features so start in reverse order.
-	 */
-	return num_mgrs - pipe - 1;
-}
-
 /* map crtc to vblank mask */
-static inline uint32_t pipe2vbl(int crtc)
-{
-	enum omap_channel channel = pipe2chan(crtc);
-	return dispc_mgr_get_vsync_irq(channel);
-}
-
-static inline int crtc2pipe(struct drm_device *dev, struct drm_crtc *crtc)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(priv->crtcs); i++)
-		if (priv->crtcs[i] == crtc)
-			return i;
-
-	BUG();  /* bogus CRTC ptr */
-	return -1;
-}
+uint32_t pipe2vbl(struct drm_crtc *crtc);
+struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder *encoder);
 
 /* should these be made into common util helpers?
  */
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 21d126d..d48df71 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -41,6 +41,13 @@ struct omap_encoder {
 	struct omap_dss_device *dssdev;
 };
 
+struct omap_dss_device *omap_encoder_get_dssdev(struct drm_encoder *encoder)
+{
+	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
+
+	return omap_encoder->dssdev;
+}
+
 static void omap_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index e01303e..9263db1 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -130,12 +130,13 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
  * Zero on success, appropriate errno if the given @crtc's vblank
  * interrupt cannot be enabled.
  */
-int omap_irq_enable_vblank(struct drm_device *dev, int crtc)
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id)
 {
 	struct omap_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc = priv->crtcs[crtc_id];
 	unsigned long flags;
 
-	DBG("dev=%p, crtc=%d", dev, crtc);
+	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
 	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
@@ -156,12 +157,13 @@ int omap_irq_enable_vblank(struct drm_device *dev, int crtc)
  * a hardware vblank counter, this routine should be a no-op, since
  * interrupts will have to stay on to keep the count accurate.
  */
-void omap_irq_disable_vblank(struct drm_device *dev, int crtc)
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id)
 {
 	struct omap_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc = priv->crtcs[crtc_id];
 	unsigned long flags;
 
-	DBG("dev=%p, crtc=%d", dev, crtc);
+	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
 	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
@@ -186,9 +188,12 @@ irqreturn_t omap_irq_handler(DRM_IRQ_ARGS)
 
 	VERB("irqs: %08x", irqstatus);
 
-	for (id = 0; id < priv->num_crtcs; id++)
-		if (irqstatus & pipe2vbl(id))
+	for (id = 0; id < priv->num_crtcs; id++) {
+		struct drm_crtc *crtc = priv->crtcs[id];
+
+		if (irqstatus & pipe2vbl(crtc))
 			drm_handle_vblank(dev, id);
+	}
 
 	spin_lock_irqsave(&list_lock, flags);
 	list_for_each_entry_safe(handler, n, &priv->irq_list, node) {
-- 
1.7.10.4

[PATCH v3 3/8] drm/omap: Make fixed resolution panels work

[Archit Taneja]

The omapdrm driver requires omapdss panel drivers to expose ops like detect,
set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
and SDI drivers. At some places, there are no checks to see if the panel driver
has these ops or not, and that leads to a crash.

The following things are done to make fixed panels work:

- The omap_connector's detect function is modified such that it considers panel
  types which are generally fixed panels as always connected(provided the panel
  driver doesn't have a detect op). Hence, the connector corresponding to these
  panels is always in a 'connected' state.

- If a panel driver doesn't have a check_timings op, assume that it supports the
  mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)

- The function omap_encoder_update shouldn't really do anything for fixed
  resolution panels, make sure that it calls set_timings only if the panel
  driver has one.

Signed-off-by: Archit Taneja <archit@ti.com>
---
v3: clear the timings local variable first before using memcmp
v2: make sure the timings we try to set for a fixed resolution panel match the
    panel's timings

 drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index c451c41..912759d 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
 			ret = connector_status_connected;
 		else
 			ret = connector_status_disconnected;
+	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
+			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
+		ret = connector_status_connected;
 	} else {
 		ret = connector_status_unknown;
 	}
@@ -189,12 +194,30 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 	struct omap_video_timings timings = {0};
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *new_mode;
-	int ret = MODE_BAD;
+	int r, ret = MODE_BAD;
 
 	copy_timings_drm_to_omap(&timings, mode);
 	mode->vrefresh = drm_mode_vrefresh(mode);
 
-	if (!dssdrv->check_timings(dssdev, &timings)) {
+	/*
+	 * if the panel driver doesn't have a check_timings, it's most likely
+	 * a fixed resolution panel, check if the timings match with the
+	 * panel's timings
+	 */
+	if (dssdrv->check_timings) {
+		r = dssdrv->check_timings(dssdev, &timings);
+	} else {
+		struct omap_video_timings t = {0};
+
+		dssdrv->get_timings(dssdev, &t);
+
+		if (memcmp(&timings, &t, sizeof(struct omap_video_timings)))
+			r = -EINVAL;
+		else
+			r = 0;
+	}
+
+	if (!r) {
 		/* check if vrefresh is still valid */
 		new_mode = drm_mode_duplicate(dev, mode);
 		new_mode->clock = timings.pixel_clock;
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index d48df71..c29451b 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -135,13 +135,26 @@ int omap_encoder_update(struct drm_encoder *encoder,
 
 	dssdev->output->manager = mgr;
 
-	ret = dssdrv->check_timings(dssdev, timings);
+	if (dssdrv->check_timings) {
+		ret = dssdrv->check_timings(dssdev, timings);
+	} else {
+		struct omap_video_timings t = {0};
+
+		dssdrv->get_timings(dssdev, &t);
+
+		if (memcmp(timings, &t, sizeof(struct omap_video_timings)))
+			ret = -EINVAL;
+		else
+			ret = 0;
+	}
+
 	if (ret) {
 		dev_err(dev->dev, "could not set timings: %d\n", ret);
 		return ret;
 	}
 
-	dssdrv->set_timings(dssdev, timings);
+	if (dssdrv->set_timings)
+		dssdrv->set_timings(dssdev, timings);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH v3 3/8] drm/omap: Make fixed resolution panels work

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2734 bytes --]

On 2013-03-26 15:45, Archit Taneja wrote:
> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
> and SDI drivers. At some places, there are no checks to see if the panel driver
> has these ops or not, and that leads to a crash.
> 
> The following things are done to make fixed panels work:
> 
> - The omap_connector's detect function is modified such that it considers panel
>   types which are generally fixed panels as always connected(provided the panel
>   driver doesn't have a detect op). Hence, the connector corresponding to these
>   panels is always in a 'connected' state.
> 
> - If a panel driver doesn't have a check_timings op, assume that it supports the
>   mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
> 
> - The function omap_encoder_update shouldn't really do anything for fixed
>   resolution panels, make sure that it calls set_timings only if the panel
>   driver has one.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> v3: clear the timings local variable first before using memcmp
> v2: make sure the timings we try to set for a fixed resolution panel match the
>     panel's timings
> 
>  drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
>  2 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index c451c41..912759d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>  			ret = connector_status_connected;
>  		else
>  			ret = connector_status_disconnected;
> +	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
> +			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
> +		ret = connector_status_connected;
>  	} else {
>  		ret = connector_status_unknown;
>  	}

Can we leave this part out? I don't like hardcoding things like that,
and if I'm not mistaken, this only affects VENC.

If the code above would use detect where available, and presume the
panel is connected in other cases, it'll be right for all other displays
but VENC, for which we have no connect information.

Or, there could be a special case for VENC, like above, which sets the
connector status to unknown for that. And connected for all others. It'd
still be hardcoded, but for much less cases.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v3 3/8] drm/omap: Make fixed resolution panels work

[Archit Taneja]

On Wednesday 27 March 2013 12:54 PM, Tomi Valkeinen wrote:
> On 2013-03-26 15:45, Archit Taneja wrote:
>> The omapdrm driver requires omapdss panel drivers to expose ops like detect,
>> set_timings and check_timings. These can be NULL for fixed panel DPI, DBI, DSI
>> and SDI drivers. At some places, there are no checks to see if the panel driver
>> has these ops or not, and that leads to a crash.
>>
>> The following things are done to make fixed panels work:
>>
>> - The omap_connector's detect function is modified such that it considers panel
>>    types which are generally fixed panels as always connected(provided the panel
>>    driver doesn't have a detect op). Hence, the connector corresponding to these
>>    panels is always in a 'connected' state.
>>
>> - If a panel driver doesn't have a check_timings op, assume that it supports the
>>    mode passed to omap_connector_mode_valid(the 'mode_valid' drm helper function)
>>
>> - The function omap_encoder_update shouldn't really do anything for fixed
>>    resolution panels, make sure that it calls set_timings only if the panel
>>    driver has one.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> v3: clear the timings local variable first before using memcmp
>> v2: make sure the timings we try to set for a fixed resolution panel match the
>>      panel's timings
>>
>>   drivers/gpu/drm/omapdrm/omap_connector.c |   27 +++++++++++++++++++++++++--
>>   drivers/gpu/drm/omapdrm/omap_encoder.c   |   17 +++++++++++++++--
>>   2 files changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
>> index c451c41..912759d 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
>> @@ -110,6 +110,11 @@ static enum drm_connector_status omap_connector_detect(
>>   			ret = connector_status_connected;
>>   		else
>>   			ret = connector_status_disconnected;
>> +	} else if (dssdev->type == OMAP_DISPLAY_TYPE_DPI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_DBI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_SDI ||
>> +			dssdev->type == OMAP_DISPLAY_TYPE_DSI) {
>> +		ret = connector_status_connected;
>>   	} else {
>>   		ret = connector_status_unknown;
>>   	}
>
> Can we leave this part out? I don't like hardcoding things like that,
> and if I'm not mistaken, this only affects VENC.
>
> If the code above would use detect where available, and presume the
> panel is connected in other cases, it'll be right for all other displays
> but VENC, for which we have no connect information.
>
> Or, there could be a special case for VENC, like above, which sets the
> connector status to unknown for that. And connected for all others. It'd
> still be hardcoded, but for much less cases.

I guess we can leave the status for VENC as always connected too, if we 
leave it as unknown by default and have no detect func for VENC, it 
would never run with omapdrm.

Archit

[PATCH v2 4/8] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

The support outputs struct for overlay managers is incorrect for OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
  output.
- the TV manager can suppport DPI, but the omapdss driver doesn't support that
  yet, we require some muxing at the DSS level, and we also need to configure
  the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
  don't support that yet.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dss_features.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id omap3630_dss_supported_outputs[] = {
 
 static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
 	/* OMAP_DSS_CHANNEL_LCD */
-	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-	OMAP_DSS_OUTPUT_DSI1,
+	OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
 
 	/* OMAP_DSS_CHANNEL_DIGIT */
-	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-	OMAP_DSS_OUTPUT_DPI,
+	OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
 
 	/* OMAP_DSS_CHANNEL_LCD2 */
 	OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-- 
1.7.10.4

[PATCH v2 5/8] omapdss: DISPC: add max pixel clock limits for LCD and TV managers

[Archit Taneja]

Each version of OMAP has a limitation on the maximum pixel clock frequency
supported by an overlay manager. This limit isn't checked by omapdss. Add
dispc feats for lcd and tv managers and check whether the target timings can
be supported or not.

The pixel clock limitations are actually more complex. They depend on which OPP
OMAP is in, and they also depend on which encoder is the manager connected to.
The OPP dependence is ignored as DSS forces the PM framework to be on OPP100
when DSS is enabled, and the encoder dependencies are ignored by DISPC for now.
These limits should come from the encoder driver.

The OMAP2 TRM doesn't mention the maximum pixel clock limit. This value is left
as half of DSS_FCLK, as OMAP2 requires the PCD to be atleast 2.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 8cfa27b..73a730a 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -69,6 +69,8 @@ struct dispc_features {
 	u8 mgr_height_start;
 	u16 mgr_width_max;
 	u16 mgr_height_max;
+	unsigned long max_lcd_pclk;
+	unsigned long max_tv_pclk;
 	int (*calc_scaling) (unsigned long pclk, unsigned long lclk,
 		const struct omap_video_timings *mgr_timings,
 		u16 width, u16 height, u16 out_width, u16 out_height,
@@ -2825,6 +2827,15 @@ static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
 	return true;
 }
 
+static bool _dispc_mgr_pclk_ok(enum omap_channel channel,
+		unsigned long pclk)
+{
+	if (dss_mgr_is_lcd(channel))
+		return pclk <= dispc.feat->max_lcd_pclk ? true : false;
+	else
+		return pclk <= dispc.feat->max_tv_pclk ? true : false;
+}
+
 bool dispc_mgr_timings_ok(enum omap_channel channel,
 		const struct omap_video_timings *timings)
 {
@@ -2832,11 +2843,13 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
 
 	timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
 
-	if (dss_mgr_is_lcd(channel))
-		timings_ok =  timings_ok && _dispc_lcd_timings_ok(timings->hsw,
-						timings->hfp, timings->hbp,
-						timings->vsw, timings->vfp,
-						timings->vbp);
+	timings_ok &= _dispc_mgr_pclk_ok(channel, timings->pixel_clock * 1000);
+
+	if (dss_mgr_is_lcd(channel)) {
+		timings_ok &= _dispc_lcd_timings_ok(timings->hsw, timings->hfp,
+				timings->hbp, timings->vsw, timings->vfp,
+				timings->vbp);
+	}
 
 	return timings_ok;
 }
@@ -3491,6 +3504,7 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.max_lcd_pclk		=	66500000,
 	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
 	.calc_core_clk		=	calc_core_clk_24xx,
 	.num_fifos		=	3,
@@ -3508,6 +3522,8 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.max_lcd_pclk		=	173000000,
+	.max_tv_pclk		=	59000000,
 	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
 	.calc_core_clk		=	calc_core_clk_34xx,
 	.num_fifos		=	3,
@@ -3525,6 +3541,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.max_lcd_pclk		=	173000000,
+	.max_tv_pclk		=	59000000,
 	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
 	.calc_core_clk		=	calc_core_clk_34xx,
 	.num_fifos		=	3,
@@ -3542,6 +3560,8 @@ static const struct dispc_features omap44xx_dispc_feats __initconst = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.max_lcd_pclk		=	170000000,
+	.max_tv_pclk		=	185625000,
 	.calc_scaling		=	dispc_ovl_calc_scaling_44xx,
 	.calc_core_clk		=	calc_core_clk_44xx,
 	.num_fifos		=	5,
@@ -3559,6 +3579,8 @@ static const struct dispc_features omap54xx_dispc_feats __initconst = {
 	.mgr_height_start	=	27,
 	.mgr_width_max		=	4096,
 	.mgr_height_max		=	4096,
+	.max_lcd_pclk		=	170000000,
+	.max_tv_pclk		=	186000000,
 	.calc_scaling		=	dispc_ovl_calc_scaling_44xx,
 	.calc_core_clk		=	calc_core_clk_44xx,
 	.num_fifos		=	5,
-- 
1.7.10.4

Re: [PATCH v2 5/8] omapdss: DISPC: add max pixel clock limits for LCD and TV managers

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

On 2013-03-26 15:45, Archit Taneja wrote:
> Each version of OMAP has a limitation on the maximum pixel clock frequency
> supported by an overlay manager. This limit isn't checked by omapdss. Add
> dispc feats for lcd and tv managers and check whether the target timings can
> be supported or not.
> 
> The pixel clock limitations are actually more complex. They depend on which OPP
> OMAP is in, and they also depend on which encoder is the manager connected to.
> The OPP dependence is ignored as DSS forces the PM framework to be on OPP100
> when DSS is enabled, and the encoder dependencies are ignored by DISPC for now.
> These limits should come from the encoder driver.
> 
> The OMAP2 TRM doesn't mention the maximum pixel clock limit. This value is left
> as half of DSS_FCLK, as OMAP2 requires the PCD to be atleast 2.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |   32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 8cfa27b..73a730a 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -69,6 +69,8 @@ struct dispc_features {
>  	u8 mgr_height_start;
>  	u16 mgr_width_max;
>  	u16 mgr_height_max;
> +	unsigned long max_lcd_pclk;
> +	unsigned long max_tv_pclk;
>  	int (*calc_scaling) (unsigned long pclk, unsigned long lclk,
>  		const struct omap_video_timings *mgr_timings,
>  		u16 width, u16 height, u16 out_width, u16 out_height,
> @@ -2825,6 +2827,15 @@ static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
>  	return true;
>  }
>  
> +static bool _dispc_mgr_pclk_ok(enum omap_channel channel,
> +		unsigned long pclk)
> +{
> +	if (dss_mgr_is_lcd(channel))
> +		return pclk <= dispc.feat->max_lcd_pclk ? true : false;
> +	else
> +		return pclk <= dispc.feat->max_tv_pclk ? true : false;
> +}
> +
>  bool dispc_mgr_timings_ok(enum omap_channel channel,
>  		const struct omap_video_timings *timings)
>  {
> @@ -2832,11 +2843,13 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
>  
>  	timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
>  
> -	if (dss_mgr_is_lcd(channel))
> -		timings_ok =  timings_ok && _dispc_lcd_timings_ok(timings->hsw,
> -						timings->hfp, timings->hbp,
> -						timings->vsw, timings->vfp,
> -						timings->vbp);
> +	timings_ok &= _dispc_mgr_pclk_ok(channel, timings->pixel_clock * 1000);
> +
> +	if (dss_mgr_is_lcd(channel)) {
> +		timings_ok &= _dispc_lcd_timings_ok(timings->hsw, timings->hfp,
> +				timings->hbp, timings->vsw, timings->vfp,
> +				timings->vbp);
> +	}
>  
>  	return timings_ok;
>  }
> @@ -3491,6 +3504,7 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
>  	.mgr_height_start	=	26,
>  	.mgr_width_max		=	2048,
>  	.mgr_height_max		=	2048,
> +	.max_lcd_pclk		=	66500000,
>  	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
>  	.calc_core_clk		=	calc_core_clk_24xx,
>  	.num_fifos		=	3,

OMAP2 has VENC output, but there's no max_tv_pclk above. This would make
VENC pclk check to fail always, wouldn't it?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 5/8] omapdss: DISPC: add max pixel clock limits for LCD and TV managers

[Archit Taneja]

On Wednesday 27 March 2013 01:00 PM, Tomi Valkeinen wrote:
> On 2013-03-26 15:45, Archit Taneja wrote:
>> Each version of OMAP has a limitation on the maximum pixel clock frequency
>> supported by an overlay manager. This limit isn't checked by omapdss. Add
>> dispc feats for lcd and tv managers and check whether the target timings can
>> be supported or not.
>>
>> The pixel clock limitations are actually more complex. They depend on which OPP
>> OMAP is in, and they also depend on which encoder is the manager connected to.
>> The OPP dependence is ignored as DSS forces the PM framework to be on OPP100
>> when DSS is enabled, and the encoder dependencies are ignored by DISPC for now.
>> These limits should come from the encoder driver.
>>
>> The OMAP2 TRM doesn't mention the maximum pixel clock limit. This value is left
>> as half of DSS_FCLK, as OMAP2 requires the PCD to be atleast 2.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dispc.c |   32 +++++++++++++++++++++++++++-----
>>   1 file changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 8cfa27b..73a730a 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -69,6 +69,8 @@ struct dispc_features {
>>   	u8 mgr_height_start;
>>   	u16 mgr_width_max;
>>   	u16 mgr_height_max;
>> +	unsigned long max_lcd_pclk;
>> +	unsigned long max_tv_pclk;
>>   	int (*calc_scaling) (unsigned long pclk, unsigned long lclk,
>>   		const struct omap_video_timings *mgr_timings,
>>   		u16 width, u16 height, u16 out_width, u16 out_height,
>> @@ -2825,6 +2827,15 @@ static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
>>   	return true;
>>   }
>>
>> +static bool _dispc_mgr_pclk_ok(enum omap_channel channel,
>> +		unsigned long pclk)
>> +{
>> +	if (dss_mgr_is_lcd(channel))
>> +		return pclk <= dispc.feat->max_lcd_pclk ? true : false;
>> +	else
>> +		return pclk <= dispc.feat->max_tv_pclk ? true : false;
>> +}
>> +
>>   bool dispc_mgr_timings_ok(enum omap_channel channel,
>>   		const struct omap_video_timings *timings)
>>   {
>> @@ -2832,11 +2843,13 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
>>
>>   	timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
>>
>> -	if (dss_mgr_is_lcd(channel))
>> -		timings_ok =  timings_ok && _dispc_lcd_timings_ok(timings->hsw,
>> -						timings->hfp, timings->hbp,
>> -						timings->vsw, timings->vfp,
>> -						timings->vbp);
>> +	timings_ok &= _dispc_mgr_pclk_ok(channel, timings->pixel_clock * 1000);
>> +
>> +	if (dss_mgr_is_lcd(channel)) {
>> +		timings_ok &= _dispc_lcd_timings_ok(timings->hsw, timings->hfp,
>> +				timings->hbp, timings->vsw, timings->vfp,
>> +				timings->vbp);
>> +	}
>>
>>   	return timings_ok;
>>   }
>> @@ -3491,6 +3504,7 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
>>   	.mgr_height_start	=	26,
>>   	.mgr_width_max		=	2048,
>>   	.mgr_height_max		=	2048,
>> +	.max_lcd_pclk		=	66500000,
>>   	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
>>   	.calc_core_clk		=	calc_core_clk_24xx,
>>   	.num_fifos		=	3,
>
> OMAP2 has VENC output, but there's no max_tv_pclk above. This would make
> VENC pclk check to fail always, wouldn't it?

oops, I missed adding it for omap2, will fix it.

Archit

[PATCH v2 6/8] omapdss: Features: Fix some parameter ranges

[Archit Taneja]

Increase the DSS_FCLK and DSI_FCLK max supported frequencies, these come because
some frequencies were increased from OMAP5 ES1 to OMAP5 ES2. We support only
OMAP5 ES2 in the kernel, so replace the ES1 values with ES2 values. Increase the
DSI PLL Fint range, this was previously just copied from the OMAP4 param range
struct.

Fix the maximum DSS_FCLK on OMAP2, it's 133 Mhz according to the TRM.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dss_features.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 7f791ae..77dbe0c 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -414,7 +414,7 @@ static const char * const omap5_dss_clk_source_names[] = {
 };
 
 static const struct dss_param_range omap2_dss_param_range[] = {
-	[FEAT_PARAM_DSS_FCK]			= { 0, 173000000 },
+	[FEAT_PARAM_DSS_FCK]			= { 0, 133000000 },
 	[FEAT_PARAM_DSS_PCD]			= { 2, 255 },
 	[FEAT_PARAM_DSIPLL_REGN]		= { 0, 0 },
 	[FEAT_PARAM_DSIPLL_REGM]		= { 0, 0 },
@@ -459,15 +459,15 @@ static const struct dss_param_range omap4_dss_param_range[] = {
 };
 
 static const struct dss_param_range omap5_dss_param_range[] = {
-	[FEAT_PARAM_DSS_FCK]			= { 0, 200000000 },
+	[FEAT_PARAM_DSS_FCK]			= { 0, 209250000 },
 	[FEAT_PARAM_DSS_PCD]			= { 1, 255 },
 	[FEAT_PARAM_DSIPLL_REGN]		= { 0, (1 << 8) - 1 },
 	[FEAT_PARAM_DSIPLL_REGM]		= { 0, (1 << 12) - 1 },
 	[FEAT_PARAM_DSIPLL_REGM_DISPC]		= { 0, (1 << 5) - 1 },
 	[FEAT_PARAM_DSIPLL_REGM_DSI]		= { 0, (1 << 5) - 1 },
-	[FEAT_PARAM_DSIPLL_FINT]		= { 500000, 2500000 },
+	[FEAT_PARAM_DSIPLL_FINT]		= { 150000, 52000000 },
 	[FEAT_PARAM_DSIPLL_LPDIV]		= { 0, (1 << 13) - 1 },
-	[FEAT_PARAM_DSI_FCK]			= { 0, 170000000 },
+	[FEAT_PARAM_DSI_FCK]			= { 0, 209250000 },
 	[FEAT_PARAM_DOWNSCALE]			= { 1, 4 },
 	[FEAT_PARAM_LINEWIDTH]			= { 1, 2048 },
 };
-- 
1.7.10.4

Re: [PATCH v2 6/8] omapdss: Features: Fix some parameter ranges

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]

On 2013-03-26 15:45, Archit Taneja wrote:
> Increase the DSS_FCLK and DSI_FCLK max supported frequencies, these come because
> some frequencies were increased from OMAP5 ES1 to OMAP5 ES2. We support only
> OMAP5 ES2 in the kernel, so replace the ES1 values with ES2 values. Increase the
> DSI PLL Fint range, this was previously just copied from the OMAP4 param range
> struct.
> 
> Fix the maximum DSS_FCLK on OMAP2, it's 133 Mhz according to the TRM.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dss_features.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index 7f791ae..77dbe0c 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -414,7 +414,7 @@ static const char * const omap5_dss_clk_source_names[] = {
>  };
>  
>  static const struct dss_param_range omap2_dss_param_range[] = {
> -	[FEAT_PARAM_DSS_FCK]			= { 0, 173000000 },
> +	[FEAT_PARAM_DSS_FCK]			= { 0, 133000000 },
>  	[FEAT_PARAM_DSS_PCD]			= { 2, 255 },
>  	[FEAT_PARAM_DSIPLL_REGN]		= { 0, 0 },
>  	[FEAT_PARAM_DSIPLL_REGM]		= { 0, 0 },
> @@ -459,15 +459,15 @@ static const struct dss_param_range omap4_dss_param_range[] = {
>  };
>  
>  static const struct dss_param_range omap5_dss_param_range[] = {
> -	[FEAT_PARAM_DSS_FCK]			= { 0, 200000000 },
> +	[FEAT_PARAM_DSS_FCK]			= { 0, 209250000 },
>  	[FEAT_PARAM_DSS_PCD]			= { 1, 255 },
>  	[FEAT_PARAM_DSIPLL_REGN]		= { 0, (1 << 8) - 1 },
>  	[FEAT_PARAM_DSIPLL_REGM]		= { 0, (1 << 12) - 1 },
>  	[FEAT_PARAM_DSIPLL_REGM_DISPC]		= { 0, (1 << 5) - 1 },
>  	[FEAT_PARAM_DSIPLL_REGM_DSI]		= { 0, (1 << 5) - 1 },
> -	[FEAT_PARAM_DSIPLL_FINT]		= { 500000, 2500000 },
> +	[FEAT_PARAM_DSIPLL_FINT]		= { 150000, 52000000 },

Just a note, I think the PLL FINT range for OMAP3/4 may be wrong also.
Some TRMs mention the FINT range being up to 52MHz or so. I don't think
it's ever very clearly stated, though...

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 6/8] omapdss: Features: Fix some parameter ranges

[Archit Taneja]

On Wednesday 27 March 2013 01:03 PM, Tomi Valkeinen wrote:
> On 2013-03-26 15:45, Archit Taneja wrote:
>> Increase the DSS_FCLK and DSI_FCLK max supported frequencies, these come because
>> some frequencies were increased from OMAP5 ES1 to OMAP5 ES2. We support only
>> OMAP5 ES2 in the kernel, so replace the ES1 values with ES2 values. Increase the
>> DSI PLL Fint range, this was previously just copied from the OMAP4 param range
>> struct.
>>
>> Fix the maximum DSS_FCLK on OMAP2, it's 133 Mhz according to the TRM.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dss_features.c |    8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
>> index 7f791ae..77dbe0c 100644
>> --- a/drivers/video/omap2/dss/dss_features.c
>> +++ b/drivers/video/omap2/dss/dss_features.c
>> @@ -414,7 +414,7 @@ static const char * const omap5_dss_clk_source_names[] = {
>>   };
>>
>>   static const struct dss_param_range omap2_dss_param_range[] = {
>> -	[FEAT_PARAM_DSS_FCK]			= { 0, 173000000 },
>> +	[FEAT_PARAM_DSS_FCK]			= { 0, 133000000 },
>>   	[FEAT_PARAM_DSS_PCD]			= { 2, 255 },
>>   	[FEAT_PARAM_DSIPLL_REGN]		= { 0, 0 },
>>   	[FEAT_PARAM_DSIPLL_REGM]		= { 0, 0 },
>> @@ -459,15 +459,15 @@ static const struct dss_param_range omap4_dss_param_range[] = {
>>   };
>>
>>   static const struct dss_param_range omap5_dss_param_range[] = {
>> -	[FEAT_PARAM_DSS_FCK]			= { 0, 200000000 },
>> +	[FEAT_PARAM_DSS_FCK]			= { 0, 209250000 },
>>   	[FEAT_PARAM_DSS_PCD]			= { 1, 255 },
>>   	[FEAT_PARAM_DSIPLL_REGN]		= { 0, (1 << 8) - 1 },
>>   	[FEAT_PARAM_DSIPLL_REGM]		= { 0, (1 << 12) - 1 },
>>   	[FEAT_PARAM_DSIPLL_REGM_DISPC]		= { 0, (1 << 5) - 1 },
>>   	[FEAT_PARAM_DSIPLL_REGM_DSI]		= { 0, (1 << 5) - 1 },
>> -	[FEAT_PARAM_DSIPLL_FINT]		= { 500000, 2500000 },
>> +	[FEAT_PARAM_DSIPLL_FINT]		= { 150000, 52000000 },
>
> Just a note, I think the PLL FINT range for OMAP3/4 may be wrong also.
> Some TRMs mention the FINT range being up to 52MHz or so. I don't think
> it's ever very clearly stated, though...

I'll drop the FINT range modification for now. It's not that critical 
anyway since we manage to lock the PLL for most frequencies anyway. I'll 
do an update later after reading more detailed specs of the PLL.

Archit

[PATCH v2 7/8] OMAPDSS: DISPC: Configure doublestride for NV12 when using 2D Tiler buffers

[Archit Taneja]

When using a DISPC video pipeline to a fetch a NV12 buffer in a 2D container, we
need to set set a doublestride bit in the video pipe's ATTRIBUTES register. This
is needed because the stride for the UV plane(using a 16 bit Tiler container) is
double the stride for the Y plane(using a 8 bit Tiler container) for the 0 or
180 degree views. The ROW_INC register is meant for the Y plane, and the HW will
calculate the row increment needed for the UV plane by using double the stride
value based on whether this bit is set or not.

Set the bit when we are using a 2D Tiler buffer and when rotation is 0 or 180
degrees. The stride value is the same for 90 and 270 degree Tiler views, hence
the bit shouldn't be set.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 73a730a..ddbf031 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1588,6 +1588,7 @@ static void dispc_ovl_set_scaling(enum omap_plane plane,
 }
 
 static void dispc_ovl_set_rotation_attrs(enum omap_plane plane, u8 rotation,
+		enum omap_dss_rotation_type rotation_type,
 		bool mirroring, enum omap_color_mode color_mode)
 {
 	bool row_repeat = false;
@@ -1638,6 +1639,15 @@ static void dispc_ovl_set_rotation_attrs(enum omap_plane plane, u8 rotation,
 	if (dss_has_feature(FEAT_ROWREPEATENABLE))
 		REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane),
 			row_repeat ? 1 : 0, 18, 18);
+
+	if (color_mode == OMAP_DSS_COLOR_NV12) {
+		bool doublestride = (rotation_type == OMAP_DSS_ROT_TILER) &&
+					(rotation == OMAP_DSS_ROT_0 ||
+					rotation == OMAP_DSS_ROT_180);
+		/* DOUBLESTRIDE */
+		REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), doublestride, 22, 22);
+	}
+
 }
 
 static int color_mode_to_bpp(enum omap_color_mode color_mode)
@@ -2516,7 +2526,8 @@ static int dispc_ovl_setup_common(enum omap_plane plane,
 		dispc_ovl_set_vid_color_conv(plane, cconv);
 	}
 
-	dispc_ovl_set_rotation_attrs(plane, rotation, mirror, color_mode);
+	dispc_ovl_set_rotation_attrs(plane, rotation, rotation_type, mirror,
+			color_mode);
 
 	dispc_ovl_set_zorder(plane, caps, zorder);
 	dispc_ovl_set_pre_mult_alpha(plane, caps, pre_mult_alpha);
-- 
1.7.10.4

[PATCH v2 8/8] OMAPDSS: DISPC: Revert to older DISPC Smart Standby mechanism for OMAP5

[Archit Taneja]

DISPC on OMAP5 has a more optimised mechanism of asserting Mstandby to achieve
more power savings when DISPC is configured in Smart Standby mode. This
mechanism leads to underflows when multiple DISPC pipes are enabled.

There is a register field which can let us revert to the older mechanism of
asserting Mstandby. Configure this field to prevent underflows.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |    7 +++++++
 drivers/video/omap2/dss/dispc.h |    1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ddbf031..b33b016 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -87,6 +87,9 @@ struct dispc_features {
 
 	/* no DISPC_IRQ_FRAMEDONETV on this SoC */
 	bool no_framedone_tv:1;
+
+	/* revert to the OMAP4 mechanism of DISPC Smart Standby operation */
+	bool mstandby_workaround:1;
 };
 
 #define DISPC_MAX_NR_FIFOS 5
@@ -3502,6 +3505,9 @@ static void _omap_dispc_initial_config(void)
 	dispc_configure_burst_sizes();
 
 	dispc_ovl_enable_zorder_planes();
+
+	if (dispc.feat->mstandby_workaround)
+		REG_FLD_MOD(DISPC_MSTANDBY_CTRL, 1, 0, 0);
 }
 
 static const struct dispc_features omap24xx_dispc_feats __initconst = {
@@ -3596,6 +3602,7 @@ static const struct dispc_features omap54xx_dispc_feats __initconst = {
 	.calc_core_clk		=	calc_core_clk_44xx,
 	.num_fifos		=	5,
 	.gfx_fifo_workaround	=	true,
+	.mstandby_workaround	=	true,
 };
 
 static int __init dispc_init_features(struct platform_device *pdev)
diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index 222363c..de4863d 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -39,6 +39,7 @@
 #define DISPC_GLOBAL_BUFFER		0x0800
 #define DISPC_CONTROL3                  0x0848
 #define DISPC_CONFIG3                   0x084C
+#define DISPC_MSTANDBY_CTRL		0x0858
 
 /* DISPC overlay registers */
 #define DISPC_OVL_BA0(n)		(DISPC_OVL_BASE(n) + \
-- 
1.7.10.4

Re: [PATCH v2 0/8] omapdss/omapdrm: Misc fixes and improvements

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

On 2013-03-26 15:45, Archit Taneja wrote:
> These are misc fixes and improvements within omapdrm and omapdss. The fixes do the
> following:
> 
> - Make omapdrm smarter to choose the right overlay managers as it's crtcs. This
>   makes sure that omapdrm is functional for OMAP platforms with different
>   combinations of panels connected to it.
> 
> - Fix certain areas in omapdrm which allow fixed resolution panels to work.
> 
> - Fix functional and pixel clock limits for DISPC, this ensures we don't try
>   to try a mode that the hardware can't support.
> 
> - Some changes that came in OMAP5 ES2 silicon.
> 
> Note: The branch is based on the DSS misc series and dsi video mode calc series
> posted by Tomi. Reference branch:
> 
> git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git for-3.10/misc_drm_dss

Which of the fixes should go for 3.9? We need those separately, based on
the mainline.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 0/8] omapdss/omapdrm: Misc fixes and improvements

[Archit Taneja]

On Wednesday 27 March 2013 01:24 PM, Tomi Valkeinen wrote:
> On 2013-03-26 15:45, Archit Taneja wrote:
>> These are misc fixes and improvements within omapdrm and omapdss. The fixes do the
>> following:
>>
>> - Make omapdrm smarter to choose the right overlay managers as it's crtcs. This
>>    makes sure that omapdrm is functional for OMAP platforms with different
>>    combinations of panels connected to it.
>>
>> - Fix certain areas in omapdrm which allow fixed resolution panels to work.
>>
>> - Fix functional and pixel clock limits for DISPC, this ensures we don't try
>>    to try a mode that the hardware can't support.
>>
>> - Some changes that came in OMAP5 ES2 silicon.
>>
>> Note: The branch is based on the DSS misc series and dsi video mode calc series
>> posted by Tomi. Reference branch:
>>
>> git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git for-3.10/misc_drm_dss
>
> Which of the fixes should go for 3.9? We need those separately, based on
> the mainline.

The most important fix is omapdrm change which allocates crtcs in a 
better way, but it needs the 'dispc_channel' parameter in outputs, so 
that can't go.

We could queue patches 7 and 8 for 3.9, they are 2 things which we would 
want to have in 3.9 itself. These should apply on mainline directly.

Archit