[PATCHv5 0/6] omapdrm: DSI command mode panel support

[PATCHv5 0/6] omapdrm: DSI command mode panel support

[Sebastian Reichel]

Hi,

Here is another round of the DSI command mode panel patchset
integrating the feedback from PATCHv4. The patches are based
on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
for OMAP3 support (it needs a workaround for a hardware bug) and
for automatic display rotation. They should get their own series,
once this patchset has landed.

Tested on Droid 4:
 * Framebuffer Console, updated at 1Hz due to blinking cursor
 * Display blanking
 * Xorg 1.19 with modesetting driver
 * Weston 5.0 with DRM backend
 * kmstest (static image)
 * No updates send when nothing needs to be sent

Known issues:
 * OMAP3 is untested and most likely broken due to missing
   workaround(s) for hardware bugs.
 * Weston 5.0 with fbdev backend does not work, since it
   uses neither page flip nor dirty ioctl. You need to use
   the drm backend.

Changes since PATCHv4:
 * Apply Acked-/Tested-by received from Tony and Pavel
 * Fix spelling/wording in commit messagess
 * Use proper multi-line comments
 * Restructure patch 4: move the whole HDMI block into a
   static sub-function, that is only called when output
   type is HDMI. Also drop the incorrect check for DVI.

Changes since PATCHv3:
 * Drop all Tested/Acked-by tags
 * Drop the rotation patches for now
 * Rebase to 4.20-rc1 + fixes from Laurent and Tony
 * Add fixes for DSI regressions introduced in 4.20-rc1
 * Store info update manual update mode in omap_crtc_state
 * Lock modesetting in omap_framebuffer_dirty
 * Directly loop through CRTCs instead of connectors in dirty function
 * Properly refresh display during page flips and get Weston working
 * Add more comments about implementation details

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 (6):
  drm/omap: use DRM_DEBUG_DRIVER instead of CORE
  drm/omap: populate DSI platform bus earlier
  drm/omap: don't check dispc timings for DSI
  drm/omap: fix incorrect union usage
  drm/omap: add framedone interrupt support
  drm/omap: add support for manually updated displays

 drivers/gpu/drm/omapdrm/dss/dsi.c        |  20 +--
 drivers/gpu/drm/omapdrm/omap_connector.c |   8 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c      | 167 ++++++++++++++++++++++-
 drivers/gpu/drm/omapdrm/omap_crtc.h      |   2 +
 drivers/gpu/drm/omapdrm/omap_drv.h       |   4 +-
 drivers/gpu/drm/omapdrm/omap_encoder.c   |  70 ++++++----
 drivers/gpu/drm/omapdrm/omap_fb.c        |  41 ++++++
 drivers/gpu/drm/omapdrm/omap_irq.c       |  25 ++++
 drivers/gpu/drm/omapdrm/omap_irq.h       |   1 +
 9 files changed, 290 insertions(+), 48 deletions(-)

-- 
2.19.1

[PATCHv5 1/6] drm/omap: use DRM_DEBUG_DRIVER instead of CORE

[Sebastian Reichel]

This macro is only used by omapdrm, which should print
debug messages using the DRIVER category instead of the
default CORE category.

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index bd7f2c227a25..3b4af517c92b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -38,8 +38,8 @@
 #include "omap_irq.h"
 #include "omap_plane.h"
 
-#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
-#define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
+#define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
+#define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__) /* verbose debug */
 
 #define MODULE_NAME     "omapdrm"
 
-- 
2.19.1

Re: [PATCHv5 1/6] drm/omap: use DRM_DEBUG_DRIVER instead of CORE

[Tomi Valkeinen]

On 21/11/18 18:09, Sebastian Reichel wrote:
> This macro is only used by omapdrm, which should print
> debug messages using the DRIVER category instead of the
> default CORE category.
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
> index bd7f2c227a25..3b4af517c92b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -38,8 +38,8 @@
>  #include "omap_irq.h"
>  #include "omap_plane.h"
>  
> -#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
> -#define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
> +#define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
> +#define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__) /* verbose debug */
>  
>  #define MODULE_NAME     "omapdrm"
>  
> 

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi

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

[PATCHv5 2/6] drm/omap: populate DSI platform bus earlier

[Sebastian Reichel]

After the changes from 4.20 the DSI encoder tries to find the
attached panel before populating the DSI bus. If the panel is
not found -EPROBE_DEFER is returned, so the DSI bus is never
populated and the panel never added.

Fix this by populating the DSI bus before searching for the
video sink in dsi_init_output().

Fixes: 27d624527d992 ("drm/omap: dss: Acquire next dssdev at probe time")
Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 0a485c5b982e..00a9c2ab9e6c 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -5418,9 +5418,15 @@ static int dsi_probe(struct platform_device *pdev)
 		dsi->num_lanes_supported = 3;
 	}
 
+	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
+	if (r) {
+		DSSERR("Failed to populate DSI child devices: %d\n", r);
+		goto err_pm_disable;
+	}
+
 	r = dsi_init_output(dsi);
 	if (r)
-		goto err_pm_disable;
+		goto err_of_depopulate;
 
 	r = dsi_probe_of(dsi);
 	if (r) {
@@ -5428,22 +5434,16 @@ static int dsi_probe(struct platform_device *pdev)
 		goto err_uninit_output;
 	}
 
-	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
-	if (r) {
-		DSSERR("Failed to populate DSI child devices: %d\n", r);
-		goto err_uninit_output;
-	}
-
 	r = component_add(&pdev->dev, &dsi_component_ops);
 	if (r)
-		goto err_of_depopulate;
+		goto err_uninit_output;
 
 	return 0;
 
-err_of_depopulate:
-	of_platform_depopulate(dev);
 err_uninit_output:
 	dsi_uninit_output(dsi);
+err_of_depopulate:
+	of_platform_depopulate(dev);
 err_pm_disable:
 	pm_runtime_disable(dev);
 	return r;
-- 
2.19.1

Re: [PATCHv5 2/6] drm/omap: populate DSI platform bus earlier

[Tomi Valkeinen]

On 21/11/18 18:09, Sebastian Reichel wrote:
> After the changes from 4.20 the DSI encoder tries to find the
> attached panel before populating the DSI bus. If the panel is
> not found -EPROBE_DEFER is returned, so the DSI bus is never
> populated and the panel never added.
> 
> Fix this by populating the DSI bus before searching for the
> video sink in dsi_init_output().
> 
> Fixes: 27d624527d992 ("drm/omap: dss: Acquire next dssdev at probe time")
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi

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

[PATCHv5 3/6] drm/omap: don't check dispc timings for DSI

[Sebastian Reichel]

While most display types only forward their VM to the DISPC, this
is not true for DSI. DSI calculates the VM for DISPC based on its
own, but it's not identical. Actually the DSI VM is not even a valid
DISPC VM making this check fail. Let's restore the old behaviour
and avoid checking the DISPC VM for DSI here.

Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly")
Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 8 +++++---
 drivers/gpu/drm/omapdrm/omap_encoder.c   | 8 +++++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index b81302c4bf9e..5c776d6211e1 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -280,9 +280,11 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 	drm_display_mode_to_videomode(mode, &vm);
 	mode->vrefresh = drm_mode_vrefresh(mode);
 
-	r = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm);
-	if (r)
-		goto done;
+	if (omap_connector->display->type != OMAP_DISPLAY_TYPE_DSI) {
+		r = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm);
+		if (r)
+			goto done;
+	}
 
 	for (dssdev = omap_connector->output; dssdev; dssdev = dssdev->next) {
 		if (!dssdev->ops->check_timings)
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 452e625f6ce3..32bbe3a80e7d 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -170,9 +170,11 @@ static int omap_encoder_atomic_check(struct drm_encoder *encoder,
 
 	drm_display_mode_to_videomode(&crtc_state->mode, &vm);
 
-	ret = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm);
-	if (ret)
-		goto done;
+	if (omap_encoder->display->type != OMAP_DISPLAY_TYPE_DSI) {
+		ret = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm);
+		if (ret)
+			goto done;
+	}
 
 	for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
 		if (!dssdev->ops->check_timings)
-- 
2.19.1

[PATCHv5 4/6] drm/omap: fix incorrect union usage

[Sebastian Reichel]

The DSI encoder sets dssdev->ops->dsi.set_config, which is stored at the
same offset as dssdev->ops->hdmi.set_hdmi_mode. The code in omap_encoder
only checks if dssdev->ops->hdmi.set_hdmi_mode is NULL. Due to the way
union works, it won't be NULL if dsi.set_config is set. This means
dsi_set_config will be called with config=hdmi_mode=false=NULL parameter
resulting in a NULL dereference. Also the dereference happens while
console is locked, so kernel hangs without any debug output without
"fb.lockless_register_fb=1" parameter.

This restructures the code, so that the HDMI mode is only configured
for HDMI output types. The new function also has a safe-guard directly
before accessing the union, that can be optimized away by the compiler
when the function is inlined and HDMI type has already been checked.

Fixes: 83910ad3f51fb ("drm/omap: Move most omap_dss_driver operations to omap_dss_device_ops")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_encoder.c | 62 +++++++++++++++-----------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 32bbe3a80e7d..f356821cd078 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -52,17 +52,48 @@ static const struct drm_encoder_funcs omap_encoder_funcs = {
 	.destroy = omap_encoder_destroy,
 };
 
+static void omap_encoder_hdmi_mode_set(struct drm_encoder *encoder,
+				       struct drm_display_mode *adjusted_mode)
+{
+	struct drm_device *dev = encoder->dev;
+	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
+	struct omap_dss_device *dssdev = omap_encoder->output;
+	struct drm_connector *connector;
+	bool hdmi_mode;
+
+	hdmi_mode = false;
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		if (connector->encoder == encoder) {
+			hdmi_mode = omap_connector_get_hdmi_mode(connector);
+			break;
+		}
+	}
+
+	/* safe-guard for accessing dssdev->ops->hdmi union */
+	if (dssdev->output_type != OMAP_DISPLAY_TYPE_HDMI)
+		return;
+
+	if (dssdev->ops->hdmi.set_hdmi_mode)
+		dssdev->ops->hdmi.set_hdmi_mode(dssdev, hdmi_mode);
+
+	if (hdmi_mode && dssdev->ops->hdmi.set_infoframe) {
+		struct hdmi_avi_infoframe avi;
+		int r;
+
+		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
+							     false);
+		if (r == 0)
+			dssdev->ops->hdmi.set_infoframe(dssdev, &avi);
+	}
+}
+
 static void omap_encoder_mode_set(struct drm_encoder *encoder,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
 {
-	struct drm_device *dev = encoder->dev;
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	struct drm_connector *connector;
 	struct omap_dss_device *dssdev;
 	struct videomode vm = { 0 };
-	bool hdmi_mode;
-	int r;
 
 	drm_display_mode_to_videomode(adjusted_mode, &vm);
 
@@ -112,27 +143,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	}
 
 	/* Set the HDMI mode and HDMI infoframe if applicable. */
-	hdmi_mode = false;
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if (connector->encoder == encoder) {
-			hdmi_mode = omap_connector_get_hdmi_mode(connector);
-			break;
-		}
-	}
-
-	dssdev = omap_encoder->output;
-
-	if (dssdev->ops->hdmi.set_hdmi_mode)
-		dssdev->ops->hdmi.set_hdmi_mode(dssdev, hdmi_mode);
-
-	if (hdmi_mode && dssdev->ops->hdmi.set_infoframe) {
-		struct hdmi_avi_infoframe avi;
-
-		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
-							     false);
-		if (r == 0)
-			dssdev->ops->hdmi.set_infoframe(dssdev, &avi);
-	}
+	if (omap_encoder->output->output_type == OMAP_DISPLAY_TYPE_HDMI)
+		omap_encoder_hdmi_mode_set(encoder, adjusted_mode);
 }
 
 static void omap_encoder_disable(struct drm_encoder *encoder)
-- 
2.19.1

Re: [PATCHv5 4/6] drm/omap: fix incorrect union usage

[Tony Lindgren]

* Sebastian Reichel <sebastian.reichel@collabora.com> [181121 16:09]:
> The DSI encoder sets dssdev->ops->dsi.set_config, which is stored at the
> same offset as dssdev->ops->hdmi.set_hdmi_mode. The code in omap_encoder
> only checks if dssdev->ops->hdmi.set_hdmi_mode is NULL. Due to the way
> union works, it won't be NULL if dsi.set_config is set. This means
> dsi_set_config will be called with config=hdmi_mode=false=NULL parameter
> resulting in a NULL dereference. Also the dereference happens while
> console is locked, so kernel hangs without any debug output without
> "fb.lockless_register_fb=1" parameter.
> 
> This restructures the code, so that the HDMI mode is only configured
> for HDMI output types. The new function also has a safe-guard directly
> before accessing the union, that can be optimized away by the compiler
> when the function is inlined and HDMI type has already been checked.
> 
> Fixes: 83910ad3f51fb ("drm/omap: Move most omap_dss_driver operations to omap_dss_device_ops")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Works for me:

Tested-by: Tony Lindgren <tony@atomide.com>

Re: [PATCHv5 4/6] drm/omap: fix incorrect union usage

[Tomi Valkeinen]

On 21/11/18 18:09, Sebastian Reichel wrote:
> The DSI encoder sets dssdev->ops->dsi.set_config, which is stored at the
> same offset as dssdev->ops->hdmi.set_hdmi_mode. The code in omap_encoder
> only checks if dssdev->ops->hdmi.set_hdmi_mode is NULL. Due to the way
> union works, it won't be NULL if dsi.set_config is set. This means
> dsi_set_config will be called with config=hdmi_mode=false=NULL parameter
> resulting in a NULL dereference. Also the dereference happens while
> console is locked, so kernel hangs without any debug output without
> "fb.lockless_register_fb=1" parameter.
> 
> This restructures the code, so that the HDMI mode is only configured
> for HDMI output types. The new function also has a safe-guard directly
> before accessing the union, that can be optimized away by the compiler
> when the function is inlined and HDMI type has already been checked.
> 
> Fixes: 83910ad3f51fb ("drm/omap: Move most omap_dss_driver operations to omap_dss_device_ops")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_encoder.c | 62 +++++++++++++++-----------
>  1 file changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 32bbe3a80e7d..f356821cd078 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -52,17 +52,48 @@ static const struct drm_encoder_funcs omap_encoder_funcs = {
>  	.destroy = omap_encoder_destroy,
>  };
>  
> +static void omap_encoder_hdmi_mode_set(struct drm_encoder *encoder,
> +				       struct drm_display_mode *adjusted_mode)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
> +	struct omap_dss_device *dssdev = omap_encoder->output;
> +	struct drm_connector *connector;
> +	bool hdmi_mode;
> +
> +	hdmi_mode = false;
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +		if (connector->encoder == encoder) {
> +			hdmi_mode = omap_connector_get_hdmi_mode(connector);
> +			break;
> +		}
> +	}
> +
> +	/* safe-guard for accessing dssdev->ops->hdmi union */
> +	if (dssdev->output_type != OMAP_DISPLAY_TYPE_HDMI)
> +		return;

If you safeguard the function, it would make sense to do this at the
very beginning of the function. But to be honest, I think this is just
extra. It's a static function, named "hdmi", so I think we can trust
that we call it correctly.

If you're ok with it, I can pick this patch up and drop the safeguard.
Or you can send a new one.

 Tomi

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

[PATCHv5 5/6] drm/omap: add framedone interrupt support

[Sebastian Reichel]

This prepares framedone interrupt handling for
manual display update support.

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 50 +++++++++++++++++++++++++++++
 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, 77 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index caffc547ef97..59ee2399f2e9 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -52,6 +52,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;
 };
 
 /* -----------------------------------------------------------------------------
@@ -231,6 +234,18 @@ static int omap_crtc_dss_register_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct drm_crtc *crtc = priv->channels[channel]->crtc;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	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;
 }
 
@@ -238,6 +253,17 @@ static void omap_crtc_dss_unregister_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct drm_crtc *crtc = priv->channels[channel]->crtc;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	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 = {
@@ -303,6 +329,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 d9de437ba9dd..d33bbb7a4f90 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -41,5 +41,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 329ad26d6d50..01dda84ca2ee 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.19.1

[PATCHv5 6/6] 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.

The display will be refreshed when something calls the dirty callback,
such as libdrm's drmModeDirtyFB(). This is currently being done at least
by the kernel console and Xorg (with modesetting driver) in their
default configuration. Weston does not implement this and the fbdev
backend does not work (display will not update). Weston's DRM backend
uses double buffering and the page flip will trigger a display refresh
and seems to work as expected.

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 117 ++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
 drivers/gpu/drm/omapdrm/omap_fb.c   |  41 ++++++++++
 3 files changed, 154 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 59ee2399f2e9..aed8d61d2783 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -33,6 +33,7 @@ struct omap_crtc_state {
 	/* Shadow values for legacy userspace support. */
 	unsigned int rotation;
 	unsigned int zpos;
+	bool manually_updated;
 };
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
@@ -52,6 +53,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;
@@ -106,21 +108,18 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 /*
  * Manager-ops, callbacks from output when they need to configure
  * the upstream part of the video pipe.
- *
- * Most of these we can ignore until we add support for command-mode
- * panels.. for video-mode the crtc-helpers already do an adequate
- * job of sequencing the setup of the video pipe in the proper order
  */
 
-/* we can probably ignore these until we support command-mode panels: */
 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);
 }
 
 /* Called only from the encoder enable/disable and suspend/resume handlers. */
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct drm_device *dev = crtc->dev;
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -132,6 +131,12 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	if (WARN_ON(omap_crtc->enabled == enable))
 		return;
 
+	if (omap_state->manually_updated) {
+		omap_irq_enable_framedone(crtc, enable);
+		omap_crtc->enabled = enable;
+		return;
+	}
+
 	if (omap_crtc->pipe->output->output_type == OMAP_DISPLAY_TYPE_HDMI) {
 		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
 		omap_crtc->enabled = enable;
@@ -353,6 +358,54 @@ 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);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
+
+	if (!omap_state->manually_updated)
+		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->pipe->display;
+	struct drm_device *dev = omap_crtc->base.dev;
+	const struct omap_dss_driver *dssdrv;
+	struct videomode vm = {0};
+	int ret;
+
+	if (!dssdev) {
+		dev_err_once(dev->dev, "missing display dssdev!");
+		return;
+	}
+
+	dssdrv = dssdev->driver;
+	if (!dssdrv || !dssdrv->update) {
+		dev_err_once(dev->dev, "missing or incorrect dssdrv!");
+		return;
+	}
+
+	if (dssdrv->sync)
+		dssdrv->sync(dssdev);
+
+	if (dssdev->ops->get_timings)
+		dssdev->ops->get_timings(dssdev, &vm);
+
+	ret = dssdrv->update(dssdev, 0, 0, vm.hactive, vm.vactive);
+	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;
@@ -402,12 +455,17 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	DBG("%s", omap_crtc->name);
 
 	priv->dispc_ops->runtime_get(priv->dispc);
 
+	/* manual updated display will not trigger vsync irq */
+	if (omap_state->manually_updated)
+		return;
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	drm_crtc_vblank_on(crtc);
 	ret = drm_crtc_vblank_get(crtc);
@@ -422,6 +480,7 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
@@ -432,6 +491,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);
 
 	priv->dispc_ops->runtime_put(priv->dispc);
@@ -487,6 +551,22 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
 }
 
+static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_dss_device *display = omap_crtc->pipe->display;
+
+	if (!display)
+		return false;
+
+	if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
+		DBG("detected manually updated display!");
+		return true;
+	}
+
+	return false;
+}
+
 static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 				struct drm_crtc_state *state)
 {
@@ -508,6 +588,9 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 		/* Mirror new values for zpos and rotation in omap_crtc_state */
 		omap_crtc_state->zpos = pri_state->zpos;
 		omap_crtc_state->rotation = pri_state->rotation;
+
+		/* Check if this CRTC is for a manually updated display */
+		omap_crtc_state->manually_updated = omap_crtc_is_manually_updated(crtc);
 	}
 
 	return 0;
@@ -523,6 +606,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	if (crtc->state->color_mgmt_changed) {
@@ -547,6 +631,15 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	DBG("%s: GO", omap_crtc->name);
 
+	if (omap_crtc_state->manually_updated) {
+		/* send new image for page flips and modeset changes */
+		spin_lock_irq(&crtc->dev->event_lock);
+		omap_crtc_flush(crtc);
+		omap_crtc_arm_event(crtc);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		return;
+	}
+
 	ret = drm_crtc_vblank_get(crtc);
 	WARN_ON(ret != 0);
 
@@ -632,6 +725,7 @@ omap_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	state->zpos = current_state->zpos;
 	state->rotation = current_state->rotation;
+	state->manually_updated = current_state->manually_updated;
 
 	return &state->base;
 }
@@ -708,6 +802,19 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 
+	/*
+	 * We want to refresh manually updated displays from dirty callback,
+	 * which is called quite often (e.g. for each drawn line). This will
+	 * be used to do the display update asynchronously to avoid blocking
+	 * the rendering process and merges multiple dirty calls into one
+	 * update if they arrive very fast. We also call this function for
+	 * atomic display updates (e.g. for page flips), which means we do
+	 * not need extra locking. Atomic updates should be synchronous, but
+	 * need to wait for the framedone interrupt anyways.
+	 */
+	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 d33bbb7a4f90..2b518c74203e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -42,5 +42,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 4d264fd554d8..d98df2091b78 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -66,8 +66,49 @@ struct omap_framebuffer {
 	struct mutex lock;
 };
 
+/* Iterator for framebuffer's CRTCs */
+static struct drm_crtc *omap_framebuffer_get_next_crtc(
+		struct drm_framebuffer *fb, struct drm_crtc *from)
+{
+	struct drm_device *dev = fb->dev;
+	struct list_head *crtc_list = &dev->mode_config.crtc_list;
+	struct drm_crtc *crtc = from;
+
+	if (!from)
+		return list_first_entry_or_null(crtc_list, typeof(*from), head);
+
+	list_for_each_entry_from(crtc, crtc_list, head) {
+		if (crtc == from)
+			continue;
+
+		if (crtc && crtc->primary->fb == fb)
+			return crtc;
+	}
+
+	return NULL;
+}
+
+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_crtc *crtc = NULL;
+
+	drm_modeset_lock_all(fb->dev);
+
+	while ((crtc = omap_framebuffer_get_next_crtc(fb, crtc)))
+		omap_crtc_flush(crtc);
+
+	drm_modeset_unlock_all(fb->dev);
+
+	return 0;
+}
+
 static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
 	.create_handle = drm_gem_fb_create_handle,
+	.dirty = omap_framebuffer_dirty,
 	.destroy = drm_gem_fb_destroy,
 };
 
-- 
2.19.1

Re: [PATCHv5 0/6] omapdrm: DSI command mode panel support

[Tomi Valkeinen]

Hi Sebastian,

On 21/11/18 18:09, Sebastian Reichel wrote:
> Hi,
> 
> Here is another round of the DSI command mode panel patchset
> integrating the feedback from PATCHv4. The patches are based
> on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
> for OMAP3 support (it needs a workaround for a hardware bug) and
> for automatic display rotation. They should get their own series,
> once this patchset has landed.

Thanks. I can pick 1, 2 and 4 as fixes. I need to look at 3 a bit more,
but it might also be a valid fix for the rcs. Although I believe the
correct fix would be to implement check_timings for dsi.c, which would
return the adjusted timings.

 Tomi

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

Re: [PATCHv5 0/6] omapdrm: DSI command mode panel support

[Sebastian Reichel]

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

Hi,

On Mon, Nov 26, 2018 at 11:34:30AM +0200, Tomi Valkeinen wrote:
> On 21/11/18 18:09, Sebastian Reichel wrote:
> > Here is another round of the DSI command mode panel patchset
> > integrating the feedback from PATCHv4. The patches are based
> > on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
> > for OMAP3 support (it needs a workaround for a hardware bug) and
> > for automatic display rotation. They should get their own series,
> > once this patchset has landed.
> 
> Thanks. I can pick 1, 2 and 4 as fixes.

Feel free to pick up any patches in any order. I can rebase the
remaining ones.

> I need to look at 3 a bit more, but it might also be a valid fix
> for the rcs. Although I believe the correct fix would be to
> implement check_timings for dsi.c, which would return the adjusted
> timings.

I wondered how a proper fix should look like. In the end I just
restored the old behaviour of not checking dispc to fix the
regression.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCHv5 0/6] omapdrm: DSI command mode panel support

[Tomi Valkeinen]

Hi Sebastian,

On 21/11/2018 18:09, Sebastian Reichel wrote:
> Hi,
> 
> Here is another round of the DSI command mode panel patchset
> integrating the feedback from PATCHv4. The patches are based
> on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
> for OMAP3 support (it needs a workaround for a hardware bug) and
> for automatic display rotation. They should get their own series,
> once this patchset has landed.

The big omapdrm bridge/panel series from Laurent has been merged to
drm-next. I think this series can now be rebased and merged.

However, all this needs to be changed to DRM bridge/panel model sooner
or later (sooner, please!), but perhaps it's best to first go with the
legacy way.

 Tomi

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

Re: [PATCHv5 0/6] omapdrm: DSI command mode panel support

[Laurent Pinchart]

On Tue, Apr 02, 2019 at 06:36:21PM +0300, Tomi Valkeinen wrote:
> On 21/11/2018 18:09, Sebastian Reichel wrote:
> > Hi,
> > 
> > Here is another round of the DSI command mode panel patchset
> > integrating the feedback from PATCHv4. The patches are based
> > on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
> > for OMAP3 support (it needs a workaround for a hardware bug) and
> > for automatic display rotation. They should get their own series,
> > once this patchset has landed.
> 
> The big omapdrm bridge/panel series from Laurent has been merged to
> drm-next. I think this series can now be rebased and merged.
> 
> However, all this needs to be changed to DRM bridge/panel model sooner
> or later (sooner, please!), but perhaps it's best to first go with the
> legacy way.

I would be OK with that, but I'd like to understand what we still need
to do to convert DSI support to DRM bridge and panel. Sebastian, if
you've looked into it, could you provide some insight ?

-- 
Regards,

Laurent Pinchart

Re: [PATCHv5 0/6] omapdrm: DSI command mode panel support

[Sebastian Reichel]

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

Hi Tomi & Laurent,

On Tue, Apr 02, 2019 at 06:55:08PM +0300, Laurent Pinchart wrote:
> On Tue, Apr 02, 2019 at 06:36:21PM +0300, Tomi Valkeinen wrote:
> > On 21/11/2018 18:09, Sebastian Reichel wrote:
> > > Hi,
> > > 
> > > Here is another round of the DSI command mode panel patchset
> > > integrating the feedback from PATCHv4. The patches are based
> > > on 4.20-rc1 + fixes from Laurent and Tony. I dropped the patches
> > > for OMAP3 support (it needs a workaround for a hardware bug) and
> > > for automatic display rotation. They should get their own series,
> > > once this patchset has landed.
> > 
> > The big omapdrm bridge/panel series from Laurent has been merged to
> > drm-next. I think this series can now be rebased and merged.
> > 
> > However, all this needs to be changed to DRM bridge/panel model sooner
> > or later (sooner, please!), but perhaps it's best to first go with the
> > legacy way.
> 
> I would be OK with that, but I'd like to understand what we still need
> to do to convert DSI support to DRM bridge and panel. Sebastian, if
> you've looked into it, could you provide some insight ?

I have a rebased & tested version of the patches getting the Droid 4
panel working in this branch (together with 2 patches adding
backlight support for testing purposes). I will send it as a patch
series in a few minutes.

https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git/log/?h=omapdrm-5.2-with-dsi

I also had a look at converting to DRM bridge/panel model and
started working on it in the following branch. It's completley
untested and probably not working at all. Also the most recent
patches are not yet properly splitted / formated. But they show
my idea of how this can be achieved:

1. Add DSI transfer() function
2. Convert custom functions to use transfer() instead
3. Convert to use mipi_dsi_host
4. Get rid of custom DSI related omapdss functions
5. at this point panel conversion can hopefully happen the
   same way as for the other modules

https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git/log/?h=omapdrm-5.2-with-dsi-untested-work-branch

P.S.: I will continue to work on this, but probably not in
the next 4 weeks. I'm currently very busy with non-kernel
tasks and still have to review > 200 power-supply patches.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv6 0/4] omapdrm: DSI command mode panel support

[Sebastian Reichel]

Hi,

Here is another round of the DSI command mode panel patchset
integrating the feedback from PATCHv5. The patches are based
on Tomi's omapdrm-5.2 tag. It does not contain the patches
required for OMAP3 support (it needs a workaround for a hardware
bug) and for automatic display rotation. They should get their
own series, once this patchset has landed. Also it does not
yet use the DRM panel/bridge things, but IMHO should be merged
before. Having this helps means 'git bisect' can be used while
converting DSI support to DRM panel/bridge style. 

Tested on Droid 4:
 * Framebuffer Console, updated at 1Hz due to blinking cursor
 * Display blanking
 * Xorg 1.19 with modesetting driver
 * Weston 5.0 with DRM backend
 * kmstest (static image)
 * No updates send when nothing needs to be sent

Known issues:
 * OMAP3 is untested and most likely broken due to missing
   workaround(s) for hardware bugs.
 * Weston 5.0 with fbdev backend does not work, since it
   uses neither page flip nor dirty ioctl. You need to use
   the drm backend.

Changes since PATCHv5:
 * Rebased to Tomi's omapdrm-5.2 tag
 * Simplified omap_framebuffer_dirty() by using
   drm_for_each_crtc()

Changes since PATCHv4:
 * Apply Acked-/Tested-by received from Tony and Pavel
 * Fix spelling/optimize commit messages
 * Use proper multi-line comments
 * Restructure patch 4: move the whole HDMI block into a
   static subfunction, that is only called when output
   type is HDMI. Also drop the incorrect check for DVI.

Changes since PATCHv3:
 * Drop all Tested/Acked-by tags
 * Drop the rotation patches for now
 * Rebase to 4.20-rc1 + fixes from Laurent and Tony
 * Add fixes for DSI regressions introduced in 4.20-rc1
 * Store info update manual update mode in omap_crtc_state
 * Lock modesetting in omap_framebuffer_dirty
 * Directly loop through CRTCs instead of connectors in dirty function
 * Properly refresh display during page flips and get Weston working
 * Add more comments about implementation details

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 (4):
  drm/omap: use DRM_DEBUG_DRIVER instead of CORE
  drm/omap: don't check dispc timings for DSI
  drm/omap: add framedone interrupt support
  drm/omap: add support for manually updated displays

 drivers/gpu/drm/omapdrm/omap_crtc.c | 182 ++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h |   2 +
 drivers/gpu/drm/omapdrm/omap_drv.h  |   4 +-
 drivers/gpu/drm/omapdrm/omap_fb.c   |  19 +++
 drivers/gpu/drm/omapdrm/omap_irq.c  |  25 ++++
 drivers/gpu/drm/omapdrm/omap_irq.h  |   1 +
 6 files changed, 222 insertions(+), 11 deletions(-)

-- 
2.20.1

[PATCHv6 1/4] drm/omap: use DRM_DEBUG_DRIVER instead of CORE

[Sebastian Reichel]

This macro is only used by omapdrm, which should print
debug messages using the DRIVER category instead of the
default CORE category.

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 3cca45cb25f3..896aa12f09b2 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -37,8 +37,8 @@
 #include "omap_irq.h"
 #include "omap_plane.h"
 
-#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
-#define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
+#define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
+#define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__) /* verbose debug */
 
 #define MODULE_NAME     "omapdrm"
 
-- 
2.20.1

[PATCHv6 2/4] drm/omap: don't check dispc timings for DSI

[Sebastian Reichel]

While most display types only forward their VM to the DISPC, this
is not true for DSI. DSI calculates the VM for DISPC based on its
own, but it's not identical. Actually the DSI VM is not even a valid
DISPC VM making this check fail. Let's restore the old behaviour
and avoid checking the DISPC VM for DSI here.

Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly")
Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5a29bf01c0e8..86827a061b0b 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -395,10 +395,20 @@ static enum drm_mode_status omap_crtc_mode_valid(struct drm_crtc *crtc,
 	int r;
 
 	drm_display_mode_to_videomode(mode, &vm);
-	r = priv->dispc_ops->mgr_check_timings(priv->dispc, omap_crtc->channel,
-					       &vm);
-	if (r)
-		return r;
+
+	/*
+	 * DSI might not call this, since the supplied mode is not a
+	 * valid DISPC mode. DSI will calculate and configure the
+	 * proper DISPC mode later.
+	 */
+	if (omap_crtc->pipe->output->next == NULL ||
+	    omap_crtc->pipe->output->next->type != OMAP_DISPLAY_TYPE_DSI) {
+		r = priv->dispc_ops->mgr_check_timings(priv->dispc,
+						       omap_crtc->channel,
+						       &vm);
+		if (r)
+			return r;
+	}
 
 	/* Check for bandwidth limit */
 	if (priv->max_bandwidth) {
-- 
2.20.1

[PATCHv6 3/4] drm/omap: add framedone interrupt support

[Sebastian Reichel]

This prepares framedone interrupt handling for
manual display update support.

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 50 +++++++++++++++++++++++++++++
 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, 77 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 86827a061b0b..68697820d189 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;
 };
 
 /* -----------------------------------------------------------------------------
@@ -230,6 +233,18 @@ static int omap_crtc_dss_register_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct drm_crtc *crtc = priv->channels[channel]->crtc;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	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;
 }
 
@@ -237,6 +252,17 @@ static void omap_crtc_dss_unregister_framedone(
 		struct omap_drm_private *priv, enum omap_channel channel,
 		void (*handler)(void *), void *data)
 {
+	struct drm_crtc *crtc = priv->channels[channel]->crtc;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	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 = {
@@ -302,6 +328,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 d9de437ba9dd..d33bbb7a4f90 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -41,5 +41,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 329ad26d6d50..01dda84ca2ee 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.20.1

[PATCHv6 4/4] 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.

The display will be refreshed when something calls the dirty callback,
such as libdrm's drmModeDirtyFB(). This is currently being done at least
by the kernel console and Xorg (with modesetting driver) in their
default configuration. Weston does not implement this and the fbdev
backend does not work (display will not update). Weston's DRM backend
uses double buffering and the page flip will also trigger a display
refresh.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 114 ++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
 drivers/gpu/drm/omapdrm/omap_fb.c   |  19 +++++
 3 files changed, 129 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 68697820d189..b59065365c9e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -32,6 +32,7 @@ struct omap_crtc_state {
 	/* Shadow values for legacy userspace support. */
 	unsigned int rotation;
 	unsigned int zpos;
+	bool manually_updated;
 };
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
@@ -51,6 +52,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;
@@ -105,21 +107,18 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 /*
  * Manager-ops, callbacks from output when they need to configure
  * the upstream part of the video pipe.
- *
- * Most of these we can ignore until we add support for command-mode
- * panels.. for video-mode the crtc-helpers already do an adequate
- * job of sequencing the setup of the video pipe in the proper order
  */
 
-/* we can probably ignore these until we support command-mode panels: */
 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);
 }
 
 /* Called only from the encoder enable/disable and suspend/resume handlers. */
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct drm_device *dev = crtc->dev;
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -131,6 +130,12 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	if (WARN_ON(omap_crtc->enabled == enable))
 		return;
 
+	if (omap_state->manually_updated) {
+		omap_irq_enable_framedone(crtc, enable);
+		omap_crtc->enabled = enable;
+		return;
+	}
+
 	if (omap_crtc->pipe->output->type == OMAP_DISPLAY_TYPE_HDMI) {
 		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
 		omap_crtc->enabled = enable;
@@ -352,6 +357,51 @@ 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);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
+
+	if (!omap_state->manually_updated)
+		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 drm_display_mode *mode = &omap_crtc->pipe->crtc->mode;
+	struct omap_dss_device *dssdev = omap_crtc->pipe->output->next;
+	struct drm_device *dev = omap_crtc->base.dev;
+	const struct omap_dss_driver *dssdrv;
+	int ret;
+
+	if (!dssdev) {
+		dev_err_once(dev->dev, "missing display dssdev!");
+		return;
+	}
+
+	dssdrv = dssdev->driver;
+	if (!dssdrv || !dssdrv->update) {
+		dev_err_once(dev->dev, "missing or incorrect dssdrv!");
+		return;
+	}
+
+	if (dssdrv->sync)
+		dssdrv->sync(dssdev);
+
+	ret = dssdrv->update(dssdev, 0, 0, mode->hdisplay, mode->vdisplay);
+	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;
@@ -401,12 +451,17 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	DBG("%s", omap_crtc->name);
 
 	priv->dispc_ops->runtime_get(priv->dispc);
 
+	/* manual updated display will not trigger vsync irq */
+	if (omap_state->manually_updated)
+		return;
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	drm_crtc_vblank_on(crtc);
 	ret = drm_crtc_vblank_get(crtc);
@@ -421,6 +476,7 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
@@ -431,6 +487,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);
 
 	priv->dispc_ops->runtime_put(priv->dispc);
@@ -501,6 +562,22 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
 }
 
+static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_dss_device *display = omap_crtc->pipe->output->next;
+
+	if (!display)
+		return false;
+
+	if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
+		DBG("detected manually updated display!");
+		return true;
+	}
+
+	return false;
+}
+
 static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 				struct drm_crtc_state *state)
 {
@@ -522,6 +599,9 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 		/* Mirror new values for zpos and rotation in omap_crtc_state */
 		omap_crtc_state->zpos = pri_state->zpos;
 		omap_crtc_state->rotation = pri_state->rotation;
+
+		/* Check if this CRTC is for a manually updated display */
+		omap_crtc_state->manually_updated = omap_crtc_is_manually_updated(crtc);
 	}
 
 	return 0;
@@ -537,6 +617,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	if (crtc->state->color_mgmt_changed) {
@@ -561,6 +642,15 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	DBG("%s: GO", omap_crtc->name);
 
+	if (omap_crtc_state->manually_updated) {
+		/* send new image for page flips and modeset changes */
+		spin_lock_irq(&crtc->dev->event_lock);
+		omap_crtc_flush(crtc);
+		omap_crtc_arm_event(crtc);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		return;
+	}
+
 	ret = drm_crtc_vblank_get(crtc);
 	WARN_ON(ret != 0);
 
@@ -646,6 +736,7 @@ omap_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	state->zpos = current_state->zpos;
 	state->rotation = current_state->rotation;
+	state->manually_updated = current_state->manually_updated;
 
 	return &state->base;
 }
@@ -722,6 +813,19 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 
+	/*
+	 * We want to refresh manually updated displays from dirty callback,
+	 * which is called quite often (e.g. for each drawn line). This will
+	 * be used to do the display update asynchronously to avoid blocking
+	 * the rendering process and merges multiple dirty calls into one
+	 * update if they arrive very fast. We also call this function for
+	 * atomic display updates (e.g. for page flips), which means we do
+	 * not need extra locking. Atomic updates should be synchronous, but
+	 * need to wait for the framedone interrupt anyways.
+	 */
+	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 d33bbb7a4f90..2b518c74203e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -42,5 +42,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 4f8eb9d08f99..b6f052f2a7e1 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -66,8 +66,27 @@ struct omap_framebuffer {
 	struct mutex lock;
 };
 
+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_crtc *crtc;
+
+	drm_modeset_lock_all(fb->dev);
+
+	drm_for_each_crtc(crtc, fb->dev)
+		omap_crtc_flush(crtc);
+
+	drm_modeset_unlock_all(fb->dev);
+
+	return 0;
+}
+
 static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
 	.create_handle = drm_gem_fb_create_handle,
+	.dirty = omap_framebuffer_dirty,
 	.destroy = drm_gem_fb_destroy,
 };
 
-- 
2.20.1

Re: [PATCHv6 4/4] drm/omap: add support for manually updated displays

[Tony Lindgren]

* Sebastian Reichel <sebastian.reichel@collabora.com> [190403 20:14]:
> 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.
> 
> The display will be refreshed when something calls the dirty callback,
> such as libdrm's drmModeDirtyFB(). This is currently being done at least
> by the kernel console and Xorg (with modesetting driver) in their
> default configuration. Weston does not implement this and the fbdev
> backend does not work (display will not update). Weston's DRM backend
> uses double buffering and the page flip will also trigger a display
> refresh.

I've tested this with Linux next and the latest lm3532
patches and it works fine as long as we leave out the
backlight = <&lcd_backlight> entry from dts like I
replied in the lm3532 tread. So as far as I'm concerned,
we're good to go:

Tested-by: Tony Lindgren <tony@atomide.com>

Re: [PATCHv6 4/4] drm/omap: add support for manually updated displays

[Pavel Machek]

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

On Wed 2019-04-03 17:11:09, Tony Lindgren wrote:
> * Sebastian Reichel <sebastian.reichel@collabora.com> [190403 20:14]:
> > 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.
> > 
> > The display will be refreshed when something calls the dirty callback,
> > such as libdrm's drmModeDirtyFB(). This is currently being done at least
> > by the kernel console and Xorg (with modesetting driver) in their
> > default configuration. Weston does not implement this and the fbdev
> > backend does not work (display will not update). Weston's DRM backend
> > uses double buffering and the page flip will also trigger a display
> > refresh.
> 
> I've tested this with Linux next and the latest lm3532
> patches and it works fine as long as we leave out the
> backlight = <&lcd_backlight> entry from dts like I
> replied in the lm3532 tread. So as far as I'm concerned,
> we're good to go:
> 
> Tested-by: Tony Lindgren <tony@atomide.com>

I've tested this on 5.2-rc1, and it is still neccessary, still needed,
and still not merged.

How can I help? Can the patches simply be picked up for drm tree?

Tested-by: Pavel Machek <pavel@ucw.cz>
									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 --]

[PATCHv6 0/4] omapdrm: DSI command mode panel support

[Sebastian Reichel]

Hi,

Here is another round of the DSI command mode panel patchset
integrating the feedback from PATCHv5. The patches are based
on v5.2-rc1 tag. It does not contain the patches required for
OMAP3 support (it needs a workaround for a hardware bug) and
for automatic display rotation. They should get their own series,
once after everything has been moved to DRM panel API. I think
DRM panel conversion should happen _after_ this series, since
otherwise there is a high risk of bricking DSI support completely.
I already started a WIP branch for converting DSI to the DRM panel
API on top of this patchset.

Tested on Droid 4:
 * Display blanking
  - automatic backlight blanking is missing (not handled by DSI)
 * Framebuffer Console, updated at 1Hz due to blinking cursor
 * Xorg 1.19 with modesetting driver
 * Weston 5.0 with DRM backend
 * kmstest (static image)
 * No updates send when nothing needs to be sent

Known issues:
 * OMAP3 is untested and most likely broken due to missing
   workaround(s) for hardware bugs.
 * Weston 5.0 with fbdev backend does not work, since it
   uses neither page flip nor dirty ioctl. You need to use
   the drm backend.

Changes since PATCHv5:
 * Rebased to v5.2-rc1
 * Simplified omap_framebuffer_dirty() by using
   drm_for_each_crtc()

Changes since PATCHv4:
 * Apply Acked-/Tested-by received from Tony and Pavel
 * Fix spelling/optimize commit messages
 * Use proper multi-line comments
 * Restructure patch 4: move the whole HDMI block into a
   static subfunction, that is only called when output
   type is HDMI. Also drop the incorrect check for DVI.

Changes since PATCHv3:
 * Drop all Tested/Acked-by tags
 * Drop the rotation patches for now
 * Rebase to 4.20-rc1 + fixes from Laurent and Tony
 * Add fixes for DSI regressions introduced in 4.20-rc1
 * Store info update manual update mode in omap_crtc_state
 * Lock modesetting in omap_framebuffer_dirty
 * Directly loop through CRTCs instead of connectors in dirty function
 * Properly refresh display during page flips and get Weston working
 * Add more comments about implementation details

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 (4):
  drm/omap: use DRM_DEBUG_DRIVER instead of CORE
  drm/omap: don't check dispc timings for DSI
  drm/omap: add framedone interrupt support
  drm/omap: add support for manually updated displays

 drivers/gpu/drm/omapdrm/omap_crtc.c | 182 ++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h |   2 +
 drivers/gpu/drm/omapdrm/omap_drv.h  |   4 +-
 drivers/gpu/drm/omapdrm/omap_fb.c   |  19 +++
 drivers/gpu/drm/omapdrm/omap_irq.c  |  25 ++++
 drivers/gpu/drm/omapdrm/omap_irq.h  |   1 +
 6 files changed, 222 insertions(+), 11 deletions(-)

-- 
2.20.1

[PATCHv6 4/4] 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.

The display will be refreshed when something calls the dirty callback,
such as libdrm's drmModeDirtyFB(). This is currently being done at least
by the kernel console and Xorg (with modesetting driver) in their
default configuration. Weston does not implement this and the fbdev
backend does not work (display will not update). Weston's DRM backend
uses double buffering and the page flip will also trigger a display
refresh.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 114 ++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_crtc.h |   1 +
 drivers/gpu/drm/omapdrm/omap_fb.c   |  19 +++++
 3 files changed, 129 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 68697820d189..b59065365c9e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -32,6 +32,7 @@ struct omap_crtc_state {
 	/* Shadow values for legacy userspace support. */
 	unsigned int rotation;
 	unsigned int zpos;
+	bool manually_updated;
 };
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
@@ -51,6 +52,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;
@@ -105,21 +107,18 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
 /*
  * Manager-ops, callbacks from output when they need to configure
  * the upstream part of the video pipe.
- *
- * Most of these we can ignore until we add support for command-mode
- * panels.. for video-mode the crtc-helpers already do an adequate
- * job of sequencing the setup of the video pipe in the proper order
  */
 
-/* we can probably ignore these until we support command-mode panels: */
 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);
 }
 
 /* Called only from the encoder enable/disable and suspend/resume handlers. */
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct drm_device *dev = crtc->dev;
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -131,6 +130,12 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	if (WARN_ON(omap_crtc->enabled == enable))
 		return;
 
+	if (omap_state->manually_updated) {
+		omap_irq_enable_framedone(crtc, enable);
+		omap_crtc->enabled = enable;
+		return;
+	}
+
 	if (omap_crtc->pipe->output->type == OMAP_DISPLAY_TYPE_HDMI) {
 		priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
 		omap_crtc->enabled = enable;
@@ -352,6 +357,51 @@ 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);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
+
+	if (!omap_state->manually_updated)
+		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 drm_display_mode *mode = &omap_crtc->pipe->crtc->mode;
+	struct omap_dss_device *dssdev = omap_crtc->pipe->output->next;
+	struct drm_device *dev = omap_crtc->base.dev;
+	const struct omap_dss_driver *dssdrv;
+	int ret;
+
+	if (!dssdev) {
+		dev_err_once(dev->dev, "missing display dssdev!");
+		return;
+	}
+
+	dssdrv = dssdev->driver;
+	if (!dssdrv || !dssdrv->update) {
+		dev_err_once(dev->dev, "missing or incorrect dssdrv!");
+		return;
+	}
+
+	if (dssdrv->sync)
+		dssdrv->sync(dssdev);
+
+	ret = dssdrv->update(dssdev, 0, 0, mode->hdisplay, mode->vdisplay);
+	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;
@@ -401,12 +451,17 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	DBG("%s", omap_crtc->name);
 
 	priv->dispc_ops->runtime_get(priv->dispc);
 
+	/* manual updated display will not trigger vsync irq */
+	if (omap_state->manually_updated)
+		return;
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	drm_crtc_vblank_on(crtc);
 	ret = drm_crtc_vblank_get(crtc);
@@ -421,6 +476,7 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	DBG("%s", omap_crtc->name);
 
@@ -431,6 +487,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);
 
 	priv->dispc_ops->runtime_put(priv->dispc);
@@ -501,6 +562,22 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
 }
 
+static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_dss_device *display = omap_crtc->pipe->output->next;
+
+	if (!display)
+		return false;
+
+	if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
+		DBG("detected manually updated display!");
+		return true;
+	}
+
+	return false;
+}
+
 static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 				struct drm_crtc_state *state)
 {
@@ -522,6 +599,9 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 		/* Mirror new values for zpos and rotation in omap_crtc_state */
 		omap_crtc_state->zpos = pri_state->zpos;
 		omap_crtc_state->rotation = pri_state->rotation;
+
+		/* Check if this CRTC is for a manually updated display */
+		omap_crtc_state->manually_updated = omap_crtc_is_manually_updated(crtc);
 	}
 
 	return 0;
@@ -537,6 +617,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	if (crtc->state->color_mgmt_changed) {
@@ -561,6 +642,15 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	DBG("%s: GO", omap_crtc->name);
 
+	if (omap_crtc_state->manually_updated) {
+		/* send new image for page flips and modeset changes */
+		spin_lock_irq(&crtc->dev->event_lock);
+		omap_crtc_flush(crtc);
+		omap_crtc_arm_event(crtc);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		return;
+	}
+
 	ret = drm_crtc_vblank_get(crtc);
 	WARN_ON(ret != 0);
 
@@ -646,6 +736,7 @@ omap_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	state->zpos = current_state->zpos;
 	state->rotation = current_state->rotation;
+	state->manually_updated = current_state->manually_updated;
 
 	return &state->base;
 }
@@ -722,6 +813,19 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 
+	/*
+	 * We want to refresh manually updated displays from dirty callback,
+	 * which is called quite often (e.g. for each drawn line). This will
+	 * be used to do the display update asynchronously to avoid blocking
+	 * the rendering process and merges multiple dirty calls into one
+	 * update if they arrive very fast. We also call this function for
+	 * atomic display updates (e.g. for page flips), which means we do
+	 * not need extra locking. Atomic updates should be synchronous, but
+	 * need to wait for the framedone interrupt anyways.
+	 */
+	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 d33bbb7a4f90..2b518c74203e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -42,5 +42,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 4f8eb9d08f99..b6f052f2a7e1 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -66,8 +66,27 @@ struct omap_framebuffer {
 	struct mutex lock;
 };
 
+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_crtc *crtc;
+
+	drm_modeset_lock_all(fb->dev);
+
+	drm_for_each_crtc(crtc, fb->dev)
+		omap_crtc_flush(crtc);
+
+	drm_modeset_unlock_all(fb->dev);
+
+	return 0;
+}
+
 static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
 	.create_handle = drm_gem_fb_create_handle,
+	.dirty = omap_framebuffer_dirty,
 	.destroy = drm_gem_fb_destroy,
 };
 
-- 
2.20.1