[PATCHv3 0/8] omapdrm: DSI command mode panel support

[PATCHv3 0/8] omapdrm: DSI command mode panel support

[Sebastian Reichel]

Hi,

These are the remaining patches from my previous patchset to get
Droid 4 (OMAP4) display working. The patches have been rebased to
linux-next tag next-20180329. I skipped the OMAP3 support patches.
They should get their own series, once this patchset has landed.

Working on Droid 4:
 * Framebuffer Console, updated at 1Hz due to blinking cursor
 * kmstest (static image)
 * Display blanking
 * Xorg with omap and modesetting driver
 * No updates send when nothing needs to be sent
 * Orientation DRM property is attached to the DSI panel

Known issues:
 * OMAP3 support is missing

Changes since PATCHv2:
 * Drop omap3 quirk patch (OMAP3 should get its own mini-series)
 * Rebase to current linux-next
 * Use existing 'rotation' DT property to set DRM orientation hint
 * Add Tested-by from Tony

Changes since PATCHv1:
 * Drop patches, that were queued by Tomi
 * Rebase to current master
 * Rework the omap3 workaround patch to only affect omap3
 * Add orientation DRM property support

-- Sebastian

Sebastian Reichel (8):
  drm/omap: add framedone interrupt support
  drm/omap: add manual update detection helper
  drm/omap: add support for manually updated displays
  drm/omap: make omap_framebuffer_get_next_connector static
  dt-bindings: panel: common: document orientation property
  drm/omap: add support for rotation hints from display drivers
  drm/omap: panel-dsi-cm: add rotation support
  ARM: dts: omap4-droid4: Add LCD panel rotation property

 .../bindings/display/panel/panel-common.txt        |  12 ++
 .../devicetree/bindings/display/panel/panel.txt    |   4 -
 arch/arm/boot/dts/omap4-droid4-xt894.dts           |   1 +
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c    |  13 ++
 drivers/gpu/drm/omapdrm/dss/omapdss.h              |   2 +
 drivers/gpu/drm/omapdrm/omap_connector.c           |  42 +++++-
 drivers/gpu/drm/omapdrm/omap_connector.h           |   1 +
 drivers/gpu/drm/omapdrm/omap_crtc.c                | 155 +++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h                |   2 +
 drivers/gpu/drm/omapdrm/omap_fb.c                  |  74 ++++++----
 drivers/gpu/drm/omapdrm/omap_fb.h                  |   2 -
 drivers/gpu/drm/omapdrm/omap_irq.c                 |  25 ++++
 drivers/gpu/drm/omapdrm/omap_irq.h                 |   1 +
 13 files changed, 292 insertions(+), 42 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt

-- 
2.16.2

[PATCHv3 1/8] drm/omap: add framedone interrupt support

[Sebastian Reichel]

This prepares framedone interrupt handling for
manual display update support.

Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 48 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/omap_crtc.h |  1 +
 drivers/gpu/drm/omapdrm/omap_irq.c  | 25 +++++++++++++++++++
 drivers/gpu/drm/omapdrm/omap_irq.h  |  1 +
 4 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 6c4d40b824e4..b893985e4efb 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -51,6 +51,9 @@ struct omap_crtc {
 	bool pending;
 	wait_queue_head_t pending_wait;
 	struct drm_pending_vblank_event *event;
+
+	void (*framedone_handler)(void *);
+	void *framedone_handler_data;
 };
 
 /* -----------------------------------------------------------------------------
@@ -256,6 +259,17 @@ static int omap_crtc_dss_register_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct omap_crtc *omap_crtc = omap_crtcs[channel];
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	if (omap_crtc->framedone_handler)
+		return -EBUSY;
+
+	dev_dbg(dev->dev, "register framedone %s", omap_crtc->name);
+
+	omap_crtc->framedone_handler = handler;
+	omap_crtc->framedone_handler_data = data;
+
 	return 0;
 }
 
@@ -263,6 +277,16 @@ static void omap_crtc_dss_unregister_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct omap_crtc *omap_crtc = omap_crtcs[channel];
+	struct drm_device *dev = omap_crtc->base.dev;
+
+	dev_dbg(dev->dev, "unregister framedone %s", omap_crtc->name);
+
+	WARN_ON(omap_crtc->framedone_handler != handler);
+	WARN_ON(omap_crtc->framedone_handler_data != data);
+
+	omap_crtc->framedone_handler = NULL;
+	omap_crtc->framedone_handler_data = NULL;
 }
 
 static const struct dss_mgr_ops mgr_ops = {
@@ -330,6 +354,30 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 	DBG("%s: apply done", omap_crtc->name);
 }
 
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	if (!omap_crtc->framedone_handler) {
+		dev_warn(omap_crtc->base.dev->dev, "no framedone handler?");
+		return;
+	}
+
+	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
+
+	spin_lock(&crtc->dev->event_lock);
+	/* Send the vblank event if one has been requested. */
+	if (omap_crtc->event) {
+		drm_crtc_send_vblank_event(crtc, omap_crtc->event);
+		omap_crtc->event = NULL;
+	}
+	omap_crtc->pending = false;
+	spin_unlock(&crtc->dev->event_lock);
+
+	/* Wake up omap_atomic_complete. */
+	wake_up(&omap_crtc->pending_wait);
+}
+
 static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index eaab2d7f0324..c748adc91665 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -39,5 +39,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 int omap_crtc_wait_pending(struct drm_crtc *crtc);
 void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
 void omap_crtc_vblank_irq(struct drm_crtc *crtc);
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);
 
 #endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index c85115049f86..0171e5e74aac 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -85,6 +85,28 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
 	return ret == 0 ? -1 : 0;
 }
 
+int omap_irq_enable_framedone(struct drm_crtc *crtc, bool enable)
+{
+	struct drm_device *dev = crtc->dev;
+	struct omap_drm_private *priv = dev->dev_private;
+	unsigned long flags;
+	enum omap_channel channel = omap_crtc_channel(crtc);
+	int framedone_irq =
+		priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel);
+
+	DBG("dev=%p, crtc=%u, enable=%d", dev, channel, enable);
+
+	spin_lock_irqsave(&priv->wait_lock, flags);
+	if (enable)
+		priv->irq_mask |= framedone_irq;
+	else
+		priv->irq_mask &= ~framedone_irq;
+	omap_irq_update(dev);
+	spin_unlock_irqrestore(&priv->wait_lock, flags);
+
+	return 0;
+}
+
 /**
  * enable_vblank - enable vblank interrupt events
  * @dev: DRM device
@@ -217,6 +239,9 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)
 
 		if (irqstatus & priv->dispc_ops->mgr_get_sync_lost_irq(priv->dispc, channel))
 			omap_crtc_error_irq(crtc, irqstatus);
+
+		if (irqstatus & priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel))
+			omap_crtc_framedone_irq(crtc, irqstatus);
 	}
 
 	omap_irq_ocp_error_handler(dev, irqstatus);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.h b/drivers/gpu/drm/omapdrm/omap_irq.h
index 9d5441468eca..02abb4ed9813 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.h
+++ b/drivers/gpu/drm/omapdrm/omap_irq.h
@@ -27,6 +27,7 @@ struct drm_device;
 struct omap_irq_wait;
 
 int omap_irq_enable_vblank(struct drm_crtc *crtc);
+int omap_irq_enable_framedone(struct drm_crtc *crtc, bool enable);
 void omap_irq_disable_vblank(struct drm_crtc *crtc);
 void omap_drm_irq_uninstall(struct drm_device *dev);
 int omap_drm_irq_install(struct drm_device *dev);
-- 
2.16.2

[PATCHv3 2/8] drm/omap: add manual update detection helper

[Sebastian Reichel]

In preparation for manually updated display support, such as DSI
command mode panels, this adds a simple helper to see if a connector
is manually updated.

Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 8 ++++++++
 drivers/gpu/drm/omapdrm/omap_connector.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index a0d7b1d905e8..a33b51484b2d 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -57,6 +57,14 @@ bool omap_connector_get_hdmi_mode(struct drm_connector *connector)
 	return omap_connector->hdmi_mode;
 }
 
+bool omap_connector_get_manually_updated(struct drm_connector *connector)
+{
+	struct omap_connector *omap_connector = to_omap_connector(connector);
+
+	return !!(omap_connector->dssdev->caps &
+		  OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE);
+}
+
 static enum drm_connector_status omap_connector_detect(
 		struct drm_connector *connector, bool force)
 {
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.h b/drivers/gpu/drm/omapdrm/omap_connector.h
index 98bbc779b302..652136d167f5 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.h
+++ b/drivers/gpu/drm/omapdrm/omap_connector.h
@@ -33,5 +33,6 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 struct drm_encoder *omap_connector_attached_encoder(
 		struct drm_connector *connector);
 bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
+bool omap_connector_get_manually_updated(struct drm_connector *connector);
 
 #endif /* __OMAPDRM_CONNECTOR_H__ */
-- 
2.16.2

[PATCHv3 3/8] drm/omap: add support for manually updated displays

[Sebastian Reichel]

This adds the required infrastructure for manually
updated displays, such as DSI command mode panels.

While those panels often support partial updates
we currently always do a full refresh. Display
will be refreshed when something calls the dirty
callback, such as libdrm's drmModeDirtyFB().

This is currently being implemented for the kernel
console and for Xorg. Weston currently does not
implement this and is known not to work on manually
updated displays.

Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 107 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
 drivers/gpu/drm/omapdrm/omap_fb.c   |  20 +++++++
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index b893985e4efb..1b91bff5bac6 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -51,6 +51,7 @@ struct omap_crtc {
 	bool pending;
 	wait_queue_head_t pending_wait;
 	struct drm_pending_vblank_event *event;
+	struct delayed_work update_work;
 
 	void (*framedone_handler)(void *);
 	void *framedone_handler_data;
@@ -146,6 +147,25 @@ static void omap_crtc_dss_disconnect(struct omap_drm_private *priv,
 static void omap_crtc_dss_start_update(struct omap_drm_private *priv,
 				       enum omap_channel channel)
 {
+	priv->dispc_ops->mgr_enable(priv->dispc, channel, true);
+}
+
+static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
+{
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	bool result = false;
+
+	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		if (connector->state->crtc != crtc)
+			continue;
+		result = omap_connector_get_manually_updated(connector);
+		break;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
+	return result;
 }
 
 /* Called only from the encoder enable/disable and suspend/resume handlers. */
@@ -157,12 +177,17 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	enum omap_channel channel = omap_crtc->channel;
 	struct omap_irq_wait *wait;
 	u32 framedone_irq, vsync_irq;
+	bool is_manual = omap_crtc_is_manually_updated(crtc);
+	enum omap_display_type type = omap_crtc_output[channel]->output_type;
 	int ret;
 
 	if (WARN_ON(omap_crtc->enabled == enable))
 		return;
 
-	if (omap_crtc_output[channel]->output_type == OMAP_DISPLAY_TYPE_HDMI) {
+	if (is_manual)
+		omap_irq_enable_framedone(crtc, enable);
+
+	if (is_manual || type == OMAP_DISPLAY_TYPE_HDMI) {
 		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
 		omap_crtc->enabled = enable;
 		return;
@@ -214,7 +239,6 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	}
 }
 
-
 static int omap_crtc_dss_enable(struct omap_drm_private *priv,
 				enum omap_channel channel)
 {
@@ -378,6 +402,53 @@ void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
 	wake_up(&omap_crtc->pending_wait);
 }
 
+void omap_crtc_flush(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	if (!omap_crtc_is_manually_updated(crtc))
+		return;
+
+	if (!delayed_work_pending(&omap_crtc->update_work))
+		schedule_delayed_work(&omap_crtc->update_work, 0);
+}
+
+static void omap_crtc_manual_display_update(struct work_struct *data)
+{
+	struct omap_crtc *omap_crtc =
+			container_of(data, struct omap_crtc, update_work.work);
+	struct omap_dss_device *dssdev = omap_crtc_output[omap_crtc->channel];
+	struct drm_device *dev = omap_crtc->base.dev;
+	struct omap_dss_driver *dssdrv;
+	int ret, width, height;
+
+	if (!dssdev || !dssdev->dst) {
+		dev_err_once(dev->dev, "missing dssdev!");
+		return;
+	}
+
+	dssdev = dssdev->dst;
+	dssdrv = dssdev->driver;
+
+	if (!dssdrv || !dssdrv->update) {
+		dev_err_once(dev->dev, "incorrect dssdrv!");
+		return;
+	}
+
+	if (dssdrv->sync)
+		dssdrv->sync(dssdev);
+
+	width = dssdev->panel.vm.hactive;
+	height = dssdev->panel.vm.vactive;
+	ret = dssdrv->update(dssdev, 0, 0, width, height);
+	if (ret < 0) {
+		spin_lock_irq(&dev->event_lock);
+		omap_crtc->pending = false;
+		spin_unlock_irq(&dev->event_lock);
+		wake_up(&omap_crtc->pending_wait);
+	}
+}
+
 static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
@@ -430,6 +501,10 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	DBG("%s", omap_crtc->name);
 
+	/* manual updated display will not trigger vsync irq */
+	if (omap_crtc_is_manually_updated(crtc))
+		return;
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	drm_crtc_vblank_on(crtc);
 	ret = drm_crtc_vblank_get(crtc);
@@ -443,6 +518,7 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
 				     struct drm_crtc_state *old_state)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
@@ -453,6 +529,11 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
 
+	cancel_delayed_work(&omap_crtc->update_work);
+
+	if (!omap_crtc_wait_pending(crtc))
+		dev_warn(dev->dev, "manual display update did not finish!");
+
 	drm_crtc_vblank_off(crtc);
 }
 
@@ -603,13 +684,20 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	DBG("%s: GO", omap_crtc->name);
 
-	ret = drm_crtc_vblank_get(crtc);
-	WARN_ON(ret != 0);
+	if (!omap_crtc_is_manually_updated(crtc)) {
+		ret = drm_crtc_vblank_get(crtc);
+		WARN_ON(ret != 0);
 
-	spin_lock_irq(&crtc->dev->event_lock);
-	priv->dispc_ops->mgr_go(priv->dispc, omap_crtc->channel);
-	omap_crtc_arm_event(crtc);
-	spin_unlock_irq(&crtc->dev->event_lock);
+		spin_lock_irq(&crtc->dev->event_lock);
+		priv->dispc_ops->mgr_go(priv->dispc, omap_crtc->channel);
+		omap_crtc_arm_event(crtc);
+		spin_unlock_irq(&crtc->dev->event_lock);
+	} else {
+		spin_lock_irq(&crtc->dev->event_lock);
+		omap_crtc_flush(crtc);
+		omap_crtc_arm_event(crtc);
+		spin_unlock_irq(&crtc->dev->event_lock);
+	}
 }
 
 static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
@@ -771,6 +859,9 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 
+	INIT_DELAYED_WORK(&omap_crtc->update_work,
+			  omap_crtc_manual_display_update);
+
 	ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
 					&omap_crtc_funcs, NULL);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index c748adc91665..300b6498d03e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -40,5 +40,6 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc);
 void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
 void omap_crtc_vblank_irq(struct drm_crtc *crtc);
 void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);
+void omap_crtc_flush(struct drm_crtc *crtc);
 
 #endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 5fd22ca73913..b65212c9a423 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -95,8 +95,28 @@ static void omap_framebuffer_destroy(struct drm_framebuffer *fb)
 	kfree(omap_fb);
 }
 
+static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
+				  struct drm_file *file_priv,
+				  unsigned flags, unsigned color,
+				  struct drm_clip_rect *clips,
+				  unsigned num_clips)
+{
+	struct drm_connector *connector = NULL;
+
+	drm_modeset_lock_all(fb->dev);
+
+	while ((connector = omap_framebuffer_get_next_connector(fb, connector)))
+		if (connector->encoder && connector->encoder->crtc)
+			omap_crtc_flush(connector->encoder->crtc);
+
+	drm_modeset_unlock_all(fb->dev);
+
+	return 0;
+}
+
 static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
 	.create_handle = omap_framebuffer_create_handle,
+	.dirty = omap_framebuffer_dirty,
 	.destroy = omap_framebuffer_destroy,
 };
 
-- 
2.16.2

[PATCHv3 4/8] drm/omap: make omap_framebuffer_get_next_connector static

[Sebastian Reichel]

omap_framebuffer_get_next_connector() is only required within omap_fb,
so let's make it static. The function had to be moved before the first
(and only) user.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 54 +++++++++++++++++++--------------------
 drivers/gpu/drm/omapdrm/omap_fb.h |  2 --
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index b65212c9a423..3f0b1f976691 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -95,6 +95,33 @@ static void omap_framebuffer_destroy(struct drm_framebuffer *fb)
 	kfree(omap_fb);
 }
 
+/* iterate thru all the connectors, returning ones that are attached
+ * to the same fb..
+ */
+static struct drm_connector *omap_framebuffer_get_next_connector(
+		struct drm_framebuffer *fb, struct drm_connector *from)
+{
+	struct drm_device *dev = fb->dev;
+	struct list_head *connector_list = &dev->mode_config.connector_list;
+	struct drm_connector *connector = from;
+
+	if (!from)
+		return list_first_entry_or_null(connector_list, typeof(*from),
+						head);
+
+	list_for_each_entry_from(connector, connector_list, head) {
+		if (connector != from) {
+			struct drm_encoder *encoder = connector->encoder;
+			struct drm_crtc *crtc = encoder ? encoder->crtc : NULL;
+			if (crtc && crtc->primary->fb == fb)
+				return connector;
+
+		}
+	}
+
+	return NULL;
+}
+
 static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
 				  struct drm_file *file_priv,
 				  unsigned flags, unsigned color,
@@ -329,33 +356,6 @@ void omap_framebuffer_unpin(struct drm_framebuffer *fb)
 	mutex_unlock(&omap_fb->lock);
 }
 
-/* iterate thru all the connectors, returning ones that are attached
- * to the same fb..
- */
-struct drm_connector *omap_framebuffer_get_next_connector(
-		struct drm_framebuffer *fb, struct drm_connector *from)
-{
-	struct drm_device *dev = fb->dev;
-	struct list_head *connector_list = &dev->mode_config.connector_list;
-	struct drm_connector *connector = from;
-
-	if (!from)
-		return list_first_entry_or_null(connector_list, typeof(*from),
-						head);
-
-	list_for_each_entry_from(connector, connector_list, head) {
-		if (connector != from) {
-			struct drm_encoder *encoder = connector->encoder;
-			struct drm_crtc *crtc = encoder ? encoder->crtc : NULL;
-			if (crtc && crtc->primary->fb == fb)
-				return connector;
-
-		}
-	}
-
-	return NULL;
-}
-
 #ifdef CONFIG_DEBUG_FS
 void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
index 94ad5f9e4404..c20cb4bc714d 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.h
+++ b/drivers/gpu/drm/omapdrm/omap_fb.h
@@ -38,8 +38,6 @@ int omap_framebuffer_pin(struct drm_framebuffer *fb);
 void omap_framebuffer_unpin(struct drm_framebuffer *fb);
 void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 		struct drm_plane_state *state, struct omap_overlay_info *info);
-struct drm_connector *omap_framebuffer_get_next_connector(
-		struct drm_framebuffer *fb, struct drm_connector *from);
 bool omap_framebuffer_supports_rotation(struct drm_framebuffer *fb);
 void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
 
-- 
2.16.2

[PATCHv3 5/8] dt-bindings: panel: common: document orientation property

[Sebastian Reichel]

Merge "rotation" property description into common panel
binding.

Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 .../devicetree/bindings/display/panel/panel-common.txt       | 12 ++++++++++++
 Documentation/devicetree/bindings/display/panel/panel.txt    |  4 ----
 2 files changed, 12 insertions(+), 4 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt

diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
index 557fa765adcb..46ff3c2d0811 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-common.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
@@ -18,6 +18,18 @@ Descriptive Properties
   physical area where images are displayed. These properties are expressed in
   millimeters and rounded to the closest unit.
 
+- rotation: Display rotation in degrees counter clockwise (0,90,180,270).
+  The following values are possible:
+
+    0 = The top side of the panel matches the top side of the
+        device's casing (default).
+   90 = The top side of the panel matches the left side of the
+        device's casing.
+  180 = The top side of the panel matches the bottom side of
+        the device's casing. The panel is mounted upside-down.
+  270 = The top side of the panel matches the right side of
+        the device's casing.
+
 - label: The label property specifies a symbolic name for the panel as a
   string suitable for use by humans. It typically contains a name inscribed on
   the system (e.g. as an affixed label) or specified in the system's
diff --git a/Documentation/devicetree/bindings/display/panel/panel.txt b/Documentation/devicetree/bindings/display/panel/panel.txt
deleted file mode 100644
index e2e6867852b8..000000000000
--- a/Documentation/devicetree/bindings/display/panel/panel.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-Common display properties
--------------------------
-
-- rotation:	Display rotation in degrees counter clockwise (0,90,180,270)
-- 
2.16.2

[PATCHv3 6/8] drm/omap: add support for rotation hints from display drivers

[Sebastian Reichel]

This adds support for setting DRM panel orientation property
based on rotation information from the display driver.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  2 ++
 drivers/gpu/drm/omapdrm/omap_connector.c | 34 +++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 14d74adb13fb..01a2a6638f70 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -554,6 +554,8 @@ struct omap_dss_driver {
 			    struct videomode *vm);
 	void (*get_size)(struct omap_dss_device *dssdev,
 			 unsigned int *width, unsigned int *height);
+	void (*get_rotation)(struct omap_dss_device *dssdev,
+				int *rotation);
 
 	int (*set_wss)(struct omap_dss_device *dssdev, u32 wss);
 	u32 (*get_wss)(struct omap_dss_device *dssdev);
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index a33b51484b2d..f8ae8bdf9211 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -249,6 +249,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	struct drm_connector *connector = NULL;
 	struct omap_connector *omap_connector;
 	bool hpd_supported = false;
+	int rotation = -1;
+	int ret;
 
 	DBG("%s", dssdev->name);
 
@@ -267,7 +269,7 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	drm_connector_helper_add(connector, &omap_connector_helper_funcs);
 
 	if (dssdev->driver->register_hpd_cb) {
-		int ret = dssdev->driver->register_hpd_cb(dssdev,
+		ret = dssdev->driver->register_hpd_cb(dssdev,
 							  omap_connector_hpd_cb,
 							  omap_connector);
 		if (!ret)
@@ -288,6 +290,36 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
+	if (dssdev->driver->get_rotation)
+		dssdev->driver->get_rotation(dssdev, &rotation);
+
+	switch (rotation) {
+	case 0:
+		connector->display_info.panel_orientation =
+			DRM_MODE_PANEL_ORIENTATION_NORMAL;
+		break;
+	case 90:
+		connector->display_info.panel_orientation =
+			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+		break;
+	case 180:
+		connector->display_info.panel_orientation =
+			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+		break;
+	case 270:
+		connector->display_info.panel_orientation =
+			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+		break;
+	default:
+		connector->display_info.panel_orientation =
+			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+		break;
+	}
+
+	ret = drm_connector_init_panel_orientation_property(connector, 0, 0);
+	if (ret)
+		DBG("%s: Failed to init orientation property (%d)", dssdev->name, ret);
+
 	return connector;
 
 fail:
-- 
2.16.2

[PATCHv3 7/8] drm/omap: panel-dsi-cm: add rotation support

[Sebastian Reichel]

From: Sebastian Reichel <sre@kernel.org>

Add support to inform the DRM subsystem about the orientation
the display has been mounted to the casing.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 428de90fced1..21a828f88d30 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -68,6 +68,7 @@ struct panel_drv_data {
 
 	int width_mm;
 	int height_mm;
+	int rotation;
 
 	struct omap_dsi_pin_config pin_config;
 
@@ -1210,6 +1211,14 @@ static void dsicm_get_size(struct omap_dss_device *dssdev,
 	*height = ddata->height_mm;
 }
 
+static void dsicm_get_rotation(struct omap_dss_device *dssdev,
+			       int *rotation)
+{
+	struct panel_drv_data *ddata = to_panel_data(dssdev);
+
+	*rotation = ddata->rotation;
+}
+
 static struct omap_dss_driver dsicm_ops = {
 	.connect	= dsicm_connect,
 	.disconnect	= dsicm_disconnect,
@@ -1223,6 +1232,7 @@ static struct omap_dss_driver dsicm_ops = {
 	.get_timings	= dsicm_get_timings,
 	.check_timings	= dsicm_check_timings,
 	.get_size	= dsicm_get_size,
+	.get_rotation   = dsicm_get_rotation,
 
 	.enable_te	= dsicm_enable_te,
 	.get_te		= dsicm_get_te,
@@ -1270,6 +1280,9 @@ static int dsicm_probe_of(struct platform_device *pdev)
 	ddata->height_mm = 0;
 	of_property_read_u32(node, "height-mm", &ddata->height_mm);
 
+	ddata->rotation = -1;
+	of_property_read_u32(node, "rotation", &ddata->rotation);
+
 	ddata->vpnl = devm_regulator_get_optional(&pdev->dev, "vpnl");
 	if (IS_ERR(ddata->vpnl)) {
 		err = PTR_ERR(ddata->vpnl);
-- 
2.16.2

[PATCHv3 8/8] ARM: dts: omap4-droid4: Add LCD panel rotation property

[Sebastian Reichel]

This adds a LCD panel rotation hint to the Droid 4. If the
display is rotated this way the keyboard can be used properly.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/arm/boot/dts/omap4-droid4-xt894.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index bdf73cbcec3a..3ed090a9db6c 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -214,6 +214,7 @@
 
 		width-mm = <50>;
 		height-mm = <89>;
+		rotation = <90>;
 		backlight = <&lcd_backlight>;
 
 		panel-timing {
-- 
2.16.2

Re: [PATCHv3 5/8] dt-bindings: panel: common: document orientation property

[Pavel Machek]

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

Hi!

> +- rotation: Display rotation in degrees counter clockwise (0,90,180,270).
> +  The following values are possible:
> +
> +    0 = The top side of the panel matches the top side of the
> +        device's casing (default).

This is still ambiguous for devices such as N900 and Droid4, which can
be used in two modes.

"For phone-like devices that can be used in two orientations, top side
is the one that is pointing up in keyboard-closed, portrait mode."

?


Best regards,
									Pavel
									
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv3 8/8] ARM: dts: omap4-droid4: Add LCD panel rotation property

[Pavel Machek]

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

On Fri 2018-03-30 19:18:22, Sebastian Reichel wrote:
> This adds a LCD panel rotation hint to the Droid 4. If the
> display is rotated this way the keyboard can be used properly.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

Hmm. Aha. And this the place where confusion matters. I believe most
users would consider top to be elsewhere on Droid 4.... but this
indeed makes sense for keyboard...

								Pavel

>  arch/arm/boot/dts/omap4-droid4-xt894.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
> index bdf73cbcec3a..3ed090a9db6c 100644
> --- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
> +++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
> @@ -214,6 +214,7 @@
>  
>  		width-mm = <50>;
>  		height-mm = <89>;
> +		rotation = <90>;
>  		backlight = <&lcd_backlight>;
>  
>  		panel-timing {

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv3 7/8] drm/omap: panel-dsi-cm: add rotation support

[Pavel Machek]

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

On Fri 2018-03-30 19:18:21, Sebastian Reichel wrote:
> From: Sebastian Reichel <sre@kernel.org>
> 
> Add support to inform the DRM subsystem about the orientation
> the display has been mounted to the casing.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

1-5,7: Reviewed-by: Pavel Machek <pavel@ucw.cz>

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv3 5/8] dt-bindings: panel: common: document orientation property

[Rob Herring]

On Fri, Mar 30, 2018 at 07:18:19PM +0200, Sebastian Reichel wrote:
> Merge "rotation" property description into common panel
> binding.
> 
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  .../devicetree/bindings/display/panel/panel-common.txt       | 12 ++++++++++++
>  Documentation/devicetree/bindings/display/panel/panel.txt    |  4 ----
>  2 files changed, 12 insertions(+), 4 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt

Reviewed-by: Rob Herring <robh@kernel.org>

Thanks for consolidating.

Rob

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Tomi Valkeinen]

Hi Sebastian,

On 30/03/18 20:18, Sebastian Reichel wrote:
> This adds the required infrastructure for manually
> updated displays, such as DSI command mode panels.
> 
> While those panels often support partial updates
> we currently always do a full refresh. Display
> will be refreshed when something calls the dirty
> callback, such as libdrm's drmModeDirtyFB().
> 
> This is currently being implemented for the kernel
> console and for Xorg. Weston currently does not
> implement this and is known not to work on manually
> updated displays.
> 
> Tested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 107 +++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
>  drivers/gpu/drm/omapdrm/omap_fb.c   |  20 +++++++
>  3 files changed, 120 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index b893985e4efb..1b91bff5bac6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -51,6 +51,7 @@ struct omap_crtc {
>  	bool pending;
>  	wait_queue_head_t pending_wait;
>  	struct drm_pending_vblank_event *event;
> +	struct delayed_work update_work;
>  
>  	void (*framedone_handler)(void *);
>  	void *framedone_handler_data;
> @@ -146,6 +147,25 @@ static void omap_crtc_dss_disconnect(struct omap_drm_private *priv,
>  static void omap_crtc_dss_start_update(struct omap_drm_private *priv,
>  				       enum omap_channel channel)
>  {
> +	priv->dispc_ops->mgr_enable(priv->dispc, channel, true);
> +}
> +
> +static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
> +{
> +	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +	bool result = false;
> +
> +	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		if (connector->state->crtc != crtc)
> +			continue;
> +		result = omap_connector_get_manually_updated(connector);
> +		break;
> +	}
> +	drm_connector_list_iter_end(&conn_iter);

It would be much nicer if the is-manual flag was somehow conveyed from
connector/encoder to the crtc when doing modesetting. I don't know how
or where, so just thinking out loud. However, if we need to do such loop
as above, I think we should just do it once, perhaps in
omap_crtc_atomic_enable, and store the value in omap_crtc's private data.

> +
> +	return result;
>  }
>  
>  /* Called only from the encoder enable/disable and suspend/resume handlers. */
> @@ -157,12 +177,17 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
>  	enum omap_channel channel = omap_crtc->channel;
>  	struct omap_irq_wait *wait;
>  	u32 framedone_irq, vsync_irq;
> +	bool is_manual = omap_crtc_is_manually_updated(crtc);
> +	enum omap_display_type type = omap_crtc_output[channel]->output_type;
>  	int ret;
>  
>  	if (WARN_ON(omap_crtc->enabled == enable))
>  		return;
>  
> -	if (omap_crtc_output[channel]->output_type == OMAP_DISPLAY_TYPE_HDMI) {
> +	if (is_manual)
> +		omap_irq_enable_framedone(crtc, enable);
> +
> +	if (is_manual || type == OMAP_DISPLAY_TYPE_HDMI) {
>  		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
>  		omap_crtc->enabled = enable;
>  		return;

This doesn't look correct, omap_crtc_dss_start_update() already sets the
enable bit for manual update displays. And you don't want to set the
enable to false with manual update displays.

HDMI handling here is already a special case due to HW issues, so I'd
rather see the manual update handled separately from the HDMI.

> @@ -214,7 +239,6 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
>  	}
>  }
>  
> -
>  static int omap_crtc_dss_enable(struct omap_drm_private *priv,
>  				enum omap_channel channel)
>  {
> @@ -378,6 +402,53 @@ void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
>  	wake_up(&omap_crtc->pending_wait);
>  }
>  
> +void omap_crtc_flush(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
> +	if (!omap_crtc_is_manually_updated(crtc))
> +		return;
> +
> +	if (!delayed_work_pending(&omap_crtc->update_work))
> +		schedule_delayed_work(&omap_crtc->update_work, 0);
> +}

Why delayed work here?

It's actually not quite clear to me how manual update displays work with
DRM...

As far as I see, we have essentially two cases: 1) single buffering,
where the userspace must set an area in the fb dirty, which then
triggers the update, 2) multi buffering, which doesn't need fb dirty,
but just a page flip which triggers the update.

In the 2) case (which I think is the optimal case which all the modern
apps should use), there's no need for delayed work or any work, and the
code flow should be very similar to the auto-update model.

Also, as Daniel mentioned in "drm/omapdrm: Nuke
omap_framebuffer_get_next_connector()" thread, the dirtying mechanism in
the series is not valid.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Daniel Vetter]

On Fri, Apr 20, 2018 at 10:09:38AM +0300, Tomi Valkeinen wrote:
> Hi Sebastian,
> 
> On 30/03/18 20:18, Sebastian Reichel wrote:
> > This adds the required infrastructure for manually
> > updated displays, such as DSI command mode panels.
> > 
> > While those panels often support partial updates
> > we currently always do a full refresh. Display
> > will be refreshed when something calls the dirty
> > callback, such as libdrm's drmModeDirtyFB().
> > 
> > This is currently being implemented for the kernel
> > console and for Xorg. Weston currently does not
> > implement this and is known not to work on manually
> > updated displays.
> > 
> > Tested-by: Tony Lindgren <tony@atomide.com>
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/omapdrm/omap_crtc.c | 107 +++++++++++++++++++++++++++++++++---
> >  drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
> >  drivers/gpu/drm/omapdrm/omap_fb.c   |  20 +++++++
> >  3 files changed, 120 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> > index b893985e4efb..1b91bff5bac6 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> > @@ -51,6 +51,7 @@ struct omap_crtc {
> >  	bool pending;
> >  	wait_queue_head_t pending_wait;
> >  	struct drm_pending_vblank_event *event;
> > +	struct delayed_work update_work;
> >  
> >  	void (*framedone_handler)(void *);
> >  	void *framedone_handler_data;
> > @@ -146,6 +147,25 @@ static void omap_crtc_dss_disconnect(struct omap_drm_private *priv,
> >  static void omap_crtc_dss_start_update(struct omap_drm_private *priv,
> >  				       enum omap_channel channel)
> >  {
> > +	priv->dispc_ops->mgr_enable(priv->dispc, channel, true);
> > +}
> > +
> > +static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
> > +{
> > +	struct drm_connector *connector;
> > +	struct drm_connector_list_iter conn_iter;
> > +	bool result = false;
> > +
> > +	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> > +	drm_for_each_connector_iter(connector, &conn_iter) {
> > +		if (connector->state->crtc != crtc)
> > +			continue;
> > +		result = omap_connector_get_manually_updated(connector);
> > +		break;
> > +	}
> > +	drm_connector_list_iter_end(&conn_iter);
> 
> It would be much nicer if the is-manual flag was somehow conveyed from
> connector/encoder to the crtc when doing modesetting. I don't know how
> or where, so just thinking out loud. However, if we need to do such loop
> as above, I think we should just do it once, perhaps in
> omap_crtc_atomic_enable, and store the value in omap_crtc's private data.

Do it in your atomic_check code, and store it as a omap private bit in
omap_crtc_state. That guarantees you that it's always up-to-date, and will
never change (except when you get a completely new state).

Recomputing derived state like this as part of your atomic_commit code
isn't recommended much.
-Daniel

> 
> > +
> > +	return result;
> >  }
> >  
> >  /* Called only from the encoder enable/disable and suspend/resume handlers. */
> > @@ -157,12 +177,17 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
> >  	enum omap_channel channel = omap_crtc->channel;
> >  	struct omap_irq_wait *wait;
> >  	u32 framedone_irq, vsync_irq;
> > +	bool is_manual = omap_crtc_is_manually_updated(crtc);
> > +	enum omap_display_type type = omap_crtc_output[channel]->output_type;
> >  	int ret;
> >  
> >  	if (WARN_ON(omap_crtc->enabled == enable))
> >  		return;
> >  
> > -	if (omap_crtc_output[channel]->output_type == OMAP_DISPLAY_TYPE_HDMI) {
> > +	if (is_manual)
> > +		omap_irq_enable_framedone(crtc, enable);
> > +
> > +	if (is_manual || type == OMAP_DISPLAY_TYPE_HDMI) {
> >  		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
> >  		omap_crtc->enabled = enable;
> >  		return;
> 
> This doesn't look correct, omap_crtc_dss_start_update() already sets the
> enable bit for manual update displays. And you don't want to set the
> enable to false with manual update displays.
> 
> HDMI handling here is already a special case due to HW issues, so I'd
> rather see the manual update handled separately from the HDMI.
> 
> > @@ -214,7 +239,6 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
> >  	}
> >  }
> >  
> > -
> >  static int omap_crtc_dss_enable(struct omap_drm_private *priv,
> >  				enum omap_channel channel)
> >  {
> > @@ -378,6 +402,53 @@ void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
> >  	wake_up(&omap_crtc->pending_wait);
> >  }
> >  
> > +void omap_crtc_flush(struct drm_crtc *crtc)
> > +{
> > +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> > +
> > +	if (!omap_crtc_is_manually_updated(crtc))
> > +		return;
> > +
> > +	if (!delayed_work_pending(&omap_crtc->update_work))
> > +		schedule_delayed_work(&omap_crtc->update_work, 0);
> > +}
> 
> Why delayed work here?
> 
> It's actually not quite clear to me how manual update displays work with
> DRM...
> 
> As far as I see, we have essentially two cases: 1) single buffering,
> where the userspace must set an area in the fb dirty, which then
> triggers the update, 2) multi buffering, which doesn't need fb dirty,
> but just a page flip which triggers the update.
> 
> In the 2) case (which I think is the optimal case which all the modern
> apps should use), there's no need for delayed work or any work, and the
> code flow should be very similar to the auto-update model.
> 
> Also, as Daniel mentioned in "drm/omapdrm: Nuke
> omap_framebuffer_get_next_connector()" thread, the dirtying mechanism in
> the series is not valid.
> 
>  Tomi
> 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Daniel Stone]

Hi Tomi,

On 20 April 2018 at 08:09, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> It's actually not quite clear to me how manual update displays work with
> DRM...
>
> As far as I see, we have essentially two cases: 1) single buffering,
> where the userspace must set an area in the fb dirty, which then
> triggers the update, 2) multi buffering, which doesn't need fb dirty,
> but just a page flip which triggers the update.
>
> In the 2) case (which I think is the optimal case which all the modern
> apps should use), there's no need for delayed work or any work, and the
> code flow should be very similar to the auto-update model.

Correct. There's been talk (and I think patches?) of adding a
per-plane dirty property, so userspace can as an optimisation inform
the kernel of the area changed between frames. But short of that, a
pageflip needs to trigger a full-plane update, with no dirtyfb
required.

Cheers,
Daniel

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Tony Lindgren]

Hi,

* Daniel Stone <daniel@fooishbar.org> [180420 10:21]:
> Hi Tomi,
> 
> On 20 April 2018 at 08:09, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > It's actually not quite clear to me how manual update displays work with
> > DRM...
> >
> > As far as I see, we have essentially two cases: 1) single buffering,
> > where the userspace must set an area in the fb dirty, which then
> > triggers the update, 2) multi buffering, which doesn't need fb dirty,
> > but just a page flip which triggers the update.
> >
> > In the 2) case (which I think is the optimal case which all the modern
> > apps should use), there's no need for delayed work or any work, and the
> > code flow should be very similar to the auto-update model.
> 
> Correct. There's been talk (and I think patches?) of adding a
> per-plane dirty property, so userspace can as an optimisation inform
> the kernel of the area changed between frames. But short of that, a
> pageflip needs to trigger a full-plane update, with no dirtyfb
> required.

For per-plane dirty property patches, which ones do you refer to?

Then for xorg, there's my second attempt on fixing the command mode
rotation at [0]. Not sure if that's enough for a fix?

It seems not very efficient to me and I don't really know where
the the per crtc dirty flag should be stored..

I can easily test patches though with a command mode LCD and normal
HDMI setup on droid 4.

Regards,

Tony

[0] https://lists.x.org/archives/xorg-devel/2018-February/055890.html

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Daniel Stone]

Hi Tony!

On 20 April 2018 at 15:25, Tony Lindgren <tony@atomide.com> wrote:
> * Daniel Stone <daniel@fooishbar.org> [180420 10:21]:
>> On 20 April 2018 at 08:09, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > It's actually not quite clear to me how manual update displays work with
>> > DRM...
>> >
>> > As far as I see, we have essentially two cases: 1) single buffering,
>> > where the userspace must set an area in the fb dirty, which then
>> > triggers the update, 2) multi buffering, which doesn't need fb dirty,
>> > but just a page flip which triggers the update.
>> >
>> > In the 2) case (which I think is the optimal case which all the modern
>> > apps should use), there's no need for delayed work or any work, and the
>> > code flow should be very similar to the auto-update model.
>>
>> Correct. There's been talk (and I think patches?) of adding a
>> per-plane dirty property, so userspace can as an optimisation inform
>> the kernel of the area changed between frames. But short of that, a
>> pageflip needs to trigger a full-plane update, with no dirtyfb
>> required.
>
> For per-plane dirty property patches, which ones do you refer to?

Here's the latest iteration of that series:
https://lists.freedesktop.org/archives/dri-devel/2018-April/171900.html
<1522885748-67122-1-git-send-email-drawat@vmware.com>

> Then for xorg, there's my second attempt on fixing the command mode
> rotation at [0]. Not sure if that's enough for a fix?
>
> It seems not very efficient to me and I don't really know where
> the the per crtc dirty flag should be stored..

I try to deny all knowledge of X11 these days, I'm afraid.

Cheers,
Daniel

Re: [PATCHv3 3/8] drm/omap: add support for manually updated displays

[Tony Lindgren]

* Daniel Stone <daniel@fooishbar.org> [180420 14:41]:
> Hi Tony!
> 
> On 20 April 2018 at 15:25, Tony Lindgren <tony@atomide.com> wrote:
> > * Daniel Stone <daniel@fooishbar.org> [180420 10:21]:
> >> On 20 April 2018 at 08:09, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > It's actually not quite clear to me how manual update displays work with
> >> > DRM...
> >> >
> >> > As far as I see, we have essentially two cases: 1) single buffering,
> >> > where the userspace must set an area in the fb dirty, which then
> >> > triggers the update, 2) multi buffering, which doesn't need fb dirty,
> >> > but just a page flip which triggers the update.
> >> >
> >> > In the 2) case (which I think is the optimal case which all the modern
> >> > apps should use), there's no need for delayed work or any work, and the
> >> > code flow should be very similar to the auto-update model.
> >>
> >> Correct. There's been talk (and I think patches?) of adding a
> >> per-plane dirty property, so userspace can as an optimisation inform
> >> the kernel of the area changed between frames. But short of that, a
> >> pageflip needs to trigger a full-plane update, with no dirtyfb
> >> required.
> >
> > For per-plane dirty property patches, which ones do you refer to?
> 
> Here's the latest iteration of that series:
> https://lists.freedesktop.org/archives/dri-devel/2018-April/171900.html
> <1522885748-67122-1-git-send-email-drawat@vmware.com>

OK thanks for the link.

> > Then for xorg, there's my second attempt on fixing the command mode
> > rotation at [0]. Not sure if that's enough for a fix?
> >
> > It seems not very efficient to me and I don't really know where
> > the the per crtc dirty flag should be stored..
> 
> I try to deny all knowledge of X11 these days, I'm afraid.

:)

Tony