[Xen-devel] [PATCH v4 00/15] Use no_vblank property for drivers without VBLANK

[Xen-devel] [PATCH v4 00/15] Use no_vblank property for drivers without VBLANK

[Thomas Zimmermann]

Instead of faking VBLANK events by themselves, drivers without VBLANK
support can enable drm_crtc_vblank.no_vblank and let DRM do the rest.
The patchset makes this official and converts over drivers.

The current implementation looks at state of a device wrt vblanking. If
vblanking has been initialized for the device, the driver is responsible
for sending out VBLANK events. Otherwise, DRM will send out the event.
The behaviour is selected by initializing no_vblank as part of
drm_atomic_helper_check_modeset().

I went through all drivers, looking for those that call send out VBLANK
events but do not call drm_vblank_init(). These are converted to the new
semantics. This affects tiny drivers; drivers for virtual hardware; and
a few others, which do not support interrupts. Xen comes with its
own VBLANK logic and now disables no_vblank explictly.

v4:
	* replaced drm_crtc_has_vblank() with drm_dev_has_vblank() (Daniel)
	* squashed patches 1 and 2
	* moved driver updates into separate patches
v3:
	* reorder and squash patches
	* set no_vblank in drm_atomic_helper_check_modeset() for *all*
			  drivers (Daniel)
	* convert all drivers to new semnatics as necessary
v2:
	* document functionality (Daniel)
	* cleanup ast (Daniel)
	* let simple-kms handle no_vblank where possible

Thomas Zimmermann (15):
  drm: Initialize struct drm_crtc_state.no_vblank from device settings
  drm/arc: Remove sending of vblank event
  drm/ast: Don't set struct drm_crtc_state.no_vblank explictly
  drm/bochs: Remove sending of vblank event
  drm/cirrus: Remove sending of vblank event
  drm/gm12u320: Remove sending of vblank event
  drm/ili9225: Remove sending of vblank event
  drm/mipi-dbi: Remove sending of vblank event
  drm/qxl: Remove sending of vblank event
  drm/repaper: Remove sending of vblank event
  drm/st7586: Remove sending of vblank event
  drm/udl: Don't set struct drm_crtc_state.no_vblank explictly
  drm/vboxvideo: Remove sending of vblank event
  drm/virtio: Remove sending of vblank event
  drm/xen: Explicitly disable automatic sending of vblank event

 drivers/gpu/drm/arc/arcpgu_crtc.c        | 16 --------------
 drivers/gpu/drm/ast/ast_mode.c           |  2 --
 drivers/gpu/drm/bochs/bochs_kms.c        |  9 --------
 drivers/gpu/drm/cirrus/cirrus.c          |  8 -------
 drivers/gpu/drm/drm_atomic_helper.c      | 10 ++++++++-
 drivers/gpu/drm/drm_mipi_dbi.c           |  9 --------
 drivers/gpu/drm/drm_vblank.c             | 28 ++++++++++++++++++++++++
 drivers/gpu/drm/qxl/qxl_display.c        | 14 ------------
 drivers/gpu/drm/tiny/gm12u320.c          |  9 --------
 drivers/gpu/drm/tiny/ili9225.c           |  9 --------
 drivers/gpu/drm/tiny/repaper.c           |  9 --------
 drivers/gpu/drm/tiny/st7586.c            |  9 --------
 drivers/gpu/drm/udl/udl_modeset.c        | 11 ----------
 drivers/gpu/drm/vboxvideo/vbox_mode.c    | 12 ----------
 drivers/gpu/drm/virtio/virtgpu_display.c |  8 -------
 drivers/gpu/drm/xen/xen_drm_front_kms.c  | 13 +++++++++++
 include/drm/drm_crtc.h                   | 27 +++++++++++++++++------
 include/drm/drm_simple_kms_helper.h      |  7 ++++--
 include/drm/drm_vblank.h                 |  1 +
 19 files changed, 76 insertions(+), 135 deletions(-)

--
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Thomas Zimmermann]

At the end of a commit, atomic helpers can generate a VBLANK event
automatically. Originally implemented for writeback connectors, the
functionality can be used by any driver and/or hardware without proper
VBLANK interrupt.

The patch updates the documentation to make this behaviour official:
settings struct drm_crtc_state.no_vblank to true enables automatic
VBLANK generation.

The new interface drm_dev_has_vblank() returns true if vblanking has
been initialized for a device, or false otherwise. This function will
be useful when initializing no_vblank in the CRTC state.

Atomic modesetting helper set the initial value of no_vblank in
drm_atomic_helper_check_modeset(). If vblanking has been initialized
for a device, no_blank is disabled. Otherwise it's enabled. Hence,
atomic helpers will automatically send out VBLANK events with any
driver that did not initialize vblanking.

v4:
	* replace drm_crtc_has_vblank() with drm_dev_has_vblank()
	* add drm_dev_crtc_has_vblank() in this patch
	* move driver changes into separate patches
v3:
	* squash all related changes patches into this patch

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 10 +++++++++-
 drivers/gpu/drm/drm_vblank.c        | 28 ++++++++++++++++++++++++++++
 include/drm/drm_crtc.h              | 27 ++++++++++++++++++++-------
 include/drm/drm_simple_kms_helper.h |  7 +++++--
 include/drm/drm_vblank.h            |  1 +
 5 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4511c2e07bb9..d7b73cd89b79 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -583,6 +583,7 @@ mode_valid(struct drm_atomic_state *state)
  * &drm_crtc_state.connectors_changed is set when a connector is added or
  * removed from the CRTC.  &drm_crtc_state.active_changed is set when
  * &drm_crtc_state.active changes, which is used for DPMS.
+ * &drm_crtc_state.no_vblank is set from the result of drm_dev_has_vblank().
  * See also: drm_atomic_crtc_needs_modeset()
  *
  * IMPORTANT:
@@ -649,6 +650,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 
 			return -EINVAL;
 		}
+
+		if (drm_dev_has_vblank(dev))
+			new_crtc_state->no_vblank = false;
+		else
+			new_crtc_state->no_vblank = true;
 	}
 
 	ret = handle_conflicting_encoders(state, false);
@@ -2215,7 +2221,9 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
  * when a job is queued, and any change to the pipeline that does not touch the
  * connector is leading to timeouts when calling
  * drm_atomic_helper_wait_for_vblanks() or
- * drm_atomic_helper_wait_for_flip_done().
+ * drm_atomic_helper_wait_for_flip_done(). In addition to writeback
+ * connectors, this function can also fake VBLANK events for CRTCs without
+ * VBLANK interrupt.
  *
  * This is part of the atomic helper support for nonblocking commits, see
  * drm_atomic_helper_setup_commit() for an overview.
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 1659b13b178c..433dec6230b1 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -69,6 +69,12 @@
  * &drm_driver.max_vblank_count. In that case the vblank core only disables the
  * vblanks after a timer has expired, which can be configured through the
  * ``vblankoffdelay`` module parameter.
+ *
+ * Drivers for hardware without support for vertical-blanking interrupts
+ * must not call drm_vblank_init(). For such drivers, atomic helpers will
+ * automatically generate vblank events as part of the display update. This
+ * functionality also can be controlled by the driver by enabling and disabling
+ * struct drm_crtc_state.no_vblank.
  */
 
 /* Retry timestamp calculation up to 3 times to satisfy
@@ -501,6 +507,28 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
 }
 EXPORT_SYMBOL(drm_vblank_init);
 
+/**
+ * drm_dev_has_vblank - test if vblanking has been initialized for
+ *                      a device
+ * @dev: the device
+ *
+ * Drivers may call this function to test if vblank support is
+ * initialized for a device. For most hardware this means that vblanking
+ * can also be enabled.
+ *
+ * Atomic helpers use this function to initialize
+ * &drm_crtc_state.no_vblank. See also drm_atomic_helper_check_modeset().
+ *
+ * Returns:
+ * True if vblanking has been initialized for the given device, false
+ * otherwise.
+ */
+bool drm_dev_has_vblank(const struct drm_device *dev)
+{
+	return dev->num_crtcs != 0;
+}
+EXPORT_SYMBOL(drm_dev_has_vblank);
+
 /**
  * drm_crtc_vblank_waitqueue - get vblank waitqueue for the CRTC
  * @crtc: which CRTC's vblank waitqueue to retrieve
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5e9b15a0e8c5..5363e31c9abe 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -174,12 +174,22 @@ struct drm_crtc_state {
 	 * @no_vblank:
 	 *
 	 * Reflects the ability of a CRTC to send VBLANK events. This state
-	 * usually depends on the pipeline configuration, and the main usuage
-	 * is CRTCs feeding a writeback connector operating in oneshot mode.
-	 * In this case the VBLANK event is only generated when a job is queued
-	 * to the writeback connector, and we want the core to fake VBLANK
-	 * events when this part of the pipeline hasn't changed but others had
-	 * or when the CRTC and connectors are being disabled.
+	 * usually depends on the pipeline configuration. If set to true, DRM
+	 * atomic helpers will sendout a fake VBLANK event during display
+	 * updates.
+	 *
+	 * One usage is for drivers and/or hardware without support for VBLANK
+	 * interrupts. Such drivers typically do not initialize vblanking
+	 * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs
+	 * without initialized vblanking, the field is initialized to true and
+	 * a VBLANK event will be send out on each update of the display
+	 * pipeline.
+	 *
+	 * Another usage is CRTCs feeding a writeback connector operating in
+	 * oneshot mode. In this case the VBLANK event is only generated when
+	 * a job is queued to the writeback connector, and we want the core
+	 * to fake VBLANK events when this part of the pipeline hasn't changed
+	 * but others had or when the CRTC and connectors are being disabled.
 	 *
 	 * __drm_atomic_helper_crtc_duplicate_state() will not reset the value
 	 * from the current state, the CRTC driver is then responsible for
@@ -335,7 +345,10 @@ struct drm_crtc_state {
 	 *  - Events for disabled CRTCs are not allowed, and drivers can ignore
 	 *    that case.
 	 *
-	 * This can be handled by the drm_crtc_send_vblank_event() function,
+	 * For very simple hardware without VBLANK interrupt, enabling
+	 * &struct drm_crtc_state.no_vblank makes DRM's atomic commit helpers
+	 * send the event at an appropriate time. For more complex hardware this
+	 * can be handled by the drm_crtc_send_vblank_event() function,
 	 * which the driver should call on the provided event upon completion of
 	 * the atomic commit. Note that if the driver supports vblank signalling
 	 * and timestamping the vblank counters and timestamps must agree with
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 15afee9cf049..e253ba7bea9d 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -100,8 +100,11 @@ struct drm_simple_display_pipe_funcs {
 	 * This is the function drivers should submit the
 	 * &drm_pending_vblank_event from. Using either
 	 * drm_crtc_arm_vblank_event(), when the driver supports vblank
-	 * interrupt handling, or drm_crtc_send_vblank_event() directly in case
-	 * the hardware lacks vblank support entirely.
+	 * interrupt handling, or drm_crtc_send_vblank_event() for more
+	 * complex case. In case the hardware lacks vblank support entirely,
+	 * drivers can set &struct drm_crtc_state.no_vblank in
+	 * &struct drm_simple_display_pipe_funcs.check and let DRM's
+	 * atomic helper fake a vblank event.
 	 */
 	void (*update)(struct drm_simple_display_pipe *pipe,
 		       struct drm_plane_state *old_plane_state);
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index c16c44052b3d..94275e93fd27 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -206,6 +206,7 @@ struct drm_vblank_crtc {
 };
 
 int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
+bool drm_dev_has_vblank(const struct drm_device *dev);
 u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
 u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
 				   ktime_t *vblanktime);
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 02/15] drm/arc: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 8ae1e1f97a73..be7c29cec318 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -9,7 +9,6 @@
 #include <drm/drm_device.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
-#include <drm/drm_vblank.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <linux/clk.h>
@@ -138,24 +137,9 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
 			      ~ARCPGU_CTRL_ENABLE_MASK);
 }
 
-static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
-				      struct drm_crtc_state *state)
-{
-	struct drm_pending_vblank_event *event = crtc->state->event;
-
-	if (event) {
-		crtc->state->event = NULL;
-
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-	}
-}
-
 static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
 	.mode_valid	= arc_pgu_crtc_mode_valid,
 	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
-	.atomic_begin	= arc_pgu_crtc_atomic_begin,
 	.atomic_enable	= arc_pgu_crtc_atomic_enable,
 	.atomic_disable	= arc_pgu_crtc_atomic_disable,
 };
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 03/15] drm/ast: Don't set struct drm_crtc_state.no_vblank explictly

[Thomas Zimmermann]

As ast does not initialize vblanking, atomic helpers initialize the
value of struct drm_crtc_state.no_vblank to be true. No need to set
it from within the driver.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/ast/ast_mode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 34608f0499eb..7810a84e7e9e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -833,8 +833,6 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 	struct ast_vbios_mode_info *vbios_mode_info;
 	struct drm_display_mode *adjusted_mode;
 
-	crtc->state->no_vblank = true;
-
 	ast_state = to_ast_crtc_state(crtc->state);
 
 	format = ast_state->format;
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 04/15] drm/bochs: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/bochs/bochs_kms.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 3f0006c2470d..ff275faee88d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -7,7 +7,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_vblank.h>
 
 #include "bochs.h"
 
@@ -57,16 +56,8 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 			      struct drm_plane_state *old_state)
 {
 	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
-	struct drm_crtc *crtc = &pipe->crtc;
 
 	bochs_plane_update(bochs, pipe->plane.state);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		crtc->state->event = NULL;
-		spin_unlock_irq(&crtc->dev->event_lock);
-	}
 }
 
 static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 05/15] drm/cirrus: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/cirrus/cirrus.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 248c9f765c45..a91fb0d7282c 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -38,7 +38,6 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
-#include <drm/drm_vblank.h>
 
 #define DRIVER_NAME "cirrus"
 #define DRIVER_DESC "qemu cirrus vga"
@@ -434,13 +433,6 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		cirrus_fb_blit_rect(pipe->plane.state->fb, &rect);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		crtc->state->event = NULL;
-		spin_unlock_irq(&crtc->dev->event_lock);
-	}
 }
 
 static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 06/15] drm/gm12u320: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/tiny/gm12u320.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 94fb1f593564..a48173441ae0 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -22,7 +22,6 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
-#include <drm/drm_vblank.h>
 
 static bool eco_mode;
 module_param(eco_mode, bool, 0644);
@@ -610,18 +609,10 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
 				 struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		gm12u320_fb_mark_dirty(pipe->plane.state->fb, &rect);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		crtc->state->event = NULL;
-		spin_unlock_irq(&crtc->dev->event_lock);
-	}
 }
 
 static const struct drm_simple_display_pipe_funcs gm12u320_pipe_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 07/15] drm/ili9225: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/tiny/ili9225.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index c66acc566c2b..802fb8dde1b6 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -26,7 +26,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_mipi_dbi.h>
 #include <drm/drm_rect.h>
-#include <drm/drm_vblank.h>
 
 #define ILI9225_DRIVER_READ_CODE	0x00
 #define ILI9225_DRIVER_OUTPUT_CONTROL	0x01
@@ -165,18 +164,10 @@ static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		ili9225_fb_dirty(state->fb, &rect);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		crtc->state->event = NULL;
-	}
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 08/15] drm/mipi-dbi: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_mipi_dbi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 16bff1be4b8a..13b753cb3f67 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -24,7 +24,6 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_rect.h>
-#include <drm/drm_vblank.h>
 #include <video/mipi_display.h>
 
 #define MIPI_DBI_MAX_SPI_READ_SPEED 2000000 /* 2MHz */
@@ -299,18 +298,10 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 			  struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		mipi_dbi_fb_dirty(state->fb, &rect);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		crtc->state->event = NULL;
-	}
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 09/15] drm/qxl: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 16d73b22f3f5..ab4f8dd00400 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -31,7 +31,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_vblank.h>
 
 #include "qxl_drv.h"
 #include "qxl_object.h"
@@ -372,19 +371,6 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc *crtc,
 static void qxl_crtc_atomic_flush(struct drm_crtc *crtc,
 				  struct drm_crtc_state *old_crtc_state)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_pending_vblank_event *event;
-	unsigned long flags;
-
-	if (crtc->state && crtc->state->event) {
-		event = crtc->state->event;
-		crtc->state->event = NULL;
-
-		spin_lock_irqsave(&dev->event_lock, flags);
-		drm_crtc_send_vblank_event(crtc, event);
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-	}
-
 	qxl_crtc_update_monitors_config(crtc, "flush");
 }
 
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 10/15] drm/repaper: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/tiny/repaper.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 76d179200775..183484595aea 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -33,7 +33,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_rect.h>
-#include <drm/drm_vblank.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -856,18 +855,10 @@ static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		repaper_fb_dirty(state->fb);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		crtc->state->event = NULL;
-	}
 }
 
 static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 11/15] drm/st7586: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/tiny/st7586.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 060cc756194f..9ef559dd3191 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -23,7 +23,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_mipi_dbi.h>
 #include <drm/drm_rect.h>
-#include <drm/drm_vblank.h>
 
 /* controller-specific commands */
 #define ST7586_DISP_MODE_GRAY	0x38
@@ -159,18 +158,10 @@ static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		st7586_fb_dirty(state->fb, &rect);
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		crtc->state->event = NULL;
-	}
 }
 
 static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 12/15] drm/udl: Don't set struct drm_crtc_state.no_vblank explictly

[Thomas Zimmermann]

As udl does not initialize vblanking, atomic helpers initialize the
value of struct drm_crtc_state.no_vblank to be true. No need to set
it from within the driver.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/udl/udl_modeset.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index 22af17959053..d59ebac70b15 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -375,8 +375,6 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	char *wrptr;
 	int color_depth = UDL_COLOR_DEPTH_16BPP;
 
-	crtc_state->no_vblank = true;
-
 	buf = (char *)udl->mode_buf;
 
 	/* This first section has to do with setting the base address on the
@@ -428,14 +426,6 @@ udl_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe)
 	udl_submit_urb(dev, urb, buf - (char *)urb->transfer_buffer);
 }
 
-static int
-udl_simple_display_pipe_check(struct drm_simple_display_pipe *pipe,
-			      struct drm_plane_state *plane_state,
-			      struct drm_crtc_state *crtc_state)
-{
-	return 0;
-}
-
 static void
 udl_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *old_plane_state)
@@ -457,7 +447,6 @@ struct drm_simple_display_pipe_funcs udl_simple_display_pipe_funcs = {
 	.mode_valid = udl_simple_display_pipe_mode_valid,
 	.enable = udl_simple_display_pipe_enable,
 	.disable = udl_simple_display_pipe_disable,
-	.check = udl_simple_display_pipe_check,
 	.update = udl_simple_display_pipe_update,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 13/15] drm/vboxvideo: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index 19612132c8a3..8b7f005c4d20 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -18,7 +18,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_vblank.h>
 
 #include "hgsmi_channels.h"
 #include "vbox_drv.h"
@@ -226,17 +225,6 @@ static void vbox_crtc_atomic_disable(struct drm_crtc *crtc,
 static void vbox_crtc_atomic_flush(struct drm_crtc *crtc,
 				   struct drm_crtc_state *old_crtc_state)
 {
-	struct drm_pending_vblank_event *event;
-	unsigned long flags;
-
-	if (crtc->state && crtc->state->event) {
-		event = crtc->state->event;
-		crtc->state->event = NULL;
-
-		spin_lock_irqsave(&crtc->dev->event_lock, flags);
-		drm_crtc_send_vblank_event(crtc, event);
-		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
-	}
 }
 
 static const struct drm_crtc_helper_funcs vbox_crtc_helper_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 14/15] drm/virtio: Remove sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. Remove the sending code from the
driver.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 0966208ec30d..ecf4ba7cc32b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -30,7 +30,6 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_vblank.h>
 
 #include "virtgpu_drv.h"
 
@@ -121,13 +120,6 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc,
 static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 					 struct drm_crtc_state *old_state)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&crtc->dev->event_lock, flags);
-	if (crtc->state->event)
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-	crtc->state->event = NULL;
-	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
 }
 
 static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Thomas Zimmermann]

The atomic helpers automatically send out fake VBLANK events if no
vblanking has been initialized. This would apply to xen, but xen has
its own vblank logic. To avoid interfering with the atomic helpers,
disable automatic vblank events explictly.

v4:
	* separate commit from core vblank changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
index 4f34c5208180..efde4561836f 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
@@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
 	return false;
 }
 
+static int display_check(struct drm_simple_display_pipe *pipe,
+			 struct drm_plane_state *plane_state,
+			 struct drm_crtc_state *crtc_state)
+{
+	/* Make sure that DRM helpers don't send VBLANK events
+	 * automatically. Xen has it's own logic to do so.
+	 */
+	crtc_state->no_vblank = false;
+
+	return 0;
+}
+
 static void display_update(struct drm_simple_display_pipe *pipe,
 			   struct drm_plane_state *old_plane_state)
 {
@@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
 	.enable = display_enable,
 	.disable = display_disable,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.check = display_check,
 	.update = display_update,
 };
 
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Daniel Vetter]

On Thu, Jan 23, 2020 at 10:21:09AM +0100, Thomas Zimmermann wrote:
> At the end of a commit, atomic helpers can generate a VBLANK event
> automatically. Originally implemented for writeback connectors, the
> functionality can be used by any driver and/or hardware without proper
> VBLANK interrupt.
> 
> The patch updates the documentation to make this behaviour official:
> settings struct drm_crtc_state.no_vblank to true enables automatic
> VBLANK generation.
> 
> The new interface drm_dev_has_vblank() returns true if vblanking has
> been initialized for a device, or false otherwise. This function will
> be useful when initializing no_vblank in the CRTC state.
> 
> Atomic modesetting helper set the initial value of no_vblank in
> drm_atomic_helper_check_modeset(). If vblanking has been initialized
> for a device, no_blank is disabled. Otherwise it's enabled. Hence,
> atomic helpers will automatically send out VBLANK events with any
> driver that did not initialize vblanking.
> 
> v4:
> 	* replace drm_crtc_has_vblank() with drm_dev_has_vblank()
> 	* add drm_dev_crtc_has_vblank() in this patch

drm_dev_has_vblank I guess?

> 	* move driver changes into separate patches
> v3:
> 	* squash all related changes patches into this patch
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 10 +++++++++-
>  drivers/gpu/drm/drm_vblank.c        | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h              | 27 ++++++++++++++++++++-------
>  include/drm/drm_simple_kms_helper.h |  7 +++++--
>  include/drm/drm_vblank.h            |  1 +
>  5 files changed, 63 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4511c2e07bb9..d7b73cd89b79 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -583,6 +583,7 @@ mode_valid(struct drm_atomic_state *state)
>   * &drm_crtc_state.connectors_changed is set when a connector is added or
>   * removed from the CRTC.  &drm_crtc_state.active_changed is set when
>   * &drm_crtc_state.active changes, which is used for DPMS.
> + * &drm_crtc_state.no_vblank is set from the result of drm_dev_has_vblank().
>   * See also: drm_atomic_crtc_needs_modeset()
>   *
>   * IMPORTANT:
> @@ -649,6 +650,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  
>  			return -EINVAL;
>  		}
> +
> +		if (drm_dev_has_vblank(dev))
> +			new_crtc_state->no_vblank = false;
> +		else
> +			new_crtc_state->no_vblank = true;
>  	}
>  
>  	ret = handle_conflicting_encoders(state, false);
> @@ -2215,7 +2221,9 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
>   * when a job is queued, and any change to the pipeline that does not touch the
>   * connector is leading to timeouts when calling
>   * drm_atomic_helper_wait_for_vblanks() or
> - * drm_atomic_helper_wait_for_flip_done().
> + * drm_atomic_helper_wait_for_flip_done(). In addition to writeback
> + * connectors, this function can also fake VBLANK events for CRTCs without
> + * VBLANK interrupt.
>   *
>   * This is part of the atomic helper support for nonblocking commits, see
>   * drm_atomic_helper_setup_commit() for an overview.
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 1659b13b178c..433dec6230b1 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -69,6 +69,12 @@
>   * &drm_driver.max_vblank_count. In that case the vblank core only disables the
>   * vblanks after a timer has expired, which can be configured through the
>   * ``vblankoffdelay`` module parameter.
> + *
> + * Drivers for hardware without support for vertical-blanking interrupts
> + * must not call drm_vblank_init(). For such drivers, atomic helpers will
> + * automatically generate vblank events as part of the display update. This
> + * functionality also can be controlled by the driver by enabling and disabling
> + * struct drm_crtc_state.no_vblank.
>   */
>  
>  /* Retry timestamp calculation up to 3 times to satisfy
> @@ -501,6 +507,28 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
>  }
>  EXPORT_SYMBOL(drm_vblank_init);
>  
> +/**
> + * drm_dev_has_vblank - test if vblanking has been initialized for
> + *                      a device
> + * @dev: the device
> + *
> + * Drivers may call this function to test if vblank support is
> + * initialized for a device. For most hardware this means that vblanking
> + * can also be enabled.
> + *
> + * Atomic helpers use this function to initialize
> + * &drm_crtc_state.no_vblank. See also drm_atomic_helper_check_modeset().
> + *
> + * Returns:
> + * True if vblanking has been initialized for the given device, false
> + * otherwise.
> + */
> +bool drm_dev_has_vblank(const struct drm_device *dev)
> +{
> +	return dev->num_crtcs != 0;
> +}
> +EXPORT_SYMBOL(drm_dev_has_vblank);
> +
>  /**
>   * drm_crtc_vblank_waitqueue - get vblank waitqueue for the CRTC
>   * @crtc: which CRTC's vblank waitqueue to retrieve
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..5363e31c9abe 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -174,12 +174,22 @@ struct drm_crtc_state {
>  	 * @no_vblank:
>  	 *
>  	 * Reflects the ability of a CRTC to send VBLANK events. This state
> -	 * usually depends on the pipeline configuration, and the main usuage
> -	 * is CRTCs feeding a writeback connector operating in oneshot mode.
> -	 * In this case the VBLANK event is only generated when a job is queued
> -	 * to the writeback connector, and we want the core to fake VBLANK
> -	 * events when this part of the pipeline hasn't changed but others had
> -	 * or when the CRTC and connectors are being disabled.
> +	 * usually depends on the pipeline configuration. If set to true, DRM
> +	 * atomic helpers will sendout a fake VBLANK event during display

s/sendout/ send out/

> +	 * updates.

maby add "... updates, after all hardware changes have been committed.
This is implemented in drm_atomic_helper_fake_vblank()."

That at least reflects the default behaviour.

> +	 *
> +	 * One usage is for drivers and/or hardware without support for VBLANK
> +	 * interrupts. Such drivers typically do not initialize vblanking
> +	 * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs

s/wit/with/

> +	 * without initialized vblanking, the field is initialized to true and

"initialized to true" is a bit a stretch, since this is done in the
helpers. Maybe instead "... this field is set to true in
drm_atomic_helper_check_modeset() and ..."

> +	 * a VBLANK event will be send out on each update of the display
> +	 * pipeline.

Maybe also reiterate that this is done through
drm_atomic_helper_fake_vblank() again? Imo for stuff done by helpers it's
important to highligh where that's done, since it's all opt-in (even if
most drivers will opt-in automatically).

> +	 *
> +	 * Another usage is CRTCs feeding a writeback connector operating in
> +	 * oneshot mode. In this case the VBLANK event is only generated when
> +	 * a job is queued to the writeback connector, and we want the core
> +	 * to fake VBLANK events when this part of the pipeline hasn't changed
> +	 * but others had or when the CRTC and connectors are being disabled.
>  	 *
>  	 * __drm_atomic_helper_crtc_duplicate_state() will not reset the value
>  	 * from the current state, the CRTC driver is then responsible for
> @@ -335,7 +345,10 @@ struct drm_crtc_state {
>  	 *  - Events for disabled CRTCs are not allowed, and drivers can ignore
>  	 *    that case.
>  	 *
> -	 * This can be handled by the drm_crtc_send_vblank_event() function,
> +	 * For very simple hardware without VBLANK interrupt, enabling
> +	 * &struct drm_crtc_state.no_vblank makes DRM's atomic commit helpers
> +	 * send the event at an appropriate time. For more complex hardware this
> +	 * can be handled by the drm_crtc_send_vblank_event() function,
>  	 * which the driver should call on the provided event upon completion of
>  	 * the atomic commit. Note that if the driver supports vblank signalling
>  	 * and timestamping the vblank counters and timestamps must agree with
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 15afee9cf049..e253ba7bea9d 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -100,8 +100,11 @@ struct drm_simple_display_pipe_funcs {
>  	 * This is the function drivers should submit the
>  	 * &drm_pending_vblank_event from. Using either
>  	 * drm_crtc_arm_vblank_event(), when the driver supports vblank
> -	 * interrupt handling, or drm_crtc_send_vblank_event() directly in case
> -	 * the hardware lacks vblank support entirely.
> +	 * interrupt handling, or drm_crtc_send_vblank_event() for more
> +	 * complex case. In case the hardware lacks vblank support entirely,
> +	 * drivers can set &struct drm_crtc_state.no_vblank in
> +	 * &struct drm_simple_display_pipe_funcs.check and let DRM's
> +	 * atomic helper fake a vblank event.
>  	 */
>  	void (*update)(struct drm_simple_display_pipe *pipe,
>  		       struct drm_plane_state *old_plane_state);
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index c16c44052b3d..94275e93fd27 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -206,6 +206,7 @@ struct drm_vblank_crtc {
>  };
>  
>  int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
> +bool drm_dev_has_vblank(const struct drm_device *dev);
>  u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
>  u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
>  				   ktime_t *vblanktime);

With the doc nits addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> -- 
> 2.24.1
> 

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Daniel Vetter]

On Thu, Jan 23, 2020 at 10:21:23AM +0100, Thomas Zimmermann wrote:
> The atomic helpers automatically send out fake VBLANK events if no
> vblanking has been initialized. This would apply to xen, but xen has
> its own vblank logic. To avoid interfering with the atomic helpers,
> disable automatic vblank events explictly.
> 
> v4:
> 	* separate commit from core vblank changes
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>

On all the driver patches:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> index 4f34c5208180..efde4561836f 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> @@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
>  	return false;
>  }
>  
> +static int display_check(struct drm_simple_display_pipe *pipe,
> +			 struct drm_plane_state *plane_state,
> +			 struct drm_crtc_state *crtc_state)
> +{
> +	/* Make sure that DRM helpers don't send VBLANK events
> +	 * automatically. Xen has it's own logic to do so.
> +	 */
> +	crtc_state->no_vblank = false;

So this here is strictly speaking dead code, because the fake_vblank
helper makes sure to only send out the event if it hasn't been processed
yet.

Which is a bit awkward, since it does this silently, potentially hiding
bugs. We already have a warning that complains if a crtc_state->event
hasn't been processed at all. But with the auto-vblank stuff here we kinda
defeat that a bit ... I think adding a WARN_ON in fake_vblank that fires
if no_vblank is set, but the event is somehow gone, would be really
useful. That would point at some serious driver snafu ...

Care to throw that on top?
-Daniel

> +
> +	return 0;
> +}
> +
>  static void display_update(struct drm_simple_display_pipe *pipe,
>  			   struct drm_plane_state *old_plane_state)
>  {
> @@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>  	.enable = display_enable,
>  	.disable = display_disable,
>  	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.check = display_check,
>  	.update = display_update,
>  };
>  
> -- 
> 2.24.1
> 

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Oleksandr Andrushchenko]

Sorry for jumping in late

On 1/23/20 11:21 AM, Thomas Zimmermann wrote:
> The atomic helpers automatically send out fake VBLANK events if no
> vblanking has been initialized. This would apply to xen, but xen has
> its own vblank logic. To avoid interfering with the atomic helpers,
> disable automatic vblank events explictly.
>
> v4:
> 	* separate commit from core vblank changes
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

> ---
>   drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> index 4f34c5208180..efde4561836f 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> @@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
>   	return false;
>   }
>   
> +static int display_check(struct drm_simple_display_pipe *pipe,
> +			 struct drm_plane_state *plane_state,
> +			 struct drm_crtc_state *crtc_state)
> +{
> +	/* Make sure that DRM helpers don't send VBLANK events
Could you please put the comment on a separate line?
> +	 * automatically. Xen has it's own logic to do so.
> +	 */
> +	crtc_state->no_vblank = false;
And it is still confusing, e.g. comment says
"Make sure that DRM helpers don't send VBLANK"
and we set "no_vblank" flag to false...
> +
> +	return 0;
> +}
> +
>   static void display_update(struct drm_simple_display_pipe *pipe,
>   			   struct drm_plane_state *old_plane_state)
>   {
> @@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>   	.enable = display_enable,
>   	.disable = display_disable,
>   	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.check = display_check,
>   	.update = display_update,
>   };
>   
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2477 bytes --]

Hi

Am 27.01.20 um 10:53 schrieb Oleksandr Andrushchenko:
> Sorry for jumping in late
> 
> On 1/23/20 11:21 AM, Thomas Zimmermann wrote:
>> The atomic helpers automatically send out fake VBLANK events if no
>> vblanking has been initialized. This would apply to xen, but xen has
>> its own vblank logic. To avoid interfering with the atomic helpers,
>> disable automatic vblank events explictly.
>>
>> v4:
>> 	* separate commit from core vblank changes
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
>> ---
>>   drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> index 4f34c5208180..efde4561836f 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> @@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
>>   	return false;
>>   }
>>   
>> +static int display_check(struct drm_simple_display_pipe *pipe,
>> +			 struct drm_plane_state *plane_state,
>> +			 struct drm_crtc_state *crtc_state)
>> +{
>> +	/* Make sure that DRM helpers don't send VBLANK events
> Could you please put the comment on a separate line?

You mean to add an empty line between comment and code?

>> +	 * automatically. Xen has it's own logic to do so.
>> +	 */
>> +	crtc_state->no_vblank = false;
> And it is still confusing, e.g. comment says
> "Make sure that DRM helpers don't send VBLANK"
> and we set "no_vblank" flag to false...

I'll rephrase and add some more context.

Best regards
Thomas

>> +
>> +	return 0;
>> +}
>> +
>>   static void display_update(struct drm_simple_display_pipe *pipe,
>>   			   struct drm_plane_state *old_plane_state)
>>   {
>> @@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>>   	.enable = display_enable,
>>   	.disable = display_disable,
>>   	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.check = display_check,
>>   	.update = display_update,
>>   };
>>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 15/15] drm/xen: Explicitly disable automatic sending of vblank event

[Oleksandr Andrushchenko]

On 1/27/20 1:59 PM, Thomas Zimmermann wrote:
> Hi
>
> Am 27.01.20 um 10:53 schrieb Oleksandr Andrushchenko:
>> Sorry for jumping in late
>>
>> On 1/23/20 11:21 AM, Thomas Zimmermann wrote:
>>> The atomic helpers automatically send out fake VBLANK events if no
>>> vblanking has been initialized. This would apply to xen, but xen has
>>> its own vblank logic. To avoid interfering with the atomic helpers,
>>> disable automatic vblank events explictly.
>>>
>>> v4:
>>> 	* separate commit from core vblank changes
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>>> ---
>>>    drivers/gpu/drm/xen/xen_drm_front_kms.c | 13 +++++++++++++
>>>    1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>>> index 4f34c5208180..efde4561836f 100644
>>> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
>>> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>>> @@ -220,6 +220,18 @@ static bool display_send_page_flip(struct drm_simple_display_pipe *pipe,
>>>    	return false;
>>>    }
>>>    
>>> +static int display_check(struct drm_simple_display_pipe *pipe,
>>> +			 struct drm_plane_state *plane_state,
>>> +			 struct drm_crtc_state *crtc_state)
>>> +{
>>> +	/* Make sure that DRM helpers don't send VBLANK events
>> Could you please put the comment on a separate line?
> You mean to add an empty line between comment and code?
>
Just like
/*
  * Make sure...
>>> +	 * automatically. Xen has it's own logic to do so.
>>> +	 */
>>> +	crtc_state->no_vblank = false;
>> And it is still confusing, e.g. comment says
>> "Make sure that DRM helpers don't send VBLANK"
>> and we set "no_vblank" flag to false...
> I'll rephrase and add some more context.
Thank you
>
> Best regards
> Thomas
>
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static void display_update(struct drm_simple_display_pipe *pipe,
>>>    			   struct drm_plane_state *old_plane_state)
>>>    {
>>> @@ -284,6 +296,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>>>    	.enable = display_enable,
>>>    	.disable = display_disable,
>>>    	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>>> +	.check = display_check,
>>>    	.update = display_update,
>>>    };
>>>    
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Emil Velikov]

Hi Thomas,

On Thu, 23 Jan 2020 at 09:21, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> @@ -174,12 +174,22 @@ struct drm_crtc_state {
>          * @no_vblank:
>          *
>          * Reflects the ability of a CRTC to send VBLANK events. This state
> -        * usually depends on the pipeline configuration, and the main usuage
> -        * is CRTCs feeding a writeback connector operating in oneshot mode.
> -        * In this case the VBLANK event is only generated when a job is queued
> -        * to the writeback connector, and we want the core to fake VBLANK
> -        * events when this part of the pipeline hasn't changed but others had
> -        * or when the CRTC and connectors are being disabled.
> +        * usually depends on the pipeline configuration. If set to true, DRM
> +        * atomic helpers will sendout a fake VBLANK event during display
> +        * updates.
> +        *
> +        * One usage is for drivers and/or hardware without support for VBLANK
> +        * interrupts. Such drivers typically do not initialize vblanking
> +        * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs
> +        * without initialized vblanking, the field is initialized to true and
> +        * a VBLANK event will be send out on each update of the display
> +        * pipeline.
> +        *
> +        * Another usage is CRTCs feeding a writeback connector operating in
> +        * oneshot mode. In this case the VBLANK event is only generated when
> +        * a job is queued to the writeback connector, and we want the core
> +        * to fake VBLANK events when this part of the pipeline hasn't changed
> +        * but others had or when the CRTC and connectors are being disabled.
>          *

Perhaps it's just me, yet the following ideas would make the topic
significantly easier and clearer.

 - adding explicit "fake" when talking about drm/atomic _helpers_
generating/sending a VBLANK event.
For example, in 15/15 the commit message says "fake", while inline
comment omits it... Leading to the confusion pointed out.

- s/no_vblank/fake_vblank/g or s/no_vblank/no_hw_vblank/g
Simple and concise. With slight inclination towards the former wording :-)

If you and Daniel agree with the rename, then the first sentence of
the description should probably be tweaked.

HTH
Emil

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3121 bytes --]

Hi Emil

Am 27.01.20 um 19:12 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Thu, 23 Jan 2020 at 09:21, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> @@ -174,12 +174,22 @@ struct drm_crtc_state {
>>          * @no_vblank:
>>          *
>>          * Reflects the ability of a CRTC to send VBLANK events. This state
>> -        * usually depends on the pipeline configuration, and the main usuage
>> -        * is CRTCs feeding a writeback connector operating in oneshot mode.
>> -        * In this case the VBLANK event is only generated when a job is queued
>> -        * to the writeback connector, and we want the core to fake VBLANK
>> -        * events when this part of the pipeline hasn't changed but others had
>> -        * or when the CRTC and connectors are being disabled.
>> +        * usually depends on the pipeline configuration. If set to true, DRM
>> +        * atomic helpers will sendout a fake VBLANK event during display
>> +        * updates.
>> +        *
>> +        * One usage is for drivers and/or hardware without support for VBLANK
>> +        * interrupts. Such drivers typically do not initialize vblanking
>> +        * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs
>> +        * without initialized vblanking, the field is initialized to true and
>> +        * a VBLANK event will be send out on each update of the display
>> +        * pipeline.
>> +        *
>> +        * Another usage is CRTCs feeding a writeback connector operating in
>> +        * oneshot mode. In this case the VBLANK event is only generated when
>> +        * a job is queued to the writeback connector, and we want the core
>> +        * to fake VBLANK events when this part of the pipeline hasn't changed
>> +        * but others had or when the CRTC and connectors are being disabled.
>>          *
> 
> Perhaps it's just me, yet the following ideas would make the topic
> significantly easier and clearer.
> 
>  - adding explicit "fake" when talking about drm/atomic _helpers_
> generating/sending a VBLANK event.
> For example, in 15/15 the commit message says "fake", while inline
> comment omits it... Leading to the confusion pointed out.

No problem on being more precise here. I'll update the docs accordingly.

> 
> - s/no_vblank/fake_vblank/g or s/no_vblank/no_hw_vblank/g
> Simple and concise. With slight inclination towards the former wording :-)

I'd prefer to not change the field's name. The current name 'no_vblank'
indicates state and lets helpers decide what to do with it. The name
'fake_vblank' indicates an instruction to the helpers, telling them what
to do. It does neither seem to fit into drm_crtc_state, nor into the
overall concept.

Best regards
Thomas

> 
> If you and Daniel agree with the rename, then the first sentence of
> the description should probably be tweaked.
> 
> HTH
> Emil
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 01/15] drm: Initialize struct drm_crtc_state.no_vblank from device settings

[Daniel Vetter]

On Mon, Jan 27, 2020 at 07:42:27PM +0100, Thomas Zimmermann wrote:
> Hi Emil
> 
> Am 27.01.20 um 19:12 schrieb Emil Velikov:
> > Hi Thomas,
> > 
> > On Thu, 23 Jan 2020 at 09:21, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > 
> >> @@ -174,12 +174,22 @@ struct drm_crtc_state {
> >>          * @no_vblank:
> >>          *
> >>          * Reflects the ability of a CRTC to send VBLANK events. This state
> >> -        * usually depends on the pipeline configuration, and the main usuage
> >> -        * is CRTCs feeding a writeback connector operating in oneshot mode.
> >> -        * In this case the VBLANK event is only generated when a job is queued
> >> -        * to the writeback connector, and we want the core to fake VBLANK
> >> -        * events when this part of the pipeline hasn't changed but others had
> >> -        * or when the CRTC and connectors are being disabled.
> >> +        * usually depends on the pipeline configuration. If set to true, DRM
> >> +        * atomic helpers will sendout a fake VBLANK event during display
> >> +        * updates.
> >> +        *
> >> +        * One usage is for drivers and/or hardware without support for VBLANK
> >> +        * interrupts. Such drivers typically do not initialize vblanking
> >> +        * (i.e., call drm_vblank_init() wit the number of CRTCs). For CRTCs
> >> +        * without initialized vblanking, the field is initialized to true and
> >> +        * a VBLANK event will be send out on each update of the display
> >> +        * pipeline.
> >> +        *
> >> +        * Another usage is CRTCs feeding a writeback connector operating in
> >> +        * oneshot mode. In this case the VBLANK event is only generated when
> >> +        * a job is queued to the writeback connector, and we want the core
> >> +        * to fake VBLANK events when this part of the pipeline hasn't changed
> >> +        * but others had or when the CRTC and connectors are being disabled.
> >>          *
> > 
> > Perhaps it's just me, yet the following ideas would make the topic
> > significantly easier and clearer.
> > 
> >  - adding explicit "fake" when talking about drm/atomic _helpers_
> > generating/sending a VBLANK event.
> > For example, in 15/15 the commit message says "fake", while inline
> > comment omits it... Leading to the confusion pointed out.
> 
> No problem on being more precise here. I'll update the docs accordingly.
> 
> > 
> > - s/no_vblank/fake_vblank/g or s/no_vblank/no_hw_vblank/g
> > Simple and concise. With slight inclination towards the former wording :-)
> 
> I'd prefer to not change the field's name. The current name 'no_vblank'
> indicates state and lets helpers decide what to do with it. The name
> 'fake_vblank' indicates an instruction to the helpers, telling them what
> to do. It does neither seem to fit into drm_crtc_state, nor into the
> overall concept.

Yeah e.g. xen has no hw vblank, but still has special processing of
events, which are kinda triggered by the "hw" (it's an event from the
compositor).

Maybe the confusion is with the helper function that generates the
fake_vblank, since it's not really a fake vblank at all, it's just "send
out this atomic completion event now, I'm not going to do it as part of
the vblank processing since no vblank". So maybe that function should be
called _send_events_i_have_no_hw_vblank, which yeah is not a great name
:-) But maybe you have an idea for that one?
-Daniel

> 
> Best regards
> Thomas
> 
> > 
> > If you and Daniel agree with the rename, then the first sentence of
> > the description should probably be tweaked.
> > 
> > HTH
> > Emil
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel